Skip to content

Find/replace: centralize UI state notifications in FindReplaceLogic #4176

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
HeikoKlare merged 1 commit into from
Jul 12, 2026
Merged

Find/replace: centralize UI state notifications in FindReplaceLogic #4176

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 44 additions & 7 deletions ....ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/FindReplaceLogic.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import java.util.ArrayList;
import java.util.Collections;
import java.util.EnumMap;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
Expand Down Expand Up @@ -63,26 +64,56 @@ public class FindReplaceLogic implements IFindReplaceLogic {
private String findString = ""; //$NON-NLS-1$
private String replaceString = ""; //$NON-NLS-1$

private final Map<SearchOptions, List<Consumer<Boolean>>> searchOptionListeners = new HashMap<>();
private final Map<SearchOptions, List<Consumer<Boolean>>> activationChangedListeners = new HashMap<>();
private final Map<SearchOptions, List<Consumer<Boolean>>> availabilityChangedListeners = new HashMap<>();

@Override
public void addSearchOptionChangedListener(SearchOptions option, Consumer<Boolean> listener) {
public void addSearchOptionActivationChangedListener(SearchOptions option, Consumer<Boolean> listener) {
Objects.requireNonNull(option);
Objects.requireNonNull(listener);
searchOptionListeners.computeIfAbsent(option, k -> new CopyOnWriteArrayList<>()).add(listener);
activationChangedListeners.computeIfAbsent(option, k -> new CopyOnWriteArrayList<>()).add(listener);
}

private void notifySearchOptionChangedListeners(SearchOptions option, boolean newState) {
List<Consumer<Boolean>> listeners = searchOptionListeners.getOrDefault(option, Collections.emptyList());
@Override
public void addSearchOptionAvailabilityChangedListener(SearchOptions option, Consumer<Boolean> listener) {
Objects.requireNonNull(option);
Objects.requireNonNull(listener);
availabilityChangedListeners.computeIfAbsent(option, k -> new CopyOnWriteArrayList<>()).add(listener);
}

private void notifyActivationChangedListeners(SearchOptions option, boolean newState) {
List<Consumer<Boolean>> listeners = activationChangedListeners.getOrDefault(option, Collections.emptyList());
listeners.forEach(l -> l.accept(newState));
}

private void notifyAvailabilityChangedListeners(SearchOptions option, boolean newAvailability) {
List<Consumer<Boolean>> listeners = availabilityChangedListeners.getOrDefault(option, Collections.emptyList());
listeners.forEach(l -> l.accept(newAvailability));
}

private Map<SearchOptions, Boolean> captureAvailabilities() {
Map<SearchOptions, Boolean> snapshot = new EnumMap<>(SearchOptions.class);
availabilityChangedListeners.keySet().forEach(option -> snapshot.put(option, isAvailable(option)));
return snapshot;
}

private void notifyAvailabilityChangedIfNeeded(Map<SearchOptions, Boolean> oldAvailabilities) {
oldAvailabilities.forEach((option, oldAvailability) -> {
boolean newAvailability = isAvailable(option);
if (oldAvailability != newAvailability) {
notifyAvailabilityChangedListeners(option, newAvailability);
}
});
}

@Override
public void setFindString(String findString) {
Map<SearchOptions, Boolean> oldAvailabilities = captureAvailabilities();
this.findString = Objects.requireNonNull(findString);
if (isAvailableAndActive(SearchOptions.INCREMENTAL)) {
performSearch(true);
}
notifyAvailabilityChangedIfNeeded(oldAvailabilities);
}

@Override
Expand All @@ -92,6 +123,7 @@ public void setReplaceString(String replaceString) {

@Override
public void activate(SearchOptions searchOption) {
Map<SearchOptions, Boolean> oldAvailabilities = captureAvailabilities();
if (!searchOptions.add(searchOption)) {
return;
}
Expand All @@ -110,11 +142,13 @@ public void activate(SearchOptions searchOption) {
default:
break;
}
notifySearchOptionChangedListeners(searchOption, true);
notifyActivationChangedListeners(searchOption, true);
notifyAvailabilityChangedIfNeeded(oldAvailabilities);
}

@Override
public void deactivate(SearchOptions searchOption) {
Map<SearchOptions, Boolean> oldAvailabilities = captureAvailabilities();
if (!searchOptions.remove(searchOption)) {
return;
}
Expand All @@ -126,7 +160,8 @@ public void deactivate(SearchOptions searchOption) {
if (searchOption == SearchOptions.FORWARD && shouldInitIncrementalBaseLocation()) {
resetIncrementalBaseLocation();
}
notifySearchOptionChangedListeners(searchOption, false);
notifyActivationChangedListeners(searchOption, false);
notifyAvailabilityChangedIfNeeded(oldAvailabilities);
}

@Override
Expand Down Expand Up @@ -613,6 +648,7 @@ public void updateTarget(IFindReplaceTarget newTarget, boolean canEditTarget) {
this.isTargetEditable = canEditTarget;

if (this.target != newTarget) {
Map<SearchOptions, Boolean> oldAvailabilities = captureAvailabilities();
if (this.target instanceof IFindReplaceTargetExtension) {
((IFindReplaceTargetExtension) this.target).endSession();
}
Expand All @@ -625,6 +661,7 @@ public void updateTarget(IFindReplaceTarget newTarget, boolean canEditTarget) {

activate(SearchOptions.GLOBAL);
}
notifyAvailabilityChangedIfNeeded(oldAvailabilities);
}

resetIncrementalBaseLocation();
Expand Down
20 changes: 19 additions & 1 deletion ...ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/IFindReplaceLogic.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,25 @@ public interface IFindReplaceLogic {
* @param option the search option to observe
* @param listener the listener to register
*/
public void addSearchOptionChangedListener(SearchOptions option, Consumer<Boolean> listener);
public void addSearchOptionActivationChangedListener(SearchOptions option, Consumer<Boolean> listener);

/**
* Registers a listener that is notified whenever the availability of the given
* search option changes. The listener is called with {@code true} when the
* option becomes available and {@code false} when it becomes unavailable.
*
* <p>
* Availability is determined by {@link #isAvailable(SearchOptions)} and depends
* on the state of the target and the compatibility of active options (e.g.
* {@link SearchOptions#WHOLE_WORD} is unavailable when
* {@link SearchOptions#REGEX} is active or the search string is not a single
* word). Listeners are only notified when availability actually changes.
* </p>
*
* @param option the search option to observe
* @param listener the listener to register
*/
public void addSearchOptionAvailabilityChangedListener(SearchOptions option, Consumer<Boolean> listener);

/**
* @param searchOption option
Expand Down
20 changes: 14 additions & 6 deletions ...teditor/src/org/eclipse/ui/internal/findandreplace/overlay/AccessibleToolItemBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,9 @@ public AccessibleToolItemBuilder withAction(FindReplaceOverlayAction newAction)

/**
* Binds a {@link SearchOptions} value to this item. When built, the item's
* selection state is initialized from the logic's current state and kept in
* sync automatically whenever the option changes.
* selection state is initialized from the logic's current activation state and
* kept in sync automatically. The item's enabled state is also initialized from
* and kept in sync with the option's availability.
*/
public AccessibleToolItemBuilder withSearchOption(SearchOptions option, IFindReplaceLogic logic) {
this.searchOption = option;
Expand All @@ -73,9 +74,10 @@ public AccessibleToolItemBuilder withSearchOption(SearchOptions option, IFindRep

/**
* Like {@link #withSearchOption(SearchOptions, IFindReplaceLogic)} but inverts
* the mapping: the item is selected when the option is <em>inactive</em>.
* Useful for options like {@link SearchOptions#GLOBAL} where a "search in
* selection" button should be selected when searching globally is turned off.
* the selection mapping: the item is selected when the option is
* <em>inactive</em>. Useful for options like {@link SearchOptions#GLOBAL} where
* a "search in selection" button should be selected when searching globally is
* turned off.
*/
public AccessibleToolItemBuilder withInvertedSearchOption(SearchOptions option, IFindReplaceLogic logic) {
this.searchOption = option;
Expand All @@ -99,11 +101,17 @@ public ToolItem build() {
if (searchOption != null) {
boolean initial = findReplaceLogic.isActive(searchOption);
toolItem.setSelection(invertSearchOption ? !initial : initial);
findReplaceLogic.addSearchOptionChangedListener(searchOption, state -> {
findReplaceLogic.addSearchOptionActivationChangedListener(searchOption, state -> {
if (!toolItem.isDisposed()) {
toolItem.setSelection(invertSearchOption ? !state : state);
}
});
toolItem.setEnabled(findReplaceLogic.isAvailable(searchOption));
findReplaceLogic.addSearchOptionAvailabilityChangedListener(searchOption, available -> {
if (!toolItem.isDisposed()) {
toolItem.setEnabled(available);
}
});
}
return toolItem;
}
Expand Down
8 changes: 4 additions & 4 deletions ...nch.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlay.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -646,10 +646,7 @@ private void createAreaSearchButton() {
private void createRegexSearchButton() {
FindReplaceOverlayAction regexAction = new FindReplaceOverlayAction(() -> {
activateInFindReplacerIf(SearchOptions.REGEX, !findReplaceLogic.isActive(SearchOptions.REGEX));
wholeWordSearchButton.setEnabled(findReplaceLogic.isAvailable(SearchOptions.WHOLE_WORD));
updateIncrementalSearch();
updateContentAssistAvailability();
decorate();
});
regexAction.addShortcuts(KeyboardShortcuts.OPTION_REGEX);
commonActions.add(regexAction);
Expand All @@ -658,6 +655,10 @@ private void createRegexSearchButton() {
.withToolTipText(FindReplaceMessages.FindReplaceOverlay_regexSearchButton_toolTip)
.withSearchOption(SearchOptions.REGEX, findReplaceLogic)
.withAction(regexAction).build();
findReplaceLogic.addSearchOptionActivationChangedListener(SearchOptions.REGEX, activated -> {
updateContentAssistAvailability();
decorate();
});
}

private void createCaseSensitiveButton() {
Expand Down Expand Up @@ -744,7 +745,6 @@ private void createSearchBar() {
searchBar.selectAll();
searchBar.addModifyListener(e -> {
updateIncrementalSearch();
wholeWordSearchButton.setEnabled(findReplaceLogic.isAvailable(SearchOptions.WHOLE_WORD));
decorate();
});
searchBar.addFocusListener(new FocusListener() {
Expand Down
78 changes: 72 additions & 6 deletions ...nch.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/FindReplaceLogicTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -987,7 +987,7 @@ public void testIncrementalSearchBackwardNoUpdateIfAlreadyOnWord() {
public void testSearchOptionListenerCalledOnActivation() {
IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(null);
List<Boolean> receivedValues= new ArrayList<>();
findReplaceLogic.addSearchOptionChangedListener(SearchOptions.CASE_SENSITIVE, receivedValues::add);
findReplaceLogic.addSearchOptionActivationChangedListener(SearchOptions.CASE_SENSITIVE, receivedValues::add);

findReplaceLogic.activate(SearchOptions.CASE_SENSITIVE);

Expand All @@ -999,7 +999,7 @@ public void testSearchOptionListenerCalledOnDeactivation() {
IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(null);
findReplaceLogic.activate(SearchOptions.CASE_SENSITIVE);
List<Boolean> receivedValues= new ArrayList<>();
findReplaceLogic.addSearchOptionChangedListener(SearchOptions.CASE_SENSITIVE, receivedValues::add);
findReplaceLogic.addSearchOptionActivationChangedListener(SearchOptions.CASE_SENSITIVE, receivedValues::add);

findReplaceLogic.deactivate(SearchOptions.CASE_SENSITIVE);

Expand All @@ -1011,7 +1011,7 @@ public void testSearchOptionListenerNotCalledWhenAlreadyActive() {
IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(null);
findReplaceLogic.activate(SearchOptions.CASE_SENSITIVE);
List<Boolean> receivedValues= new ArrayList<>();
findReplaceLogic.addSearchOptionChangedListener(SearchOptions.CASE_SENSITIVE, receivedValues::add);
findReplaceLogic.addSearchOptionActivationChangedListener(SearchOptions.CASE_SENSITIVE, receivedValues::add);

findReplaceLogic.activate(SearchOptions.CASE_SENSITIVE);

Expand All @@ -1023,7 +1023,7 @@ public void testSearchOptionListenerNotCalledWhenAlreadyInactive() {
IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(null);
// CASE_SENSITIVE is inactive by default
List<Boolean> receivedValues= new ArrayList<>();
findReplaceLogic.addSearchOptionChangedListener(SearchOptions.CASE_SENSITIVE, receivedValues::add);
findReplaceLogic.addSearchOptionActivationChangedListener(SearchOptions.CASE_SENSITIVE, receivedValues::add);

findReplaceLogic.deactivate(SearchOptions.CASE_SENSITIVE);

Expand All @@ -1035,15 +1035,81 @@ public void testMultipleListenersForSameOptionAllNotified() {
IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(null);
List<Boolean> received1= new ArrayList<>();
List<Boolean> received2= new ArrayList<>();
findReplaceLogic.addSearchOptionChangedListener(SearchOptions.CASE_SENSITIVE, received1::add);
findReplaceLogic.addSearchOptionChangedListener(SearchOptions.CASE_SENSITIVE, received2::add);
findReplaceLogic.addSearchOptionActivationChangedListener(SearchOptions.CASE_SENSITIVE, received1::add);
findReplaceLogic.addSearchOptionActivationChangedListener(SearchOptions.CASE_SENSITIVE, received2::add);

findReplaceLogic.activate(SearchOptions.CASE_SENSITIVE);

assertThat(received1, is(List.of(true)));
assertThat(received2, is(List.of(true)));
}

@Test
public void testAvailabilityListenerCalledWhenRegexActivationMakesWholeWordUnavailable() {
TextViewer textViewer= setupTextViewer("");
IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(textViewer);
findReplaceLogic.setFindString("word");
List<Boolean> receivedValues= new ArrayList<>();
findReplaceLogic.addSearchOptionAvailabilityChangedListener(SearchOptions.WHOLE_WORD, receivedValues::add);

findReplaceLogic.activate(SearchOptions.REGEX);

assertThat(receivedValues, is(List.of(false)));
}

@Test
public void testAvailabilityListenerCalledWhenRegexDeactivationMakesWholeWordAvailable() {
TextViewer textViewer= setupTextViewer("");
IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(textViewer);
findReplaceLogic.setFindString("word");
findReplaceLogic.activate(SearchOptions.REGEX);
List<Boolean> receivedValues= new ArrayList<>();
findReplaceLogic.addSearchOptionAvailabilityChangedListener(SearchOptions.WHOLE_WORD, receivedValues::add);

findReplaceLogic.deactivate(SearchOptions.REGEX);

assertThat(receivedValues, is(List.of(true)));
}

@Test
public void testAvailabilityListenerCalledWhenFindStringBecomesNonWord() {
IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(null);
findReplaceLogic.setFindString("word");
List<Boolean> receivedValues= new ArrayList<>();
findReplaceLogic.addSearchOptionAvailabilityChangedListener(SearchOptions.WHOLE_WORD, receivedValues::add);

findReplaceLogic.setFindString("hello world");

assertThat(receivedValues, is(List.of(false)));
}

@Test
public void testAvailabilityListenerCalledWhenFindStringBecomesWord() {
IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(null);
findReplaceLogic.setFindString("hello world");
List<Boolean> receivedValues= new ArrayList<>();
findReplaceLogic.addSearchOptionAvailabilityChangedListener(SearchOptions.WHOLE_WORD, receivedValues::add);

findReplaceLogic.setFindString("word");

assertThat(receivedValues, is(List.of(true)));
}

@Test
public void testAvailabilityListenerNotCalledWhenAvailabilityUnchanged() {
TextViewer textViewer= setupTextViewer("");
IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(textViewer);
findReplaceLogic.setFindString("hello world");
List<Boolean> receivedValues= new ArrayList<>();
findReplaceLogic.addSearchOptionAvailabilityChangedListener(SearchOptions.WHOLE_WORD, receivedValues::add);

// WHOLE_WORD is already unavailable due to non-word string; activating REGEX
// does not change its availability
findReplaceLogic.activate(SearchOptions.REGEX);

assertTrue(receivedValues.isEmpty());
}

private void expectStatusEmpty(IFindReplaceLogic findReplaceLogic) {
assertThat(findReplaceLogic.getStatus(), instanceOf(NoStatus.class));
}
Expand Down
Loading