From 5b3453d5dfd00be94cbaeb76d164448794fdcecc Mon Sep 17 00:00:00 2001 From: Deepika Udayagiri Date: Tue, 30 Sep 2025 18:37:36 +0530 Subject: [PATCH 1/2] This pr is refactoring find/replace overlay to restore selection when search text is cleared. --- .../findandreplace/FindReplaceLogic.java | 25 ++++- .../findandreplace/FindReplaceLogicTest.java | 103 +++++++++++++++++- 2 files changed, 125 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 76616e5c21d..89b04193129 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 @@ -50,7 +50,7 @@ public class FindReplaceLogic implements IFindReplaceLogic { private IFindReplaceStatus status; private IFindReplaceTarget target; private Point incrementalBaseLocation; - + private Point restoreBaseLocation = new Point(0, 0); private boolean isTargetSupportingRegEx; private boolean isTargetEditable; private final Set searchOptions = new HashSet<>(); @@ -60,6 +60,12 @@ public class FindReplaceLogic implements IFindReplaceLogic { @Override public void setFindString(String findString) { + if (this.findString.isEmpty() && !findString.isEmpty()) { + // User just started a new search after clearing previous search. + if (target != null) { + restoreBaseLocation = target.getSelection(); + } + } this.findString = Objects.requireNonNull(findString); if (isAvailableAndActive(SearchOptions.INCREMENTAL)) { performSearch(true); @@ -324,6 +330,9 @@ public boolean performSearch() { private boolean performSearch(boolean updateFromIncrementalBaseLocation) { resetStatus(); if (findString.isEmpty()) { + if (isAvailableAndActive(SearchOptions.INCREMENTAL)) { + restoreSelectionIfEmpty(); + } return false; } @@ -338,6 +347,20 @@ private boolean performSearch(boolean updateFromIncrementalBaseLocation) { return somethingFound; } + /** + * Restores the original caret/selection position when the search field becomes + * empty. + */ + private void restoreSelectionIfEmpty() { + if (restoreBaseLocation == null) { + return; + } + incrementalBaseLocation = restoreBaseLocation; + if (target instanceof IFindReplaceTargetExtension extension) { + extension.setSelection(restoreBaseLocation.x, restoreBaseLocation.y); + } + } + /** * Replaces all occurrences of the user's findString with the replace string. * Returns the number of replacements that occur. 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 992b4aefb8a..1f0fac2fa8e 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 @@ -826,13 +826,112 @@ public void testResetIncrementalBaseLocation() { findReplaceLogic.activate(SearchOptions.FORWARD); findReplaceLogic.activate(SearchOptions.WRAP); findReplaceLogic.activate(SearchOptions.INCREMENTAL); - findReplaceLogic.setFindString("test"); assertThat(textViewer.getSelectedRange(), is(new Point(0, 4))); textViewer.setSelectedRange(5, 0); findReplaceLogic.resetIncrementalBaseLocation(); findReplaceLogic.performSearch(); assertThat(textViewer.getSelectedRange(), is(new Point(5, 4))); + textViewer.setSelectedRange(7, 0); + findReplaceLogic.resetIncrementalBaseLocation(); + findReplaceLogic.performSearch(); + assertThat(textViewer.getSelectedRange(), is(new Point(10, 4))); + textViewer.setSelectedRange(10, 0); + findReplaceLogic.resetIncrementalBaseLocation(); + findReplaceLogic.performSearch(); + assertThat(textViewer.getSelectedRange(), is(new Point(10, 4))); + // Verify that after clearing the search, caret goes back to last active location + findReplaceLogic.setFindString(""); + findReplaceLogic.performSearch(); + assertThat(textViewer.getSelectedRange(), is(new Point(0, 0))); + } + + @Test + public void testRestoreBaseLocationCapturedOnNewSearch() { + String setupString= "alpha beta gamma"; + TextViewer textViewer= setupTextViewer(setupString); + textViewer.setSelectedRange(6, 0); // caret after "alpha " + + IFindReplaceLogic logic= setupFindReplaceLogicObject(textViewer); + logic.activate(SearchOptions.FORWARD); + logic.activate(SearchOptions.INCREMENTAL); + + // Start a new search + logic.setFindString("gamma"); + assertThat(textViewer.getSelectedRange(), is(new Point(11, 5))); // found "gamma" + + // Now clear the search — caret should return to where it started (6, 0) + logic.setFindString(""); + logic.performSearch(); + assertThat(textViewer.getSelectedRange(), is(new Point(6, 0))); + } + + @Test + public void testCaretRestoredWhenSearchCleared() { + String setupString= "alpha beta gamma"; + TextViewer textViewer= setupTextViewer(setupString); + textViewer.setSelectedRange(0, 0); + + IFindReplaceLogic logic= setupFindReplaceLogicObject(textViewer); + logic.activate(SearchOptions.FORWARD); + logic.activate(SearchOptions.INCREMENTAL); + + logic.setFindString("beta"); + assertThat(textViewer.getSelectedRange(), is(new Point(6, 4))); // found "beta" + + // Clear the search field — should restore caret + logic.setFindString(""); + logic.performSearch(); + + // Expect caret restored to starting location (0,0) + assertThat(textViewer.getSelectedRange(), is(new Point(0, 0))); + } + + @Test + public void testRestoreBaseLocationRefreshedBetweenSessions() { + String setupString= "alpha beta gamma"; + TextViewer textViewer= setupTextViewer(setupString); + IFindReplaceLogic logic= setupFindReplaceLogicObject(textViewer); + logic.activate(SearchOptions.FORWARD); + logic.activate(SearchOptions.INCREMENTAL); + + // --- First search session --- + textViewer.setSelectedRange(0, 0); + logic.setFindString("alpha"); + assertThat(textViewer.getSelectedRange(), is(new Point(0, 5))); + + // Clear the search → caret returns to (0, 0) + logic.setFindString(""); + logic.performSearch(); + assertThat(textViewer.getSelectedRange(), is(new Point(0, 0))); + + // --- Second search session --- + textViewer.setSelectedRange(6, 0); + logic.setFindString("beta"); + assertThat(textViewer.getSelectedRange(), is(new Point(6, 4))); + + // Clear again → caret returns to (6, 0) + logic.setFindString(""); + logic.performSearch(); + assertThat(textViewer.getSelectedRange(), is(new Point(6, 0))); + } + + @Test + public void testNoRestoreOnEmptyFindStringWhenNotIncremental() { + String setupString= "test\ntest\ntest"; + TextViewer textViewer= setupTextViewer(setupString); + textViewer.setSelectedRange(5, 0); // Set initial selection + IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(textViewer); + findReplaceLogic.activate(SearchOptions.FORWARD); + findReplaceLogic.activate(SearchOptions.WRAP); + // Do NOT activate incremental mode + + findReplaceLogic.resetIncrementalBaseLocation(); + findReplaceLogic.setFindString("test"); + assertThat(textViewer.getSelectedRange(), is(new Point(5, 0))); + findReplaceLogic.setFindString(""); // Clear the find string + // Should not restore selection + assertThat(textViewer.getSelectedRange(), is(new Point(5, 0))); // Changes as per pr3379 } @Test @@ -867,7 +966,7 @@ public void testSetFindString_incrementalActive() { assertEquals(new Point(0, 2), findReplaceLogic.getTarget().getSelection()); findReplaceLogic.setFindString(""); // this clears the incremental search, but the "old search" still remains active - assertEquals(new Point(0, 2), findReplaceLogic.getTarget().getSelection()); + assertEquals(new Point(0, 0), findReplaceLogic.getTarget().getSelection()); // after #3379 pr } @Test From 2fee44e2f835a42c2f5f81b0f54a9e037500e5d7 Mon Sep 17 00:00:00 2001 From: Deepika Udayagiri Date: Thu, 6 Nov 2025 22:55:36 +0530 Subject: [PATCH 2/2] Removed the logic around restoreBaseLocation. --- .../findandreplace/FindReplaceLogic.java | 16 ++-- .../findandreplace/FindReplaceLogicTest.java | 84 ++++--------------- 2 files changed, 22 insertions(+), 78 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 89b04193129..79c9188426a 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 @@ -50,7 +50,6 @@ public class FindReplaceLogic implements IFindReplaceLogic { private IFindReplaceStatus status; private IFindReplaceTarget target; private Point incrementalBaseLocation; - private Point restoreBaseLocation = new Point(0, 0); private boolean isTargetSupportingRegEx; private boolean isTargetEditable; private final Set searchOptions = new HashSet<>(); @@ -60,12 +59,6 @@ public class FindReplaceLogic implements IFindReplaceLogic { @Override public void setFindString(String findString) { - if (this.findString.isEmpty() && !findString.isEmpty()) { - // User just started a new search after clearing previous search. - if (target != null) { - restoreBaseLocation = target.getSelection(); - } - } this.findString = Objects.requireNonNull(findString); if (isAvailableAndActive(SearchOptions.INCREMENTAL)) { performSearch(true); @@ -352,12 +345,15 @@ private boolean performSearch(boolean updateFromIncrementalBaseLocation) { * empty. */ private void restoreSelectionIfEmpty() { - if (restoreBaseLocation == null) { + if (!isAvailableAndActive(SearchOptions.INCREMENTAL)) { + System.out.println("Incremental mode not active — skipping restore."); + return; + } + if (incrementalBaseLocation == null) { return; } - incrementalBaseLocation = restoreBaseLocation; if (target instanceof IFindReplaceTargetExtension extension) { - extension.setSelection(restoreBaseLocation.x, restoreBaseLocation.y); + extension.setSelection(incrementalBaseLocation.x, incrementalBaseLocation.y); } } 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 1f0fac2fa8e..69313af5c8f 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 @@ -826,6 +826,7 @@ public void testResetIncrementalBaseLocation() { findReplaceLogic.activate(SearchOptions.FORWARD); findReplaceLogic.activate(SearchOptions.WRAP); findReplaceLogic.activate(SearchOptions.INCREMENTAL); + findReplaceLogic.setFindString("test"); assertThat(textViewer.getSelectedRange(), is(new Point(0, 4))); textViewer.setSelectedRange(5, 0); @@ -843,77 +844,24 @@ public void testResetIncrementalBaseLocation() { // Verify that after clearing the search, caret goes back to last active location findReplaceLogic.setFindString(""); findReplaceLogic.performSearch(); - assertThat(textViewer.getSelectedRange(), is(new Point(0, 0))); - } - - @Test - public void testRestoreBaseLocationCapturedOnNewSearch() { - String setupString= "alpha beta gamma"; - TextViewer textViewer= setupTextViewer(setupString); - textViewer.setSelectedRange(6, 0); // caret after "alpha " - - IFindReplaceLogic logic= setupFindReplaceLogicObject(textViewer); - logic.activate(SearchOptions.FORWARD); - logic.activate(SearchOptions.INCREMENTAL); - - // Start a new search - logic.setFindString("gamma"); - assertThat(textViewer.getSelectedRange(), is(new Point(11, 5))); // found "gamma" - - // Now clear the search — caret should return to where it started (6, 0) - logic.setFindString(""); - logic.performSearch(); - assertThat(textViewer.getSelectedRange(), is(new Point(6, 0))); - } - - @Test - public void testCaretRestoredWhenSearchCleared() { - String setupString= "alpha beta gamma"; - TextViewer textViewer= setupTextViewer(setupString); - textViewer.setSelectedRange(0, 0); - - IFindReplaceLogic logic= setupFindReplaceLogicObject(textViewer); - logic.activate(SearchOptions.FORWARD); - logic.activate(SearchOptions.INCREMENTAL); - - logic.setFindString("beta"); - assertThat(textViewer.getSelectedRange(), is(new Point(6, 4))); // found "beta" - - // Clear the search field — should restore caret - logic.setFindString(""); - logic.performSearch(); - - // Expect caret restored to starting location (0,0) - assertThat(textViewer.getSelectedRange(), is(new Point(0, 0))); + assertThat(textViewer.getSelectedRange(), is(new Point(10, 4))); } @Test - public void testRestoreBaseLocationRefreshedBetweenSessions() { - String setupString= "alpha beta gamma"; + public void testRestoreSelectionOnEmptyFindStringInIncrementalMode() { + String setupString= "test\ntest\ntest"; TextViewer textViewer= setupTextViewer(setupString); - IFindReplaceLogic logic= setupFindReplaceLogicObject(textViewer); - logic.activate(SearchOptions.FORWARD); - logic.activate(SearchOptions.INCREMENTAL); + textViewer.setSelectedRange(5, 0); // Set initial selection + IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(textViewer); + findReplaceLogic.activate(SearchOptions.FORWARD); + findReplaceLogic.activate(SearchOptions.WRAP); + findReplaceLogic.activate(SearchOptions.INCREMENTAL); - // --- First search session --- - textViewer.setSelectedRange(0, 0); - logic.setFindString("alpha"); - assertThat(textViewer.getSelectedRange(), is(new Point(0, 5))); - - // Clear the search → caret returns to (0, 0) - logic.setFindString(""); - logic.performSearch(); - assertThat(textViewer.getSelectedRange(), is(new Point(0, 0))); - - // --- Second search session --- - textViewer.setSelectedRange(6, 0); - logic.setFindString("beta"); - assertThat(textViewer.getSelectedRange(), is(new Point(6, 4))); - - // Clear again → caret returns to (6, 0) - logic.setFindString(""); - logic.performSearch(); - assertThat(textViewer.getSelectedRange(), is(new Point(6, 0))); + findReplaceLogic.resetIncrementalBaseLocation(); // Set base location + findReplaceLogic.setFindString("test"); + assertThat(textViewer.getSelectedRange(), is(new Point(5, 4))); + findReplaceLogic.setFindString(""); // Clear the find string + assertThat(textViewer.getSelectedRange(), is(new Point(5, 0))); // Restored to base location } @Test @@ -928,10 +876,10 @@ public void testNoRestoreOnEmptyFindStringWhenNotIncremental() { findReplaceLogic.resetIncrementalBaseLocation(); findReplaceLogic.setFindString("test"); - assertThat(textViewer.getSelectedRange(), is(new Point(5, 0))); + assertThat(textViewer.getSelectedRange(), is(new Point(5, 0))); // As per pr3379 findReplaceLogic.setFindString(""); // Clear the find string // Should not restore selection - assertThat(textViewer.getSelectedRange(), is(new Point(5, 0))); // Changes as per pr3379 + assertThat(textViewer.getSelectedRange(), is(new Point(5, 0))); // Remains unchanged } @Test