Skip to content

Find/Replace overlay: handle the editor's find next and find previous - #4355

Open
HeikoKlare wants to merge 1 commit into
eclipse-platform:masterfrom
HeikoKlare:findreplace-overlay-find-next-previous
Open

Find/Replace overlay: handle the editor's find next and find previous#4355
HeikoKlare wants to merge 1 commit into
eclipse-platform:masterfrom
HeikoKlare:findreplace-overlay-find-next-previous

Conversation

@HeikoKlare

Copy link
Copy Markdown
Contributor

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

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Test Results

   858 files  +  286     858 suites  +286   54m 11s ⏱️ + 2m 4s
 8 311 tests +    3   8 068 ✅ +    8  243 💤  -   5  0 ❌ ±0 
20 802 runs  +6 940  20 132 ✅ +6 725  670 💤 +215  0 ❌ ±0 

Results for commit c703b23. ± Comparison against base commit ee7ae1f.

♻️ This comment has been updated with latest results.

@HeikoKlare
HeikoKlare marked this pull request as ready for review September 7, 2026 14:38
@HeikoKlare
HeikoKlare requested a lite review from Copilot September 7, 2026 14:38

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 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.

@HeikoKlare
HeikoKlare force-pushed the findreplace-overlay-find-next-previous branch from 4fa1868 to 4c4d909 Compare September 9, 2026 09:40
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>
@HeikoKlare
HeikoKlare force-pushed the findreplace-overlay-find-next-previous branch from 4c4d909 to c703b23 Compare September 9, 2026 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants