Skip to content

Commit 84ad5a4

Browse files
HeikoKlareCopilot
HeikoKlare
and
Copilot
committed
Find/Replace overlay: unify action responsibility in FindReplaceOverlayAction
Each overlay button previously had its operation and keyboard shortcut hint managed separately through the builder (withOperation / withShortcuts). This meant the relationship between what a button does and what shortcut triggers it was spread across the call site. A FindReplaceOverlayAction is now created before building the tool item and passed as a single entity via withAction(), making the action the sole source of truth for both the operation and its shortcut bindings. Shortcut registration on input fields iterates the typed action lists (commonActions, searchActions, replaceActions) directly rather than delegating through the toolbar controls, and AccessibleToolBar no longer needs a registerActionShortcutsAtControl method. AccessibleToolItem's action field is initialized as null rather than a no-op dummy; setToolTipText falls back to the plain text for items that receive no action. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 2597f7f commit 84ad5a4

6 files changed

Lines changed: 105 additions & 91 deletions

File tree

‎bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/AccessibleToolBar.java‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -65,11 +65,6 @@ public void setBackground(Color color) {
6565
}
6666
}
6767

68-
void registerActionShortcutsAtControl(Control control) {
69-
for (AccessibleToolItem item : accessibleToolItems) {
70-
item.registerActionShortcutsAtControl(control);
71-
}
72-
}
7368

