FindReplaceOverlay: use search-as-you-type in regex mode eclipse-plat…

…form#1911 Currently, the FindReplaceLogic disables the incremental mode (i.e., search-as-you-type) in case the regex search option is activated. This makes the FindReplaceOverlay, which uses search-as-you-type in all other situations, behave non-intuitively when the regex option is activated. In order to achieve consistent, user-expectable behavior of the find and replace functionality and because of lacking arguments against using search-as-you-type in regex mode, this change makes the regex search mode independent from the incremental mode of the FindReplaceLogic. In consequence, the FindReplaceOverlay always uses search-as-you-type, no matter which search options are activated. In addition, explicit test code for that exceptional behavior is removed or adapted. Contributes to eclipse-platform#2066 Contributes to eclipse-platform#2646 Fixes eclipse-platform#1911
ptziegler · Jan 7, 2025 · 58bb65c · 58bb65c
1 parent 36c6b93
commit 58bb65c
Show file tree

Hide file tree

Showing 5 changed files with 43 additions and 34 deletions.
diff --git a/....ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/FindReplaceLogic.java b/....ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/FindReplaceLogic.java
@@ -132,12 +132,11 @@ private void resetStatus() {
 	@Override
 	public boolean isAvailable(SearchOptions searchOption) {
 		switch (searchOption) {
-		case INCREMENTAL:
-			return !isAvailableAndActive(SearchOptions.REGEX);
 		case REGEX:
 			return isTargetSupportingRegEx;
 		case WHOLE_WORD:
 			return !isAvailableAndActive(SearchOptions.REGEX) && isWord(findString);
+		case INCREMENTAL:
 		case CASE_SENSITIVE:
 		case FORWARD:
 		case GLOBAL:

diff --git a/.../org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/texteditor/FindReplaceDialog.java b/.../org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/texteditor/FindReplaceDialog.java
@@ -589,6 +589,7 @@ public void widgetSelected(SelectionEvent e) {
 					return;
 				}
 				findReplaceLogic.activate(SearchOptions.GLOBAL);
+				updateFindString();
 			}
 
 			@Override
@@ -608,6 +609,7 @@ public void widgetSelected(SelectionEvent e) {
 					return;
 				}
 				findReplaceLogic.deactivate(SearchOptions.GLOBAL);
+				updateFindString();
 			}
 
 			@Override
@@ -650,11 +652,7 @@ private Composite createInputPanel(Composite parent) {
 				ITextEditorActionDefinitionIds.CONTENT_ASSIST_PROPOSALS, new char[0], true);
 		setGridData(fFindField, SWT.FILL, true, SWT.CENTER, false);
 		addDecorationMargin(fFindField);
-		fFindModifyListener = new InputModifyListener(() -> {
-			if (okToUse(fFindField)) {
-				findReplaceLogic.setFindString(fFindField.getText());
-			}
-		});
+		fFindModifyListener = new InputModifyListener(this::updateFindString);
 		fFindField.addModifyListener(fFindModifyListener);
 
 		fReplaceLabel = new Label(panel, SWT.LEFT);
@@ -680,6 +678,12 @@ private Composite createInputPanel(Composite parent) {
 		return panel;
 	}
 
+	private void updateFindString() {
+		if (okToUse(fFindField)) {
+			findReplaceLogic.setFindString(fFindField.getText());
+		}
+	}
+
 	/**
 	 * Creates the functional options part of the options defining section of the
 	 * find replace dialog.
@@ -708,6 +712,7 @@ private Composite createOptionsGroup(Composite parent) {
 			public void widgetSelected(SelectionEvent e) {
 				setupFindReplaceLogic();
 				storeSettings();
+				updateFindString();
 			}
 
 			@Override
@@ -745,6 +750,7 @@ public void widgetDefaultSelected(SelectionEvent e) {
 			public void widgetSelected(SelectionEvent e) {
 				setupFindReplaceLogic();
 				storeSettings();
+				updateFindString();
 			}
 
 			@Override
@@ -770,7 +776,7 @@ public void widgetSelected(SelectionEvent e) {
 				storeSettings();
 				updateButtonState();
 				setContentAssistsEnablement(newState);
-				fIncrementalCheckBox.setEnabled(findReplaceLogic.isAvailable(SearchOptions.INCREMENTAL));
+				updateFindString();
 			}
 		});
 		storeButtonWithMnemonicInMap(fIsRegExCheckBox);
@@ -779,9 +785,9 @@ public void widgetSelected(SelectionEvent e) {
 			@Override
 			public void widgetSelected(SelectionEvent e) {
 				updateButtonState();
+				updateFindString();
 			}
 		});
-		fIncrementalCheckBox.setEnabled(findReplaceLogic.isAvailable(SearchOptions.INCREMENTAL));
 		return panel;
 	}
 

diff --git a/...kbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/FindReplaceUITest.java b/...kbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/FindReplaceUITest.java
@@ -212,17 +212,25 @@ public void testFindWithWholeWordEnabledWithMultipleWords() {
 	@Test
 	public void testRegExSearch() {
 		initializeTextViewerWithFindReplaceUI("abc");
-		dialog.select(SearchOptions.REGEX);
+		dialog.select(SearchOptions.INCREMENTAL);
 		dialog.setFindText("(a|bc)");
+		dialog.select(SearchOptions.REGEX);
 
 		IFindReplaceTarget target= getFindReplaceTarget();
-		dialog.simulateKeyboardInteractionInFindInputField(SWT.CR, false);
 		assertEquals(0, (target.getSelection()).x);
 		assertEquals(1, (target.getSelection()).y);
 
 		dialog.simulateKeyboardInteractionInFindInputField(SWT.CR, false);
 		assertEquals(1, (target.getSelection()).x);
 		assertEquals(2, (target.getSelection()).y);
+
+		dialog.simulateKeyboardInteractionInFindInputField(SWT.CR, false);
+		assertEquals(0, (target.getSelection()).x);
+		assertEquals(1, (target.getSelection()).y);
+
+		dialog.setFindText("b|c");
+		assertEquals(1, (target.getSelection()).x);
+		assertEquals(1, (target.getSelection()).y);
 	}
 
 	@Test

diff --git a/...itor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayTest.java b/...itor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayTest.java
@@ -160,8 +160,7 @@ public void testSearchBackwardsWithRegEx() {
 
 		OverlayAccess dialog= getDialog();
 		dialog.select(SearchOptions.REGEX);
-		dialog.setFindText("text"); // with RegEx enabled, there is no incremental search!
-		dialog.pressSearch(true);
+		dialog.setFindText("text");
 		assertThat(target.getSelection().y, is(4));
 		dialog.pressSearch(true);
 		assertThat(target.getSelection().x, is("text ".length()));

diff --git a/...texteditor.tests/src/org/eclipse/ui/workbench/texteditor/tests/FindReplaceDialogTest.java b/...texteditor.tests/src/org/eclipse/ui/workbench/texteditor/tests/FindReplaceDialogTest.java
@@ -163,18 +163,6 @@ public void testReplaceAndFindAfterInitializingFindWithSelectedString() {
 		assertEquals(4, (target.getSelection()).y);
 	}
 
-	@Test
-	public void testIncrementalSearchOnlyEnabledWhenAllowed() {
-		initializeTextViewerWithFindReplaceUI("text text text");
-		DialogAccess dialog= getDialog();
-
-		dialog.select(SearchOptions.INCREMENTAL);
-		dialog.select(SearchOptions.REGEX);
-
-		dialog.assertSelected(SearchOptions.INCREMENTAL);
-		dialog.assertDisabled(SearchOptions.INCREMENTAL);
-	}
-
 	/*
 	 * Test for https://github.com/eclipse-platform/eclipse.platform.ui/pull/1805#pullrequestreview-1993772378
 	 */
@@ -191,15 +179,6 @@ public void testIncrementalSearchOptionRecoveredCorrectly() {
 		dialog= getDialog();
 		dialog.assertSelected(SearchOptions.INCREMENTAL);
 		dialog.assertEnabled(SearchOptions.INCREMENTAL);
-
-		dialog.select(SearchOptions.REGEX);
-		dialog.assertSelected(SearchOptions.INCREMENTAL);
-		dialog.assertDisabled(SearchOptions.INCREMENTAL);
-
-		reopenFindReplaceUIForTextViewer();
-		dialog= getDialog();
-		dialog.assertSelected(SearchOptions.INCREMENTAL);
-		dialog.assertDisabled(SearchOptions.INCREMENTAL);
 	}
 
 	@Test
@@ -225,4 +204,22 @@ public void testFindWithWholeWordEnabledWithMultipleWordsNotIncremental() {
 		assertEquals(0, (target.getSelection()).x);
 		assertEquals(dialog.getFindText().length(), (target.getSelection()).y);
 	}
+
+	@Test
+	public void testRegExSearch_nonIncremental() {
+		initializeTextViewerWithFindReplaceUI("abc");
+		DialogAccess dialog= getDialog();
+		dialog.setFindText("(a|bc)");
+		dialog.select(SearchOptions.REGEX);
+
+		IFindReplaceTarget target= getFindReplaceTarget();
+		dialog.simulateKeyboardInteractionInFindInputField(SWT.CR, false);
+		assertEquals(0, (target.getSelection()).x);
+		assertEquals(1, (target.getSelection()).y);
+
+		dialog.simulateKeyboardInteractionInFindInputField(SWT.CR, false);
+		assertEquals(1, (target.getSelection()).x);
+		assertEquals(2, (target.getSelection()).y);
+	}
+
 }