From b9109c8815ba7b1b3e06fa493277e316146f81f9 Mon Sep 17 00:00:00 2001 From: Heiko Klare Date: Thu, 16 Jul 2026 11:19:00 +0200 Subject: [PATCH] Find/replace overlay: move action management to command class The management of actions for the find/replace overlay, the registration of key shortcuts and the initialization of according tooltips with the shortcuts is currently part of the FindReplaceOverlay itself. In addition, the tooltip texts are static throughout the overlay lifecycle and show conflicting shortcuts, such as Enter for both the "search forward" and "replace" button, even though only either of them is registered, depending on which of the input fields has focus. With this change, the responsibility for the action management with the activation and deactivation of their key bindings is moved to the FindReplaceOverlayCommandSupport. This prepares for the provision of the actions as Eclipse handlers in order to allow rebinding shortcuts. Contributes to https://github.com/eclipse-platform/eclipse.platform.ui/issues/2015 --- .../overlay/AccessibleToolItem.java | 18 +++++-- .../overlay/FindReplaceOverlay.java | 50 ++++++++--------- .../overlay/FindReplaceOverlayAction.java | 29 +++++++--- .../FindReplaceOverlayCommandSupport.java | 53 ++++++++++++++++++- 4 files changed, 109 insertions(+), 41 deletions(-) diff --git a/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/AccessibleToolItem.java b/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/AccessibleToolItem.java index 71a665242a2..864b828a9da 100644 --- a/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/AccessibleToolItem.java +++ b/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/AccessibleToolItem.java @@ -23,7 +23,8 @@ class AccessibleToolItem { private final ToolItem toolItem; - private FindReplaceOverlayAction action; + private String baseToolTipText; + AccessibleToolItem(Composite parent, int styleBits) { ToolBar toolbar = new ToolBar(parent, SWT.FLAT | SWT.HORIZONTAL); @@ -44,13 +45,22 @@ void setImage(Image image) { } void setToolTipText(String text) { - toolItem.setToolTipText(action != null ? action.addShortcutHintToTooltipText(text) : text); + this.baseToolTipText = text; + toolItem.setToolTipText(text); } - void setAction(FindReplaceOverlayAction newAction) { - this.action = newAction; + void setAction(FindReplaceOverlayAction action) { setToolTipText(toolItem.getToolTipText()); toolItem.addSelectionListener(SelectionListener.widgetSelectedAdapter(__ -> action.execute())); + action.addShortcutHintListener(hint -> { + if (!toolItem.isDisposed()) { + String tooltipWithHint = baseToolTipText; + if (!hint.isEmpty()) { + tooltipWithHint += " (" + hint + ")"; //$NON-NLS-1$//$NON-NLS-2$ + } + toolItem.setToolTipText(tooltipWithHint); + } + }); } } 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 efb18ffe6da..76a18cb7415 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 @@ -13,7 +13,6 @@ *******************************************************************************/ package org.eclipse.ui.internal.findandreplace.overlay; -import java.util.ArrayList; import java.util.List; import java.util.concurrent.atomic.AtomicReference; @@ -136,10 +135,6 @@ private static final class KeyboardShortcuts { private ToolItem replaceButton; private ToolItem replaceAllButton; - private final List commonActions = new ArrayList<>(); - private final List searchActions = new ArrayList<>(); - private final List replaceActions = new ArrayList<>(); - private Color widgetBackgroundColor; private Color overlayBackgroundColor; private Color normalTextForegroundColor; @@ -154,12 +149,16 @@ private static final class KeyboardShortcuts { private final FocusListener targetActionActivationHandling = new FocusListener() { @Override public void focusGained(FocusEvent e) { - commandSupport.overlayActivated(); + if (e.widget == searchBar.getTextBar()) { + commandSupport.searchBarActivated(); + } else if (replaceBar != null && e.widget == replaceBar.getTextBar()) { + commandSupport.replaceBarActivated(); + } } @Override public void focusLost(FocusEvent e) { - commandSupport.overlayDeactivated(); + commandSupport.searchOrReplaceBarDeactivated(); } }; @@ -415,14 +414,8 @@ private void createContainerAndSearchControls(Composite parent) { } private void initializeSearchShortcutHandlers() { - registerActionShortcutsAtControl(commonActions, searchBar); - registerActionShortcutsAtControl(searchActions, searchBar); - } - - private void registerActionShortcutsAtControl(List actions, Control control) { - for (FindReplaceOverlayAction action : actions) { - FindReplaceShortcutUtil.registerActionShortcutsAtControl(action, control); - } + commandSupport.registerCommonActionShortcutsAtControl(searchBar); + commandSupport.registerSearchActionShortcutsAtControl(searchBar); } /** @@ -498,7 +491,7 @@ private void createReplaceToggle() { FindReplaceOverlayAction replaceToggleAction = new FindReplaceOverlayAction(() -> setReplaceVisible(!replaceBarOpen)); replaceToggleAction.addShortcuts(KeyboardShortcuts.TOGGLE_REPLACE); - commonActions.add(replaceToggleAction); + commandSupport.registerCommonAction(replaceToggleAction); replaceToggle = new AccessibleToolItemBuilder(replaceToggleTools) .withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_OPEN_REPLACE_AREA)) @@ -530,7 +523,7 @@ private void createSearchTools() { FindReplaceOverlayAction searchBackwardAction = new FindReplaceOverlayAction(() -> performSearch(false)); searchBackwardAction.addShortcuts(KeyboardShortcuts.SEARCH_BACKWARD); - searchActions.add(searchBackwardAction); + commandSupport.registerSearchAction(searchBackwardAction); searchBackwardButton = new AccessibleToolItemBuilder(searchTools).withStyleBits(SWT.PUSH) .withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_FIND_PREV)) .withToolTipText(FindReplaceMessages.FindReplaceOverlay_upSearchButton_toolTip) @@ -538,7 +531,7 @@ private void createSearchTools() { FindReplaceOverlayAction searchForwardAction = new FindReplaceOverlayAction(() -> performSearch(true)); searchForwardAction.addShortcuts(KeyboardShortcuts.SEARCH_FORWARD); - searchActions.add(searchForwardAction); + commandSupport.registerSearchAction(searchForwardAction); searchForwardButton = new AccessibleToolItemBuilder(searchTools).withStyleBits(SWT.PUSH) .withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_FIND_NEXT)) .withToolTipText(FindReplaceMessages.FindReplaceOverlay_downSearchButton_toolTip) @@ -547,7 +540,7 @@ private void createSearchTools() { FindReplaceOverlayAction selectAllAction = new FindReplaceOverlayAction(this::performSelectAll); selectAllAction.addShortcuts(KeyboardShortcuts.SEARCH_ALL); - searchActions.add(selectAllAction); + commandSupport.registerSearchAction(selectAllAction); selectAllButton = new AccessibleToolItemBuilder(searchTools).withStyleBits(SWT.PUSH) .withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_SEARCH_ALL)) .withToolTipText(FindReplaceMessages.FindReplaceOverlay_searchAllButton_toolTip) @@ -560,7 +553,7 @@ private void createCloseTools() { FindReplaceOverlayAction closeAction = new FindReplaceOverlayAction(this::close); closeAction.addShortcuts(KeyboardShortcuts.CLOSE); - commonActions.add(closeAction); + commandSupport.registerCommonAction(closeAction); // Close button new AccessibleToolItemBuilder(closeTools).withStyleBits(SWT.PUSH) @@ -573,7 +566,7 @@ private void createAreaSearchButton() { FindReplaceOverlaySearchOptionAction searchInSelectionAction = new FindReplaceOverlaySearchOptionAction(SearchOptions.GLOBAL, findReplaceLogic); searchInSelectionAction.addExecutionListener(this::updateIncrementalSearch); searchInSelectionAction.addShortcuts(KeyboardShortcuts.OPTION_SEARCH_IN_SELECTION); - commonActions.add(searchInSelectionAction); + commandSupport.registerCommonAction(searchInSelectionAction); searchInSelectionButton = new AccessibleToolItemBuilder(searchTools).withStyleBits(SWT.CHECK) .withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_SEARCH_IN_AREA)) .withToolTipText(FindReplaceMessages.FindReplaceOverlay_searchInSelectionButton_toolTip) @@ -585,7 +578,7 @@ private void createRegexSearchButton() { findReplaceLogic); regexAction.addExecutionListener(this::updateIncrementalSearch); regexAction.addShortcuts(KeyboardShortcuts.OPTION_REGEX); - commonActions.add(regexAction); + commandSupport.registerCommonAction(regexAction); regexSearchButton = new AccessibleToolItemBuilder(searchTools).withStyleBits(SWT.CHECK) .withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_FIND_REGEX)) .withToolTipText(FindReplaceMessages.FindReplaceOverlay_regexSearchButton_toolTip) @@ -601,7 +594,7 @@ private void createCaseSensitiveButton() { SearchOptions.CASE_SENSITIVE, findReplaceLogic); caseSensitiveAction.addExecutionListener(this::updateIncrementalSearch); caseSensitiveAction.addShortcuts(KeyboardShortcuts.OPTION_CASE_SENSITIVE); - commonActions.add(caseSensitiveAction); + commandSupport.registerCommonAction(caseSensitiveAction); caseSensitiveSearchButton = new AccessibleToolItemBuilder(searchTools).withStyleBits(SWT.CHECK) .withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_CASE_SENSITIVE)) .withToolTipText(FindReplaceMessages.FindReplaceOverlay_caseSensitiveButton_toolTip) @@ -613,7 +606,7 @@ private void createWholeWordsButton() { SearchOptions.WHOLE_WORD, findReplaceLogic); wholeWordAction.addExecutionListener(this::updateIncrementalSearch); wholeWordAction.addShortcuts(KeyboardShortcuts.OPTION_WHOLE_WORD); - commonActions.add(wholeWordAction); + commandSupport.registerCommonAction(wholeWordAction); wholeWordSearchButton = new AccessibleToolItemBuilder(searchTools).withStyleBits(SWT.CHECK) .withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_WHOLE_WORD)) .withToolTipText(FindReplaceMessages.FindReplaceOverlay_wholeWordsButton_toolTip) @@ -634,7 +627,7 @@ private void createReplaceTools() { performSingleReplace(); }); replaceAction.addShortcuts(KeyboardShortcuts.SEARCH_FORWARD); - replaceActions.add(replaceAction); + commandSupport.registerReplaceAction(replaceAction); replaceButton = new AccessibleToolItemBuilder(replaceTools).withStyleBits(SWT.PUSH) .withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_REPLACE)) .withToolTipText(FindReplaceMessages.FindReplaceOverlay_replaceButton_toolTip) @@ -648,7 +641,7 @@ private void createReplaceTools() { performReplaceAll(); }); replaceAllAction.addShortcuts(KeyboardShortcuts.SEARCH_ALL); - replaceActions.add(replaceAllAction); + commandSupport.registerReplaceAction(replaceAllAction); replaceAllButton = new AccessibleToolItemBuilder(replaceTools).withStyleBits(SWT.PUSH) .withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_REPLACE_ALL)) .withToolTipText(FindReplaceMessages.FindReplaceOverlay_replaceAllButton_toolTip) @@ -754,6 +747,7 @@ private void hideReplace() { return; } customFocusOrder.dispose(); + commandSupport.unregisterReplaceActions(); searchBar.forceFocus(); contentAssistReplaceField = null; replaceBarOpen = false; @@ -776,8 +770,8 @@ private void createReplaceDialog() { } private void initializeReplaceShortcutHandlers() { - registerActionShortcutsAtControl(commonActions, replaceBar); - registerActionShortcutsAtControl(replaceActions, replaceBar); + commandSupport.registerCommonActionShortcutsAtControl(replaceBar); + commandSupport.registerReplaceActionShortcutsAtControl(replaceBar); } private void enableSearchTools(boolean enable) { diff --git a/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayAction.java b/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayAction.java index 28d66801bef..9440853f829 100644 --- a/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayAction.java +++ b/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayAction.java @@ -13,6 +13,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.function.Consumer; import org.eclipse.jface.bindings.keys.KeyStroke; @@ -21,6 +22,8 @@ class FindReplaceOverlayAction { private final List executionListeners = new ArrayList<>(); + private final List> shortcutHintListeners = new ArrayList<>(); + private final List shortcuts = new ArrayList<>(); FindReplaceOverlayAction(Runnable operation) { @@ -48,13 +51,6 @@ boolean executeIfMatching(KeyStroke keystroke) { return false; } - String addShortcutHintToTooltipText(String originalTooltipText) { - if (shortcuts.isEmpty()) { - return originalTooltipText; - } - return originalTooltipText + " (" + shortcuts.get(0).format() + ")"; //$NON-NLS-1$ //$NON-NLS-2$ - } - void addExecutionListener(Runnable listener) { executionListeners.add(listener); } @@ -65,4 +61,23 @@ void notifyExecutionListeners() { } } + void addShortcutHintListener(Consumer listener) { + shortcutHintListeners.add(listener); + } + + void activateKeyBinding() { + shortcutHintListeners.forEach(listener -> listener.accept(getShortcutHint())); + } + + private String getShortcutHint() { + if (shortcuts.isEmpty()) { + return ""; //$NON-NLS-1$ + } + return shortcuts.get(0).format(); // $NON-NLS-1$ + } + + void deactivateKeyBinding() { + shortcutHintListeners.forEach(listener -> listener.accept("")); //$NON-NLS-1$ + } + } diff --git a/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayCommandSupport.java b/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayCommandSupport.java index 50ed200703b..a45b44aeaa4 100644 --- a/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayCommandSupport.java +++ b/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayCommandSupport.java @@ -11,10 +11,13 @@ package org.eclipse.ui.internal.findandreplace.overlay; import java.lang.reflect.Method; +import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; +import org.eclipse.swt.widgets.Control; + import org.eclipse.core.runtime.ILog; import org.eclipse.jface.action.IAction; @@ -37,15 +40,61 @@ class FindReplaceOverlayCommandSupport { private final IWorkbenchPart targetPart; private DeactivateGlobalActionHandlers globalActionHandlerDeaction; + private List commonActions = new ArrayList<>(); + private List searchActions = new ArrayList<>(); + private List replaceActions = new ArrayList<>(); + FindReplaceOverlayCommandSupport(IWorkbenchPart targetPart) { this.targetPart = targetPart; } - void overlayActivated() { + void registerCommonAction(FindReplaceOverlayAction action) { + this.commonActions.add(action); + } + + void registerSearchAction(FindReplaceOverlayAction action) { + this.searchActions.add(action); + } + + void registerReplaceAction(FindReplaceOverlayAction action) { + this.replaceActions.add(action); + } + + void unregisterReplaceActions() { + this.replaceActions.clear(); + } + + void registerCommonActionShortcutsAtControl(Control control) { + commonActions.forEach(action -> FindReplaceShortcutUtil.registerActionShortcutsAtControl(action, control)); + } + + void registerSearchActionShortcutsAtControl(Control control) { + searchActions.forEach(action -> FindReplaceShortcutUtil.registerActionShortcutsAtControl(action, control)); + } + + void registerReplaceActionShortcutsAtControl(Control control) { + replaceActions.forEach(action -> FindReplaceShortcutUtil.registerActionShortcutsAtControl(action, control)); + } + + void searchBarActivated() { + searchOrReplaceBarActivated(); + searchActions.forEach(FindReplaceOverlayAction::activateKeyBinding); + } + + void replaceBarActivated() { + searchOrReplaceBarActivated(); + replaceActions.forEach(FindReplaceOverlayAction::activateKeyBinding); + } + + private void searchOrReplaceBarActivated() { setTextEditorActionsActivated(false); + commonActions.forEach(FindReplaceOverlayAction::activateKeyBinding); } - void overlayDeactivated() { + void searchOrReplaceBarDeactivated() { + commonActions.forEach(FindReplaceOverlayAction::deactivateKeyBinding); + searchActions.forEach(FindReplaceOverlayAction::deactivateKeyBinding); + replaceActions.forEach(FindReplaceOverlayAction::deactivateKeyBinding); setTextEditorActionsActivated(true); }