From 3ebee11fa9be5ef3e1fb488be70f3efed97d6a33 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 | 18 ++- .../findandreplace/FindReplaceLogicTest.java | 124 +++++++++++++++++- 2 files changed, 139 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..fadff8e1f21 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,26 @@ 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 = 0; + if (!isActive(SearchOptions.CASE_SENSITIVE)) { + patternFlags |= Pattern.CASE_INSENSITIVE | Pattern.UNICODE_CASE; + } + 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..b5aa6720efc 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 @@ -19,6 +19,7 @@ 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 +197,98 @@ 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 testPerformSelectAndReplaceRegExWithLinebreaks() { + TextViewer textViewer= setupTextViewer(""" + Hello + World + !"""); + IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(textViewer); + findReplaceLogic.activate(SearchOptions.FORWARD); + findReplaceLogic.activate(SearchOptions.REGEX); + findReplaceLogic.deactivate(SearchOptions.WRAP); + + assertTrue(findReplaceLogic.performSearch("o$")); + boolean status= findReplaceLogic.performSelectAndReplace("o$", "o!"); + assertTrue(status); + assertThat(textViewer.getDocument().get(), equalTo(""" + Hello! + World + !""")); + expectStatusEmpty(findReplaceLogic); + + status= findReplaceLogic.performSelectAndReplace(""" + d + !""", "d!"); + assertTrue(status); + assertThat(textViewer.getDocument().get(), equalTo(""" + Hello! + World!""")); + expectStatusEmpty(findReplaceLogic); + + status= findReplaceLogic.performSelectAndReplace(""" + """, " "); + assertEquals("Status wasn't correctly returned", false, status); + assertEquals("Text shouldn't have been changed", """ + Hello! + World!""", textViewer.getDocument().get()); + } + + @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 +306,6 @@ public void testPerformSelectAndReplaceBackward() { expectStatusEmpty(findReplaceLogic); } - @Test public void testPerformReplaceAndFind() { TextViewer textViewer= setupTextViewer("HelloWorld!"); @@ -237,6 +329,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");