Find/replace: move handling of performing incremental search into logic

Currently, consumers of the FindReplaceLogic are responsible for
properly handling incremental searches by executing a search when the
input text changes and by disabling incremental mode when an ordinary
search shall be performed.

With this change, incremental "search-as-you-type" is automatically
performed by the FindReplaceLogic when the incremental search option is
activated. Explicitly performing a search does require to deactivate the
incremental mode anymore.

Author: HeikoKlare

bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/FindReplaceLogic.java

@@ -322,26 +322,24 @@ private boolean prepareTargetForEditing() {
 
 	@Override
 	public boolean performSearch() {
-		return performSearch(true);
+		boolean result = performSearch(false);
+		resetIncrementalBaseLocation();
+		return result;
 	}
 
-	private boolean performSearch(boolean validateSearchOptions) {
+	private boolean performSearch(boolean updateFromIncrementalBaseLocation) {
 		resetStatus();
-
-		if (validateSearchOptions && !isAvailable(SearchOptions.INCREMENTAL) && isActive(SearchOptions.INCREMENTAL)) {
-			return false; // Do nothing if search options are not compatible
+		if (findString.isEmpty()) {
+			return false;
 		}
-		boolean somethingFound = false;
-
-		if (findString != null && !findString.isEmpty()) {
 
-			try {
-				somethingFound = findNext();
-			} catch (PatternSyntaxException ex) {
-				status = new InvalidRegExStatus(ex);
-			} catch (IllegalStateException ex) {
-				// we don't keep state in this dialog
-			}
+		boolean somethingFound = false;
+		try {
+			return somethingFound = findNext(updateFromIncrementalBaseLocation);
+		} catch (PatternSyntaxException ex) {
+			status = new InvalidRegExStatus(ex);
+		} catch (IllegalStateException ex) {
+			// we don't keep state in this dialog
 		}
 		return somethingFound;
 	}
@@ -482,21 +480,13 @@ private Point replaceSelection() {
 		return target.getSelection();
 	}
 
-	/**
-	 * Returns whether the specified search string can be found using the given
-	 * options.
-	 *
-	 * @return <code>true</code> if the search string can be found using the given
-	 *         options
-	 *
-	 */
-	private boolean findNext() {
+	private boolean findNext(boolean updateFromIncrementalBaseLocation) {
 
 		if (target == null) {
 			return false;
 		}
 
-		int findReplacePosition = calculateFindBeginningOffset();
+		int findReplacePosition = calculateFindBeginningOffset(updateFromIncrementalBaseLocation);
 
 		int index = findIndex(findReplacePosition);
 
@@ -515,27 +505,33 @@ private boolean findNext() {
 		return true;
 	}
 
-	private int calculateFindBeginningOffset() {
+	private int calculateFindBeginningOffset(boolean updateFromExistingBaseLocation) {
 		Point r = null;
-		if (isActive(SearchOptions.INCREMENTAL)) {
+		if (updateFromExistingBaseLocation) {
 			r = incrementalBaseLocation;
 		} else {
 			r = target.getSelection();
 		}
 
 		int findReplacePosition = r.x;
-		if (isActive(SearchOptions.FORWARD) && !isActive(SearchOptions.INCREMENTAL)
-				|| isActive(SearchOptions.INCREMENTAL) && !isActive(SearchOptions.FORWARD)) {
+		if (!isActive(SearchOptions.FORWARD)) {
 			findReplacePosition += r.y;
 		}
+		if (!updateFromExistingBaseLocation) {
+			if (isActive(SearchOptions.FORWARD)) {
+				findReplacePosition += r.y;
+			} else {
+				findReplacePosition -= r.y;
+			}
+		}
 		return findReplacePosition;
 	}
 
 	@Override
 	public boolean performReplaceAndFind() {
 		resetStatus();
 		if (performSelectAndReplace()) {
-			performSearch(false);
+			performSearch();
 			return true;
 		}
 		return false;
@@ -545,7 +541,7 @@ public boolean performReplaceAndFind() {
 	public boolean performSelectAndReplace() {
 		resetStatus();
 		if (!isFindStringSelected()) {
-			performSearch(false);
+			performSearch();
 		}
 		if (getStatus().wasSuccessful()) {
 			if (!prepareTargetForEditing()) {

bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/IFindReplaceLogic.java

@@ -26,9 +26,14 @@
 public interface IFindReplaceLogic {
 
 	/**
-	 * Sets the string to be used for searching in find and replace operations.
+	 * Sets the string to be used for searching in find and replace operations. In
+	 * case the {@link SearchOptions#INCREMENTAL} is active, this also triggers an
+	 * incremental search operation, starting from the current incremental base
+	 * location. This also updates the search status accordingly.
 	 *
 	 * @param findString the find string to use, must not be null
+	 *
+	 * @see #getStatus()
 	 */
 	public void setFindString(String findString);
 
@@ -107,8 +112,6 @@ public interface IFindReplaceLogic {
 	 * Locates the current find string in the target. If incremental search is
 	 * activated, the search will be performed starting from an incremental search
 	 * position, which can be reset using {@link #resetIncrementalBaseLocation()}.
-	 * If incremental search is activated and RegEx search is activated, nothing
-	 * happens.
 	 *
 	 * @return Whether the string was found in the target
 	 */

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

@@ -635,7 +635,6 @@ private void createSearchBar() {
 			wholeWordSearchButton.setEnabled(findReplaceLogic.isAvailable(SearchOptions.WHOLE_WORD));
 
 			showUserFeedback(normalTextForegroundColor, true);
-			findReplaceLogic.setFindString(searchBar.getText());
 			updateIncrementalSearch();
 		});
 		searchBar.addFocusListener(new FocusListener() {
@@ -656,7 +655,7 @@ public void focusLost(FocusEvent e) {
 	}
 
 	private void updateIncrementalSearch() {
-		findReplaceLogic.performSearch();
+		findReplaceLogic.setFindString(searchBar.getText());
 		evaluateFindReplaceStatus();
 	}
 
@@ -964,10 +963,8 @@ private void performSingleReplace() {
 	private void performSearch(boolean forward) {
 		boolean oldForwardSearchSetting = findReplaceLogic.isActive(SearchOptions.FORWARD);
 		activateInFindReplacerIf(SearchOptions.FORWARD, forward);
-		findReplaceLogic.deactivate(SearchOptions.INCREMENTAL);
 		findReplaceLogic.performSearch();
 		activateInFindReplacerIf(SearchOptions.FORWARD, oldForwardSearchSetting);
-		findReplaceLogic.activate(SearchOptions.INCREMENTAL);
 		evaluateFindReplaceStatus();
 		searchBar.storeHistory();
 	}

bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/texteditor/FindReplaceDialog.java

@@ -118,8 +118,7 @@ public void shellDeactivated(ShellEvent e) {
 	}
 
 	/**
-	 * Modify listener to update the find logic with the current input and update
-	 * the search result in case of incremental search.
+	 * Modify listener to update the find logic with the current input.
 	 *
 	 * @since 2.0
 	 */
@@ -639,9 +638,6 @@ private Composite createInputPanel(Composite parent) {
 		fFindModifyListener = new InputModifyListener(() -> {
 			if (okToUse(fFindField)) {
 				findReplaceLogic.setFindString(fFindField.getText());
-				if (findReplaceLogic.isActive(SearchOptions.INCREMENTAL)) {
-					findReplaceLogic.performSearch();
-				}
 			}
 		});
 		fFindField.addModifyListener(fFindModifyListener);

tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/FindReplaceLogicTest.java

@@ -81,7 +81,6 @@ private TextViewer setupTextViewer(String contentText) {
 		return textViewer;
 	}
 
-
 	private void setFindAndReplaceString(IFindReplaceLogic findReplaceLogic, String findString, String replaceString) {
 		findReplaceLogic.setFindString(findString);
 		findReplaceLogic.setReplaceString(replaceString);
@@ -430,7 +429,6 @@ public void testPerformSearchAndReplaceRegEx_incrementalActive() {
 		findReplaceLogic.activate(SearchOptions.REGEX);
 
 		findReplaceLogic.setFindString("text");
-		findReplaceLogic.performSearch();
 		textViewer.setSelectedRange(0, 0);
 
 		findReplaceLogic.setReplaceString("");
@@ -769,25 +767,38 @@ public void testResetIncrementalBaseLocation() {
 	}
 
 	@Test
-	public void testPerformIncrementalSearch() {
+	public void testSetFindString_incrementalInactive() {
+		TextViewer textViewer= setupTextViewer("Test Test Test Test");
+		IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(textViewer);
+		findReplaceLogic.activate(SearchOptions.FORWARD);
+
+		assertEquals(new Point(0, 0), findReplaceLogic.getTarget().getSelection());
+		findReplaceLogic.setFindString("Test");
+		assertEquals(new Point(0, 0), findReplaceLogic.getTarget().getSelection());
+	}
+
+	@Test
+	public void testSetFindString_incrementalActive() {
 		TextViewer textViewer= setupTextViewer("Test Test Test Test");
 		IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(textViewer);
 		findReplaceLogic.activate(SearchOptions.INCREMENTAL);
 		findReplaceLogic.activate(SearchOptions.FORWARD);
+		assertEquals(new Point(0, 0), findReplaceLogic.getTarget().getSelection());
 
 		findReplaceLogic.setFindString("Test");
-		findReplaceLogic.performSearch();
-		assertThat(findReplaceLogic.getTarget().getSelection().x, is(0));
-		assertThat(findReplaceLogic.getTarget().getSelection().y, is(4));
+		assertEquals(new Point(0, 4), findReplaceLogic.getTarget().getSelection());
 
-		findReplaceLogic.performSearch(); // incremental search is idempotent
-		assertThat(findReplaceLogic.getTarget().getSelection().x, is(0));
-		assertThat(findReplaceLogic.getTarget().getSelection().y, is(4));
+		findReplaceLogic.setFindString("Test"); // incremental search is idempotent
+		assertEquals(new Point(0, 4), findReplaceLogic.getTarget().getSelection());
 
-		findReplaceLogic.setFindString("");
-		findReplaceLogic.performSearch(); // this clears the incremental search, but the "old search" still remains active
-		assertThat(findReplaceLogic.getTarget().getSelection().x, is(0));
-		assertThat(findReplaceLogic.getTarget().getSelection().y, is(4));
+		findReplaceLogic.setFindString("T");
+		assertEquals(new Point(0, 1), findReplaceLogic.getTarget().getSelection());
+
+		findReplaceLogic.setFindString("Te");
+		assertEquals(new Point(0, 2), findReplaceLogic.getTarget().getSelection());
+
+		findReplaceLogic.setFindString(""); // this clears the incremental search, but the "old search" still remains active
+		assertEquals(new Point(0, 2), findReplaceLogic.getTarget().getSelection());
 	}
 
 	@Test
@@ -798,7 +809,6 @@ public void testIncrementBaseLocationWithRegEx() {
 		findReplaceLogic.activate(SearchOptions.FORWARD);
 
 		findReplaceLogic.setFindString("Test");
-		findReplaceLogic.performSearch();
 		assertThat(findReplaceLogic.getTarget().getSelection(), is(new Point(0, 4)));
 
 		findReplaceLogic.activate(SearchOptions.REGEX);
@@ -812,10 +822,10 @@ public void testIncrementBaseLocationWithRegEx() {
 		assertThat(findReplaceLogic.getTarget().getSelection(), is(new Point(10, 4)));
 		findReplaceLogic.deactivate(SearchOptions.REGEX);
 
-		findReplaceLogic.performSearch();
+		findReplaceLogic.setFindString("Test");
 		assertThat(findReplaceLogic.getTarget().getSelection(), is(new Point(10, 4)));
 		findReplaceLogic.performSearch();
-		assertThat(findReplaceLogic.getTarget().getSelection(), is(new Point(10, 4)));
+		assertThat(findReplaceLogic.getTarget().getSelection(), is(new Point(15, 4)));
 	}
 
 	@Test
@@ -827,7 +837,6 @@ public void testIncrementalSearchNoUpdateIfAlreadyOnWord() {
 		findReplaceLogic.activate(SearchOptions.INCREMENTAL);
 		textViewer.setSelectedRange(0, 0);
 		findReplaceLogic.setFindString("hello");
-		findReplaceLogic.performSearch();
 		assertThat(findReplaceLogic.getTarget().getSelection(), is(new Point(0, 5)));
 	}
 
@@ -840,7 +849,6 @@ public void testIncrementalSearchBackwardNoUpdateIfAlreadyOnWord() {
 		findReplaceLogic.activate(SearchOptions.INCREMENTAL);
 		textViewer.setSelectedRange(5, 0);
 		findReplaceLogic.setFindString("hello");
-		findReplaceLogic.performSearch();
 		assertThat(findReplaceLogic.getTarget().getSelection(), is(new Point(5, 5)));
 	}
 

tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayTest.java

@@ -20,6 +20,8 @@
 
 import org.junit.Test;
 
+import org.eclipse.swt.graphics.Point;
+
 import org.eclipse.core.runtime.preferences.IEclipsePreferences;
 import org.eclipse.core.runtime.preferences.InstanceScope;
 
@@ -88,10 +90,13 @@ public void testIncrementalSearchUpdatesAfterChangingOptions() {
 		initializeTextViewerWithFindReplaceUI("alinee\naLinee\nline\nline");
 		OverlayAccess dialog= getDialog();
 		IFindReplaceTarget target= dialog.getTarget();
-
 		dialog.setFindText("Line");
+		assertThat(dialog.getTarget().getSelectionText(), is("line"));
+		assertEquals(new Point(1,4), dialog.getTarget().getSelection());
+
 		dialog.select(SearchOptions.CASE_SENSITIVE);
 		assertThat(dialog.getTarget().getSelectionText(), is("Line"));
+		assertEquals(new Point(8,4), dialog.getTarget().getSelection());
 
 		dialog.unselect(SearchOptions.CASE_SENSITIVE);
 		assertEquals(1, (target.getSelection()).x);