Find/Replace overlay: handle the editor's find next and find previous - #4355
Open
HeikoKlare wants to merge 1 commit into
Open
Find/Replace overlay: handle the editor's find next and find previous#4355HeikoKlare wants to merge 1 commit into
HeikoKlare wants to merge 1 commit into
Conversation
While one of the overlay's input fields has focus, the editor's actions are deactivated so that its commands do not act on the document instead of on the field. That takes away their handlers, but not the bindings of those commands: the ones declared without a context id live in the window scope, which stays active. Find Next and Find Previous therefore still resolved to their command and then found nothing to run, so their key bindings did not work from within the overlay at all. Both commands stay meaningful inside the overlay, so instead of leaving them unhandled they are now bound to the overlay's own search commands and do from there what its search buttons do. Adopting the command id rather than its key sequence keeps this working when the user rebinds the command, and running the overlay's own operation gives the semantics the user expects in a search field: the term being typed is searched for, not the previously persisted one the editor's own action would use, and an empty field does nothing, just like clicking the buttons. The behavior of those keys inside editors is unaffected. There they keep running the editor's own actions, which continue to use the search settings last defined in the overlay, including after moving on to another file or editor. This extends the small, explicit set of the editor's commands the overlay already adopted for find/replace and content assist. Which commands belong to that set stays a deliberate choice per command, since adopting one only makes sense where the overlay has a sensible meaning for it. Contributes to eclipse-platform#2737 Assisted-by: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The change is isolated to overlay command handling and is validated by new focused end-to-end tests covering the intended keybinding and command-id behavior.
Pull request overview
This PR improves the Eclipse text editor Find/Replace overlay so that the existing Find Next / Find Previous commands (typically bound to Ctrl+K / Ctrl+Shift+K) continue to work when focus is inside the overlay’s input fields, by routing those commands to the overlay’s own search behavior rather than leaving them unhandled.
Changes:
- Register overlay-scoped handlers for the workbench Find Next / Find Previous command IDs to invoke the overlay’s forward/backward search.
- Add end-to-end UI tests ensuring the commands work via keypress and via command execution-by-id, and that an empty search field is a no-op.
File summaries
| File | Description |
|---|---|
| bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlay.java | Activates overlay handlers for org.eclipse.ui.edit.findNext/findPrevious while the overlay is focused, delegating to existing performSearch(...) logic. |
| tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayInEditorTest.java | Adds coverage for Ctrl+K / Ctrl+Shift+K behavior in the focused search field, command execution by ID, and empty-search no-op behavior. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The Find Next and Find Previous key bindings currently do nothing while the Find/Replace overlay has focus. While one of its input fields has focus, the overlay deactivates the editor's actions so that the editor's commands do not act on the document instead of on the field, which takes the handlers of those two commands away. Their bindings are declared without a context id and therefore live in the window scope, which stays active, so a key press still resolves to the command and then finds nothing to run.
Both commands stay meaningful inside the overlay, so they are now bound to the overlay's own search commands and do from there what its search buttons do. Adopting the command id rather than the key sequence keeps this working when the user rebinds the command, and running the overlay's own operation searches for the term currently typed, with an empty field doing nothing, just like clicking the buttons. This extends the small set of editor commands the overlay already adopts for find/replace and content assist.
The behavior of those keys inside editors is unchanged. There they keep running the editor's own Find Next and Find Previous, which continue to use the search settings last defined in the overlay, including after moving on to another file or editor.
Added tests cover pressing the keys in the focused search field, executing the commands by id, and the empty search field doing nothing.
Contributes to #2737