7469
Control getFirstControl() {
7570
Control[] children = getChildren();

‎bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/AccessibleToolItem.java‎

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,24 +10,20 @@
1010
*******************************************************************************/
1111
package org.eclipse.ui.internal.findandreplace.overlay;
1212

13-
import java.util.List;
14-
1513
import org.eclipse.swt.SWT;
1614
import org.eclipse.swt.events.SelectionListener;
1715
import org.eclipse.swt.graphics.Color;
1816
import org.eclipse.swt.graphics.Image;
1917
import org.eclipse.swt.widgets.Composite;
20-
import org.eclipse.swt.widgets.Control;
2118
import org.eclipse.swt.widgets.ToolBar;
2219
import org.eclipse.swt.widgets.ToolItem;
2320

24-
import org.eclipse.jface.bindings.keys.KeyStroke;
2521
import org.eclipse.jface.layout.GridDataFactory;
2622

2723
class AccessibleToolItem {
2824
private final ToolItem toolItem;
2925

30-
private FindReplaceOverlayAction action = new FindReplaceOverlayAction(null);
26+
private FindReplaceOverlayAction action;
3127

3228
AccessibleToolItem(Composite parent, int styleBits) {
3329
ToolBar toolbar = new ToolBar(parent, SWT.FLAT | SWT.HORIZONTAL);
@@ -48,18 +44,13 @@ void setImage(Image image) {
4844
}
4945

5046
void setToolTipText(String text) {
51-
toolItem.setToolTipText(action.addShortcutHintToTooltipText(text));
47+
toolItem.setToolTipText(action != null ? action.addShortcutHintToTooltipText(text) : text);
5248
}
5349

54-
void setOperation(Runnable operation, List<KeyStroke> shortcuts) {
55-
action = new FindReplaceOverlayAction(operation);
56-
action.addShortcuts(shortcuts);
50+
void setAction(FindReplaceOverlayAction newAction) {
51+
this.action = newAction;
5752
setToolTipText(toolItem.getToolTipText());
58-
toolItem.addSelectionListener(SelectionListener.widgetSelectedAdapter(__ -> operation.run()));
59-
}
60-
61-
void registerActionShortcutsAtControl(Control control) {
62-
FindReplaceShortcutUtil.registerActionShortcutsAtControl(action, control);
53+
toolItem.addSelectionListener(SelectionListener.widgetSelectedAdapter(__ -> action.execute()));
6354
}
6455

6556
}

‎bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/AccessibleToolItemBuilder.java‎

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,12 @@
1313
*******************************************************************************/
1414
package org.eclipse.ui.internal.findandreplace.overlay;
1515

16-
import java.util.Collections;
17-
import java.util.List;
1816
import java.util.Objects;
1917

2018
import org.eclipse.swt.SWT;
2119
import org.eclipse.swt.graphics.Image;
2220
import org.eclipse.swt.widgets.ToolItem;
2321

24-
import org.eclipse.jface.bindings.keys.KeyStroke;
25-
2622
import org.eclipse.ui.internal.findandreplace.IFindReplaceLogic;
2723
import org.eclipse.ui.internal.findandreplace.SearchOptions;
2824

@@ -34,8 +30,7 @@ class AccessibleToolItemBuilder {
3430
private int styleBits = SWT.NONE;
3531
private Image image;
3632
private String toolTipText;
37-
private List<KeyStroke> shortcuts = Collections.emptyList();
38-
private Runnable operation;
33+
private FindReplaceOverlayAction action;
3934
private SearchOptions searchOption;
4035
private IFindReplaceLogic findReplaceLogic;
4136
private boolean invertSearchOption;
@@ -59,13 +54,8 @@ public AccessibleToolItemBuilder withToolTipText(String newToolTipText) {
5954
return this;
6055
}
6156

62-
public AccessibleToolItemBuilder withShortcuts(List<KeyStroke> newShortcuts) {
63-
this.shortcuts = newShortcuts;
64-
return this;
65-
}
66-
67-
public AccessibleToolItemBuilder withOperation(Runnable newOperation) {
68-
this.operation = newOperation;
57+
public AccessibleToolItemBuilder withAction(FindReplaceOverlayAction newAction) {
58+
this.action = newAction;
6959
return this;
7060
}
7161

@@ -102,8 +92,8 @@ public ToolItem build() {
10292
if (toolTipText != null) {
10393
accessibleToolItem.setToolTipText(toolTipText);
10494
}
105-
if (operation != null) {
106-
accessibleToolItem.setOperation(operation, shortcuts);
95+
if (action != null) {
96+
accessibleToolItem.setAction(action);
10797
}
10898
ToolItem toolItem = accessibleToolItem.getToolItem();
10999
if (searchOption != null) {

‎bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlay.java‎

Lines changed: 88 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
package org.eclipse.ui.internal.findandreplace.overlay;
1515

1616
import java.lang.reflect.Method;
17+
import java.util.ArrayList;
1718
import java.util.HashMap;
1819
import java.util.List;
1920
import java.util.Map;
@@ -145,6 +146,10 @@ private static final class KeyboardShortcuts {
145146
private ToolItem replaceButton;
146147
private ToolItem replaceAllButton;
147148

149+
private final List<FindReplaceOverlayAction> commonActions = new ArrayList<>();
150+
private final List<FindReplaceOverlayAction> searchActions = new ArrayList<>();
151+
private final List<FindReplaceOverlayAction> replaceActions = new ArrayList<>();
152+
148153
private Color widgetBackgroundColor;
149154
private Color overlayBackgroundColor;
150155
private Color normalTextForegroundColor;
@@ -470,9 +475,14 @@ private void createContainerAndSearchControls(Composite parent) {
470475
}
471476

472477
private void initializeSearchShortcutHandlers() {
473-
searchTools.registerActionShortcutsAtControl(searchBar);
474-
closeTools.registerActionShortcutsAtControl(searchBar);
475-
replaceToggleTools.registerActionShortcutsAtControl(searchBar);
478+
registerActionShortcutsAtControl(commonActions, searchBar);
479+
registerActionShortcutsAtControl(searchActions, searchBar);
480+
}
481+
482+
private void registerActionShortcutsAtControl(List<FindReplaceOverlayAction> actions, Control control) {
483+
for (FindReplaceOverlayAction action : actions) {
484+
FindReplaceShortcutUtil.registerActionShortcutsAtControl(action, control);
485+
}
476486
}
477487

478488
/**
@@ -546,11 +556,14 @@ private void createReplaceToggle() {
546556
.applyTo(replaceToggleTools);
547557
replaceToggleTools.addMouseListener(MouseListener.mouseDownAdapter(__ -> setReplaceVisible(!replaceBarOpen)));
548558

559+
FindReplaceOverlayAction replaceToggleAction = new FindReplaceOverlayAction(() -> setReplaceVisible(!replaceBarOpen));
560+
replaceToggleAction.addShortcuts(KeyboardShortcuts.TOGGLE_REPLACE);
561+
commonActions.add(replaceToggleAction);
562+
549563
replaceToggle = new AccessibleToolItemBuilder(replaceToggleTools)
550-
.withShortcuts(KeyboardShortcuts.TOGGLE_REPLACE)
551564
.withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_OPEN_REPLACE_AREA))
552565
.withToolTipText(FindReplaceMessages.FindReplaceOverlay_replaceToggle_toolTip)
553-
.withOperation(() -> setReplaceVisible(!replaceBarOpen)).withShortcuts(KeyboardShortcuts.TOGGLE_REPLACE)
566+
.withAction(replaceToggleAction)
554567
.build();
555568
}
556569

@@ -575,90 +588,104 @@ private void createSearchTools() {
575588

576589
searchTools.createToolItem(SWT.SEPARATOR);
577590

591+
FindReplaceOverlayAction searchBackwardAction = new FindReplaceOverlayAction(() -> performSearch(false));
592+
searchBackwardAction.addShortcuts(KeyboardShortcuts.SEARCH_BACKWARD);
593+
searchActions.add(searchBackwardAction);
578594
searchBackwardButton = new AccessibleToolItemBuilder(searchTools).withStyleBits(SWT.PUSH)
579595
.withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_FIND_PREV))
580596
.withToolTipText(FindReplaceMessages.FindReplaceOverlay_upSearchButton_toolTip)
581-
.withOperation(() -> performSearch(false))
582-
.withShortcuts(KeyboardShortcuts.SEARCH_BACKWARD).build();
597+
.withAction(searchBackwardAction).build();
583598

599+
FindReplaceOverlayAction searchForwardAction = new FindReplaceOverlayAction(() -> performSearch(true));
600+
searchForwardAction.addShortcuts(KeyboardShortcuts.SEARCH_FORWARD);
601+
searchActions.add(searchForwardAction);
584602
searchForwardButton = new AccessibleToolItemBuilder(searchTools).withStyleBits(SWT.PUSH)
585603
.withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_FIND_NEXT))
586604
.withToolTipText(FindReplaceMessages.FindReplaceOverlay_downSearchButton_toolTip)
587-
.withOperation(() -> performSearch(true))
588-
.withShortcuts(KeyboardShortcuts.SEARCH_FORWARD).build();
605+
.withAction(searchForwardAction).build();
589606
searchForwardButton.setSelection(true); // by default, search down
590607

608+
FindReplaceOverlayAction selectAllAction = new FindReplaceOverlayAction(this::performSelectAll);
609+
selectAllAction.addShortcuts(KeyboardShortcuts.SEARCH_ALL);
610+
searchActions.add(selectAllAction);
591611
selectAllButton = new AccessibleToolItemBuilder(searchTools).withStyleBits(SWT.PUSH)
592612
.withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_SEARCH_ALL))
593613
.withToolTipText(FindReplaceMessages.FindReplaceOverlay_searchAllButton_toolTip)
594-
.withOperation(this::performSelectAll).withShortcuts(KeyboardShortcuts.SEARCH_ALL).build();
614+
.withAction(selectAllAction).build();
595615
}
596616

597617
private void createCloseTools() {
598618
closeTools = new AccessibleToolBar(searchContainer);
599619
GridDataFactory.fillDefaults().grab(false, true).align(GridData.END, GridData.END).applyTo(closeTools);
600620

621+
FindReplaceOverlayAction closeAction = new FindReplaceOverlayAction(this::close);
622+
closeAction.addShortcuts(KeyboardShortcuts.CLOSE);
623+
commonActions.add(closeAction);
624+
601625
// Close button
602626
new AccessibleToolItemBuilder(closeTools).withStyleBits(SWT.PUSH)
603627
.withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_CLOSE))
604628
.withToolTipText(FindReplaceMessages.FindReplaceOverlay_closeButton_toolTip) //
605-
.withOperation(this::close)
606-
.withShortcuts(KeyboardShortcuts.CLOSE).build();
629+
.withAction(closeAction).build();
607630
}
608631

609632
private void createAreaSearchButton() {
633+
FindReplaceOverlayAction searchInSelectionAction = new FindReplaceOverlayAction(() -> {
634+
activateInFindReplacerIf(SearchOptions.GLOBAL, !findReplaceLogic.isActive(SearchOptions.GLOBAL));
635+
updateIncrementalSearch();
636+
});
637+
searchInSelectionAction.addShortcuts(KeyboardShortcuts.OPTION_SEARCH_IN_SELECTION);
638+
commonActions.add(searchInSelectionAction);
610639
searchInSelectionButton = new AccessibleToolItemBuilder(searchTools).withStyleBits(SWT.CHECK)
611640
.withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_SEARCH_IN_AREA))
612641
.withToolTipText(FindReplaceMessages.FindReplaceOverlay_searchInSelectionButton_toolTip)
613-
.withOperation(() -> {
614-
activateInFindReplacerIf(SearchOptions.GLOBAL, !findReplaceLogic.isActive(SearchOptions.GLOBAL));
615-
updateIncrementalSearch();
616-
})
617-
.withShortcuts(KeyboardShortcuts.OPTION_SEARCH_IN_SELECTION)
618642
.withInvertedSearchOption(SearchOptions.GLOBAL, findReplaceLogic)
619-
.build();
643+
.withAction(searchInSelectionAction).build();
620644
}
621645

622646
private void createRegexSearchButton() {
647+
FindReplaceOverlayAction regexAction = new FindReplaceOverlayAction(() -> {
648+
activateInFindReplacerIf(SearchOptions.REGEX, !findReplaceLogic.isActive(SearchOptions.REGEX));
649+
wholeWordSearchButton.setEnabled(findReplaceLogic.isAvailable(SearchOptions.WHOLE_WORD));
650+
updateIncrementalSearch();
651+
updateContentAssistAvailability();
652+
decorate();
653+
});
654+
regexAction.addShortcuts(KeyboardShortcuts.OPTION_REGEX);
655+
commonActions.add(regexAction);
623656
regexSearchButton = new AccessibleToolItemBuilder(searchTools).withStyleBits(SWT.CHECK)
624657
.withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_FIND_REGEX))
625658
.withToolTipText(FindReplaceMessages.FindReplaceOverlay_regexSearchButton_toolTip)
626-
.withOperation(() -> {
627-
activateInFindReplacerIf(SearchOptions.REGEX, !findReplaceLogic.isActive(SearchOptions.REGEX));
628-
wholeWordSearchButton.setEnabled(findReplaceLogic.isAvailable(SearchOptions.WHOLE_WORD));
629-
updateIncrementalSearch();
630-
updateContentAssistAvailability();
631-
decorate();
632-
})
633-
.withShortcuts(KeyboardShortcuts.OPTION_REGEX)
634659
.withSearchOption(SearchOptions.REGEX, findReplaceLogic)
635-
.build();
660+
.withAction(regexAction).build();
636661
}
637662

638663
private void createCaseSensitiveButton() {
664+
FindReplaceOverlayAction caseSensitiveAction = new FindReplaceOverlayAction(() -> {
665+
activateInFindReplacerIf(SearchOptions.CASE_SENSITIVE, !findReplaceLogic.isActive(SearchOptions.CASE_SENSITIVE));
666+
updateIncrementalSearch();
667+
});
668+
caseSensitiveAction.addShortcuts(KeyboardShortcuts.OPTION_CASE_SENSITIVE);
669+
commonActions.add(caseSensitiveAction);
639670
caseSensitiveSearchButton = new AccessibleToolItemBuilder(searchTools).withStyleBits(SWT.CHECK)
640671
.withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_CASE_SENSITIVE))
641672
.withToolTipText(FindReplaceMessages.FindReplaceOverlay_caseSensitiveButton_toolTip)
642-
.withOperation(() -> {
643-
activateInFindReplacerIf(SearchOptions.CASE_SENSITIVE, !findReplaceLogic.isActive(SearchOptions.CASE_SENSITIVE));
644-
updateIncrementalSearch();
645-
})
646-
.withShortcuts(KeyboardShortcuts.OPTION_CASE_SENSITIVE)
647673
.withSearchOption(SearchOptions.CASE_SENSITIVE, findReplaceLogic)
648-
.build();
674+
.withAction(caseSensitiveAction).build();
649675
}
650676

651677
private void createWholeWordsButton() {
678+
FindReplaceOverlayAction wholeWordAction = new FindReplaceOverlayAction(() -> {
679+
activateInFindReplacerIf(SearchOptions.WHOLE_WORD, !findReplaceLogic.isActive(SearchOptions.WHOLE_WORD));
680+
updateIncrementalSearch();
681+
});
682+
wholeWordAction.addShortcuts(KeyboardShortcuts.OPTION_WHOLE_WORD);
683+
commonActions.add(wholeWordAction);
652684
wholeWordSearchButton = new AccessibleToolItemBuilder(searchTools).withStyleBits(SWT.CHECK)
653685
.withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_WHOLE_WORD))
654686
.withToolTipText(FindReplaceMessages.FindReplaceOverlay_wholeWordsButton_toolTip)
655-
.withOperation(() -> {
656-
activateInFindReplacerIf(SearchOptions.WHOLE_WORD, !findReplaceLogic.isActive(SearchOptions.WHOLE_WORD));
657-
updateIncrementalSearch();
658-
})
659-
.withShortcuts(KeyboardShortcuts.OPTION_WHOLE_WORD)
660687
.withSearchOption(SearchOptions.WHOLE_WORD, findReplaceLogic)
661-
.build();
688+
.withAction(wholeWordAction).build();
662689
}
663690

664691
private void createReplaceTools() {
@@ -667,27 +694,33 @@ private void createReplaceTools() {
667694
replaceTools.createToolItem(SWT.SEPARATOR);
668695

669696
GridDataFactory.fillDefaults().grab(false, true).align(GridData.CENTER, GridData.END).applyTo(replaceTools);
697+
FindReplaceOverlayAction replaceAction = new FindReplaceOverlayAction(() -> {
698+
if (getFindString().isEmpty()) {
699+
applyErrorColor(replaceBar);
700+
return;
701+
}
702+
performSingleReplace();
703+
});
704+
replaceAction.addShortcuts(KeyboardShortcuts.SEARCH_FORWARD);
705+
replaceActions.add(replaceAction);
670706
replaceButton = new AccessibleToolItemBuilder(replaceTools).withStyleBits(SWT.PUSH)
671707
.withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_REPLACE))
672708
.withToolTipText(FindReplaceMessages.FindReplaceOverlay_replaceButton_toolTip)
673-
.withOperation(() -> {
674-
if (getFindString().isEmpty()) {
675-
applyErrorColor(replaceBar);
676-
return;
677-
}
678-
performSingleReplace();
679-
}).withShortcuts(KeyboardShortcuts.SEARCH_FORWARD).build();
709+
.withAction(replaceAction).build();
680710

711+
FindReplaceOverlayAction replaceAllAction = new FindReplaceOverlayAction(() -> {
712+
if (getFindString().isEmpty()) {
713+
applyErrorColor(replaceBar);
714+
return;
715+
}
716+
performReplaceAll();
717+
});
718+
replaceAllAction.addShortcuts(KeyboardShortcuts.SEARCH_ALL);
719+
replaceActions.add(replaceAllAction);
681720
replaceAllButton = new AccessibleToolItemBuilder(replaceTools).withStyleBits(SWT.PUSH)
682721
.withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_REPLACE_ALL))
683722
.withToolTipText(FindReplaceMessages.FindReplaceOverlay_replaceAllButton_toolTip)
684-
.withOperation(() -> {
685-
if (getFindString().isEmpty()) {
686-
applyErrorColor(replaceBar);
687-
return;
688-
}
689-
performReplaceAll();
690-
}).withShortcuts(KeyboardShortcuts.SEARCH_ALL).build();
723+
.withAction(replaceAllAction).build();
691724
}
692725

693726
private ContentAssistCommandAdapter createContentAssistField(HistoryTextWrapper control, boolean isFind) {
@@ -812,9 +845,8 @@ private void createReplaceDialog() {
812845
}
813846

814847
private void initializeReplaceShortcutHandlers() {
815-
replaceTools.registerActionShortcutsAtControl(replaceBar);
816-
closeTools.registerActionShortcutsAtControl(replaceBar);
817-
replaceToggleTools.registerActionShortcutsAtControl(replaceBar);
848+
registerActionShortcutsAtControl(commonActions, replaceBar);
849+
registerActionShortcutsAtControl(replaceActions, replaceBar);
818850
}
819851

820852
private void enableSearchTools(boolean enable) {

‎bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayAction.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
package org.eclipse.ui.internal.findandreplace.overlay;
1212

1313
import java.util.ArrayList;
14+
import java.util.Collections;
1415
import java.util.List;
1516

1617
import org.eclipse.jface.bindings.keys.KeyStroke;
@@ -32,6 +33,10 @@ void execute() {
3233
operation.run();
3334
}
3435

36+
List<KeyStroke> getShortcuts() {
37+
return Collections.unmodifiableList(shortcuts);
38+
}
39+
3540
boolean executeIfMatching(KeyStroke keystroke) {
3641
if (shortcuts.stream().anyMatch(keystroke::equals)) {
3742
execute();

‎bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/HistoryTextWrapper.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,11 @@ public HistoryTextWrapper(HistoryStore history, Composite parent, int style) {
5757
textBar = new Text(this, style);
5858
GridDataFactory.fillDefaults().grab(true, true).align(GridData.FILL, GridData.CENTER).applyTo(textBar);
5959
tools = new AccessibleToolBar(this);
60+
FindReplaceOverlayAction action = new FindReplaceOverlayAction(this::createHistoryMenuDropdown);
6061
dropDown = new AccessibleToolItemBuilder(tools).withStyleBits(SWT.PUSH)
6162
.withToolTipText(FindReplaceMessages.FindReplaceOverlay_searchHistory_toolTip)
6263
.withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_OPEN_HISTORY))
63-
.withOperation(this::createHistoryMenuDropdown)
64+
.withAction(action)
6465
.build();
6566

6667
listenForKeyboardHistoryNavigation();

0 commit comments

Comments
 (0)