eclipse.platform.ui icon indicating copy to clipboard operation
eclipse.platform.ui copied to clipboard

feat : Add global search result navigation shortcuts

Open NikkiAung opened this issue 3 months ago • 17 comments

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 https://github.com/eclipse-platform/eclipse.platform.ui/issues/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 avatar Sep 20 '25 00:09 NikkiAung

Here is the demo after making some code changes.

https://github.com/user-attachments/assets/a5f6d352-f4ec-4606-9c90-8ff185f55473

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 avatar Sep 25 '25 03:09 NikkiAung

@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 avatar Oct 01 '25 08:10 vogella

/gemini review

vogella avatar Oct 01 '25 08:10 vogella

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! :)

image

NikkiAung avatar Oct 01 '25 20:10 NikkiAung

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

NikkiAung avatar Oct 15 '25 23:10 NikkiAung

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?

vogella avatar Oct 16 '25 08:10 vogella

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 avatar Oct 16 '25 21:10 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?

NikkiAung avatar Oct 19 '25 02:10 NikkiAung

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

vogella avatar Oct 22 '25 15:10 vogella

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:

image

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?

NikkiAung avatar Oct 23 '25 00:10 NikkiAung

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

vogella avatar Oct 23 '25 15:10 vogella

Test Results

 3 024 files   3 024 suites   2h 19m 25s ⏱️  8 244 tests  7 995 ✅ 249 💤 0 ❌ 23 652 runs  22 858 ✅ 794 💤 0 ❌

Results for commit 9c716bf1.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar Oct 23 '25 17:10 github-actions[bot]

Just updated!

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

Lmk how it goes. :)

NikkiAung avatar Oct 24 '25 05:10 NikkiAung

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

vogella avatar Oct 24 '25 16:10 vogella

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

vogella avatar Oct 24 '25 16:10 vogella

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 avatar Oct 25 '25 04:10 NikkiAung

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

NikkiAung avatar Nov 07 '25 04:11 NikkiAung