From c703b2310c7035d3462aab9b24fd24e429cea50b Mon Sep 17 00:00:00 2001 From: Heiko Klare Date: Mon, 7 Sep 2026 10:06:46 +0200 Subject: [PATCH] Find/Replace overlay: handle the editor's find next and find previous 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 https://github.com/eclipse-platform/eclipse.platform.ui/issues/2737 Assisted-by: Claude Opus 5 --- .../overlay/FindReplaceOverlay.java | 9 ++ .../FindReplaceOverlayInEditorTest.java | 83 +++++++++++++++++++ 2 files changed, 92 insertions(+) diff --git a/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlay.java b/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlay.java index 00cba692e3c..61385c1e391 100644 --- a/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlay.java +++ b/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlay.java @@ -67,6 +67,7 @@ import org.eclipse.ui.texteditor.FindReplaceAction; import org.eclipse.ui.texteditor.IAbstractTextEditorHelpContextIds; import org.eclipse.ui.texteditor.ITextEditorActionDefinitionIds; +import org.eclipse.ui.texteditor.IWorkbenchActionDefinitionIds; import org.eclipse.ui.texteditor.StatusTextEditor; public class FindReplaceOverlay { @@ -520,6 +521,14 @@ private void createSearchTools() { .withAction(searchForwardAction).build(); searchForwardButton.setData(ID_DATA_KEY, "searchForward"); //$NON-NLS-1$ + // Also search on the editor's find next/previous: their bindings stay reachable + // while the overlay has focus, but the editor's handlers for them do not, and + // searching for what is typed in the field is what the user expects there. + commandSupport.registerAction(new FindReplaceOverlayAction(() -> performSearch(true), + IWorkbenchActionDefinitionIds.FIND_NEXT)); + commandSupport.registerAction(new FindReplaceOverlayAction(() -> performSearch(false), + IWorkbenchActionDefinitionIds.FIND_PREVIOUS)); + FindReplaceOverlayAction selectAllAction = new FindReplaceOverlayAction(this::performSelectAll, FindReplaceOverlayCommandSupport.CMD_SELECT_ALL); commandSupport.registerAction(selectAllAction); diff --git a/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayInEditorTest.java b/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayInEditorTest.java index 15ffd328627..23591488abc 100644 --- a/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayInEditorTest.java +++ b/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayInEditorTest.java @@ -33,6 +33,10 @@ import org.eclipse.swt.widgets.Display; import org.eclipse.swt.widgets.Text; +import org.eclipse.jface.bindings.TriggerSequence; +import org.eclipse.jface.bindings.keys.KeySequence; +import org.eclipse.jface.bindings.keys.KeyStroke; + import org.eclipse.jface.text.IDocument; import org.eclipse.jface.text.ITextSelection; @@ -42,8 +46,10 @@ import org.eclipse.ui.handlers.IHandlerService; import org.eclipse.ui.intro.IIntroManager; import org.eclipse.ui.intro.IIntroPart; +import org.eclipse.ui.keys.IBindingService; import org.eclipse.ui.texteditor.FindReplaceAction; +import org.eclipse.ui.texteditor.IWorkbenchActionDefinitionIds; import org.eclipse.ui.texteditor.StatusTextEditor; /** @@ -207,6 +213,67 @@ public void testEnterMeansSomethingElseInEachInputField() throws Exception { "Enter in the replace field must replace the current match in the document"); //$NON-NLS-1$ } + /** + * The editor's Find Next and Find Previous stay meaningful while an input field + * has focus, so they must search on for what is typed there, exactly as the + * overlay's own search buttons do. + */ + @Test + public void testFindNextAndPreviousSearchForTheTypedTerm() { + focusSearchField(); + searchField.setText("word"); //$NON-NLS-1$ + processPendingEvents(); + int firstMatch = editorSelectionOffset(); + + typeKeyBoundTo(IWorkbenchActionDefinitionIds.FIND_NEXT); + + int secondMatch = editorSelectionOffset(); + assertNotEquals(firstMatch, secondMatch, "Find Next must move on to the next match"); //$NON-NLS-1$ + + typeKeyBoundTo(IWorkbenchActionDefinitionIds.FIND_PREVIOUS); + + assertEquals(firstMatch, editorSelectionOffset(), "Find Previous must move back to the previous match"); //$NON-NLS-1$ + assertEquals(CONTENT, documentText(), "finding must not change the document"); //$NON-NLS-1$ + } + + /** + * Find Next is bound in the window scope, so its key keeps resolving while the + * overlay has focus. It must find a handler there, no matter which key the + * command is bound to, which is what executing it by id asserts. + */ + @Test + public void testFindNextIsHandledByTheOverlayWhileItHasFocus() throws Exception { + focusSearchField(); + searchField.setText("word"); //$NON-NLS-1$ + processPendingEvents(); + int firstMatch = editorSelectionOffset(); + + executeCommand(IWorkbenchActionDefinitionIds.FIND_NEXT); + + assertNotEquals(firstMatch, editorSelectionOffset(), + "Find Next must be handled while the overlay has focus"); //$NON-NLS-1$ + } + + /** + * An empty search field is a no-op for the overlay's search buttons, so it has to + * be one for the adopted commands too, rather than falling back to the previously + * searched term the editor's own Find Next would use. + */ + @Test + public void testFindNextDoesNothingWhileTheSearchFieldIsEmpty() { + focusSearchField(); + searchField.setText("word"); //$NON-NLS-1$ + processPendingEvents(); + searchField.setText(""); //$NON-NLS-1$ + processPendingEvents(); + int selectionBefore = editorSelectionOffset(); + + typeKeyBoundTo(IWorkbenchActionDefinitionIds.FIND_NEXT); + + assertEquals(selectionBefore, editorSelectionOffset(), + "Find Next must not search on an empty search field"); //$NON-NLS-1$ + } + /** * Commands of the surrounding window, Save among them, must stay executable * while an input field has focus: only the editor's own are out of place there. @@ -291,6 +358,22 @@ private static void executeCommand(String commandId) throws Exception { processPendingEvents(); } + /** + * Types into the search field whatever key the given command is bound to, rather + * than a sequence spelled out here: which key that is belongs to the key bindings + * and may change there, whereas what this asserts is that the command reaches the + * overlay at all. + */ + private void typeKeyBoundTo(String commandId) { + TriggerSequence binding = PlatformUI.getWorkbench().getService(IBindingService.class) + .getBestActiveBindingFor(commandId); + assertTrue(binding instanceof KeySequence, () -> commandId + " is expected to be bound to a key: " + binding); //$NON-NLS-1$ + KeyStroke[] keyStrokes = ((KeySequence) binding).getKeyStrokes(); + assertEquals(1, keyStrokes.length, + () -> commandId + " is expected to be bound to a single key stroke: " + binding); //$NON-NLS-1$ + type(searchField, keyStrokes[0].getModifierKeys(), keyStrokes[0].getNaturalKey()); + } + /** * Types a key and lets what it triggered run, but without waiting on top: the * stroke itself is carried out while it is delivered, and waiting after every one