Find/Replace overlay: make the overlay the active part while focused - #4293
Open
HeikoKlare wants to merge 2 commits into
Open
Find/Replace overlay: make the overlay the active part while focused#4293HeikoKlare wants to merge 2 commits into
HeikoKlare wants to merge 2 commits into
Conversation
The overlay's existing tests drive it on a bare text viewer, which has no workbench part behind it. Everything about how the overlay and its host editor compete for a keystroke depends on there being a part, so none of it is currently covered: the overlay could start letting the editor act on keys typed into its input fields without any test noticing. Adds an end-to-end test that opens a real editor and reaches the overlay through FindReplaceAction, the way a user does. It observes only where a command ends up taking effect, never how that is arranged, so that it keeps describing what the overlay owes its users regardless of how the arbitration is implemented: keys typed into an input field do not act on the document, Select All applies to the focused field, Enter finds in the search field and replaces in the replace one, window commands such as Save still reach the editor, the editor stays the active part, and it acts on its keys again once the overlay loses focus. Key strokes are delivered by notifying the widget rather than by posting native events, so the display filter the key binding dispatcher installs is exercised without depending on the operating system to deliver anything. Because that path is easy to get wrong in a way that would make the test vacuous, the test also asserts that a key the overlay does bind takes effect. A real editor is needed but the workspace and the IDE are not, so the test contributes a minimal editor of its own rather than opening a file. Extensions from this bundle were previously ignored because it was not marked as a singleton. Assisted-by: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR changes how the Find/Replace overlay integrates with Eclipse’s command/keybinding resolution so that, while a Find/Replace input field is focused, the overlay effectively becomes the active participant in the handler/context chain (without becoming an actual workbench part), preventing editor handlers/keybindings from competing with text entry. It also adds end-to-end regression coverage and records the architectural rationale as an ADR.
Changes:
- Introduce
FindReplaceOverlayContextSupportto activate an overlay-ownedIEclipseContext(shadowingactivePartId) while an overlay field is focused, and to manage overlay keybinding contexts. - Remove the previous reflection-based / action-bar suppression mechanisms from
FindReplaceOverlayCommandSupport, delegating focus-driven context behavior to the new context support. - Add an end-to-end editor-based UI test bundle contribution (via
plugin.xml) and a new ADR documenting the decision and alternatives.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/workbench/texteditor/tests/WorkbenchTextEditorTestSuite.java | Adds the new end-to-end overlay-in-editor test to the suite. |
| tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/TestTextEditorInput.java | New in-memory IEditorInput to support a minimal test editor. |
| tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/TestTextEditor.java | New minimal StatusTextEditor with in-memory document provider and text editor keybinding scope. |
| tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayInEditorTest.java | New end-to-end test validating command routing between overlay fields and host editor. |
| tests/org.eclipse.ui.workbench.texteditor.tests/plugin.xml | Contributes the minimal test editor via the editors extension point. |
| tests/org.eclipse.ui.workbench.texteditor.tests/META-INF/MANIFEST.MF | Marks the test bundle as a singleton to ensure plugin.xml is read. |
| tests/org.eclipse.ui.workbench.texteditor.tests/build.properties | Includes plugin.xml in the built test bundle. |
| docs/adr/0001-find-replace-overlay-key-handling.md | ADR documenting the chosen context-based approach and alternatives/measurements. |
| bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayContextSupport.java | New overlay-owned E4 context + activePartId shadowing + keybinding context switching. |
| bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayCommandSupport.java | Removes reflection/workarounds; delegates focus-driven context behavior to the new context support. |
| bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlay.java | Hooks command support disposal to overlay container disposal; removes focus tracking via IFocusService. |
| bundles/org.eclipse.ui.workbench.texteditor/plugin.xml | Adjusts overlay context parent to avoid inheriting the editor scope. |
| bundles/org.eclipse.ui.workbench.texteditor/META-INF/MANIFEST.MF | Adds required E4 bundles for the new context/model usage. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The overlay's control is parented into the editor's widget tree, so focusing an input field does not change the active part. The editor's key bindings and command handlers therefore stayed in effect and consumed keys meant for the input fields, which was worked around by reflectively disabling the editor's action activation and by nulling out its global action handlers. That workaround reached into private API, applied only to AbstractTextEditor, and covered only the six retargetable actions rather than the full set of conflicting commands. Instead of suppressing the editor's commands one by one, the overlay now takes the editor out of the resolution path while an input field has focus. Both conditions that decide whether one of the editor's handlers wins have to be addressed, because the editor's commands are spread over both: its key binding scopes and part-level handlers are reachable through the editor's part context, while its retargetable actions live in the window context and are guarded by an expression over the active part id. Activating a context that is a sibling of the editor's part context removes the former, and declaring that context's own id as the active part id makes the latter evaluate to false. The context is placed below the window context rather than below the application context, so that window-scoped commands and services remain available. Only the active part id is overridden, not the active part itself, so the overlay and anything invoked from it still operate on the editor, and so do contributions keyed on the active part, the active editor or the selection. With no editor handler left in the resolution path, keys the platform does not otherwise handle reach the native text widget, and the workbench-wide default handlers for cut, copy, paste and select all act on the focused input field, so those also work from the Edit menu again. The overlay's key binding scopes are activated in that same context rather than at the workbench context service. Scopes are collected along the chain between the active leaf and the root, so a scope activated there is active exactly while the context is the active leaf: the overlay's shared scope is activated once and never deactivated, and only the per-field scope is switched as focus moves between the input fields. Both kinds of context are consequently owned by one class, leaving the command support with handler activation and shortcut hints. Since the overlay reports itself as the active part, that is also what scopes its own command handlers, which are activated once at the workbench. The active part id changes at exactly the moments focus enters and leaves an input field, and it says which overlay is focused, so the overlays of different editors no longer need to be told apart by inspecting the focus control's widget hierarchy and no focus tracking has to be registered for the input fields at all. The overlay's shared scope no longer declares the text editor scope as its parent. Parent scopes are resolved when building the set a binding lookup runs against, so that parent would have reintroduced the editor's bindings regardless of the context topology, and it also tied the overlay to text editors. None of this changes what the overlay owes its users, so the end-to-end test added by the preceding commit passes unchanged before and after. Assisted-by: Claude Opus 5 <noreply@anthropic.com>
HeikoKlare
force-pushed
the
findreplace-overlay-own-part-context
branch
from
August 28, 2026 18:20
bd16768 to
4cce1ed
Compare
HeikoKlare
marked this pull request as ready for review
August 28, 2026 18:55
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.
Important
This is based on and contains #4292, which contributes regression tests for exactly the command-related behavior changed here, and it should be merged after it. Until #4292 is merged, its commit shows up in this pull request as well; only the second commit belongs to this change.
Rationale
The Find/Replace overlay is drawn inside the widget tree of the editor it searches in, so focusing one of its input fields does not change the active workbench part. The editor stays active, and its key bindings and command handlers therefore compete for every keystroke with the field the user is typing into. Getting that wrong is very visible: undo applying to the document instead of to the search field, Select All selecting the whole file, Delete editing the document.
So far this was handled by reflectively calling the private
AbstractTextEditor#setActionActivation(false)while an input field has focus, and by nulling out the editor's global action handlers for multi-page editors. Both are workarounds rather than a mechanism: they reach into private API, they only work for editors derived fromAbstractTextEditor, and they cover the six retargetable actions rather than the full set of commands that conflict with typing into a text field.That coupling is also what keeps the overlay tied to one kind of editor. It is currently offered only for
StatusTextEditor, andFindReplaceOverlaybranches on that type in two further places.Concept
Instead of suppressing the editor's commands one at a time, the overlay is given an
IEclipseContextof its own, which it activates while one of its input fields has focus and in which it publishes its own id as the active part id.Two independent conditions decide whether one of the editor's handlers wins a command, and the editor's commands are spread over both. Its key binding scopes and part-level handlers are only reachable through the editor's part context, and activating a context that is a sibling of that context takes them off the active chain. Its retargetable actions are not registered there at all but in the window context, guarded by an expression over the active part id, which publishing the overlay's own id makes evaluate to false.
Together this leaves no editor handler in the command resolution path, so command enablement no longer depends on the editor type, on a list of commands to suppress, or on reflection. Keys that the platform does not otherwise handle reach the native text widget, and the workbench's own default handlers for cut, copy, paste and select all act on the focused input field, so those work from the Edit menu again. The overlay's own key binding scopes are activated in that same context, which also gives them their lifetime.
What this enables
With command enablement independent of the editor type, the remaining
instanceof StatusTextEditorchecks can be removed in follow-up changes, which is what would let the overlay be used with editors other than text editors:Neither is addressed here. This change removes the reason those checks exist.
Alternatives
A number of architecturally quite different approaches were considered, and several were built as proofs of concept and measured against each other, among them suppressing the editor's commands individually, giving the overlay its own shell, and modelling it as a real workbench part. The approach proposed here turned out to be both the simplest and the most flexible: it needs no command list, no reflection, no knowledge of the editor, and it leaves the workbench's notion of the active part, the active editor and the selection untouched, so nothing else in the IDE observes a change while the overlay has focus.
Because the reasoning behind that comparison is not recoverable from the code, the insights and the decision are recorded as an architecture decision record in
docs/adr/0001-find-replace-overlay-key-handling.md, including the alternatives, what was measured about them, and the consequences of the chosen one.🤖 Generated with Claude Code