Find/Replace overlay: make the overlay the active part while focused - #4293
Open
HeikoKlare wants to merge 1 commit into
Open
Find/Replace overlay: make the overlay the active part while focused#4293HeikoKlare wants to merge 1 commit into
HeikoKlare wants to merge 1 commit into
Conversation
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.
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
HeikoKlare
force-pushed
the
findreplace-overlay-own-part-context
branch
8 times, most recently
from
September 7, 2026 10:12
f46e596 to
aba96ba
Compare
HeikoKlare
force-pushed
the
findreplace-overlay-own-part-context
branch
2 times, most recently
from
September 7, 2026 16:00
de1e075 to
ef67651
Compare
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. The active leaf is handed back when an input field loses focus only while the overlay still holds it. Losing the focus to another part is not such a case: the workbench activates the part under the mouse before the focus leaves the field, so that part already owns the leaf by then and has to keep it. Handing it to the editor anyway would raise the editor's key binding scope beside the one the part coming up brings, and while both are up every stroke the two scopes have in common is an unresolvable binding conflict, as observed for the zoom commands the console and the text editor both bind. None of this changes what the overlay owes its users, so the existing end-to-end tests pass unchanged before and after. The one behaviour not covered by them is that leaving the overlay for another part reports no binding conflict, which could not be reproduced outside a running IDE: it needs the other part to hold its key binding scope across the switch, which a part whose scope follows the active leaf does not do, so every fixture tried settles on a single scope and never conflicts. It is described in the architecture decision record instead of being pinned by a test that would pass either way. Assisted-by: Claude Opus 5 <noreply@anthropic.com>
HeikoKlare
force-pushed
the
findreplace-overlay-own-part-context
branch
from
September 7, 2026 17:58
ef67651 to
23e5c8a
Compare
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.
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.
With the simple removal of the
StatusTextEditorlimitation, immediately theConsoleview would have the find/replace overlay available: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