From ef617153f0f19d3fba071fffe2e930107e670e3f Mon Sep 17 00:00:00 2001 From: Heiko Klare Date: Thu, 18 Jan 2024 13:08:46 +0100 Subject: [PATCH] Make "Select All" in FindReplaceDialog work for read-only targets #1029 The FindReplaceDialog does currently not find any results when performing a "Select All" operation for read-only targets, such as a console view. The reason is that the initial implementation reused "Replace All" functionality, including the check for the target of being editable. While this is reasonable for replace operations, it is not for select-only operations. This change enables "Select All" functionality for read-only targets. It refactors the "Select All" and "Replace All" functionalities to reuse their common logic why adapting both for their specific purpose. It also adds a regression test for performing "Select All" operations on read-only targets. Fixes https://github.com/eclipse-platform/eclipse.platform.ui/issues/1029 --- .../findandreplace/FindReplaceLogic.java | 107 ++++++++---------- .../findandreplace/FindReplaceLogicTest.java | 9 ++ 2 files changed, 57 insertions(+), 59 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 ae03912e9d1..8cf7db6e0e9 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 @@ -398,79 +398,68 @@ private boolean performSearch(boolean mustInitIncrementalBaseLocation, String fi * */ private int replaceAll(String findString, String replaceString) { - boolean oldSearchDirectionIsForward = isActive(SearchOptions.FORWARD); - activate(SearchOptions.FORWARD); // since we operate on the whole text, we can just start from top left and work - // forward - regardless of internal settings - int replaceCount = 0; - int findReplacePosition = 0; - - findReplacePosition = 0; - if (!prepareTargetForEditing()) { - return replaceCount; - } - - if (target instanceof IFindReplaceTargetExtension) { - ((IFindReplaceTargetExtension) target).setReplaceAllMode(true); + return 0; } - try { - int index = 0; - while (index != -1) { - index = findAndSelect(findReplacePosition, findString); - if (index != -1) { // substring not contained from current position - Point selection = replaceSelection(replaceString, isRegExSearchAvailableAndActive()); - replaceCount++; - findReplacePosition = selection.x + selection.y; + List replacements = new ArrayList<>(); + executeInForwardMode(() -> { + executeWithReplaceAllEnabled(() -> { + Point currentSelection = new Point(0, 0); + while (findAndSelect(currentSelection.x + currentSelection.y, findString) != -1) { + currentSelection = replaceSelection(replaceString, isRegExSearchAvailableAndActive()); + replacements.add(currentSelection); } - } - } finally { - if (target instanceof IFindReplaceTargetExtension) { - ((IFindReplaceTargetExtension) target).setReplaceAllMode(false); - } - if (!oldSearchDirectionIsForward) { + }); + }); + return replacements.size(); + } + + private void executeInForwardMode(Runnable runnable) { + if (isActive(SearchOptions.FORWARD)) { + runnable.run(); + } else { + activate(SearchOptions.FORWARD); + try { + runnable.run(); + } finally { deactivate(SearchOptions.FORWARD); } } + } - return replaceCount; + private void executeWithReplaceAllEnabled(Runnable runnable) { + if (target instanceof IFindReplaceTargetExtension selectableTarget) { + selectableTarget.setReplaceAllMode(true); + try { + runnable.run(); + } finally { + selectableTarget.setReplaceAllMode(false); + } + } else { + runnable.run(); + } } /** - * @param findString the String to select as part of the search - * @return The amount of selected Elements + * @param findString the string to select as part of the search + * @return the number of selected elements */ private int selectAll(String findString) { - boolean oldSearchDirectionIsForward = isActive(SearchOptions.FORWARD); - activate(SearchOptions.FORWARD); // since we operate on the whole text, we can just start from top left and work - // forward - regardless of internal settings - int selectCount = 0; - int position = 0; - - if (!prepareTargetForEditing()) { - return selectCount; - } - - List selectedRegions = new ArrayList<>(); - int index = 0; - do { - index = findAndSelect(position, findString); - if (index != -1) { // substring not contained from current position - Point selection = target.getSelection(); - selectedRegions.add(new Region(selection.x, selection.y)); - selectCount++; - position = selection.x + selection.y; + List selections = new ArrayList<>(); + executeInForwardMode(() -> { + Point currentSeletion = new Point(0, 0); + while (findAndSelect(currentSeletion.x + currentSeletion.y, findString) != -1) { + currentSeletion = target.getSelection(); + selections.add(currentSeletion); } - } while (index != -1); - if (target instanceof IFindReplaceTargetExtension4) { - ((IFindReplaceTargetExtension4) target).setSelection(selectedRegions.toArray(IRegion[]::new)); - } - - if (!oldSearchDirectionIsForward) { - deactivate(SearchOptions.FORWARD); - } - - return selectCount; + if (target instanceof IFindReplaceTargetExtension4 selectableTarget) { + IRegion[] selectedRegions = selections.stream().map(selection -> new Region(selection.x, selection.y)) + .toArray(IRegion[]::new); + selectableTarget.setSelection(selectedRegions); + } + }); + return selections.size(); } /** 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 7ae3ae22bdd..8a8cf22bf31 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 @@ -288,6 +288,15 @@ public void testPerformSelectAllBackward() { expectStatusIsFindAllWithCount(findReplaceLogic, 1); } + @Test + public void testPerformSelectAllOnReadonlyTarget() { + TextViewer textViewer= setupTextViewer("Ab Ab"); + textViewer.setEditable(false); + IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(textViewer); + findReplaceLogic.performSelectAll("Ab", Display.getCurrent()); + expectStatusIsFindAllWithCount(findReplaceLogic, 2); + } + @Test public void testSelectWholeWords() { TextViewer textViewer= setupTextViewer("Hello World of get and getters, set and setters");