Skip to content

Commit

Permalink
Move activation logic for SearchOptions.INCREMENTAL to FindReplaceLogic
Browse files Browse the repository at this point in the history 
when "incremental search" is selected and "RegEx-Search" is as well,
the dialog/overlay will disable the option for "whole
words". This PR extracts that functionality into the Find/Replace-Logic.

Extracted from a Review-Comment in #1791
  • Loading branch information
@HeikoKlare
Maximilian Wittmer authored and HeikoKlare committed Apr 17, 2024
1 parent d03f4ff commit f391341
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 14 deletions.
8 changes: 6 additions & 2 deletions ....ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/FindReplaceLogic.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,11 +114,15 @@ private void resetStatus() {
status = null;
}

@Override
public boolean isIncrementalSearchAvailable() {
return !isRegExSearchAvailableAndActive();
}

@Override
public boolean isWholeWordSearchAvailable(String findString) {
return !isRegExSearchAvailableAndActive() && isWord(findString);
}

/**
* Tests whether each character in the given string is a letter.
*
Expand Down Expand Up @@ -686,7 +690,7 @@ private void statusLineMessage(String message) {
public void performIncrementalSearch(String searchString) {
resetStatus();

if (isActive(SearchOptions.INCREMENTAL) && !isRegExSearchAvailableAndActive()) {
if (isActive(SearchOptions.INCREMENTAL) && isIncrementalSearchAvailable()) {
if (searchString.equals("") && target != null) { //$NON-NLS-1$
// empty selection at base location
int offset = incrementalBaseLocation.x;
Expand Down
6 changes: 6 additions & 0 deletions ...ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/IFindReplaceLogic.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,12 @@ public interface IFindReplaceLogic {
*/
public boolean isRegExSearchAvailableAndActive();

/**
* {@return whether incremental search may be performed by the
* find/replace-logic based on the currently active options}
*/
public boolean isIncrementalSearchAvailable();

/**
* Updates the search result after the Text was Modified. Used in combination
* with <code>setIncrementalSearch(true)</code>. This method specifically allows
Expand Down
9 changes: 4 additions & 5 deletions .../org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/texteditor/FindReplaceDialog.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -725,11 +725,11 @@ public void widgetDefaultSelected(SelectionEvent e) {
@Override
public void widgetSelected(SelectionEvent e) {
boolean newState = fIsRegExCheckBox.getSelection();
fIncrementalCheckBox.setEnabled(!newState);
setupFindReplaceLogic();
storeSettings();
updateButtonState();
setContentAssistsEnablement(newState);
fIncrementalCheckBox.setEnabled(findReplaceLogic.isIncrementalSearchAvailable());
}
});
storeButtonWithMnemonicInMap(fIsRegExCheckBox);
Expand All @@ -740,7 +740,7 @@ public void widgetSelected(SelectionEvent e) {
updateButtonState();
}
});
fIncrementalCheckBox.setEnabled(!findReplaceLogic.isRegExSearchAvailableAndActive());
fIncrementalCheckBox.setEnabled(findReplaceLogic.isIncrementalSearchAvailable());
return panel;
}

Expand Down Expand Up @@ -1165,7 +1165,7 @@ public void updateTarget(IFindReplaceTarget target, boolean isTargetEditable, bo
}

if (okToUse(fIncrementalCheckBox)) {
fIncrementalCheckBox.setEnabled(!findReplaceLogic.isRegExSearchAvailableAndActive());
fIncrementalCheckBox.setEnabled(findReplaceLogic.isIncrementalSearchAvailable());
}

if (okToUse(fReplaceLabel)) {
Expand Down Expand Up @@ -1264,8 +1264,7 @@ private void setupFindReplaceLogic() {
activateInFindReplaceLogicIf(SearchOptions.CASE_SENSITIVE, fCaseCheckBox.getSelection());
activateInFindReplaceLogicIf(SearchOptions.REGEX, fIsRegExCheckBox.getSelection());
activateInFindReplaceLogicIf(SearchOptions.WHOLE_WORD, fWholeWordCheckBox.getSelection());
activateInFindReplaceLogicIf(SearchOptions.INCREMENTAL,
fIncrementalCheckBox.getEnabled() && fIncrementalCheckBox.getSelection());
activateInFindReplaceLogicIf(SearchOptions.INCREMENTAL, fIncrementalCheckBox.getSelection());
}

/**
Expand Down
68 changes: 61 additions & 7 deletions ...texteditor.tests/src/org/eclipse/ui/workbench/texteditor/tests/FindReplaceDialogTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,8 @@ private class DialogAccess {

private Runnable closeOperation;

DialogAccess(Accessor findReplaceDialogAccessor) {

DialogAccess(Accessor findReplaceDialogAccessor, boolean checkInitialConfiguration) {
findReplaceLogic= (FindReplaceLogic) findReplaceDialogAccessor.get("findReplaceLogic");
findCombo= (Combo) findReplaceDialogAccessor.get("fFindField");
forwardRadioButton= (Button) findReplaceDialogAccessor.get("fForwardRadioButton");
Expand All @@ -111,7 +112,9 @@ private class DialogAccess {
replaceFindButton= (Button) findReplaceDialogAccessor.get("fReplaceFindButton");
shellRetriever= () -> ((Shell) findReplaceDialogAccessor.get("fActiveShell"));
closeOperation= () -> findReplaceDialogAccessor.invoke("close", null);
assertInitialConfiguration();
if (checkInitialConfiguration) {
assertInitialConfiguration();
}
}

void restoreInitialConfiguration() {
Expand Down Expand Up @@ -148,9 +151,13 @@ private void assertInitialConfiguration() {
assertFalse(regExCheckBox.getSelection());
}

void close() {
void closeAndRestore() {
restoreInitialConfiguration();
assertInitialConfiguration();
close();
}

void close() {
closeOperation.run();
}

Expand All @@ -171,7 +178,7 @@ private void openFindReplaceDialog() {
Shell shell= PlatformUI.getWorkbench().getActiveWorkbenchWindow().getShell();
Accessor dialogAccessor= new Accessor("org.eclipse.ui.texteditor.FindReplaceDialog", getClass().getClassLoader(), new Object[] { shell });
dialogAccessor.invoke("create", null);
dialog= new DialogAccess(dialogAccessor);
dialog= new DialogAccess(dialogAccessor, true);
}

private void openTextViewerAndFindReplaceDialog() {
Expand All @@ -190,6 +197,10 @@ private void openTextViewer(String content) {
}

private void openFindReplaceDialogForTextViewer() {
openFindReplaceDialogForTextViewer(true);
}

private void openFindReplaceDialogForTextViewer(boolean checkInitialConfiguration) {
Accessor fFindReplaceAction;
fFindReplaceAction= new Accessor("org.eclipse.ui.texteditor.FindReplaceAction", getClass().getClassLoader(),
new Class[] { ResourceBundle.class, String.class, Shell.class, IFindReplaceTarget.class },
Expand All @@ -203,13 +214,13 @@ private void openFindReplaceDialogForTextViewer() {
Accessor fFindReplaceDialogStubAccessor= new Accessor(fFindReplaceDialogStub, "org.eclipse.ui.texteditor.FindReplaceAction$FindReplaceDialogStub", getClass().getClassLoader());

Accessor dialogAccessor= new Accessor(fFindReplaceDialogStubAccessor.invoke("getDialog", null), "org.eclipse.ui.texteditor.FindReplaceDialog", getClass().getClassLoader());
dialog= new DialogAccess(dialogAccessor);
dialog= new DialogAccess(dialogAccessor, checkInitialConfiguration);
}

@After
public void tearDown() throws Exception {
if (dialog != null) {
dialog.close();
dialog.closeAndRestore();
dialog= null;
}

Expand Down Expand Up @@ -268,10 +279,11 @@ public void testFocusNotChangedWhenEnterPressed() {
simulateEnterInFindInputField(false);
dialog.ensureHasFocusOnGTK();

if (Util.isMac())
if (Util.isMac()) {
/* On the Mac, checkboxes only take focus if "Full Keyboard Access" is enabled in the System Preferences.
* Let's not assume that someone pressed Ctrl+F7 on every test machine... */
return;
}

