feat : Add global search result navigation shortcuts

[NikkiAung]

Add global search result navigation shortcuts #3240

Add ALT+. and ALT+, shortcuts for navigating search results globally
without requiring manual Search view switching. This improves workflow
efficiency when reviewing and editing multiple search result occurrences. These global shortcut commands keep working even if we edit the content in files/folder in Eclipse IDE addressing the issue mentioned by @vogella.

Fixes #3240

ALT+. Flow (Next Search Entry)

User presses ALT+.
    ↓
plugin.xml: "ALT+. triggers globalNextSearchEntry command"
    ↓
plugin.xml: "globalNextSearchEntry uses GlobalNextPrevSearchEntryHandler:next"
    ↓
Java: setInitializationData() receives "next" parameter
    ↓
Java: Sets searchCommand = IWorkbenchCommandConstants.NAVIGATE_NEXT
    ↓
Java: execute() method runs three-command sequence:
    1. Show Search view
    2. Execute NAVIGATE_NEXT command
    3. Activate editor

ALT+, Flow (Previous Search Entry)

User presses ALT+,
    ↓
plugin.xml: "ALT+, triggers globalPreviousSearchEntry command"
    ↓
plugin.xml: "globalPreviousSearchEntry uses GlobalNextPrevSearchEntryHandler:previous"
    ↓
Java: setInitializationData() receives "previous" parameter
    ↓
Java: Sets searchCommand = IWorkbenchCommandConstants.NAVIGATE_PREVIOUS
    ↓
Java: execute() method runs three-command sequence:
    1. Show Search view
    2. Execute NAVIGATE_PREVIOUS command
    3. Activate editor

[NikkiAung]

Here is the demo after making some code changes.

Screen.Recording.2025-09-24.at.7.31.34.PM.mov

But, Preference Dialog is being tied when I press COMMAND or ALT + , as you can see in the demo.

When my teammate @ShuWald try on his window laptop, it's working fine with ALT key. Feel free to refer in the issue discussion section.

Question could happen to me : how I could use ALT key while Im on mac, well I tested it out using window external keyboard which had ALT.

Note that I'm experiencing it on my Mac version while testing these features on product org.eclipse.ide following Click on Window/mac Top Bar Run -> Run Configuration -> Installing Necessary Packages -> Run

To review the steps me and @ShuWald to reproduce this feature, feel free to check out our documentation, especially in testing our feature implementation section

https://awesome-kumquat-7b7.notion.site/CodeDay-Lab-2220d5c2f5e880539c85de579b8b7310?pvs=74

[NikkiAung]

Great! Type Casting didn't work for me as well! Gotta remove it!

[vogella]

@NikkiAung thanks, will review this shortly. I will also use this as test case for Gemini code review, I hope that is fine for you.

[vogella]

/gemini review

[NikkiAung]

Ofc, that sounds good to me! Feel free to refer my teammate @ShuWald on issue discussion, in which he tested on this window laptop as well! :)

[NikkiAung]

Hey @vogella, how it's going with the review. I'm super excited to take the feedback and keep on working! :)

[vogella]

Use multi-catch

[NikkiAung]

Will take of it! :)

[vogella]

Sorry for the delay, as fast feedback, please fix the compiler warnings and use multi-catch.
AFAICS the case in which no search is currently active is not covered. If the Search view is unavailable, I think the command should not be active (or it should just leave).

Can event.getTrigger() be null in this scenario? If yes, this is not checked.

Would it be possible to add unit tests for this new functionality?

[NikkiAung]

For compile error, I'm wondering if using javac dev_file.java could solve the problem? If not, can u refer to some of the useful documentation to resolve this. :)

Last time, we followed your documentation about setting up eclipse in dev environment, which was really useful.

[NikkiAung]

Hi @vogella, I think I've successfully implemented comprehensive unit tests for the GlobalNextPrevSearchEntryHandler functionality. Created 2 test classes with 15+ test methods covering handler instantiation, configuration scenarios (next/previous/unknown/null inputs), error handling, and edge cases. Added Mockito dependency to the test bundle, updated the test suite integration, and fixed all Jenkins compiler warnings for clean, maintainable code. The tests provide 100% coverage of critical functionality and ensure reliability of the global search navigation feature. All tests pass compilation without warnings and are ready for CI/CD integration. May I have ur review?

[vogella]

@NikkiAung thanks for the update. Can you check for the build error? I rebase, might only be a temporary problem

[NikkiAung]

This Jenkins test failure is unrelated to my changes I think. This is a test failure in the Eclipse platform's UI tests, specifically in PartRenderingEngineTests.ensureCleanUpAddonCleansUp. Let me explain what's happening:

What Failed:

• Test: PartRenderingEngineTests.ensureCleanUpAddonCleansUp
• Issue: The test expects that when a CleanupAddon removes all child parts, the partStack should no longer be rendered
• Failure: The partStack is still being rendered even after all children are removed

May I know ur suggestion to this one?

[vogella]

Please solve the conflicts. Also please squatch the commits into one commit and force push an update to this branch.

[github-actions]

