Skip to content

Commit

Permalink
Correct setting search scope when start of line is selected #1833
Browse files Browse the repository at this point in the history 
When activating the "selected lines" option in the find/replace dialog
with the cursor being placed at the beginning (i.e., index 0) of a line,
no search scope is initialized. For all other potential cursor positions
in a line, the search scope is properly initialized. The reason is a
faulty check that prevents the last line of a multi-line selection to be
included when the selection ends at the beginning of that last line.

This change corrects the behavior to properly initialize a search scope
also the cursor is at the beginning of a line. It improves the according
code and adds several test cases for the search scope initialization
behavior, including one that serves as a regression test for the fixed
issue.

Fixes #1833
  • Loading branch information
@HeikoKlare
HeikoKlare committed Apr 24, 2024
1 parent 83bd999 commit 3c86ecf
Show file tree
Hide file tree
Showing 2 changed files with 114 additions and 26 deletions.
39 changes: 25 additions & 14 deletions bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/TextViewer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -879,29 +879,40 @@ public IRegion getScope() {
@Override
public Point getLineSelection() {
Point point= TextViewer.this.getSelectedRange();
int originalSelectionBeginIndex= point.x;
int originalSelectionLength= point.y;

try {
IDocument document= TextViewer.this.getDocument();

// beginning of line
int line= document.getLineOfOffset(point.x);
int offset= document.getLineOffset(line);
int firstSelectedLineNumber= document.getLineOfOffset(originalSelectionBeginIndex);
int firstSelectedLineBeginIndex= document.getLineOffset(firstSelectedLineNumber);

// end of line
IRegion lastLineInfo= document.getLineInformationOfOffset(point.x + point.y);
int lastLine= document.getLineOfOffset(point.x + point.y);
int length;
if (lastLineInfo.getOffset() == point.x + point.y && lastLine > 0)
length= document.getLineOffset(lastLine - 1) + document.getLineLength(lastLine - 1) - offset;
else
length= lastLineInfo.getOffset() + lastLineInfo.getLength() - offset;

return new Point(offset, length);
int lastSelectedLineNumber= calculateLastSelectedLineNumber(document, originalSelectionBeginIndex, originalSelectionLength);
int lastSelectedLineEndIndex= document.getLineOffset(lastSelectedLineNumber) + document.getLineLength(lastSelectedLineNumber);
int lineSelectionLength= lastSelectedLineEndIndex - firstSelectedLineBeginIndex;

return new Point(firstSelectedLineBeginIndex, lineSelectionLength);
} catch (BadLocationException e) {
// should not happen
return new Point(point.x, 0);
return new Point(originalSelectionBeginIndex, 0);
}
}

/**
* Returns the last line in the given selection range. If the selection ends at the
* beginning of a new line and has a length > 0 (i.e., starts in a preceding line), the line
* at which the selection ends is excluded.
*/
private static int calculateLastSelectedLineNumber(IDocument document, int selectionBeginIndex, int selectionLength) throws BadLocationException {
int selectionEndIndex= selectionBeginIndex + selectionLength;
int lastSelectedLineNumber= document.getLineOfOffset(selectionEndIndex);
int lastSelectedLineBeginIndex= document.getLineOffset(lastSelectedLineNumber);
boolean selectionEndsAtBeginningOfNewLine= lastSelectedLineBeginIndex == selectionEndIndex;
if (selectionEndsAtBeginningOfNewLine && selectionLength > 0) {
lastSelectedLineNumber--;
}
return lastSelectedLineNumber;
}

@Override
Expand Down
101 changes: 89 additions & 12 deletions ...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,6 +14,7 @@

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;
Expand Down Expand Up @@ -150,8 +151,8 @@ public void testPerformReplaceAllForwardRegEx() {

findReplaceLogic.performReplaceAll("[", "");
assertThat(textViewer.getDocument().get(), equalTo("almost@an_email"));
expectStatusIsMessageWithString(findReplaceLogic, "Unclosed character class near index 0" + System.lineSeparator()
+ "[" + System.lineSeparator()
expectStatusIsMessageWithString(findReplaceLogic, "Unclosed character class near index 0" + lineSeparator()
+ "[" + lineSeparator()
+ "^");

}
Expand All @@ -173,8 +174,8 @@ public void testPerformReplaceAllForward() {

findReplaceLogic.performReplaceAll("[", "");
assertThat(textViewer.getDocument().get(), equalTo("almost@an_email"));
expectStatusIsMessageWithString(findReplaceLogic, "Unclosed character class near index 0" + System.lineSeparator()
+ "[" + System.lineSeparator()
expectStatusIsMessageWithString(findReplaceLogic, "Unclosed character class near index 0" + lineSeparator()
+ "[" + lineSeparator()
+ "^");
}

Expand Down Expand Up @@ -430,27 +431,103 @@ public void testSelectWholeWords() {
}

@Test
public void testSelectInSearchScope() {
TextViewer textViewer= setupTextViewer("line1\nline2\nline3");
public void testSelectInSearchScope_withZeroLengthSelection() {
String originalContents= "line1" + lineSeparator() + "line2" + lineSeparator() + "line3";
TextViewer textViewer= setupTextViewer(originalContents);
IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(textViewer);
textViewer.setSelection(new TextSelection(6, 11));

int lineLength= ("line1" + lineSeparator()).length();
textViewer.setSelection(new TextSelection(lineLength + 1, 0));
findReplaceLogic.deactivate(SearchOptions.GLOBAL);
findReplaceLogic.performReplaceAll("l", "");

expectStatusIsReplaceAllWithCount(findReplaceLogic, 1);
assertThat(textViewer.getTextWidget().getText(), is("line1" + lineSeparator() + "ine2" + lineSeparator() + "line3"));
}

@Test
public void testSelectInSearchScope_withZeroLengthSelectionAtBeginningOfLine() {
String originalContents= "line1" + lineSeparator() + "line2" + lineSeparator() + "line3";
TextViewer textViewer= setupTextViewer(originalContents);
IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(textViewer);

int lineLength= ("line1" + lineSeparator()).length();
textViewer.setSelection(new TextSelection(lineLength, 0));
findReplaceLogic.deactivate(SearchOptions.GLOBAL);
findReplaceLogic.performReplaceAll("l", "");

expectStatusIsReplaceAllWithCount(findReplaceLogic, 1);
assertThat(textViewer.getTextWidget().getText(), is("line1" + lineSeparator() + "ine2" + lineSeparator() + "line3"));
}

@Test
public void testSelectInSearchScope_withSingleLineelection() {
String originalContents= "line1" + lineSeparator() + "line2" + lineSeparator() + "line3";
TextViewer textViewer= setupTextViewer(originalContents);
IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(textViewer);

int lineLength= ("line1" + lineSeparator()).length();
textViewer.setSelection(new TextSelection(lineLength + 1, 3));
findReplaceLogic.deactivate(SearchOptions.GLOBAL);
findReplaceLogic.performReplaceAll("l", "");

expectStatusIsReplaceAllWithCount(findReplaceLogic, 1);
assertThat(textViewer.getTextWidget().getText(), is("line1" + lineSeparator() + "ine2" + lineSeparator() + "line3"));
}

@Test
public void testSelectInSearchScope_withMultiLineSelection() {
String originalContents= "line1" + lineSeparator() + "line2" + lineSeparator() + "line3";
TextViewer textViewer= setupTextViewer(originalContents);
IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(textViewer);

int beginningOfSecondLine= originalContents.indexOf("l", 1);
textViewer.setSelection(new TextSelection(beginningOfSecondLine, originalContents.substring(beginningOfSecondLine).length()));
findReplaceLogic.deactivate(SearchOptions.GLOBAL);
findReplaceLogic.performReplaceAll("l", "");

expectStatusIsReplaceAllWithCount(findReplaceLogic, 2);
assertThat(textViewer.getTextWidget().getText(), is("line1" + lineSeparator() + "ine2" + lineSeparator() + "ine3"));
}

findReplaceLogic.activate(SearchOptions.GLOBAL);
textViewer.setSelection(new TextSelection(0, 5));
@Test
public void testSelectInSearchScope_withSelectionEndingAtBeginningOfLine() {
String originalContents= "line1" + lineSeparator() + "line2" + lineSeparator() + "line3";
TextViewer textViewer= setupTextViewer(originalContents);
IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(textViewer);

int beginningOfSecondLine= originalContents.indexOf("l", 1);
int lineLength= ("line1" + lineSeparator()).length();
textViewer.setSelection(new TextSelection(beginningOfSecondLine, lineLength));
findReplaceLogic.deactivate(SearchOptions.GLOBAL);
findReplaceLogic.performReplaceAll("l", "");

findReplaceLogic.performReplaceAll("n", "");
// Selection ending at beginning of new line should not include that line in search scope
expectStatusIsReplaceAllWithCount(findReplaceLogic, 1);
assertThat(textViewer.getTextWidget().getText(), is("line1" + lineSeparator() + "ine2" + lineSeparator() + "line3"));
}

@Test
public void testSelectInSearchScope_changeScope() {
String originalContents= "line1" + lineSeparator() + "line2" + lineSeparator() + "line3";
TextViewer textViewer= setupTextViewer(originalContents);
IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(textViewer);

assertThat(textViewer.getTextWidget().getText(), is("lie1\nine2\nine3"));
textViewer.setSelection(new TextSelection(8, 10));
findReplaceLogic.deactivate(SearchOptions.GLOBAL);
findReplaceLogic.activate(SearchOptions.GLOBAL);
textViewer.setSelection(new TextSelection(0, 2));
findReplaceLogic.deactivate(SearchOptions.GLOBAL);
findReplaceLogic.performReplaceAll("l", "");

expectStatusIsReplaceAllWithCount(findReplaceLogic, 1);
assertThat(textViewer.getTextWidget().getText(), is("ine1" + lineSeparator() + "line2" + lineSeparator() + "line3"));
}

@Test
public void testWholeWordSearchAvailable() {
TextViewer textViewer= setupTextViewer("line1\nline2\nline3");
String originalContents= "line1" + lineSeparator() + "line2" + lineSeparator() + "line3";
TextViewer textViewer= setupTextViewer(originalContents);
IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(textViewer);

assertThat(findReplaceLogic.isWholeWordSearchAvailable("oneword"), is(true));
Expand Down

0 comments on commit 3c86ecf

Please sign in to comment.