From 127756b4914f5f905cd4baa149f1e57e1f779a2a Mon Sep 17 00:00:00 2001 From: Heiko Klare Date: Mon, 22 Jan 2024 19:57:39 +0100 Subject: [PATCH] Fix replace behavior of FindReplaceLogic/Dialog for RegEx search #1540 The replace functionality of the FindAndReplaceLogic checks whether the current selection fits to the current search string and otherwise performs a find operation first. To this end, it compares the text selection with the search string, even if regex search is used. In regex search mode, this check will usually fail and a find operation is performed even though a correct string is already selected. This results in the replacement of the next instead of the current match. With this change, the check for whether the currently selected string fits to the search string properly considers regex search mode. In consequence, all replace operations consider the current selection for replacement if fitting. It also adds according regression tests. Fixes https://github.com/eclipse-platform/eclipse.platform.ui/issues/1540 --- .../findandreplace/FindReplaceLogic.java | 15 ++- .../findandreplace/FindReplaceLogicTest.java | 112 +++++++++++++++++- 2 files changed, 124 insertions(+), 3 deletions(-) diff --git a/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/FindReplaceLogic.java b/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/FindReplaceLogic.java index 8cf7db6e0e9..ef85098b1d6 100644 --- a/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/FindReplaceLogic.java +++ b/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/FindReplaceLogic.java @@ -18,6 +18,7 @@ import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.regex.Pattern; import java.util.regex.PatternSyntaxException; import org.eclipse.swt.custom.BusyIndicator; @@ -582,13 +583,23 @@ public boolean performReplaceAndFind(String findString, String replaceString) { @Override public boolean performSelectAndReplace(String findString, String replaceString) { resetStatus(); - boolean needToSelectFirst = !getCurrentSelection().equals(findString); - if (needToSelectFirst) { + if (!isFindStringSelected(findString)) { performSearch(findString); } return replaceSelection(replaceString); } + private boolean isFindStringSelected(String findString) { + String selectedString = getCurrentSelection(); + if (isRegExSearchAvailableAndActive()) { + int patternFlags = isActive(SearchOptions.CASE_SENSITIVE) ? 0 : Pattern.CASE_INSENSITIVE; + Pattern pattern = Pattern.compile(findString, patternFlags); + return pattern.matcher(selectedString).find(); + } else { + return getCurrentSelection().equals(findString); + } + } + @Override public void updateTarget(IFindReplaceTarget newTarget, boolean canEditTarget) { resetStatus(); diff --git a/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/FindReplaceLogicTest.java b/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/FindReplaceLogicTest.java index 8a8cf22bf31..b32ed3d10fb 100644 --- a/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/FindReplaceLogicTest.java +++ b/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/FindReplaceLogicTest.java @@ -14,11 +14,13 @@ package org.eclipse.ui.internal.findandreplace; +import static java.lang.System.lineSeparator; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import org.junit.After; import org.junit.Before; @@ -196,6 +198,85 @@ public void testPerformSelectAndReplace() { expectStatusEmpty(findReplaceLogic); } + @Test + public void testPerformSelectAndReplaceRegEx() { + TextViewer textViewer= setupTextViewer("HelloWorld!"); + IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(textViewer); + findReplaceLogic.activate(SearchOptions.FORWARD); + findReplaceLogic.activate(SearchOptions.REGEX); + + findReplaceLogic.performSearch("<(\\w*)>"); + boolean status= findReplaceLogic.performSelectAndReplace("<(\\w*)>", " "); + assertTrue(status); + assertThat(textViewer.getDocument().get(), equalTo("Hello World!")); + expectStatusEmpty(findReplaceLogic); + + status= findReplaceLogic.performSelectAndReplace("<(\\w*)>", " "); + assertTrue(status); + assertThat(textViewer.getDocument().get(), equalTo("Hello World !")); + expectStatusEmpty(findReplaceLogic); + + status= findReplaceLogic.performSelectAndReplace("<(\\w*)>", " "); + assertEquals("Status wasn't correctly returned", false, status); + assertEquals("Text shouldn't have been changed", "Hello World !", textViewer.getDocument().get()); + expectStatusIsCode(findReplaceLogic, FindStatus.StatusCode.NO_MATCH); + } + + @Test + public void testPerformSelectAndReplaceRegExWithNewline() { + TextViewer textViewer= setupTextViewer("Hello" + lineSeparator() + "World" + lineSeparator() + "!"); + IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(textViewer); + findReplaceLogic.activate(SearchOptions.FORWARD); + findReplaceLogic.activate(SearchOptions.REGEX); + + findReplaceLogic.performSearch("o" + lineSeparator() + "W"); + boolean status= findReplaceLogic.performSelectAndReplace("o" + lineSeparator() + "W", "o W"); + assertTrue(status); + assertThat(textViewer.getDocument().get(), equalTo("Hello World" + lineSeparator() + "!")); + expectStatusEmpty(findReplaceLogic); + + status= findReplaceLogic.performSelectAndReplace("d" + lineSeparator() + "!", "d !"); + assertTrue(status); + assertThat(textViewer.getDocument().get(), equalTo("Hello World !")); + expectStatusEmpty(findReplaceLogic); + + status= findReplaceLogic.performSelectAndReplace(lineSeparator(), " "); + assertEquals("Status wasn't correctly returned", false, status); + assertEquals("Text shouldn't have been changed", "Hello World !", textViewer.getDocument().get()); + expectStatusIsCode(findReplaceLogic, FindStatus.StatusCode.NO_MATCH); + } + + @Test + public void testPerformSelectAndReplaceWithConfigurationChanges() { + TextViewer textViewer= setupTextViewer("HelloWorld!!"); + IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(textViewer); + findReplaceLogic.activate(SearchOptions.FORWARD); + findReplaceLogic.activate(SearchOptions.REGEX); + + findReplaceLogic.performSearch("<(\\w*)>"); + boolean status= findReplaceLogic.performSelectAndReplace("<(\\w*)>", " "); + assertTrue(status); + assertThat(textViewer.getDocument().get(), equalTo("Hello World!!")); + expectStatusEmpty(findReplaceLogic); + + findReplaceLogic.deactivate(SearchOptions.REGEX); + status= findReplaceLogic.performSelectAndReplace("", " "); + assertTrue(status); + assertThat(textViewer.getDocument().get(), equalTo("Hello World !!")); + expectStatusEmpty(findReplaceLogic); + + findReplaceLogic.activate(SearchOptions.REGEX); + status= findReplaceLogic.performSelectAndReplace("<(\\w*)>", " "); + assertTrue(status); + assertThat(textViewer.getDocument().get(), equalTo("Hello World ! !")); + expectStatusEmpty(findReplaceLogic); + + status= findReplaceLogic.performSelectAndReplace("<(\\w*)>", " "); + assertEquals("Status wasn't correctly returned", false, status); + assertEquals("Text shouldn't have been changed", "Hello World ! !", textViewer.getDocument().get()); + expectStatusIsCode(findReplaceLogic, FindStatus.StatusCode.NO_MATCH); + } + @Test public void testPerformSelectAndReplaceBackward() { TextViewer textViewer= setupTextViewer("HelloWorld!"); @@ -213,7 +294,6 @@ public void testPerformSelectAndReplaceBackward() { expectStatusEmpty(findReplaceLogic); } - @Test public void testPerformReplaceAndFind() { TextViewer textViewer= setupTextViewer("HelloWorld!"); @@ -237,6 +317,36 @@ public void testPerformReplaceAndFind() { expectStatusIsCode(findReplaceLogic, FindStatus.StatusCode.NO_MATCH); } + @Test + public void testPerformReplaceAndFindRegEx() { + TextViewer textViewer= setupTextViewer("HelloWorld!!"); + IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(textViewer); + findReplaceLogic.activate(SearchOptions.FORWARD); + findReplaceLogic.activate(SearchOptions.REGEX); + + boolean status= findReplaceLogic.performReplaceAndFind("<(\\w*)>", " "); + assertTrue(status); + assertThat(textViewer.getDocument().get(), equalTo("Hello World!!")); + assertThat(findReplaceLogic.getTarget().getSelectionText(), equalTo("")); + expectStatusEmpty(findReplaceLogic); + + status= findReplaceLogic.performReplaceAndFind("<(\\w*)>", " "); + assertTrue(status); + assertThat(textViewer.getDocument().get(), equalTo("Hello World !!")); + assertThat(findReplaceLogic.getTarget().getSelectionText(), equalTo("")); + expectStatusEmpty(findReplaceLogic); + + status= findReplaceLogic.performReplaceAndFind("<(\\w)>", " "); + assertTrue(status); + assertThat(textViewer.getDocument().get(), equalTo("Hello World ! !")); + expectStatusIsCode(findReplaceLogic, FindStatus.StatusCode.NO_MATCH); + + status= findReplaceLogic.performReplaceAndFind("<(\\w*)>", " "); + assertEquals("Status wasn't correctly returned", false, status); + assertEquals("Text shouldn't have been changed", "Hello World ! !", textViewer.getDocument().get()); + expectStatusIsCode(findReplaceLogic, FindStatus.StatusCode.NO_MATCH); + } + @Test public void testPerformSelectAllForward() { TextViewer textViewer= setupTextViewer("AbAbAbAb");