assertTrue(dialog.findCombo.isFocusControl());

Expand Down Expand Up @@ -414,6 +426,48 @@ public void testActivateWholeWordsAndSearchForTwoWords() {
assertFalse(dialog.wholeWordCheckBox.getEnabled());
}

@Test
public void testIncrementalSearchOnlyEnabledWhenAllowed() {
openTextViewer("text text text");
openFindReplaceDialogForTextViewer();

dialog.incrementalCheckBox.setSelection(true);
select(dialog.regExCheckBox);
runEventQueue();
assertTrue(dialog.incrementalCheckBox.getSelection());
assertFalse(dialog.incrementalCheckBox.getEnabled());
}

/*
* Test for https://github.com/eclipse-platform/eclipse.platform.ui/pull/1805#pullrequestreview-1993772378
*/
@Test
public void testIncrementalSearchOptionRecoveredCorrectly() {
openTextViewer("text text text");
openFindReplaceDialogForTextViewer();

select(dialog.incrementalCheckBox);
assertTrue(dialog.incrementalCheckBox.getSelection());
assertTrue(dialog.incrementalCheckBox.getEnabled());

dialog.close();
openFindReplaceDialogForTextViewer(false);

assertTrue(dialog.incrementalCheckBox.getSelection());
assertTrue(dialog.incrementalCheckBox.getEnabled());

select(dialog.incrementalCheckBox);
select(dialog.regExCheckBox);
assertTrue(dialog.incrementalCheckBox.getSelection());
assertFalse(dialog.incrementalCheckBox.getEnabled());

dialog.close();
openFindReplaceDialogForTextViewer(false);

assertTrue(dialog.incrementalCheckBox.getSelection());
assertFalse(dialog.incrementalCheckBox.getEnabled());
}

private static void select(Button button) {
button.setSelection(true);
button.notifyListeners(SWT.Selection, null);
Expand Down

0 comments on commit f391341

Please sign in to comment.