Skip to content

Commit

Permalink
Fix replace behavior of FindReplaceLogic/Dialog for RegEx search #1540
Browse files Browse the repository at this point in the history 
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 #1540
  • Loading branch information
@HeikoKlare
HeikoKlare committed Jan 29, 2024
1 parent ce26ac0 commit 127756b
Show file tree
Hide file tree
Showing 2 changed files with 124 additions and 3 deletions.
15 changes: 13 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 @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
112 changes: 111 additions & 1 deletion ...nch.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/FindReplaceLogicTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -196,6 +198,85 @@ public void testPerformSelectAndReplace() {
expectStatusEmpty(findReplaceLogic);
}

@Test
public void testPerformSelectAndReplaceRegEx() {
TextViewer textViewer= setupTextViewer("Hello<replace>World<replace>!");
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<replace>!"));
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("Hello<replace>World<replace>!<replace>!");
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<replace>!<replace>!"));
expectStatusEmpty(findReplaceLogic);

findReplaceLogic.deactivate(SearchOptions.REGEX);
status= findReplaceLogic.performSelectAndReplace("<replace>", " ");
assertTrue(status);
assertThat(textViewer.getDocument().get(), equalTo("Hello World !<replace>!"));
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("Hello<replace>World<replace>!");
Expand All @@ -213,7 +294,6 @@ public void testPerformSelectAndReplaceBackward() {
expectStatusEmpty(findReplaceLogic);
}


@Test
public void testPerformReplaceAndFind() {
TextViewer textViewer= setupTextViewer("Hello<replace>World<replace>!");
Expand All @@ -237,6 +317,36 @@ public void testPerformReplaceAndFind() {
expectStatusIsCode(findReplaceLogic, FindStatus.StatusCode.NO_MATCH);
}

@Test
public void testPerformReplaceAndFindRegEx() {
TextViewer textViewer= setupTextViewer("Hello<replace>World<replace>!<r>!");
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<replace>!<r>!"));
assertThat(findReplaceLogic.getTarget().getSelectionText(), equalTo("<replace>"));
expectStatusEmpty(findReplaceLogic);

status= findReplaceLogic.performReplaceAndFind("<(\\w*)>", " ");
assertTrue(status);
assertThat(textViewer.getDocument().get(), equalTo("Hello World !<r>!"));
assertThat(findReplaceLogic.getTarget().getSelectionText(), equalTo("<r>"));
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");
Expand Down

0 comments on commit 127756b

Please sign in to comment.