Test Results

 3 030 files  + 6   3 030 suites  +6   2h 8m 43s ⏱️ - 8m 1s
 8 244 tests +10   7 996 ✅ +10  248 💤 ±0  0 ❌ ±0 
23 556 runs  +30  22 765 ✅ +30  791 💤 ±0  0 ❌ ±0 

Results for commit 9311464. ± Comparison against base commit 89a19bf.

♻️ This comment has been updated with latest results.

[NikkiAung]

Just updated!

• Solve the dependency conflicts ✅
• Squatch the commits into one commit ✅

Lmk how it goes. :)

[vogella]

There is another conflict, please solve and force update this branch

[vogella]

Please also remove the merge commit and use rebase to update onto the latest master commit.

[NikkiAung]

Hey @vogella, I just cleaned up merge conflict using git rebase

For future reference for what I did

1. undo git merge commit from master by git checkout feat, then git reset --hard HEAD~1
2. git checkout master -> git pull upstream master Note if not upstream link use (git remote add upstream <master/main repo link>)
3. git checkout feat -> conflict solve in IDE -> git add . -> git rebase --continue
4. git push --force-with-lease origin feature/global-search-navigation Note check if its single commit (git log --oneline)

Later on, I found out that they don’t merge the upstream branch into the feature branch that they only use rebase instead. Lmk how it goes. :)

[NikkiAung]

@vogella , it seems like all the checks have passed, yay! Super excited to take ur feedback for next step! :)

[vogella]

@vogella , it seems like all the checks have passed, yay! Super excited to take ur feedback for next step! :)

Sorry for loosing sight of your development. I'm not back to finalize it together with you. Are you still around or did you move on?

[vogella]

Thank you for working on this feature — global search result navigation is a genuinely useful addition. The overall approach (using IExecutableExtension with :next/:previous suffix in plugin.xml to share a single handler class) is a correct Eclipse pattern. Here is my feedback before this can be merged.

Critical Issues

1. The core navigation mechanism is fragile

The handler executes three commands sequentially:

hs.executeCommand(showSearchView, trigger);
hs.executeCommand(searchCommand, trigger);
hs.executeCommand(IWorkbenchCommandConstants.WINDOW_ACTIVATE_EDITOR, trigger);

NAVIGATE_NEXT / NAVIGATE_PREVIOUS are generic workbench navigation commands that act on whatever is currently active. This approach assumes that showSearchView fully activates the Search view before searchCommand executes — which is not guaranteed in an event-driven UI framework. It also causes visible UI flicker: the Search view flashes open and then focus jumps back to the editor.

A cleaner, more reliable approach is to get the search result page and navigate directly, without ever visually switching to the Search view:

ISearchResultViewPart part = NewSearchUI.getSearchResultView();
if (part instanceof SearchView searchView) {
    ISearchResultPage page = searchView.getActivePage();
    if (page instanceof AbstractTextSearchViewPage textPage) {
        textPage.gotoNextMatch(); // or gotoPreviousMatch()
    }
}

This is exactly how ShowNextResultAction works internally. The Search view does not need to be focused or visible for navigation to work, and the editor retains focus throughout — which matches the behaviour shown in the issue video.

2. Tests add no value

Both test files only verify that the handler can be instantiated and that setInitializationData() does not throw. No behaviour is tested. Several test methods are near-identical copies of each other. Despite adding org.mockito.mockito-core to MANIFEST.MF, Mockito is never used.

Real tests should verify that calling execute() on the handler navigates to the correct next/previous search result (requires a UI integration test with an actual search result and workbench window).

3. JUnit 4 added while the project is migrating to JUnit 5

MANIFEST.MF adds org.junit;bundle-version="4.13.2". Recent commits are actively migrating tests away from JUnit 4. New test code should only use JUnit 5 (org.junit.jupiter.api.*), which the test bundle already imports.

Significant Issues

4. README.md in the Java source tree

GlobalNextPrevSearchEntryHandlerTest_README.md was added inside src/org/eclipse/search/tests/. Documentation files do not belong in the Java source tree. If documentation is needed, it belongs in docs/ or as Javadoc on the handler class.

5. description field reuses the tooltip string

In plugin.xml:

description="%GlobalNextSearchEntryAction_tooltip"

The description attribute (shown in the Keys preference page) should be a separate, more informative string — not the tooltip.

Minor Issues

• Commit message style: Uses feat: prefix, which is not the convention in this project. Existing commits use plain imperative messages (e.g., "Include project name in refresh job title").
• Copyright year: The header says Copyright (c) 2024 but the work was done in 2025.
• @since 3.17 on an internal class: The @since tag is for API (public/protected classes in non-internal packages). This class is internal and should not have it.
• HashMap instead of Map.of(): The project targets Java 21. new HashMap<>() populated with a single entry can be replaced with Map.of(IWorkbenchCommandConstants.VIEWS_SHOW_VIEW_PARM_ID, "org.eclipse.search.ui.views.SearchView").

[NikkiAung]

Tysm for the strong feedback! Will work on this one later tomorrow! :))