From b06fb4736ef968ed4b7a143084dad7f86b1bfd86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Kubitz?= Date: Wed, 28 Aug 2024 08:47:01 +0200 Subject: [PATCH 01/23] fix workspace considered not writable on a smb home share #2225 https://github.com/eclipse-platform/eclipse.platform.ui/issues/2225 java.io.File.canWrite() and java.nio.file.Files.isWritable(Path) can not be trusted on windows. they may return wrong values. see for example JDK-8282720, JDK-8148211, JDK-8154915 => allow the user to press "Launch" even if jdk states the directory is not writable --- .../internal/ide/ChooseWorkspaceDialog.java | 38 +++++++++++-------- .../ui/internal/ide/messages.properties | 2 +- 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/ide/ChooseWorkspaceDialog.java b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/ide/ChooseWorkspaceDialog.java index 05cde034235..e8807e90963 100644 --- a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/ide/ChooseWorkspaceDialog.java +++ b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/ide/ChooseWorkspaceDialog.java @@ -474,9 +474,6 @@ protected String getUnexpectedPathHint() { } String normalisedPath = path.normalize().toString(); String normalisedPathWithSeperator = normalisedPath + File.separator; - if (!isWritable(path)) { - return NLS.bind(IDEWorkbenchMessages.ChooseWorkspaceDialog_NotWriteablePathWarning, normalisedPath); - } if (normalisedPathWithSeperator.contains(TILDE)) { return NLS.bind(IDEWorkbenchMessages.ChooseWorkspaceDialog_TildeNonExpandedWarning, normalisedPath); } @@ -484,23 +481,31 @@ protected String getUnexpectedPathHint() { && !workspaceLocation.equalsIgnoreCase(normalisedPathWithSeperator)) { return NLS.bind(IDEWorkbenchMessages.ChooseWorkspaceDialog_ResolvedAbsolutePath, normalisedPath); } + if (!maybeWritable(path)) { + return NLS.bind(IDEWorkbenchMessages.ChooseWorkspaceDialog_NotWriteablePathWarning, normalisedPath); + } } return ""; //$NON-NLS-1$ } - /** - * @param path - * @return - */ - private boolean isWritable(Path path) { - if (Files.exists(path)) { - return Files.isWritable(path); - } - Path parent = path.getParent(); - if (parent != null) { - return isWritable(parent); + /** the returned value may be wrong **/ + private boolean maybeWritable(Path path) { + try { + if (Files.exists(path)) { + // both java.io.File.canWrite() and + // java.nio.file.Files.isWritable(Path) + // can not be trusted on windows. they may return wrong values. + // for example JDK-8282720, JDK-8148211, JDK-8154915 + return Files.isWritable(path); + } + Path parent = path.getParent(); + if (parent == null) { + return false; + } + return maybeWritable(parent); + } catch (SecurityException se) { + return false; } - return true; } protected Composite createBrowseComposite(Composite parent) { @@ -545,7 +550,8 @@ protected Combo createPathCombo(Composite panel) { */ private boolean isValidPath(String path) { try { - return isWritable(new File(path).toPath()); + Path.of(path); + return true; } catch (InvalidPathException e) { return false; } diff --git a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/ide/messages.properties b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/ide/messages.properties index d55a6f59376..3f9b127093c 100644 --- a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/ide/messages.properties +++ b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/ide/messages.properties @@ -1049,7 +1049,7 @@ ChooseWorkspaceDialog_recentWorkspaces=&Recent Workspaces ChooseWorkspaceDialog_ResolvedAbsolutePath=Full path: {0} ChooseWorkspaceDialog_TildeNonExpandedWarning=\u26A0\uFE0F '~' is not expanded, full path: {0} ChooseWorkspaceDialog_InvalidPathWarning=\u26A0\uFE0F The path is invalid on this system: {0} -ChooseWorkspaceDialog_NotWriteablePathWarning=\u26A0\uFE0F The path is not writable by the current user: {0} +ChooseWorkspaceDialog_NotWriteablePathWarning=\u26A0\uFE0F The path may not be writable by the current user: {0} ChooseWorkspaceDialog_useDefaultMessage=&Use this as the default and do not ask again ChooseWorkspaceWithSettingsDialog_SettingsGroupName=&Copy Settings From 06f82a4cacc65c9a12c974f6403338c92d1f6d17 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sun, 1 Sep 2024 20:34:12 +0530 Subject: [PATCH 02/23] Update for release 4.34 (#2230) Co-authored-by: eclipse-releng-bot <98035380+eclipse-releng-bot@users.noreply.github.com> --- examples/pom.xml | 2 +- pom.xml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/pom.xml b/examples/pom.xml index 48748d474e8..e87bb500749 100644 --- a/examples/pom.xml +++ b/examples/pom.xml @@ -14,7 +14,7 @@ org.eclipse.platform eclipse.platform.ui - 4.33.0-SNAPSHOT + 4.34.0-SNAPSHOT eclipse.platform.ui.examples pom diff --git a/pom.xml b/pom.xml index c87923b468f..021376e71f9 100644 --- a/pom.xml +++ b/pom.xml @@ -16,7 +16,7 @@ org.eclipse eclipse-platform-parent - 4.33.0-SNAPSHOT + 4.34.0-SNAPSHOT ../eclipse-platform-parent From b96413b80add077ee86ea888235f86887c3bbee4 Mon Sep 17 00:00:00 2001 From: Rahul Mohanan <121536011+MohananRahul@users.noreply.github.com> Date: Mon, 2 Sep 2024 17:32:24 +0530 Subject: [PATCH 03/23] Update POM version (#2233) --- features/org.eclipse.e4.rcp/feature.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/org.eclipse.e4.rcp/feature.xml b/features/org.eclipse.e4.rcp/feature.xml index d7bd0a8289f..f67c0434f03 100644 --- a/features/org.eclipse.e4.rcp/feature.xml +++ b/features/org.eclipse.e4.rcp/feature.xml @@ -2,7 +2,7 @@ From 6b00c3ed5e930f15edb9700557e7694e42412890 Mon Sep 17 00:00:00 2001 From: fedejeanne Date: Tue, 3 Sep 2024 09:31:25 +0200 Subject: [PATCH 04/23] Add more details to error message Add more details to one of the error messages in ServiceLocator::registerService(...) --- .../org/eclipse/ui/internal/services/ServiceLocator.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/services/ServiceLocator.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/services/ServiceLocator.java index 1263fd7f191..fa5e04ff28e 100644 --- a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/services/ServiceLocator.java +++ b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/services/ServiceLocator.java @@ -265,7 +265,8 @@ private void registerService(Class api, Object service, boolean saveInContext } if (!api.isInstance(service)) { - throw new IllegalArgumentException("The service does not implement the given interface"); //$NON-NLS-1$ + throw new IllegalArgumentException( + "The service '" + service + "' does not implement the given interface: '" + api.getName() + "'"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ } if (isDisposed()) { IllegalStateException ex = new IllegalStateException("An attempt was made to register service " + service //$NON-NLS-1$ From 7a1a3f1c05683f6ecbfa4eab156c6cf4cac91e16 Mon Sep 17 00:00:00 2001 From: fedejeanne Date: Tue, 3 Sep 2024 14:44:50 +0200 Subject: [PATCH 05/23] Bump version --- bundles/org.eclipse.ui.workbench/META-INF/MANIFEST.MF | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundles/org.eclipse.ui.workbench/META-INF/MANIFEST.MF b/bundles/org.eclipse.ui.workbench/META-INF/MANIFEST.MF index 353a8fa9c43..261f4fe712f 100644 --- a/bundles/org.eclipse.ui.workbench/META-INF/MANIFEST.MF +++ b/bundles/org.eclipse.ui.workbench/META-INF/MANIFEST.MF @@ -2,7 +2,7 @@ Manifest-Version: 1.0 Bundle-ManifestVersion: 2 Bundle-Name: %pluginName Bundle-SymbolicName: org.eclipse.ui.workbench; singleton:=true -Bundle-Version: 3.133.0.qualifier +Bundle-Version: 3.133.100.qualifier Bundle-Activator: org.eclipse.ui.internal.WorkbenchPlugin Bundle-ActivationPolicy: lazy Bundle-Vendor: %providerName From d9d7d2d69361b70090729e74cd73a07a6b9a996f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Kubitz?= Date: Wed, 4 Sep 2024 13:13:49 +0200 Subject: [PATCH 06/23] fix IFindReplaceLogic.java:184: error: unexpected end tag:

reported in jenkins build log: [ERROR] MavenReportException: Error while generating Javadoc --- .../org.eclipse.ui.workbench.texteditor/META-INF/MANIFEST.MF | 2 +- .../eclipse/ui/internal/findandreplace/IFindReplaceLogic.java | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/bundles/org.eclipse.ui.workbench.texteditor/META-INF/MANIFEST.MF b/bundles/org.eclipse.ui.workbench.texteditor/META-INF/MANIFEST.MF index cf12407210d..b58b26bfbfc 100644 --- a/bundles/org.eclipse.ui.workbench.texteditor/META-INF/MANIFEST.MF +++ b/bundles/org.eclipse.ui.workbench.texteditor/META-INF/MANIFEST.MF @@ -2,7 +2,7 @@ Manifest-Version: 1.0 Bundle-ManifestVersion: 2 Bundle-Name: %pluginName Bundle-SymbolicName: org.eclipse.ui.workbench.texteditor; singleton:=true -Bundle-Version: 3.18.0.qualifier +Bundle-Version: 3.18.100.qualifier Bundle-Activator: org.eclipse.ui.internal.texteditor.TextEditorPlugin Bundle-ActivationPolicy: lazy Bundle-Vendor: %providerName diff --git a/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/IFindReplaceLogic.java b/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/IFindReplaceLogic.java index 304cbfc07f4..6bb72d363f3 100644 --- a/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/IFindReplaceLogic.java +++ b/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/IFindReplaceLogic.java @@ -181,7 +181,6 @@ public interface IFindReplaceLogic { *
  • Backward search operations will use the end of the selection as the * starting point.
    • * - *

    */ void resetIncrementalBaseLocation(); From b6441169f712a036bbd2bf43d7857826020e21c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Kubitz?= Date: Wed, 4 Sep 2024 12:32:20 +0200 Subject: [PATCH 07/23] reduce some "Potential resource leak: '' may not be closed" --- bundles/org.eclipse.ui.ide/META-INF/MANIFEST.MF | 2 +- .../actions/CopyFilesAndFoldersOperation.java | 8 +++++++- .../actions/MoveFilesAndFoldersOperation.java | 9 ++++++++- .../eclipse/ui/ide/undo/WorkspaceUndoUtil.java | 9 ++++++++- .../datatransfer/ArchiveFileManipulations.java | 8 ++++---- .../internal/wizards/datatransfer/TarFile.java | 5 ++++- .../wizards/datatransfer/TarFileExporter.java | 15 ++++++--------- .../WizardArchiveFileResourceImportPage1.java | 17 +++++++++-------- 8 files changed, 47 insertions(+), 26 deletions(-) diff --git a/bundles/org.eclipse.ui.ide/META-INF/MANIFEST.MF b/bundles/org.eclipse.ui.ide/META-INF/MANIFEST.MF index 64b62e924a6..c43ddcf24b9 100644 --- a/bundles/org.eclipse.ui.ide/META-INF/MANIFEST.MF +++ b/bundles/org.eclipse.ui.ide/META-INF/MANIFEST.MF @@ -2,7 +2,7 @@ Manifest-Version: 1.0 Bundle-ManifestVersion: 2 Bundle-Name: %Plugin.name Bundle-SymbolicName: org.eclipse.ui.ide; singleton:=true -Bundle-Version: 3.22.300.qualifier +Bundle-Version: 3.22.400.qualifier Bundle-Activator: org.eclipse.ui.internal.ide.IDEWorkbenchPlugin Bundle-ActivationPolicy: lazy Bundle-Vendor: %Plugin.providerName diff --git a/bundles/org.eclipse.ui.ide/extensions/org/eclipse/ui/actions/CopyFilesAndFoldersOperation.java b/bundles/org.eclipse.ui.ide/extensions/org/eclipse/ui/actions/CopyFilesAndFoldersOperation.java index 7c393308ff2..3a930c095a8 100644 --- a/bundles/org.eclipse.ui.ide/extensions/org/eclipse/ui/actions/CopyFilesAndFoldersOperation.java +++ b/bundles/org.eclipse.ui.ide/extensions/org/eclipse/ui/actions/CopyFilesAndFoldersOperation.java @@ -17,6 +17,8 @@ package org.eclipse.ui.actions; import java.io.File; +import java.io.IOException; +import java.io.InputStream; import java.lang.reflect.InvocationTargetException; import java.math.BigDecimal; import java.net.URI; @@ -528,7 +530,11 @@ private void copyExisting(IResource source, IResource existing, IProgressMonitor IFile sourceFile = getFile(source); if (sourceFile != null) { - existingFile.setContents(sourceFile.getContents(), IResource.KEEP_HISTORY, subMonitor.split(1)); + try (InputStream contents = sourceFile.getContents()) { + existingFile.setContents(contents, IResource.KEEP_HISTORY, subMonitor.split(1)); + } catch (IOException closeException) { + // never happens + } } } } diff --git a/bundles/org.eclipse.ui.ide/extensions/org/eclipse/ui/actions/MoveFilesAndFoldersOperation.java b/bundles/org.eclipse.ui.ide/extensions/org/eclipse/ui/actions/MoveFilesAndFoldersOperation.java index db4b51cb9f4..2df676e168f 100644 --- a/bundles/org.eclipse.ui.ide/extensions/org/eclipse/ui/actions/MoveFilesAndFoldersOperation.java +++ b/bundles/org.eclipse.ui.ide/extensions/org/eclipse/ui/actions/MoveFilesAndFoldersOperation.java @@ -13,6 +13,9 @@ *******************************************************************************/ package org.eclipse.ui.actions; +import java.io.IOException; +import java.io.InputStream; + import org.eclipse.core.resources.IContainer; import org.eclipse.core.resources.IFile; import org.eclipse.core.resources.IResource; @@ -199,7 +202,11 @@ private void moveExisting(IResource source, IResource existing, IProgressMonitor IFile sourceFile = getFile(source); if (sourceFile != null) { - existingFile.setContents(sourceFile.getContents(), IResource.KEEP_HISTORY, subMonitor.split(1)); + try (InputStream contents = sourceFile.getContents()) { + existingFile.setContents(contents, IResource.KEEP_HISTORY, subMonitor.split(1)); + } catch (IOException closeException) { + // never happens + } delete(sourceFile, subMonitor.split(1)); } } diff --git a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/ide/undo/WorkspaceUndoUtil.java b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/ide/undo/WorkspaceUndoUtil.java index e2bea300925..971f91d019c 100644 --- a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/ide/undo/WorkspaceUndoUtil.java +++ b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/ide/undo/WorkspaceUndoUtil.java @@ -15,6 +15,8 @@ *******************************************************************************/ package org.eclipse.ui.ide.undo; +import java.io.IOException; +import java.io.InputStream; import java.net.URI; import java.util.ArrayList; import java.util.Arrays; @@ -760,7 +762,12 @@ private static IResourceSnapshot copyOverExistingResource( // restored. IResourceSnapshot fileDescription = ResourceSnapshotFactory.fromResource(existingFile); // Reset the contents to that of the file being moved - existingFile.setContents(file.getContents(), IResource.KEEP_HISTORY, subMonitor.split(1)); + try (InputStream contents = file.getContents()) { + existingFile.setContents(contents, IResource.KEEP_HISTORY, subMonitor.split(1)); + } catch (IOException closeException) { + // never happens + } + fileDescription.recordStateFromHistory(subMonitor.split(1)); // Now delete the source file if requested // We don't need to remember anything about it, because diff --git a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/wizards/datatransfer/ArchiveFileManipulations.java b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/wizards/datatransfer/ArchiveFileManipulations.java index c50052971c1..d3461b58256 100644 --- a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/wizards/datatransfer/ArchiveFileManipulations.java +++ b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/wizards/datatransfer/ArchiveFileManipulations.java @@ -82,11 +82,11 @@ public static boolean isZipFile(String fileName) { */ @SuppressWarnings("resource") public static void closeStructureProvider(ILeveledImportStructureProvider structureProvider, Shell shell) { - if (structureProvider instanceof ZipLeveledStructureProvider) { - closeZipFile(((ZipLeveledStructureProvider) structureProvider).getZipFile(), shell); + if (structureProvider instanceof ZipLeveledStructureProvider zsp) { + closeZipFile(zsp.getZipFile(), shell); } - if (structureProvider instanceof TarLeveledStructureProvider) { - closeTarFile(((TarLeveledStructureProvider) structureProvider).getTarFile(), shell); + if (structureProvider instanceof TarLeveledStructureProvider tsp) { + closeTarFile(tsp.getTarFile(), shell); } } diff --git a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/wizards/datatransfer/TarFile.java b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/wizards/datatransfer/TarFile.java index 3053fe21f6c..38468e32cf9 100644 --- a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/wizards/datatransfer/TarFile.java +++ b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/wizards/datatransfer/TarFile.java @@ -32,7 +32,7 @@ */ public class TarFile implements AutoCloseable { private final File file; - private TarInputStream entryEnumerationStream; + private final TarInputStream entryEnumerationStream; private TarEntry curEntry; private TarInputStream entryStream; @@ -52,6 +52,9 @@ public TarFile(File file) throws TarException, IOException { //If it is not compressed we close //the old one and recreate in.close(); + in = null; + } + if (in == null) { in = new FileInputStream(file); } try { diff --git a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/wizards/datatransfer/TarFileExporter.java b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/wizards/datatransfer/TarFileExporter.java index 0362fcf61fd..02f57390935 100644 --- a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/wizards/datatransfer/TarFileExporter.java +++ b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/wizards/datatransfer/TarFileExporter.java @@ -34,9 +34,8 @@ * @since 3.1 */ public class TarFileExporter implements IFileExporter { - private TarOutputStream outputStream; - private GZIPOutputStream gzipOutputStream; - private boolean resolveLinks; + private final TarOutputStream outputStream; + private final boolean resolveLinks; /** @@ -52,12 +51,13 @@ public class TarFileExporter implements IFileExporter { */ public TarFileExporter(String filename, boolean compress, boolean resolveLinks) throws IOException { this.resolveLinks = resolveLinks; + TarOutputStream tos; if (compress) { - gzipOutputStream = new GZIPOutputStream(new FileOutputStream(filename)); - outputStream = new TarOutputStream(new BufferedOutputStream(gzipOutputStream)); + tos = new TarOutputStream(new BufferedOutputStream(new GZIPOutputStream(new FileOutputStream(filename)))); } else { - outputStream = new TarOutputStream(new BufferedOutputStream(new FileOutputStream(filename))); + tos = new TarOutputStream(new BufferedOutputStream(new FileOutputStream(filename))); } + outputStream = tos; } /** @@ -69,9 +69,6 @@ public TarFileExporter(String filename, boolean compress, boolean resolveLinks) @Override public void finished() throws IOException { outputStream.close(); - if(gzipOutputStream != null) { - gzipOutputStream.close(); - } } /** diff --git a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/wizards/datatransfer/WizardArchiveFileResourceImportPage1.java b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/wizards/datatransfer/WizardArchiveFileResourceImportPage1.java index 1e028a8cc29..52db25ea3d4 100644 --- a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/wizards/datatransfer/WizardArchiveFileResourceImportPage1.java +++ b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/wizards/datatransfer/WizardArchiveFileResourceImportPage1.java @@ -145,12 +145,15 @@ private boolean validateSourceFile(String fileName) { * and is valid (ie.- proper format) */ private boolean ensureZipSourceIsValid() { - ZipFile specifiedFile = getSpecifiedZipSourceFile(); - if (specifiedFile == null) { - setErrorMessage(DataTransferMessages.ZipImport_badFormat); + try (ZipFile specifiedFile = getSpecifiedZipSourceFile()) { + if (specifiedFile == null) { + setErrorMessage(DataTransferMessages.ZipImport_badFormat); + return false; + } + return ArchiveFileManipulations.closeZipFile(specifiedFile, getShell()); + } catch (IOException closeException) { return false; } - return ArchiveFileManipulations.closeZipFile(specifiedFile, getShell()); } private boolean ensureTarSourceIsValid() { @@ -403,12 +406,10 @@ private ILeveledImportStructureProvider extracted() { ILeveledImportStructureProvider importStructureProvider = null; if (ArchiveFileManipulations.isTarFile(sourceNameField.getText())) { if( ensureTarSourceIsValid()) { - TarFile tarFile = getSpecifiedTarSourceFile(); - importStructureProvider = new TarLeveledStructureProvider(tarFile); + importStructureProvider = new TarLeveledStructureProvider(getSpecifiedTarSourceFile()); } } else if(ensureZipSourceIsValid()) { - ZipFile zipFile = getSpecifiedZipSourceFile(); - importStructureProvider = new ZipLeveledStructureProvider(zipFile); + importStructureProvider = new ZipLeveledStructureProvider(getSpecifiedZipSourceFile()); } return importStructureProvider; } From 021c995919ef423cb8453423d97289006aeba15a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Kubitz?= Date: Thu, 29 Aug 2024 12:29:10 +0200 Subject: [PATCH 08/23] AbstractTextEditor: resolve "Potential null pointer access" "The variable nextAnnotationPosition may be null at this location" The code assumed "distance=Integer.MAX_VALUE" will cause short circuit "currentDistance < distance" while theoretically equality could happen with currentDistance=Integer.MAX_VALUE. --- .../src/org/eclipse/ui/texteditor/AbstractTextEditor.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/texteditor/AbstractTextEditor.java b/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/texteditor/AbstractTextEditor.java index 8a4af550a4f..a518ed85f3a 100644 --- a/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/texteditor/AbstractTextEditor.java +++ b/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/texteditor/AbstractTextEditor.java @@ -6998,7 +6998,8 @@ protected Annotation findAnnotation(final int offset, final int length, boolean if (currentDistance < 0) currentDistance= endOfDocument + currentDistance; - if (currentDistance < distance || currentDistance == distance && p.length < nextAnnotationPosition.length) { + if (nextAnnotationPosition == null || currentDistance < distance + || currentDistance == distance && p.length < nextAnnotationPosition.length) { distance= currentDistance; nextAnnotation= a; nextAnnotationPosition= p; @@ -7008,7 +7009,8 @@ protected Annotation findAnnotation(final int offset, final int length, boolean if (currentDistance < 0) currentDistance= endOfDocument + currentDistance; - if (currentDistance < distance || currentDistance == distance && p.length < nextAnnotationPosition.length) { + if (nextAnnotationPosition == null || currentDistance < distance + || currentDistance == distance && p.length < nextAnnotationPosition.length) { distance= currentDistance; nextAnnotation= a; nextAnnotationPosition= p; From 445b93626d9fc863e6617cd484b785586efbfe38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Kubitz?= Date: Tue, 20 Aug 2024 17:47:22 +0200 Subject: [PATCH 09/23] fix "Implicit narrowing conversion in compound assignment" warnings All false positives, but avoidable with integer arithmetic instead of floating point. https://github.com/eclipse-platform/eclipse.platform.ui/security/code-scanning/16 https://github.com/eclipse-platform/eclipse.platform.ui/security/code-scanning/15 https://github.com/eclipse-platform/eclipse.platform.ui/security/code-scanning/2 https://github.com/eclipse-platform/eclipse.platform.ui/security/code-scanning/1 --- .../refactoring/RefactoringDescriptor.java | 2 +- .../RefactoringDescriptorProxy.java | 2 +- .../ide/dialogs/ResourceFilterGroup.java | 36 +++++++++---------- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/bundles/org.eclipse.ltk.core.refactoring/src/org/eclipse/ltk/core/refactoring/RefactoringDescriptor.java b/bundles/org.eclipse.ltk.core.refactoring/src/org/eclipse/ltk/core/refactoring/RefactoringDescriptor.java index 6b63fc2285f..1e61d32a89d 100644 --- a/bundles/org.eclipse.ltk.core.refactoring/src/org/eclipse/ltk/core/refactoring/RefactoringDescriptor.java +++ b/bundles/org.eclipse.ltk.core.refactoring/src/org/eclipse/ltk/core/refactoring/RefactoringDescriptor.java @@ -357,7 +357,7 @@ public final long getTimeStamp() { public final int hashCode() { int code= getDescription().hashCode(); if (fTimeStamp >= 0) - code+= (17 * fTimeStamp); + code+= 17 * Long.hashCode(fTimeStamp); return code; } diff --git a/bundles/org.eclipse.ltk.core.refactoring/src/org/eclipse/ltk/core/refactoring/RefactoringDescriptorProxy.java b/bundles/org.eclipse.ltk.core.refactoring/src/org/eclipse/ltk/core/refactoring/RefactoringDescriptorProxy.java index 1db7e197d5a..d8be3122bba 100644 --- a/bundles/org.eclipse.ltk.core.refactoring/src/org/eclipse/ltk/core/refactoring/RefactoringDescriptorProxy.java +++ b/bundles/org.eclipse.ltk.core.refactoring/src/org/eclipse/ltk/core/refactoring/RefactoringDescriptorProxy.java @@ -108,7 +108,7 @@ public final int hashCode() { int code= getDescription().hashCode(); final long stamp= getTimeStamp(); if (stamp >= 0) - code+= (17 * stamp); + code+= 17 * Long.hashCode(stamp); return code; } diff --git a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/ide/dialogs/ResourceFilterGroup.java b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/ide/dialogs/ResourceFilterGroup.java index 6716e23b2d1..14eab90961c 100644 --- a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/ide/dialogs/ResourceFilterGroup.java +++ b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/ide/dialogs/ResourceFilterGroup.java @@ -2772,8 +2772,8 @@ public void widgetSelected(SelectionEvent e) { setupDescriptionText(null); } - private String[] timeIntervalPrefixes = {"s", "m", "h", "d"}; //$NON-NLS-1$//$NON-NLS-2$//$NON-NLS-3$ //$NON-NLS-4$ - private double[] timeIntervalScale = {60, 60, 24}; + private static final String[] TIME_INTERVAL_PREFIXES = { "s", "m", "h", "d" }; //$NON-NLS-1$//$NON-NLS-2$//$NON-NLS-3$ //$NON-NLS-4$ + private static final long[] TIME_INTERVAL_SCALE = { 60, 60, 24 }; private String convertToEditableTimeInterval(String string) { if (string.isEmpty()) @@ -2786,22 +2786,22 @@ private String convertToEditableTimeInterval(String string) { } if (value == 0) return Long.toString(0); - for (int i = 0; i < timeIntervalPrefixes.length - 1; i++) { - if (value % timeIntervalScale[i] != 0) - return Long.toString(value) + timeIntervalPrefixes[i]; - value /= timeIntervalScale[i]; + for (int i = 0; i < TIME_INTERVAL_PREFIXES.length - 1; i++) { + if (value % TIME_INTERVAL_SCALE[i] != 0) + return Long.toString(value) + TIME_INTERVAL_PREFIXES[i]; + value /= TIME_INTERVAL_SCALE[i]; } - return Long.toString(value) + timeIntervalPrefixes[timeIntervalPrefixes.length - 1]; + return Long.toString(value) + TIME_INTERVAL_PREFIXES[TIME_INTERVAL_PREFIXES.length - 1]; } private String convertFromEditableTimeInterval(String string) { if (string.isEmpty()) return string; - for (int i = 1; i < timeIntervalPrefixes.length; i++) { - if (string.endsWith(timeIntervalPrefixes[i])) { + for (int i = 1; i < TIME_INTERVAL_PREFIXES.length; i++) { + if (string.endsWith(TIME_INTERVAL_PREFIXES[i])) { long value = Long.parseLong(string.substring(0, string.length() - 1)); for (int j = 0; j < i; j++) - value *= timeIntervalScale[j]; + value *= TIME_INTERVAL_SCALE[j]; return Long.toString(value); } } @@ -2810,7 +2810,7 @@ private String convertFromEditableTimeInterval(String string) { } - private String[] lengthPrefixes = { "", "k", "m", "g" }; //$NON-NLS-1$//$NON-NLS-2$//$NON-NLS-3$//$NON-NLS-4$ + private static final String[] METRIC_PREFIXES = { "", "k", "m", "g" }; //$NON-NLS-1$//$NON-NLS-2$//$NON-NLS-3$//$NON-NLS-4$ // converts "32768" to "32k" private String convertToEditableLength(String string) { @@ -2824,23 +2824,23 @@ private String convertToEditableLength(String string) { } if (value == 0) return Long.toString(0); - for (int i = 0; i < lengthPrefixes.length; i++) { + for (int i = 0; i < METRIC_PREFIXES.length; i++) { if (value % 1024 != 0) - return Long.toString(value) + lengthPrefixes[i]; - if ((i + 1) < lengthPrefixes.length) + return Long.toString(value) + METRIC_PREFIXES[i]; + if ((i + 1) < METRIC_PREFIXES.length) value /= 1024; } - return Long.toString(value) + lengthPrefixes[lengthPrefixes.length - 1]; + return Long.toString(value) + METRIC_PREFIXES[METRIC_PREFIXES.length - 1]; } // converts "32k" to "32768" private String convertFromEditableLength(String string) throws NumberFormatException { if (string.isEmpty()) return string; - for (int i = 1; i < lengthPrefixes.length; i++) { - if (string.endsWith(lengthPrefixes[i])) { + for (int i = 1; i < METRIC_PREFIXES.length; i++) { + if (string.endsWith(METRIC_PREFIXES[i])) { long value = Long.parseLong(string.substring(0, string.length() - 1)); - value *= Math.pow(1024, i); + value *= 2 << (10 * i - 1); return Long.toString(value); } } From 0aeff4d64c4e2a939ec0ae2f0f1c616c96dcb8e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Kubitz?= Date: Thu, 5 Sep 2024 12:40:24 +0200 Subject: [PATCH 10/23] version bump --- bundles/org.eclipse.ltk.core.refactoring/META-INF/MANIFEST.MF | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundles/org.eclipse.ltk.core.refactoring/META-INF/MANIFEST.MF b/bundles/org.eclipse.ltk.core.refactoring/META-INF/MANIFEST.MF index 49d207da620..3d9e6012b46 100644 --- a/bundles/org.eclipse.ltk.core.refactoring/META-INF/MANIFEST.MF +++ b/bundles/org.eclipse.ltk.core.refactoring/META-INF/MANIFEST.MF @@ -3,7 +3,7 @@ Automatic-Module-Name: org.eclipse.ltk.core.refactoring Bundle-ManifestVersion: 2 Bundle-Name: %pluginName Bundle-SymbolicName: org.eclipse.ltk.core.refactoring; singleton:=true -Bundle-Version: 3.14.500.qualifier +Bundle-Version: 3.14.600.qualifier Bundle-Activator: org.eclipse.ltk.internal.core.refactoring.RefactoringCorePlugin Bundle-ActivationPolicy: lazy Bundle-Vendor: %providerName From 081ad821cf9555deca5d1ca4c88f3c742d72a152 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Kubitz?= Date: Thu, 29 Aug 2024 11:39:20 +0200 Subject: [PATCH 11/23] Resolve problem markers "Unlikely argument type for equals()" trick the compiler to ignore false positives: * IHyperlinkSegment is related to ParagraphSegment * ITabbedPropertySheetPageContributor is related to IWorkbenchPart as they have common implementations. --- bundles/org.eclipse.ui.forms/META-INF/MANIFEST.MF | 2 +- .../eclipse/ui/internal/forms/widgets/Paragraph.java | 4 ++-- .../META-INF/MANIFEST.MF | 2 +- .../properties/tabbed/TabbedPropertySheetPage.java | 11 +++++------ 4 files changed, 9 insertions(+), 10 deletions(-) diff --git a/bundles/org.eclipse.ui.forms/META-INF/MANIFEST.MF b/bundles/org.eclipse.ui.forms/META-INF/MANIFEST.MF index d966324b750..51ad30fc4a7 100644 --- a/bundles/org.eclipse.ui.forms/META-INF/MANIFEST.MF +++ b/bundles/org.eclipse.ui.forms/META-INF/MANIFEST.MF @@ -2,7 +2,7 @@ Manifest-Version: 1.0 Bundle-ManifestVersion: 2 Bundle-Name: %name Bundle-SymbolicName: org.eclipse.ui.forms;singleton:=true -Bundle-Version: 3.13.300.qualifier +Bundle-Version: 3.13.400.qualifier Bundle-Vendor: %provider-name Bundle-Localization: plugin Export-Package: org.eclipse.ui.forms, diff --git a/bundles/org.eclipse.ui.forms/src/org/eclipse/ui/internal/forms/widgets/Paragraph.java b/bundles/org.eclipse.ui.forms/src/org/eclipse/ui/internal/forms/widgets/Paragraph.java index 8b26dbf39bd..a1f96d180a4 100644 --- a/bundles/org.eclipse.ui.forms/src/org/eclipse/ui/internal/forms/widgets/Paragraph.java +++ b/bundles/org.eclipse.ui.forms/src/org/eclipse/ui/internal/forms/widgets/Paragraph.java @@ -162,7 +162,7 @@ public void layout(GC gc, int width, Locator loc, int lineHeight, computeRowHeights(gc, width, loc, lineHeight, resourceTable); for (ParagraphSegment segment : segments) { boolean doSelect = false; - if (selectedLink != null && segment.equals(selectedLink)) + if (selectedLink instanceof ParagraphSegment sl && segment.equals(sl)) doSelect = true; segment.layout(gc, width, loc, resourceTable, doSelect); } @@ -182,7 +182,7 @@ public void paint(GC gc, Rectangle repaintRegion, if (!segment.intersects(repaintRegion)) continue; boolean doSelect = false; - if (selectedLink != null && segment.equals(selectedLink)) + if (selectedLink instanceof ParagraphSegment sl && segment.equals(sl)) doSelect = true; segment.paint(gc, false, resourceTable, doSelect, selData, repaintRegion); } diff --git a/bundles/org.eclipse.ui.views.properties.tabbed/META-INF/MANIFEST.MF b/bundles/org.eclipse.ui.views.properties.tabbed/META-INF/MANIFEST.MF index dcc4ac61bf1..8d9275b13c5 100644 --- a/bundles/org.eclipse.ui.views.properties.tabbed/META-INF/MANIFEST.MF +++ b/bundles/org.eclipse.ui.views.properties.tabbed/META-INF/MANIFEST.MF @@ -2,7 +2,7 @@ Manifest-Version: 1.0 Bundle-ManifestVersion: 2 Bundle-Name: %Plugin.name Bundle-SymbolicName: org.eclipse.ui.views.properties.tabbed;singleton:=true -Bundle-Version: 3.10.300.qualifier +Bundle-Version: 3.10.400.qualifier Bundle-Vendor: %Plugin.providerName Bundle-Localization: plugin Export-Package: org.eclipse.ui.internal.views.properties.tabbed;x-internal:=true, diff --git a/bundles/org.eclipse.ui.views.properties.tabbed/src/org/eclipse/ui/views/properties/tabbed/TabbedPropertySheetPage.java b/bundles/org.eclipse.ui.views.properties.tabbed/src/org/eclipse/ui/views/properties/tabbed/TabbedPropertySheetPage.java index 5092dee24ee..2117b2c7823 100644 --- a/bundles/org.eclipse.ui.views.properties.tabbed/src/org/eclipse/ui/views/properties/tabbed/TabbedPropertySheetPage.java +++ b/bundles/org.eclipse.ui.views.properties.tabbed/src/org/eclipse/ui/views/properties/tabbed/TabbedPropertySheetPage.java @@ -315,8 +315,8 @@ protected void handlePartActivated(IWorkbenchPart part) { * The properties view has been activated and the current page is this * instance of TabbedPropertySheetPage */ - boolean thisActivated = part instanceof PropertySheet - && ((PropertySheet) part).getCurrentPage() == this; + boolean thisActivated = part instanceof PropertySheet propertySheet + && propertySheet.getCurrentPage() == this; /* * When the active part changes and the part does not provide a @@ -325,16 +325,15 @@ protected void handlePartActivated(IWorkbenchPart part) { * of these events since we want to send aboutToBeHidden() and * aboutToBeShown() when the property sheet is hidden or shown. */ - if (!thisActivated && !part.equals(contributor) + if (!thisActivated && !(part instanceof ITabbedPropertySheetPageContributor p && p.equals(contributor)) && !part.getSite().getId().equals(contributor.getContributorId())) { /* * Is the part is a IContributedContentsView for the contributor, * for example, outline view. */ IContributedContentsView view = Adapters.adapt(part, IContributedContentsView.class); - if (view == null - || (view.getContributingPart() != null && !view - .getContributingPart().equals(contributor))) { + if (!(view.getContributingPart() instanceof ITabbedPropertySheetPageContributor cp + && cp.equals(contributor))) { if (activePropertySheet) { if (currentTab != null) { currentTab.aboutToBeHidden(); From 4af4df453fe6fb1de6d487975b987931eaeb9605 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Kubitz?= Date: Mon, 26 Aug 2024 16:58:25 +0200 Subject: [PATCH 12/23] search: fail fast for binary file types without opening the file #2180 https://github.com/eclipse-platform/eclipse.platform.ui/issues/2180 assumes that files are binary if they have a IContentType that is not text --- .../internal/core/text/TextSearchVisitor.java | 34 ++++++++++++------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/bundles/org.eclipse.search.core/search/org/eclipse/search/internal/core/text/TextSearchVisitor.java b/bundles/org.eclipse.search.core/search/org/eclipse/search/internal/core/text/TextSearchVisitor.java index 2fb97742442..8a0354bf93f 100644 --- a/bundles/org.eclipse.search.core/search/org/eclipse/search/internal/core/text/TextSearchVisitor.java +++ b/bundles/org.eclipse.search.core/search/org/eclipse/search/internal/core/text/TextSearchVisitor.java @@ -40,7 +40,6 @@ import org.eclipse.core.runtime.Platform; import org.eclipse.core.runtime.Status; import org.eclipse.core.runtime.SubMonitor; -import org.eclipse.core.runtime.content.IContentDescription; import org.eclipse.core.runtime.content.IContentType; import org.eclipse.core.runtime.content.IContentTypeManager; import org.eclipse.core.runtime.jobs.Job; @@ -205,8 +204,13 @@ public IStatus processFile(List sameFiles, IProgressMonitor monitor) { occurences = locateMatches(file, charsequence, matcher, monitor); } else { try { + boolean reportTextOnly = !fCollector.reportBinaryFile(file); + if (reportTextOnly && hasBinaryContentType(file)) { + // fail fast for binary file types without opening the file + return Status.OK_STATUS; + } charsequence = fileCharSequenceProvider.newCharSequence(file); - if (hasBinaryContent(charsequence, file) && !fCollector.reportBinaryFile(file)) { + if (reportTextOnly && hasBinaryContent(charsequence)) { return Status.OK_STATUS; } occurences = locateMatches(file, charsequence, matcher, monitor); @@ -435,21 +439,25 @@ public IStatus search(TextSearchScope scope, IProgressMonitor monitor) { return search(scope.evaluateFilesInScope(fStatus), monitor); } - private boolean hasBinaryContent(CharSequence seq, IFile file) throws CoreException { - if (seq instanceof String) { - if (!((String) seq).contains("\0")) { //$NON-NLS-1$ - // fail fast to avoid file.getContentDescription(): - return false; + private final IContentType TEXT_TYPE = Platform.getContentTypeManager().getContentType(IContentTypeManager.CT_TEXT); + + private boolean hasBinaryContentType(IFile file) { + IContentType[] contentTypes = Platform.getContentTypeManager().findContentTypesFor(file.getName()); + for (IContentType contentType : contentTypes) { + if (contentType.isKindOf(TEXT_TYPE)) { + return false; // is text } } - IContentDescription desc= file.getContentDescription(); - if (desc != null) { - IContentType contentType= desc.getContentType(); - if (contentType != null && contentType.isKindOf(Platform.getContentTypeManager().getContentType(IContentTypeManager.CT_TEXT))) { - return false; - } + if (contentTypes.length > 0) { + return true; // has some not text type } + return false; // unknown + } + private boolean hasBinaryContent(CharSequence seq) { + if (seq instanceof String s) { + return (s.contains("\0")); //$NON-NLS-1$ + } // avoid calling seq.length() at it runs through the complete file, // thus it would do so for all binary files. try { From 1697843388a268b79849f24d957d5b177806ef30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Kubitz?= Date: Thu, 5 Sep 2024 12:47:07 +0200 Subject: [PATCH 13/23] version bump --- bundles/org.eclipse.search.core/META-INF/MANIFEST.MF | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundles/org.eclipse.search.core/META-INF/MANIFEST.MF b/bundles/org.eclipse.search.core/META-INF/MANIFEST.MF index 99d9b34cf3e..8b5a65960b0 100644 --- a/bundles/org.eclipse.search.core/META-INF/MANIFEST.MF +++ b/bundles/org.eclipse.search.core/META-INF/MANIFEST.MF @@ -2,7 +2,7 @@ Manifest-Version: 1.0 Bundle-ManifestVersion: 2 Bundle-Name: %pluginName Bundle-SymbolicName: org.eclipse.search.core;singleton:=true -Bundle-Version: 3.16.300.qualifier +Bundle-Version: 3.16.400.qualifier Bundle-Activator: org.eclipse.search.internal.core.SearchCorePlugin Bundle-ActivationPolicy: lazy Bundle-Vendor: %providerName From 41abe79f4a9bd1611622e4a77598f848c0187e3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Kubitz?= Date: Mon, 26 Aug 2024 12:35:13 +0200 Subject: [PATCH 14/23] Error Log: do not pop up on info/warning/freeze by default #2218 As it is counterproductive to pop up error log when something took too long. https://github.com/eclipse-platform/eclipse.platform.ui/issues/2218 --- bundles/org.eclipse.ui.monitoring/META-INF/MANIFEST.MF | 2 +- .../preferences/MonitoringPreferenceInitializer.java | 2 +- .../src/org/eclipse/ui/internal/views/log/LogView.java | 5 +++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/bundles/org.eclipse.ui.monitoring/META-INF/MANIFEST.MF b/bundles/org.eclipse.ui.monitoring/META-INF/MANIFEST.MF index 82fb67653e5..daf7dc1df95 100644 --- a/bundles/org.eclipse.ui.monitoring/META-INF/MANIFEST.MF +++ b/bundles/org.eclipse.ui.monitoring/META-INF/MANIFEST.MF @@ -6,7 +6,7 @@ Bundle-Name: %Bundle-Name Bundle-RequiredExecutionEnvironment: JavaSE-17 Bundle-SymbolicName: org.eclipse.ui.monitoring;singleton:=true Bundle-Vendor: %Bundle-Vendor -Bundle-Version: 1.3.300.qualifier +Bundle-Version: 1.3.400.qualifier Export-Package: org.eclipse.ui.internal.monitoring;x-internal:=true, org.eclipse.ui.internal.monitoring.preferences;x-internal:=true, org.eclipse.ui.monitoring;x-internal:=true diff --git a/bundles/org.eclipse.ui.monitoring/src/org/eclipse/ui/internal/monitoring/preferences/MonitoringPreferenceInitializer.java b/bundles/org.eclipse.ui.monitoring/src/org/eclipse/ui/internal/monitoring/preferences/MonitoringPreferenceInitializer.java index 79fdea0a60c..517b13154e9 100644 --- a/bundles/org.eclipse.ui.monitoring/src/org/eclipse/ui/internal/monitoring/preferences/MonitoringPreferenceInitializer.java +++ b/bundles/org.eclipse.ui.monitoring/src/org/eclipse/ui/internal/monitoring/preferences/MonitoringPreferenceInitializer.java @@ -30,7 +30,7 @@ public void initializeDefaultPreferences() { store.setDefault(PreferenceConstants.MONITORING_ENABLED, false); store.setDefault(PreferenceConstants.LONG_EVENT_WARNING_THRESHOLD_MILLIS, 500); // 0.5 sec - store.setDefault(PreferenceConstants.LONG_EVENT_ERROR_THRESHOLD_MILLIS, 1000); // 2 sec + store.setDefault(PreferenceConstants.LONG_EVENT_ERROR_THRESHOLD_MILLIS, 100_000); // 100 sec store.setDefault(PreferenceConstants.MAX_STACK_SAMPLES, 3); store.setDefault(PreferenceConstants.DEADLOCK_REPORTING_THRESHOLD_MILLIS, 5 * 60 * 1000); // 5 min diff --git a/bundles/org.eclipse.ui.views.log/src/org/eclipse/ui/internal/views/log/LogView.java b/bundles/org.eclipse.ui.views.log/src/org/eclipse/ui/internal/views/log/LogView.java index a94fcc1b3b6..4debe5f649a 100644 --- a/bundles/org.eclipse.ui.views.log/src/org/eclipse/ui/internal/views/log/LogView.java +++ b/bundles/org.eclipse.ui.views.log/src/org/eclipse/ui/internal/views/log/LogView.java @@ -1762,9 +1762,10 @@ private void readSettings() { fMemento.putInteger(P_COLUMN_1, getColumnWidthPreference(instancePrefs, defaultPrefs, P_COLUMN_1, 300)); fMemento.putInteger(P_COLUMN_2, getColumnWidthPreference(instancePrefs, defaultPrefs, P_COLUMN_2, 150)); fMemento.putInteger(P_COLUMN_3, getColumnWidthPreference(instancePrefs, defaultPrefs, P_COLUMN_3, 150)); - fMemento.putBoolean(P_ACTIVATE, instancePrefs.getBoolean(P_ACTIVATE, defaultPrefs.getBoolean(P_ACTIVATE, true))); + fMemento.putBoolean(P_ACTIVATE, + instancePrefs.getBoolean(P_ACTIVATE, defaultPrefs.getBoolean(P_ACTIVATE, false))); fMemento.putBoolean(P_ACTIVATE_WARN, - instancePrefs.getBoolean(P_ACTIVATE_WARN, defaultPrefs.getBoolean(P_ACTIVATE_WARN, true))); + instancePrefs.getBoolean(P_ACTIVATE_WARN, defaultPrefs.getBoolean(P_ACTIVATE_WARN, false))); fMemento.putBoolean(P_ACTIVATE_ERRROR, instancePrefs.getBoolean(P_ACTIVATE_ERRROR, defaultPrefs.getBoolean(P_ACTIVATE_ERRROR, true))); fMemento.putInteger(P_ORDER_VALUE, instancePrefs.getInt(P_ORDER_VALUE, defaultPrefs.getInt(P_ORDER_VALUE, DESCENDING))); From 31a902749e53ae095fb98c40459ba8cfc70d6d0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Kubitz?= Date: Thu, 5 Sep 2024 13:11:19 +0200 Subject: [PATCH 15/23] version bump --- bundles/org.eclipse.ui.views.log/META-INF/MANIFEST.MF | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundles/org.eclipse.ui.views.log/META-INF/MANIFEST.MF b/bundles/org.eclipse.ui.views.log/META-INF/MANIFEST.MF index ebf2da6b7b4..3ad0cdd6f52 100644 --- a/bundles/org.eclipse.ui.views.log/META-INF/MANIFEST.MF +++ b/bundles/org.eclipse.ui.views.log/META-INF/MANIFEST.MF @@ -2,7 +2,7 @@ Manifest-Version: 1.0 Bundle-ManifestVersion: 2 Bundle-Name: %name Bundle-SymbolicName: org.eclipse.ui.views.log;singleton:=true -Bundle-Version: 1.4.500.qualifier +Bundle-Version: 1.4.600.qualifier Bundle-Activator: org.eclipse.ui.internal.views.log.Activator Bundle-Vendor: %provider-name Require-Bundle: org.eclipse.core.runtime;bundle-version="[3.29.0,4.0.0)", From b3a93e3818449c931d3de57a8a8c1547e56d77cb Mon Sep 17 00:00:00 2001 From: Heiko Klare Date: Fri, 6 Sep 2024 09:58:42 +0200 Subject: [PATCH 16/23] Find/replace overlay: unify async executions, avoid NPEs #2244 Several operation in the find/replace overlay need to be executed asynchronously as they may be called in OS callback which can lead to deadlocks in GTK. Currently, this processing is distributed and repeated across the class and did not check preconditions (in particular the dialog being opened and its shell being present) appropriate in every case. In some cases, when the overlay is already or temporarily closed but an asynchronous task is still to be executed, that task may fail with an NPE. This change moves the functionality for the asynchronous processing to a central place, ensuring that the precondition requiring the shell to exist to always be evaluated. Factoring out this functionality also makes explicit that the called operation is executed only if the overlay respectively its shell are currently open. It also fixes the mentioned NPEs. Fixes https://github.com/eclipse-platform/eclipse.platform.ui/issues/2244 --- .../overlay/FindReplaceOverlay.java | 44 +++++++++++-------- 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlay.java b/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlay.java index 7e131500202..9a733b6a94b 100644 --- a/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlay.java +++ b/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlay.java @@ -16,7 +16,7 @@ import static org.eclipse.ui.internal.findandreplace.overlay.FindReplaceShortcutUtil.registerActionShortcutsAtControl; import java.util.List; -import java.util.function.Supplier; +import java.util.function.Consumer; import org.osgi.framework.FrameworkUtil; @@ -156,7 +156,7 @@ public FindReplaceOverlay(Shell parent, IWorkbenchPart part, IFindReplaceTarget setShellStyle(SWT.MODELESS); setBlockOnOpen(false); targetPart = part; - targetPartVisibilityHandler = new TargetPartVisibilityHandler(targetPart, this::getShell, this::close, + targetPartVisibilityHandler = new TargetPartVisibilityHandler(targetPart, this::asyncExecIfOpen, this::close, this::updatePlacementAndVisibility); } @@ -195,21 +195,26 @@ private void performSelectAll() { private ControlListener shellMovementListener = new ControlListener() { @Override public void controlMoved(ControlEvent e) { - asyncUpdatePlacementAndVisibility(); + asyncExecIfOpen(FindReplaceOverlay.this::updatePlacementAndVisibility); } @Override public void controlResized(ControlEvent e) { - asyncUpdatePlacementAndVisibility(); + asyncExecIfOpen(FindReplaceOverlay.this::updatePlacementAndVisibility); } }; - private PaintListener widgetMovementListener = __ -> asyncUpdatePlacementAndVisibility(); + private PaintListener widgetMovementListener = __ -> asyncExecIfOpen( + FindReplaceOverlay.this::updatePlacementAndVisibility); - private void asyncUpdatePlacementAndVisibility() { + private void asyncExecIfOpen(Runnable operation) { Shell shell = getShell(); if (shell != null) { - shell.getDisplay().asyncExec(this::updatePlacementAndVisibility); + shell.getDisplay().asyncExec(() -> { + if (getShell() != null) { + operation.run(); + } + }); } } @@ -228,17 +233,18 @@ public void shellDeactivated(ShellEvent e) { private static class TargetPartVisibilityHandler implements IPartListener2, IPageChangedListener { private final IWorkbenchPart targetPart; private final IWorkbenchPart topLevelPart; - private final Supplier shellProvider; + private final Consumer asyncExecIfOpen; private final Runnable closeCallback; private final Runnable placementUpdateCallback; private boolean isTopLevelVisible = true; private boolean isNestedLevelVisible = true; - TargetPartVisibilityHandler(IWorkbenchPart targetPart, Supplier shellProvider, Runnable closeCallback, + TargetPartVisibilityHandler(IWorkbenchPart targetPart, Consumer asyncExecIfOpen, + Runnable closeCallback, Runnable placementUpdateCallback) { this.targetPart = targetPart; - this.shellProvider = shellProvider; + this.asyncExecIfOpen = asyncExecIfOpen; this.closeCallback = closeCallback; this.placementUpdateCallback = placementUpdateCallback; if (targetPart != null && targetPart.getSite() instanceof MultiPageEditorSite multiEditorSite) { @@ -252,7 +258,7 @@ private static class TargetPartVisibilityHandler implements IPartListener2, IPag public void partBroughtToTop(IWorkbenchPartReference partRef) { if (partRef.getPart(false) == topLevelPart && !isTopLevelVisible) { this.isTopLevelVisible = true; - shellProvider.get().getDisplay().asyncExec(this::adaptToPartActivationChange); + asyncExecIfOpen.accept(this::adaptToPartActivationChange); } } @@ -260,7 +266,7 @@ public void partBroughtToTop(IWorkbenchPartReference partRef) { public void partVisible(IWorkbenchPartReference partRef) { if (partRef.getPart(false) == topLevelPart && !isTopLevelVisible) { this.isTopLevelVisible = true; - shellProvider.get().getDisplay().asyncExec(this::adaptToPartActivationChange); + asyncExecIfOpen.accept(this::adaptToPartActivationChange); } } @@ -268,7 +274,7 @@ public void partVisible(IWorkbenchPartReference partRef) { public void partHidden(IWorkbenchPartReference partRef) { if (partRef.getPart(false) == topLevelPart && isTopLevelVisible) { this.isTopLevelVisible = false; - shellProvider.get().getDisplay().asyncExec(this::adaptToPartActivationChange); + asyncExecIfOpen.accept(this::adaptToPartActivationChange); } } @@ -285,13 +291,13 @@ public void pageChanged(PageChangedEvent event) { boolean isPageVisible = event.getSelectedPage() == targetPart; if (isNestedLevelVisible != isPageVisible) { this.isNestedLevelVisible = isPageVisible; - shellProvider.get().getDisplay().asyncExec(this::adaptToPartActivationChange); + asyncExecIfOpen.accept(this::adaptToPartActivationChange); } } } private void adaptToPartActivationChange() { - if (shellProvider.get() == null || targetPart.getSite().getPart() == null) { + if (targetPart.getSite().getPart() == null) { return; } placementUpdateCallback.run(); @@ -299,12 +305,12 @@ private void adaptToPartActivationChange() { if (!isTargetVisible()) { targetPart.getSite().getShell().setActive(); targetPart.setFocus(); - shellProvider.get().getDisplay().asyncExec(this::focusTargetWidget); + asyncExecIfOpen.accept(this::focusTargetWidget); } } private void focusTargetWidget() { - if (shellProvider.get() == null || targetPart.getSite().getPart() == null) { + if (targetPart.getSite().getPart() == null) { return; } if (targetPart instanceof StatusTextEditor textEditor) { @@ -845,7 +851,7 @@ private void updatePlacementAndVisibility() { return; } if (isInvalidTargetShell()) { - getShell().getDisplay().asyncExec(() -> { + asyncExecIfOpen(() -> { if (isInvalidTargetShell()) { close(); setParentShell(targetPart.getSite().getShell()); @@ -883,7 +889,7 @@ private boolean isInvalidTargetShell() { if (isInvalidTargetPart()) { return false; } - return getShell() == null || !targetPart.getSite().getShell().equals(getShell().getParent()); + return !targetPart.getSite().getShell().equals(getShell().getParent()); } private Rectangle calculateAbsoluteControlBounds(Control control) { From cd670d0680ee3b58f1aec88e3e4c541eab937911 Mon Sep 17 00:00:00 2001 From: Heiko Klare Date: Sun, 18 Aug 2024 12:51:27 +0200 Subject: [PATCH 17/23] Find/replace: keep find/replace input state in FindReplaceLogic The FindReplaceLogic is a stateful representation of a search context for a search target. It currently already has a state of the used search options. Still, the find and replace string used for performing operations on the target are passed to each operation instead of keeping it in the logic object itself. Keeping this information in the logic object simplifies its interface and allows to encapsulate more of the incremental logic in the logic class, as it then has the information about when the search input changes. In addition, it works towards a clear model (logic) and view (dialog/overlay) separation. This change adds the find and replace input string state to the FindReplaceLogic class and simplifies its interface accordingly. --- .../findandreplace/FindReplaceLogic.java | 140 +++++----- .../findandreplace/IFindReplaceLogic.java | 62 ++--- .../overlay/FindReplaceOverlay.java | 28 +- .../ui/texteditor/FindReplaceDialog.java | 85 ++++-- .../findandreplace/FindReplaceLogicTest.java | 249 +++++++++++------- .../findandreplace/overlay/OverlayAccess.java | 2 +- .../texteditor/tests/DialogAccess.java | 1 + 7 files changed, 326 insertions(+), 241 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 8099d514bb2..1288a2f5328 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 @@ -17,6 +17,7 @@ import java.util.ArrayList; import java.util.HashSet; import java.util.List; +import java.util.Objects; import java.util.Set; import java.util.regex.Pattern; import java.util.regex.PatternSyntaxException; @@ -54,6 +55,22 @@ public class FindReplaceLogic implements IFindReplaceLogic { private boolean isTargetEditable; private Set searchOptions = new HashSet<>(); + private String findString = ""; //$NON-NLS-1$ + private String replaceString = ""; //$NON-NLS-1$ + + @Override + public void setFindString(String findString) { + this.findString = Objects.requireNonNull(findString); + if (isIncrementalSearchAvailable() && isActive(SearchOptions.INCREMENTAL)) { + performSearch(true); + } + } + + @Override + public void setReplaceString(String replaceString) { + this.replaceString = Objects.requireNonNull(replaceString); + } + @Override public void activate(SearchOptions searchOption) { if (!searchOptions.add(searchOption)) { @@ -118,7 +135,7 @@ public boolean isIncrementalSearchAvailable() { } @Override - public boolean isWholeWordSearchAvailable(String findString) { + public boolean isWholeWordSearchAvailable() { return !isRegExSearchAvailableAndActive() && isWord(findString); } /** @@ -213,12 +230,12 @@ private IEditorStatusLine getStatusLineManager() { } @Override - public void performReplaceAll(String findString, String replaceString) { + public void performReplaceAll() { resetStatus(); if (findString != null && !findString.isEmpty()) { try { - int replaceCount = replaceAll(findString, replaceString == null ? "" : replaceString); //$NON-NLS-1$ + int replaceCount = replaceAll(); if (replaceCount != 0) { if (replaceCount == 1) { // not plural statusLineMessage(FindReplaceMessages.FindReplace_Status_replacement_label); @@ -243,12 +260,12 @@ public void performReplaceAll(String findString, String replaceString) { } @Override - public void performSelectAll(String findString) { + public void performSelectAll() { resetStatus(); if (findString != null && !findString.isEmpty()) { try { - int selectCount = selectAll(findString); + int selectCount = selectAll(); if (selectCount != 0) { if (selectCount == 1) { // not plural statusLineMessage(FindReplaceMessages.FindReplace_Status_selection_label); @@ -293,43 +310,12 @@ private boolean prepareTargetForEditing() { return isEditable(); } - /** - * Replaces the current selection of the target with the user's replace string. - * - * @param replaceString the String to replace the selection with - * - * @return true if the operation was successful - */ - private boolean replaceSelection(String replaceString) { - - if (!prepareTargetForEditing()) { - return false; - } - - if (replaceString == null) { - replaceString = ""; //$NON-NLS-1$ - } - - boolean replaced; - try { - replaceSelection(replaceString, isRegExSearchAvailableAndActive()); - replaced = true; - } catch (PatternSyntaxException ex) { - status = new InvalidRegExStatus(ex); - replaced = false; - } catch (IllegalStateException ex) { - replaced = false; - } - - return replaced; - } - @Override - public boolean performSearch(String findString) { - return performSearch(findString, true); + public boolean performSearch() { + return performSearch(true); } - private boolean performSearch(String findString, boolean validateSearchOptions) { + private boolean performSearch(boolean validateSearchOptions) { resetStatus(); if (validateSearchOptions && (isActive(SearchOptions.INCREMENTAL) && !isIncrementalSearchAvailable())) { @@ -340,7 +326,7 @@ private boolean performSearch(String findString, boolean validateSearchOptions) if (findString != null && !findString.isEmpty()) { try { - somethingFound = findNext(findString); + somethingFound = findNext(); } catch (PatternSyntaxException ex) { status = new InvalidRegExStatus(ex); } catch (IllegalStateException ex) { @@ -354,13 +340,10 @@ private boolean performSearch(String findString, boolean validateSearchOptions) * Replaces all occurrences of the user's findString with the replace string. * Returns the number of replacements that occur. * - * @param findString the string to search for - * @param replaceString the replacement string - * expression * @return the number of occurrences * */ - private int replaceAll(String findString, String replaceString) { + private int replaceAll() { if (!prepareTargetForEditing()) { return 0; } @@ -369,8 +352,8 @@ private int replaceAll(String findString, String replaceString) { executeInForwardMode(() -> { executeWithReplaceAllEnabled(() -> { Point currentSelection = new Point(0, 0); - while (findAndSelect(currentSelection.x + currentSelection.y, findString) != -1) { - currentSelection = replaceSelection(replaceString, isRegExSearchAvailableAndActive()); + while (findAndSelect(currentSelection.x + currentSelection.y) != -1) { + currentSelection = replaceSelection(); replacements.add(currentSelection); } }); @@ -405,14 +388,13 @@ private void executeWithReplaceAllEnabled(Runnable runnable) { } /** - * @param findString the string to select as part of the search * @return the number of selected elements */ - private int selectAll(String findString) { + private int selectAll() { List selections = new ArrayList<>(); executeInForwardMode(() -> { Point currentSeletion = new Point(0, 0); - while (findAndSelect(currentSeletion.x + currentSeletion.y, findString) != -1) { + while (findAndSelect(currentSeletion.x + currentSeletion.y) != -1) { currentSeletion = target.getSelection(); selections.add(currentSeletion); } @@ -429,18 +411,17 @@ private int selectAll(String findString) { * Returns the position of the specified search string, or -1 if * the string can not be found when searching using the given options. * - * @param findString the string to search for * @param startPosition the position at which to start the search * @return the occurrence of the find string following the options or * -1 if nothing found */ - private int findIndex(String findString, int startPosition) { + private int findIndex(int startPosition) { int index = 0; if (isActive(SearchOptions.FORWARD)) { - index = findAndSelect(startPosition, findString); + index = findAndSelect(startPosition); } else { index = startPosition == 0 ? -1 - : findAndSelect(startPosition - 1, findString); + : findAndSelect(startPosition - 1); } if (index == -1) { @@ -448,7 +429,7 @@ private int findIndex(String findString, int startPosition) { if (isActive(SearchOptions.WRAP)) { statusLineMessage(FindReplaceMessages.FindReplace_Status_wrapped_label); status = new FindStatus(FindStatus.StatusCode.WRAPPED); - index = findAndSelect(-1, findString); + index = findAndSelect(-1); } else { status = new FindStatus(FindStatus.StatusCode.NO_MATCH); } @@ -457,8 +438,8 @@ private int findIndex(String findString, int startPosition) { } @Override - public int findAndSelect(int offset, String findString) { - boolean wholeWordSearch = isActive(SearchOptions.WHOLE_WORD) && isWholeWordSearchAvailable(findString); + public int findAndSelect(int offset) { + boolean wholeWordSearch = isActive(SearchOptions.WHOLE_WORD) && isWholeWordSearchAvailable(); boolean forwardSearch = isActive(SearchOptions.FORWARD); boolean caseSensitiveSearch = isActive(SearchOptions.CASE_SENSITIVE); boolean regexSearch = isActive(SearchOptions.REGEX); @@ -473,20 +454,17 @@ public int findAndSelect(int offset, String findString) { } /** - * Replaces the selection with replaceString. If - * regExReplace is true, replaceString is - * a regex replace pattern which will get expanded if the underlying target - * supports it. Returns the region of the inserted text; note that the returned - * selection covers the expanded pattern in case of regex replace. + * Replaces the selection with the current replace string. If the regex search + * option has been activated, the replace string is considered as regex replace + * pattern which will get expanded if the underlying target supports it. Returns + * the region of the inserted text; note that the returned selection covers the + * expanded pattern in case of regex replace. * - * @param replaceString the replace string (or a regex pattern) - * @param regExReplace true if replaceString is a - * pattern * @return the selection after replacing, i.e. the inserted text */ - private Point replaceSelection(String replaceString, boolean regExReplace) { + private Point replaceSelection() { if (target instanceof IFindReplaceTargetExtension3) - ((IFindReplaceTargetExtension3) target).replaceSelection(replaceString, regExReplace); + ((IFindReplaceTargetExtension3) target).replaceSelection(replaceString, isRegExSearchAvailableAndActive()); else target.replaceSelection(replaceString); @@ -497,12 +475,11 @@ private Point replaceSelection(String replaceString, boolean regExReplace) { * Returns whether the specified search string can be found using the given * options. * - * @param findString the string to search for * @return true if the search string can be found using the given * options * */ - private boolean findNext(String findString) { + private boolean findNext() { if (target == null) { return false; @@ -510,7 +487,7 @@ private boolean findNext(String findString) { int findReplacePosition = calculateFindBeginningOffset(); - int index = findIndex(findString, findReplacePosition); + int index = findIndex(findReplacePosition); if (index == -1) { String msg = NLSUtility.format(FindReplaceMessages.FindReplace_Status_noMatchWithValue_label, findString); @@ -544,28 +521,37 @@ private int calculateFindBeginningOffset() { } @Override - public boolean performReplaceAndFind(String findString, String replaceString) { + public boolean performReplaceAndFind() { resetStatus(); - if (performSelectAndReplace(findString, replaceString)) { - performSearch(findString, false); + if (performSelectAndReplace()) { + performSearch(false); return true; } return false; } @Override - public boolean performSelectAndReplace(String findString, String replaceString) { + public boolean performSelectAndReplace() { resetStatus(); - if (!isFindStringSelected(findString)) { - performSearch(findString, false); + if (!isFindStringSelected()) { + performSearch(false); } if (getStatus().wasSuccessful()) { - return replaceSelection(replaceString); + if (!prepareTargetForEditing()) { + return false; + } + try { + replaceSelection(); + return true; + } catch (PatternSyntaxException ex) { + status = new InvalidRegExStatus(ex); + } catch (IllegalStateException ex) { + } } return false; } - private boolean isFindStringSelected(String findString) { + private boolean isFindStringSelected() { String selectedString = getCurrentSelection(); if (isRegExSearchAvailableAndActive()) { int patternFlags = 0; diff --git a/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/IFindReplaceLogic.java b/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/IFindReplaceLogic.java index 6bb72d363f3..d0eb24b9ce1 100644 --- a/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/IFindReplaceLogic.java +++ b/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/IFindReplaceLogic.java @@ -25,6 +25,20 @@ */ public interface IFindReplaceLogic { + /** + * Sets the string to be used for searching in find and replace operations. + * + * @param findString the find string to use, must not be null + */ + public void setFindString(String findString); + + /** + * Sets the string to be used as a replacement in replace operations. + * + * @param replaceString the replace string to use, must not be null + */ + public void setReplaceString(String replaceString); + /** * Activate a search option * @@ -71,65 +85,54 @@ public interface IFindReplaceLogic { public boolean isIncrementalSearchAvailable(); /** - * Replaces all occurrences of the user's findString with the replace string. + * Replaces all occurrences of the current find string with the replace string. * Indicate to the user the number of replacements that occur. - * - * @param findString The string that will be replaced - * @param replaceString The string that will replace the findString */ - public void performReplaceAll(String findString, String replaceString); + public void performReplaceAll(); /** - * Selects all occurrences of findString. - * - * @param findString The String to find and select + * Selects all occurrences of the current find string. */ - public void performSelectAll(String findString); + public void performSelectAll(); /** - * Locates the user's findString in the target. If incremental search is + * Locates the current find string in the target. If incremental search is * activated, the search will be performed starting from an incremental search * position, which can be reset using {@link #resetIncrementalBaseLocation()}. * If incremental search is activated and RegEx search is activated, nothing * happens. * - * @param searchString the String to search for * @return Whether the string was found in the target - * */ - public boolean performSearch(String searchString); + public boolean performSearch(); /** - * Searches for a string starting at the given offset and using the specified - * search directives. If a string has been found it is selected and its start - * offset is returned. + * Searches for the current find string starting at the given offset and using + * the specified search directives. If a string has been found it is selected + * and its start offset is returned. * - * @param offset the offset at which searching starts - * @param findString the string which should be found + * @param offset the offset at which searching starts * @return the position of the specified string, or -1 if the string has not * been found */ - public int findAndSelect(int offset, String findString); + public int findAndSelect(int offset); /** - * Replaces the selection and jumps to the next occurrence of findString - * instantly. Will not fail in case the selection is invalidated, eg. after a - * replace operation or after the target was updated + * Replaces the current selection if it matches the find string or performs a + * search and does a replacement. It then performs another search for the + * current find string. Will not fail in case the selection is invalidated, + * e.g., after a replace operation or after the target was updated. * - * @param findString the string to replace - * @param replaceString the string to put in place of findString * @return whether a replacement has been performed */ - public boolean performReplaceAndFind(String findString, String replaceString); + public boolean performReplaceAndFind(); /** * Selects first and then replaces the next occurrence. * - * @param findString the string to replace - * @param replaceString the new string that will replace the findString * @return whether a replacement has been performed */ - public boolean performSelectAndReplace(String findString, String replaceString); + public boolean performSelectAndReplace(); /** * Updates the target on which to perform Find/Replace-operations on. @@ -158,10 +161,9 @@ public interface IFindReplaceLogic { * false if not. Searching for whole words requires the given find * string to be an entire word and the regex search option to be disabled. * - * @param findString the string that is currently being searched for. * @return true if the search can be restricted to whole words */ - public boolean isWholeWordSearchAvailable(String findString); + public boolean isWholeWordSearchAvailable(); /** * Initializes the anchor used as the starting point for incremental searching. diff --git a/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlay.java b/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlay.java index 9a733b6a94b..2747b861dfd 100644 --- a/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlay.java +++ b/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlay.java @@ -180,7 +180,7 @@ private void createFindReplaceLogic(IFindReplaceTarget target) { private void performReplaceAll() { BusyIndicator.showWhile(getShell() != null ? getShell().getDisplay() : Display.getCurrent(), - () -> findReplaceLogic.performReplaceAll(getFindString(), getReplaceString())); + findReplaceLogic::performReplaceAll); evaluateFindReplaceStatus(); replaceBar.storeHistory(); searchBar.storeHistory(); @@ -188,7 +188,7 @@ private void performReplaceAll() { private void performSelectAll() { BusyIndicator.showWhile(getShell() != null ? getShell().getDisplay() : Display.getCurrent(), - () -> findReplaceLogic.performSelectAll(getFindString())); + findReplaceLogic::performSelectAll); searchBar.storeHistory(); } @@ -556,7 +556,7 @@ private void createRegexSearchButton() { .withToolTipText(FindReplaceMessages.FindReplaceOverlay_regexSearchButton_toolTip) .withOperation(() -> { activateInFindReplacerIf(SearchOptions.REGEX, regexSearchButton.getSelection()); - wholeWordSearchButton.setEnabled(findReplaceLogic.isWholeWordSearchAvailable(getFindString())); + wholeWordSearchButton.setEnabled(findReplaceLogic.isWholeWordSearchAvailable()); updateIncrementalSearch(); updateContentAssistAvailability(); }).withShortcuts(KeyboardShortcuts.OPTION_REGEX).build(); @@ -632,9 +632,10 @@ private void createSearchBar() { searchBar.forceFocus(); searchBar.selectAll(); searchBar.addModifyListener(e -> { - wholeWordSearchButton.setEnabled(findReplaceLogic.isWholeWordSearchAvailable(getFindString())); + wholeWordSearchButton.setEnabled(findReplaceLogic.isWholeWordSearchAvailable()); showUserFeedback(normalTextForegroundColor, true); + findReplaceLogic.setFindString(searchBar.getText()); updateIncrementalSearch(); }); searchBar.addFocusListener(new FocusListener() { @@ -655,7 +656,7 @@ public void focusLost(FocusEvent e) { } private void updateIncrementalSearch() { - findReplaceLogic.performSearch(getFindString()); + findReplaceLogic.performSearch(); evaluateFindReplaceStatus(); } @@ -664,6 +665,9 @@ private void createReplaceBar() { replaceBar = new HistoryTextWrapper(replaceHistory, replaceBarContainer, SWT.SINGLE); GridDataFactory.fillDefaults().grab(true, false).align(SWT.FILL, SWT.END).applyTo(replaceBar); replaceBar.setMessage(FindReplaceMessages.FindReplaceOverlay_replaceBar_message); + replaceBar.addModifyListener(e -> { + findReplaceLogic.setReplaceString(replaceBar.getText()); + }); replaceBar.addFocusListener(FocusListener.focusLostAdapter(e -> { replaceBar.setForeground(normalTextForegroundColor); searchBar.setForeground(normalTextForegroundColor); @@ -950,16 +954,8 @@ private String getFindString() { return searchBar.getText(); } - private String getReplaceString() { - if (!okayToUse(replaceBar)) { - return ""; //$NON-NLS-1$ - } - return replaceBar.getText(); - - } - private void performSingleReplace() { - findReplaceLogic.performReplaceAndFind(getFindString(), getReplaceString()); + findReplaceLogic.performReplaceAndFind(); evaluateFindReplaceStatus(); replaceBar.storeHistory(); searchBar.storeHistory(); @@ -969,7 +965,7 @@ private void performSearch(boolean forward) { boolean oldForwardSearchSetting = findReplaceLogic.isActive(SearchOptions.FORWARD); activateInFindReplacerIf(SearchOptions.FORWARD, forward); findReplaceLogic.deactivate(SearchOptions.INCREMENTAL); - findReplaceLogic.performSearch(getFindString()); + findReplaceLogic.performSearch(); activateInFindReplacerIf(SearchOptions.FORWARD, oldForwardSearchSetting); findReplaceLogic.activate(SearchOptions.INCREMENTAL); evaluateFindReplaceStatus(); @@ -986,7 +982,7 @@ private void updateFromTargetSelection() { selectionText = FindReplaceDocumentAdapter.escapeForRegExPattern(selectionText); } searchBar.setText(selectionText); - findReplaceLogic.findAndSelect(findReplaceLogic.getTarget().getSelection().x, searchBar.getText()); + findReplaceLogic.findAndSelect(findReplaceLogic.getTarget().getSelection().x); } searchBar.setSelection(0, searchBar.getText().length()); } diff --git a/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/texteditor/FindReplaceDialog.java b/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/texteditor/FindReplaceDialog.java index e7ec0f682f2..157de2177b0 100644 --- a/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/texteditor/FindReplaceDialog.java +++ b/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/texteditor/FindReplaceDialog.java @@ -117,34 +117,35 @@ public void shellDeactivated(ShellEvent e) { } - private final FindModifyListener fFindModifyListener = new FindModifyListener(); - /** - * Modify listener to update the search result in case of incremental search. + * Modify listener to update the find logic with the current input and update + * the search result in case of incremental search. * * @since 2.0 */ - private class FindModifyListener implements ModifyListener { + private class InputModifyListener implements ModifyListener { + + private Runnable modificationHandler; // XXX: Workaround for Combo bug on Linux (see bug 404202 and bug 410603) private boolean fIgnoreNextEvent; + private InputModifyListener(Runnable modificationHandler) { + this.modificationHandler = modificationHandler; + } + private void ignoreNextEvent() { fIgnoreNextEvent = true; } @Override public void modifyText(ModifyEvent e) { - // XXX: Workaround for Combo bug on Linux (see bug 404202 and bug 410603) if (fIgnoreNextEvent) { fIgnoreNextEvent = false; return; } - - if (findReplaceLogic.isActive(SearchOptions.INCREMENTAL)) { - findReplaceLogic.performSearch(getFindString()); - } + modificationHandler.run(); evaluateFindReplaceStatus(); updateButtonState(!findReplaceLogic.isActive(SearchOptions.INCREMENTAL)); @@ -175,6 +176,7 @@ public void modifyText(ModifyEvent e) { private Button fReplaceSelectionButton, fReplaceFindButton, fFindNextButton, fReplaceAllButton, fSelectAllButton; private Combo fFindField, fReplaceField; + private InputModifyListener fFindModifyListener, fReplaceModifyListener; /** * Find and replace command adapters. @@ -260,7 +262,9 @@ public void create() { fFindField.removeModifyListener(fFindModifyListener); updateCombo(fFindField, findHistory.get()); fFindField.addModifyListener(fFindModifyListener); + fReplaceField.removeModifyListener(fReplaceModifyListener); updateCombo(fReplaceField, replaceHistory.get()); + fReplaceField.addModifyListener(fReplaceModifyListener); // get find string initFindString(); @@ -298,7 +302,7 @@ public void widgetSelected(SelectionEvent e) { activateInFindReplaceLogicIf(SearchOptions.FORWARD, eventRequiresInverseSearchDirection != forwardSearchActivated); findReplaceLogic.deactivate(SearchOptions.INCREMENTAL); - boolean somethingFound = findReplaceLogic.performSearch(getFindString()); + boolean somethingFound = findReplaceLogic.performSearch(); activateInFindReplaceLogicIf(SearchOptions.INCREMENTAL, incrementalSearchActivated); activateInFindReplaceLogicIf(SearchOptions.FORWARD, forwardSearchActivated); @@ -315,7 +319,7 @@ public void widgetSelected(SelectionEvent e) { @Override public void widgetSelected(SelectionEvent e) { BusyIndicator.showWhile(fActiveShell != null ? fActiveShell.getDisplay() : Display.getCurrent(), - () -> findReplaceLogic.performSelectAll(getFindString())); + findReplaceLogic::performSelectAll); writeSelection(); updateButtonState(); updateFindAndReplaceHistory(); @@ -331,7 +335,7 @@ public void widgetSelected(SelectionEvent e) { new SelectionAdapter() { @Override public void widgetSelected(SelectionEvent e) { - if (findReplaceLogic.performReplaceAndFind(getFindString(), getReplaceString())) { + if (findReplaceLogic.performReplaceAndFind()) { writeSelection(); } updateButtonState(); @@ -345,7 +349,7 @@ public void widgetSelected(SelectionEvent e) { false, new SelectionAdapter() { @Override public void widgetSelected(SelectionEvent e) { - if (findReplaceLogic.performSelectAndReplace(getFindString(), getReplaceString())) { + if (findReplaceLogic.performSelectAndReplace()) { writeSelection(); } updateButtonState(); @@ -360,7 +364,7 @@ public void widgetSelected(SelectionEvent e) { @Override public void widgetSelected(SelectionEvent e) { BusyIndicator.showWhile(fActiveShell != null ? fActiveShell.getDisplay() : Display.getCurrent(), - () -> findReplaceLogic.performReplaceAll(getFindString(), getReplaceString())); + () -> findReplaceLogic.performReplaceAll()); writeSelection(); updateButtonState(); updateFindAndReplaceHistory(); @@ -632,6 +636,14 @@ private Composite createInputPanel(Composite parent) { ITextEditorActionDefinitionIds.CONTENT_ASSIST_PROPOSALS, new char[0], true); setGridData(fFindField, SWT.FILL, true, SWT.CENTER, false); addDecorationMargin(fFindField); + fFindModifyListener = new InputModifyListener(() -> { + if (okToUse(fFindField)) { + findReplaceLogic.setFindString(fFindField.getText()); + if (findReplaceLogic.isActive(SearchOptions.INCREMENTAL)) { + findReplaceLogic.performSearch(); + } + } + }); fFindField.addModifyListener(fFindModifyListener); fReplaceLabel = new Label(panel, SWT.LEFT); @@ -646,6 +658,12 @@ private Composite createInputPanel(Composite parent) { ITextEditorActionDefinitionIds.CONTENT_ASSIST_PROPOSALS, new char[0], true); setGridData(fReplaceField, SWT.FILL, true, SWT.CENTER, false); addDecorationMargin(fReplaceField); + fReplaceModifyListener = new InputModifyListener(() -> { + if (okToUse(fReplaceField)) { + findReplaceLogic.setReplaceString(fReplaceField.getText()); + } + }); + fReplaceField.addModifyListener(fReplaceModifyListener); fReplaceField.addModifyListener(listener); return panel; @@ -741,7 +759,7 @@ public void widgetSelected(SelectionEvent e) { } }); storeButtonWithMnemonicInMap(fIsRegExCheckBox); - fWholeWordCheckBox.setEnabled(findReplaceLogic.isWholeWordSearchAvailable(getFindString())); + fWholeWordCheckBox.setEnabled(findReplaceLogic.isWholeWordSearchAvailable()); fWholeWordCheckBox.addSelectionListener(new SelectionAdapter() { @Override public void widgetSelected(SelectionEvent e) { @@ -859,11 +877,13 @@ public boolean close() { * Removes focus changed listener from browser and stores settings for re-open. */ private void handleDialogClose() { - // remove listeners if (okToUse(fFindField)) { fFindField.removeModifyListener(fFindModifyListener); } + if (okToUse(fReplaceField)) { + fReplaceField.removeModifyListener(fReplaceModifyListener); + } if (fParentShell != null) { fParentShell.removeShellListener(fActivationListener); @@ -916,14 +936,12 @@ private void initFindString() { return; } - fFindField.removeModifyListener(fFindModifyListener); if (hasTargetSelection()) { initFindStringFromSelection(); } else { initFindStringFromHistory(); } fFindField.setSelection(new Point(0, fFindField.getText().length())); - fFindField.addModifyListener(fFindModifyListener); } private boolean hasTargetSelection() { @@ -947,7 +965,7 @@ private void initFindStringFromSelection() { if (isSingleLineInput) { // initialize search with current selection to allow for execution of replace // operations - findReplaceLogic.findAndSelect(findReplaceLogic.getTarget().getSelection().x, fFindField.getText()); + findReplaceLogic.findAndSelect(findReplaceLogic.getTarget().getSelection().x); } else { fGlobalRadioButton.setSelection(false); fSelectedRangeRadioButton.setSelection(true); @@ -1074,7 +1092,7 @@ private void updateButtonState(boolean disableReplace) { boolean isSelectionGoodForReplace = selectionString != "" //$NON-NLS-1$ || !isRegExSearchAvailableAndActive; - fWholeWordCheckBox.setEnabled(findReplaceLogic.isWholeWordSearchAvailable(getFindString())); + fWholeWordCheckBox.setEnabled(findReplaceLogic.isWholeWordSearchAvailable()); fFindNextButton.setEnabled(enable && isFindStringSet); fSelectAllButton.setEnabled(enable && isFindStringSet && (target instanceof IFindReplaceTargetExtension4)); fReplaceSelectionButton.setEnabled( @@ -1106,10 +1124,7 @@ private void updateCombo(Combo combo, Iterable content) { */ private void updateFindAndReplaceHistory() { updateFindHistory(); - if (okToUse(fReplaceField)) { - updateHistory(fReplaceField, replaceHistory); - } - + updateReplaceHistory(); } /** @@ -1120,14 +1135,32 @@ private void updateFindHistory() { fFindField.removeModifyListener(fFindModifyListener); // XXX: Workaround for Combo bug on Linux (see bug 404202 and bug 410603) - if (Util.isLinux()) + if (Util.isLinux()) { fFindModifyListener.ignoreNextEvent(); + } updateHistory(fFindField, findHistory); fFindField.addModifyListener(fFindModifyListener); } } + /** + * Called after executed find action to update the history. + */ + private void updateReplaceHistory() { + if (okToUse(fReplaceField)) { + fReplaceField.removeModifyListener(fReplaceModifyListener); + + // XXX: Workaround for Combo bug on Linux (see bug 404202 and bug 410603) + if (Util.isLinux()) { + fReplaceModifyListener.ignoreNextEvent(); + } + + updateHistory(fReplaceField, replaceHistory); + fReplaceField.addModifyListener(fReplaceModifyListener); + } + } + /** * Updates the combo with the history. * @@ -1170,7 +1203,7 @@ public void updateTarget(IFindReplaceTarget target, boolean isTargetEditable, bo } if (okToUse(fWholeWordCheckBox)) { - fWholeWordCheckBox.setEnabled(findReplaceLogic.isWholeWordSearchAvailable(getFindString())); + fWholeWordCheckBox.setEnabled(findReplaceLogic.isWholeWordSearchAvailable()); } if (okToUse(fIncrementalCheckBox)) { 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 67181162ae6..430e7d415cf 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 @@ -21,6 +21,7 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyInt; @@ -30,6 +31,8 @@ import static org.mockito.Mockito.when; import static org.mockito.Mockito.withSettings; +import java.util.function.Predicate; + import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -78,6 +81,12 @@ private TextViewer setupTextViewer(String contentText) { return textViewer; } + + private void setFindAndReplaceString(IFindReplaceLogic findReplaceLogic, String findString, String replaceString) { + findReplaceLogic.setFindString(findString); + findReplaceLogic.setReplaceString(replaceString); + } + @After public void disposeShell() { if (parentShell != null) { @@ -117,37 +126,43 @@ public void testPerformReplaceAllForwards() { private void performReplaceAllBaseTestcases(IFindReplaceLogic findReplaceLogic, TextViewer textViewer) { textViewer.setDocument(new Document("aaaa")); - findReplaceLogic.performReplaceAll("a", "b"); + setFindAndReplaceString(findReplaceLogic, "a", "b"); + findReplaceLogic.performReplaceAll(); assertThat(textViewer.getDocument().get(), equalTo("bbbb")); expectStatusIsReplaceAllWithCount(findReplaceLogic, 4); - findReplaceLogic.performReplaceAll("b", "aa"); + setFindAndReplaceString(findReplaceLogic, "b", "aa"); + findReplaceLogic.performReplaceAll(); assertThat(textViewer.getDocument().get(), equalTo("aaaaaaaa")); expectStatusIsReplaceAllWithCount(findReplaceLogic, 4); - findReplaceLogic.performReplaceAll("b", "c"); + setFindAndReplaceString(findReplaceLogic, "b", "c"); + findReplaceLogic.performReplaceAll(); assertThat(textViewer.getDocument().get(), equalTo("aaaaaaaa")); expectStatusIsCode(findReplaceLogic, FindStatus.StatusCode.NO_MATCH); - findReplaceLogic.performReplaceAll("aaaaaaaa", "d"); // https://github.com/eclipse-platform/eclipse.platform.ui/issues/1203 + setFindAndReplaceString(findReplaceLogic, "aaaaaaaa", "d"); + findReplaceLogic.performReplaceAll(); // https://github.com/eclipse-platform/eclipse.platform.ui/issues/1203 assertThat(textViewer.getDocument().get(), equalTo("d")); expectStatusIsReplaceAllWithCount(findReplaceLogic, 1); - findReplaceLogic.performReplaceAll("d", null); + setFindAndReplaceString(findReplaceLogic, "d", ""); + findReplaceLogic.performReplaceAll(); assertThat(textViewer.getDocument().get(), equalTo("")); expectStatusIsReplaceAllWithCount(findReplaceLogic, 1); textViewer.getDocument().set("f"); - findReplaceLogic.performReplaceAll("f", ""); + setFindAndReplaceString(findReplaceLogic, "f", ""); + findReplaceLogic.performReplaceAll(); assertThat(textViewer.getDocument().get(), equalTo("")); expectStatusIsReplaceAllWithCount(findReplaceLogic, 1); - IFindReplaceTarget mockFindReplaceTarget= Mockito.mock(IFindReplaceTarget.class); Mockito.when(mockFindReplaceTarget.isEditable()).thenReturn(false); findReplaceLogic.updateTarget(mockFindReplaceTarget, false); - findReplaceLogic.performReplaceAll("a", "b"); + setFindAndReplaceString(findReplaceLogic, "a", "b"); + findReplaceLogic.performReplaceAll(); expectStatusIsCode(findReplaceLogic, FindStatus.StatusCode.NO_MATCH); } @@ -158,20 +173,22 @@ public void testPerformReplaceAllForwardRegEx() { findReplaceLogic.activate(SearchOptions.REGEX); findReplaceLogic.activate(SearchOptions.FORWARD); - findReplaceLogic.performReplaceAll(".+\\@.+\\.com", ""); + setFindAndReplaceString(findReplaceLogic, ".+\\@.+\\.com", ""); + findReplaceLogic.performReplaceAll(); assertThat(textViewer.getDocument().get(), equalTo(" looks.almost@like_an_email")); expectStatusIsReplaceAllWithCount(findReplaceLogic, 1); - findReplaceLogic.performReplaceAll("( looks.)|(like_)", ""); + setFindAndReplaceString(findReplaceLogic, "( looks.)|(like_)", ""); + findReplaceLogic.performReplaceAll(); assertThat(textViewer.getDocument().get(), equalTo("almost@an_email")); expectStatusIsReplaceAllWithCount(findReplaceLogic, 2); - findReplaceLogic.performReplaceAll("[", ""); + setFindAndReplaceString(findReplaceLogic, "[", ""); + findReplaceLogic.performReplaceAll(); assertThat(textViewer.getDocument().get(), equalTo("almost@an_email")); expectStatusIsMessageWithString(findReplaceLogic, "Unclosed character class near index 0" + lineSeparator() + "[" + lineSeparator() + "^"); - } @Test @@ -181,15 +198,18 @@ public void testPerformReplaceAllForward() { findReplaceLogic.activate(SearchOptions.REGEX); findReplaceLogic.activate(SearchOptions.FORWARD); - findReplaceLogic.performReplaceAll(".+\\@.+\\.com", ""); + setFindAndReplaceString(findReplaceLogic, ".+\\@.+\\.com", ""); + findReplaceLogic.performReplaceAll(); assertThat(textViewer.getDocument().get(), equalTo(" looks.almost@like_an_email")); expectStatusIsReplaceAllWithCount(findReplaceLogic, 1); - findReplaceLogic.performReplaceAll("( looks.)|(like_)", ""); + setFindAndReplaceString(findReplaceLogic, "( looks.)|(like_)", ""); + findReplaceLogic.performReplaceAll(); assertThat(textViewer.getDocument().get(), equalTo("almost@an_email")); expectStatusIsReplaceAllWithCount(findReplaceLogic, 2); - findReplaceLogic.performReplaceAll("[", ""); + setFindAndReplaceString(findReplaceLogic, "[", ""); + findReplaceLogic.performReplaceAll(); assertThat(textViewer.getDocument().get(), equalTo("almost@an_email")); expectStatusIsMessageWithString(findReplaceLogic, "Unclosed character class near index 0" + lineSeparator() + "[" + lineSeparator() @@ -201,13 +221,14 @@ public void testPerformSelectAndReplace() { TextViewer textViewer= setupTextViewer("HelloWorld!"); IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(textViewer); findReplaceLogic.activate(SearchOptions.FORWARD); + setFindAndReplaceString(findReplaceLogic, "", " "); - findReplaceLogic.performSearch(""); // select first, then replace. We don't need to perform a second search - findReplaceLogic.performSelectAndReplace("", " "); + findReplaceLogic.performSearch(); // select first, then replace. We don't need to perform a second search + findReplaceLogic.performSelectAndReplace(); assertThat(textViewer.getDocument().get(), equalTo("Hello World!")); expectStatusEmpty(findReplaceLogic); - findReplaceLogic.performSelectAndReplace("", " "); // perform the search yourself and replace that automatically + findReplaceLogic.performSelectAndReplace(); // perform the search yourself and replace that automatically assertThat(textViewer.getDocument().get(), equalTo("Hello World !")); expectStatusEmpty(findReplaceLogic); } @@ -218,19 +239,20 @@ public void testPerformSelectAndReplaceRegEx() { IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(textViewer); findReplaceLogic.activate(SearchOptions.FORWARD); findReplaceLogic.activate(SearchOptions.REGEX); + setFindAndReplaceString(findReplaceLogic, "<(\\w*)>", " "); - findReplaceLogic.performSearch("<(\\w*)>"); - boolean status= findReplaceLogic.performSelectAndReplace("<(\\w*)>", " "); + findReplaceLogic.performSearch(); + boolean status= findReplaceLogic.performSelectAndReplace(); assertTrue(status); assertThat(textViewer.getDocument().get(), equalTo("Hello World!")); expectStatusEmpty(findReplaceLogic); - status= findReplaceLogic.performSelectAndReplace("<(\\w*)>", " "); + status= findReplaceLogic.performSelectAndReplace(); assertTrue(status); assertThat(textViewer.getDocument().get(), equalTo("Hello World !")); expectStatusEmpty(findReplaceLogic); - status= findReplaceLogic.performSelectAndReplace("<(\\w*)>", " "); + status= findReplaceLogic.performSelectAndReplace(); 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); @@ -247,8 +269,8 @@ public void testPerformSelectAndReplaceRegExWithLinebreaks() { findReplaceLogic.activate(SearchOptions.REGEX); findReplaceLogic.deactivate(SearchOptions.WRAP); - assertTrue(findReplaceLogic.performSearch("o$")); - boolean status= findReplaceLogic.performSelectAndReplace("o$", "o!"); + setFindAndReplaceString(findReplaceLogic, "o$", "o!"); + boolean status= findReplaceLogic.performSelectAndReplace(); assertTrue(status); assertThat(textViewer.getDocument().get(), equalTo(""" Hello! @@ -256,17 +278,19 @@ public void testPerformSelectAndReplaceRegExWithLinebreaks() { !""")); expectStatusEmpty(findReplaceLogic); - status= findReplaceLogic.performSelectAndReplace(""" + setFindAndReplaceString(findReplaceLogic, """ d !""", "d!"); + status= findReplaceLogic.performSelectAndReplace(); assertTrue(status); assertThat(textViewer.getDocument().get(), equalTo(""" Hello! World!""")); expectStatusEmpty(findReplaceLogic); - status= findReplaceLogic.performSelectAndReplace(""" + setFindAndReplaceString(findReplaceLogic, """ """, " "); + status= findReplaceLogic.performSelectAndReplace(); assertEquals("Status wasn't correctly returned", false, status); assertEquals("Text shouldn't have been changed", """ Hello! @@ -280,25 +304,27 @@ public void testPerformSelectAndReplaceWithConfigurationChanges() { findReplaceLogic.activate(SearchOptions.FORWARD); findReplaceLogic.activate(SearchOptions.REGEX); - findReplaceLogic.performSearch("<(\\w*)>"); - boolean status= findReplaceLogic.performSelectAndReplace("<(\\w*)>", " "); + setFindAndReplaceString(findReplaceLogic, "<(\\w*)>", " "); + boolean status= findReplaceLogic.performSelectAndReplace(); assertTrue(status); assertThat(textViewer.getDocument().get(), equalTo("Hello World!!")); expectStatusEmpty(findReplaceLogic); + setFindAndReplaceString(findReplaceLogic, "", " "); findReplaceLogic.deactivate(SearchOptions.REGEX); - status= findReplaceLogic.performSelectAndReplace("", " "); + status= findReplaceLogic.performSelectAndReplace(); assertTrue(status); assertThat(textViewer.getDocument().get(), equalTo("Hello World !!")); expectStatusEmpty(findReplaceLogic); + setFindAndReplaceString(findReplaceLogic, "<(\\w*)>", " "); findReplaceLogic.activate(SearchOptions.REGEX); - status= findReplaceLogic.performSelectAndReplace("<(\\w*)>", " "); + status= findReplaceLogic.performSelectAndReplace(); assertTrue(status); assertThat(textViewer.getDocument().get(), equalTo("Hello World ! !")); expectStatusEmpty(findReplaceLogic); - status= findReplaceLogic.performSelectAndReplace("<(\\w*)>", " "); + status= findReplaceLogic.performSelectAndReplace(); 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); @@ -310,13 +336,14 @@ public void testPerformSelectAndReplaceBackward() { IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(textViewer); findReplaceLogic.deactivate(SearchOptions.FORWARD); findReplaceLogic.activate(SearchOptions.WRAP); // this only works if the search was wrapped + setFindAndReplaceString(findReplaceLogic, "", " "); - findReplaceLogic.performSearch(""); // select first, then replace. We don't need to perform a second search + findReplaceLogic.performSearch(); // select first, then replace. We don't need to perform a second search expectStatusIsCode(findReplaceLogic, FindStatus.StatusCode.WRAPPED); - findReplaceLogic.performSelectAndReplace("", " "); + findReplaceLogic.performSelectAndReplace(); assertThat(textViewer.getDocument().get(), equalTo("HelloWorld !")); - findReplaceLogic.performSelectAndReplace("", " "); // perform the search yourself and replace that automatically + findReplaceLogic.performSelectAndReplace(); // perform the search yourself and replace that automatically assertThat(textViewer.getDocument().get(), equalTo("Hello World !")); expectStatusEmpty(findReplaceLogic); } @@ -326,19 +353,20 @@ public void testPerformReplaceAndFind() { TextViewer textViewer= setupTextViewer("HelloWorld!"); IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(textViewer); findReplaceLogic.activate(SearchOptions.FORWARD); + setFindAndReplaceString(findReplaceLogic, "", " "); - boolean status= findReplaceLogic.performReplaceAndFind("", " "); + boolean status= findReplaceLogic.performReplaceAndFind(); assertThat(status, is(true)); assertThat(textViewer.getDocument().get(), equalTo("Hello World!")); assertThat(findReplaceLogic.getTarget().getSelectionText(), equalTo("")); expectStatusEmpty(findReplaceLogic); - status= findReplaceLogic.performReplaceAndFind("", " "); + status= findReplaceLogic.performReplaceAndFind(); assertThat(status, is(true)); assertThat(textViewer.getDocument().get(), equalTo("Hello World !")); expectStatusIsCode(findReplaceLogic, FindStatus.StatusCode.NO_MATCH); - status= findReplaceLogic.performReplaceAndFind("", " "); + status= findReplaceLogic.performReplaceAndFind(); 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); @@ -366,24 +394,28 @@ public void testPerformReplaceAndFindRegEx_incrementalActive() { } private void executeReplaceAndFindRegExTest(TextViewer textViewer, IFindReplaceLogic findReplaceLogic) { - boolean status= findReplaceLogic.performReplaceAndFind("<(\\w*)>", " "); + setFindAndReplaceString(findReplaceLogic, "<(\\w*)>", " "); + + boolean status= findReplaceLogic.performReplaceAndFind(); assertTrue(status); assertThat(textViewer.getDocument().get(), equalTo("Hello World!!")); assertThat(findReplaceLogic.getTarget().getSelectionText(), equalTo("")); expectStatusEmpty(findReplaceLogic); - status= findReplaceLogic.performReplaceAndFind("<(\\w*)>", " "); + status= findReplaceLogic.performReplaceAndFind(); assertTrue(status); assertThat(textViewer.getDocument().get(), equalTo("Hello World !!")); assertThat(findReplaceLogic.getTarget().getSelectionText(), equalTo("")); expectStatusEmpty(findReplaceLogic); - status= findReplaceLogic.performReplaceAndFind("<(\\w)>", " "); + setFindAndReplaceString(findReplaceLogic, "<(\\w)>", " "); + status= findReplaceLogic.performReplaceAndFind(); assertTrue(status); assertThat(textViewer.getDocument().get(), equalTo("Hello World ! !")); expectStatusIsCode(findReplaceLogic, FindStatus.StatusCode.NO_MATCH); - status= findReplaceLogic.performReplaceAndFind("<(\\w*)>", " "); + setFindAndReplaceString(findReplaceLogic, "<(\\w*)>", " "); + status= findReplaceLogic.performReplaceAndFind(); 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); @@ -397,10 +429,12 @@ public void testPerformSearchAndReplaceRegEx_incrementalActive() { findReplaceLogic.activate(SearchOptions.INCREMENTAL); findReplaceLogic.activate(SearchOptions.REGEX); - findReplaceLogic.performSearch("text"); + findReplaceLogic.setFindString("text"); + findReplaceLogic.performSearch(); textViewer.setSelectedRange(0, 0); - findReplaceLogic.performSelectAndReplace("text", ""); + findReplaceLogic.setReplaceString(""); + findReplaceLogic.performSelectAndReplace(); assertEquals("some ", textViewer.getDocument().get()); } @@ -411,15 +445,18 @@ public void testPerformSelectAllForward() { IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(textViewer); findReplaceLogic.activate(SearchOptions.FORWARD); - findReplaceLogic.performSelectAll("c"); + findReplaceLogic.setFindString("c"); + findReplaceLogic.performSelectAll(); expectStatusIsCode(findReplaceLogic, FindStatus.StatusCode.NO_MATCH); - findReplaceLogic.performSelectAll("b"); + findReplaceLogic.setFindString("b"); + findReplaceLogic.performSelectAll(); expectStatusIsFindAllWithCount(findReplaceLogic, 4); // I don't have access to getAllSelectionPoints or similar (not yet implemented), so I cannot really test for correct behavior // related to https://github.com/eclipse-platform/eclipse.platform.ui/issues/1047 - findReplaceLogic.performSelectAll("AbAbAbAb"); + findReplaceLogic.setFindString("AbAbAbAb"); + findReplaceLogic.performSelectAll(); expectStatusIsFindAllWithCount(findReplaceLogic, 1); } @@ -430,13 +467,16 @@ public void testPerformSelectAllRegEx() { findReplaceLogic.activate(SearchOptions.FORWARD); findReplaceLogic.activate(SearchOptions.REGEX); - findReplaceLogic.performSelectAll("c.*"); + findReplaceLogic.setFindString("c.*"); + findReplaceLogic.performSelectAll(); expectStatusIsCode(findReplaceLogic, FindStatus.StatusCode.NO_MATCH); - findReplaceLogic.performSelectAll("(Ab)*"); + findReplaceLogic.setFindString("(Ab)*"); + findReplaceLogic.performSelectAll(); expectStatusIsFindAllWithCount(findReplaceLogic, 1); - findReplaceLogic.performSelectAll("Ab(Ab)+Ab(Ab)+(Ab)+"); + findReplaceLogic.setFindString("Ab(Ab)+Ab(Ab)+(Ab)+"); + findReplaceLogic.performSelectAll(); expectStatusIsCode(findReplaceLogic, FindStatus.StatusCode.NO_MATCH); } @@ -447,12 +487,14 @@ public void testPerformSelectAllBackward() { IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(textViewer); findReplaceLogic.deactivate(SearchOptions.FORWARD); - findReplaceLogic.performSelectAll("b"); + findReplaceLogic.setFindString("b"); + findReplaceLogic.performSelectAll(); expectStatusIsFindAllWithCount(findReplaceLogic, 4); // I don't have access to getAllSelectionPoints or similar (not yet implemented), so I cannot really test for correct behavior // related to https://github.com/eclipse-platform/eclipse.platform.ui/issues/1047 - findReplaceLogic.performSelectAll("AbAbAbAb"); + findReplaceLogic.setFindString("AbAbAbAb"); + findReplaceLogic.performSelectAll(); expectStatusIsFindAllWithCount(findReplaceLogic, 1); } @@ -461,7 +503,8 @@ public void testPerformSelectAllOnReadonlyTarget() { TextViewer textViewer= setupTextViewer("Ab Ab"); textViewer.setEditable(false); IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(textViewer); - findReplaceLogic.performSelectAll("Ab"); + findReplaceLogic.setFindString("Ab"); + findReplaceLogic.performSelectAll(); expectStatusIsFindAllWithCount(findReplaceLogic, 2); } @@ -473,8 +516,9 @@ public void testSelectWholeWords() { findReplaceLogic.activate(SearchOptions.WHOLE_WORD); findReplaceLogic.deactivate(SearchOptions.WRAP); - findReplaceLogic.performSearch("get"); - findReplaceLogic.performSearch("get"); + findReplaceLogic.setFindString("get"); + findReplaceLogic.performSearch(); + findReplaceLogic.performSearch(); expectStatusIsCode(findReplaceLogic, FindStatus.StatusCode.NO_MATCH); } @@ -487,7 +531,8 @@ public void testSelectInSearchScope_withZeroLengthSelection() { int lineLength= ("line1" + lineSeparator()).length(); textViewer.setSelection(new TextSelection(lineLength + 1, 0)); findReplaceLogic.deactivate(SearchOptions.GLOBAL); - findReplaceLogic.performReplaceAll("l", ""); + setFindAndReplaceString(findReplaceLogic, "l", ""); + findReplaceLogic.performReplaceAll(); expectStatusIsReplaceAllWithCount(findReplaceLogic, 1); assertThat(textViewer.getTextWidget().getText(), is("line1" + lineSeparator() + "ine2" + lineSeparator() + "line3")); @@ -502,7 +547,8 @@ public void testSelectInSearchScope_withZeroLengthSelectionAtBeginningOfLine() { int lineLength= ("line1" + lineSeparator()).length(); textViewer.setSelection(new TextSelection(lineLength, 0)); findReplaceLogic.deactivate(SearchOptions.GLOBAL); - findReplaceLogic.performReplaceAll("l", ""); + setFindAndReplaceString(findReplaceLogic, "l", ""); + findReplaceLogic.performReplaceAll(); expectStatusIsReplaceAllWithCount(findReplaceLogic, 1); assertThat(textViewer.getTextWidget().getText(), is("line1" + lineSeparator() + "ine2" + lineSeparator() + "line3")); @@ -517,7 +563,8 @@ public void testSelectInSearchScope_withSingleLineelection() { int lineLength= ("line1" + lineSeparator()).length(); textViewer.setSelection(new TextSelection(lineLength + 1, 3)); findReplaceLogic.deactivate(SearchOptions.GLOBAL); - findReplaceLogic.performReplaceAll("l", ""); + setFindAndReplaceString(findReplaceLogic, "l", ""); + findReplaceLogic.performReplaceAll(); expectStatusIsReplaceAllWithCount(findReplaceLogic, 1); assertThat(textViewer.getTextWidget().getText(), is("line1" + lineSeparator() + "ine2" + lineSeparator() + "line3")); @@ -532,7 +579,8 @@ public void testSelectInSearchScope_withMultiLineSelection() { int beginningOfSecondLine= originalContents.indexOf("l", 1); textViewer.setSelection(new TextSelection(beginningOfSecondLine, originalContents.substring(beginningOfSecondLine).length())); findReplaceLogic.deactivate(SearchOptions.GLOBAL); - findReplaceLogic.performReplaceAll("l", ""); + setFindAndReplaceString(findReplaceLogic, "l", ""); + findReplaceLogic.performReplaceAll(); expectStatusIsReplaceAllWithCount(findReplaceLogic, 2); assertThat(textViewer.getTextWidget().getText(), is("line1" + lineSeparator() + "ine2" + lineSeparator() + "ine3")); @@ -548,7 +596,8 @@ public void testSelectInSearchScope_withSelectionEndingAtBeginningOfLine() { int lineLength= ("line1" + lineSeparator()).length(); textViewer.setSelection(new TextSelection(beginningOfSecondLine, lineLength)); findReplaceLogic.deactivate(SearchOptions.GLOBAL); - findReplaceLogic.performReplaceAll("l", ""); + setFindAndReplaceString(findReplaceLogic, "l", ""); + findReplaceLogic.performReplaceAll(); // Selection ending at beginning of new line should not include that line in search scope expectStatusIsReplaceAllWithCount(findReplaceLogic, 1); @@ -566,7 +615,8 @@ public void testSelectInSearchScope_changeScope() { findReplaceLogic.activate(SearchOptions.GLOBAL); textViewer.setSelection(new TextSelection(0, 2)); findReplaceLogic.deactivate(SearchOptions.GLOBAL); - findReplaceLogic.performReplaceAll("l", ""); + setFindAndReplaceString(findReplaceLogic, "l", ""); + findReplaceLogic.performReplaceAll(); expectStatusIsReplaceAllWithCount(findReplaceLogic, 1); assertThat(textViewer.getTextWidget().getText(), is("ine1" + lineSeparator() + "line2" + lineSeparator() + "line3")); @@ -578,21 +628,26 @@ public void testWholeWordSearchAvailable() { TextViewer textViewer= setupTextViewer(originalContents); IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(textViewer); - assertThat(findReplaceLogic.isWholeWordSearchAvailable("oneword"), is(true)); - assertThat(findReplaceLogic.isWholeWordSearchAvailable("stilläoneäword"), is(true)); - assertThat(findReplaceLogic.isWholeWordSearchAvailable("two.words"), is(false)); - assertThat(findReplaceLogic.isWholeWordSearchAvailable("two words"), is(false)); - assertThat(findReplaceLogic.isWholeWordSearchAvailable("oneword"), is(true)); - assertThat(findReplaceLogic.isWholeWordSearchAvailable("twöwords"), is(true)); + Predicate isConsideredWholeWord= string -> { + findReplaceLogic.setFindString(string); + return findReplaceLogic.isWholeWordSearchAvailable(); + }; + + assertTrue(isConsideredWholeWord.test("oneword")); + assertTrue(isConsideredWholeWord.test("stilläoneäword")); + assertFalse(isConsideredWholeWord.test("two.words")); + assertFalse(isConsideredWholeWord.test("two words")); + assertTrue(isConsideredWholeWord.test("oneword")); + assertTrue(isConsideredWholeWord.test("twöwords")); findReplaceLogic.activate(SearchOptions.REGEX); - assertThat(findReplaceLogic.isWholeWordSearchAvailable("oneword"), is(false)); - assertThat(findReplaceLogic.isWholeWordSearchAvailable("stilläoneöword"), is(false)); - assertThat(findReplaceLogic.isWholeWordSearchAvailable("two.words"), is(false)); - assertThat(findReplaceLogic.isWholeWordSearchAvailable("two words"), is(false)); + assertFalse(isConsideredWholeWord.test("oneword")); + assertFalse(isConsideredWholeWord.test("stilläoneäword")); + assertFalse(isConsideredWholeWord.test("two.words")); + assertFalse(isConsideredWholeWord.test("two words")); - assertThat(findReplaceLogic.isWholeWordSearchAvailable(""), is(false)); + assertFalse(isConsideredWholeWord.test("")); } @Test @@ -604,15 +659,16 @@ public void testReplaceInScopeStaysInScope() { findReplaceLogic.activate(SearchOptions.FORWARD); findReplaceLogic.deactivate(SearchOptions.GLOBAL); findReplaceLogic.activate(SearchOptions.WRAP); - findReplaceLogic.performSelectAndReplace(LINE_STRING, ""); + setFindAndReplaceString(findReplaceLogic, LINE_STRING, ""); + findReplaceLogic.performSelectAndReplace(); assertThat(textViewer.getTextWidget().getText(), is(LINE_STRING + lineSeparator() + lineSeparator() + LINE_STRING)); expectStatusEmpty(findReplaceLogic); - findReplaceLogic.performSelectAndReplace(LINE_STRING, ""); + findReplaceLogic.performSelectAndReplace(); assertThat(textViewer.getTextWidget().getText(), is(LINE_STRING + lineSeparator() + lineSeparator())); expectStatusEmpty(findReplaceLogic); - findReplaceLogic.performSelectAndReplace(LINE_STRING, ""); + findReplaceLogic.performSelectAndReplace(); assertThat(textViewer.getTextWidget().getText(), is(LINE_STRING + lineSeparator() + lineSeparator())); expectStatusIsCode(findReplaceLogic, StatusCode.NO_MATCH); } @@ -625,7 +681,8 @@ public void testSearchInScopeBeginsSearchInScope() { IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(textViewer); findReplaceLogic.activate(SearchOptions.FORWARD); findReplaceLogic.deactivate(SearchOptions.GLOBAL); - findReplaceLogic.performSearch(LINE_STRING); + findReplaceLogic.setFindString(LINE_STRING); + findReplaceLogic.performSearch(); expectStatusEmpty(findReplaceLogic); assertThat(findReplaceLogic.getTarget().getSelection().x, not(is(0))); assertThat(findReplaceLogic.getTarget().getSelection().x, not(is(textViewer.getDocument().get().length() - LINE_STRING_LENGTH))); @@ -655,7 +712,8 @@ public void onlySelectAndReplacesIfFindSuccessfulOnCustomTarget() { IFindReplaceLogic findReplaceLogic= new FindReplaceLogic(); findReplaceLogic.updateTarget(mockedTarget, true); - findReplaceLogic.performSelectAndReplace("NOTFOUND", ""); + setFindAndReplaceString(findReplaceLogic, "NOTFOUND", ""); + findReplaceLogic.performSelectAndReplace(); verify((IFindReplaceTargetExtension3) mockedTarget, never()).replaceSelection(anyString(), anyBoolean()); } @@ -667,9 +725,11 @@ public void testCanReplaceAfterWrap() { IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(textViewer); findReplaceLogic.activate(SearchOptions.FORWARD); findReplaceLogic.activate(SearchOptions.WRAP); - findReplaceLogic.performSelectAndReplace(LINE_STRING, ""); + setFindAndReplaceString(findReplaceLogic, LINE_STRING, ""); + + findReplaceLogic.performSelectAndReplace(); assertThat(textViewer.getTextWidget().getText(), is(LINE_STRING + lineSeparator())); - findReplaceLogic.performSelectAndReplace(LINE_STRING, ""); + findReplaceLogic.performSelectAndReplace(); assertThat(textViewer.getTextWidget().getText(), is(lineSeparator())); } @@ -681,7 +741,8 @@ public void testDontSelectAndReplaceIfFindNotSuccessful() { IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(textViewer); findReplaceLogic.activate(SearchOptions.FORWARD); findReplaceLogic.activate(SearchOptions.WRAP); - findReplaceLogic.performSelectAndReplace("NOTFOUND", ""); + setFindAndReplaceString(findReplaceLogic, "NOTFOUND", ""); + findReplaceLogic.performSelectAndReplace(); // ensure nothing was replaced assertThat(textViewer.getTextWidget().getText(), is(setupString)); // ensure the selection was not overridden @@ -698,11 +759,12 @@ public void testResetIncrementalBaseLocation() { findReplaceLogic.activate(SearchOptions.FORWARD); findReplaceLogic.activate(SearchOptions.WRAP); findReplaceLogic.activate(SearchOptions.INCREMENTAL); - findReplaceLogic.performSearch("test"); + + findReplaceLogic.setFindString("test"); assertThat(textViewer.getSelectedRange(), is(new Point(0, 4))); textViewer.setSelectedRange(5, 0); findReplaceLogic.resetIncrementalBaseLocation(); - findReplaceLogic.performSearch("test"); + findReplaceLogic.performSearch(); assertThat(textViewer.getSelectedRange(), is(new Point(5, 4))); } @@ -713,15 +775,17 @@ public void testPerformIncrementalSearch() { findReplaceLogic.activate(SearchOptions.INCREMENTAL); findReplaceLogic.activate(SearchOptions.FORWARD); - findReplaceLogic.performSearch("Test"); + findReplaceLogic.setFindString("Test"); + findReplaceLogic.performSearch(); assertThat(findReplaceLogic.getTarget().getSelection().x, is(0)); assertThat(findReplaceLogic.getTarget().getSelection().y, is(4)); - findReplaceLogic.performSearch("Test"); // incremental search is idempotent + findReplaceLogic.performSearch(); // incremental search is idempotent assertThat(findReplaceLogic.getTarget().getSelection().x, is(0)); assertThat(findReplaceLogic.getTarget().getSelection().y, is(4)); - findReplaceLogic.performSearch(""); // this clears the incremental search, but the "old search" still remains active + findReplaceLogic.setFindString(""); + findReplaceLogic.performSearch(); // this clears the incremental search, but the "old search" still remains active assertThat(findReplaceLogic.getTarget().getSelection().x, is(0)); assertThat(findReplaceLogic.getTarget().getSelection().y, is(4)); } @@ -733,23 +797,24 @@ public void testIncrementBaseLocationWithRegEx() { findReplaceLogic.activate(SearchOptions.INCREMENTAL); findReplaceLogic.activate(SearchOptions.FORWARD); - findReplaceLogic.performSearch("Test"); + findReplaceLogic.setFindString("Test"); + findReplaceLogic.performSearch(); assertThat(findReplaceLogic.getTarget().getSelection(), is(new Point(0, 4))); findReplaceLogic.activate(SearchOptions.REGEX); findReplaceLogic.deactivate(SearchOptions.INCREMENTAL); - findReplaceLogic.performSearch("Test"); + findReplaceLogic.performSearch(); findReplaceLogic.activate(SearchOptions.INCREMENTAL); assertThat(findReplaceLogic.getTarget().getSelection(), is(new Point(5, 4))); findReplaceLogic.deactivate(SearchOptions.INCREMENTAL); - findReplaceLogic.performSearch("Test"); + findReplaceLogic.performSearch(); findReplaceLogic.activate(SearchOptions.INCREMENTAL); assertThat(findReplaceLogic.getTarget().getSelection(), is(new Point(10, 4))); findReplaceLogic.deactivate(SearchOptions.REGEX); - findReplaceLogic.performSearch("Test"); + findReplaceLogic.performSearch(); assertThat(findReplaceLogic.getTarget().getSelection(), is(new Point(10, 4))); - findReplaceLogic.performSearch("Test"); + findReplaceLogic.performSearch(); assertThat(findReplaceLogic.getTarget().getSelection(), is(new Point(10, 4))); } @@ -761,7 +826,8 @@ public void testIncrementalSearchNoUpdateIfAlreadyOnWord() { textViewer.setSelectedRange(0, 4); findReplaceLogic.activate(SearchOptions.INCREMENTAL); textViewer.setSelectedRange(0, 0); - findReplaceLogic.performSearch("hello"); + findReplaceLogic.setFindString("hello"); + findReplaceLogic.performSearch(); assertThat(findReplaceLogic.getTarget().getSelection(), is(new Point(0, 5))); } @@ -773,7 +839,8 @@ public void testIncrementalSearchBackwardNoUpdateIfAlreadyOnWord() { textViewer.setSelectedRange(5, 5); findReplaceLogic.activate(SearchOptions.INCREMENTAL); textViewer.setSelectedRange(5, 0); - findReplaceLogic.performSearch("hello"); + findReplaceLogic.setFindString("hello"); + findReplaceLogic.performSearch(); assertThat(findReplaceLogic.getTarget().getSelection(), is(new Point(5, 5))); } diff --git a/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/OverlayAccess.java b/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/OverlayAccess.java index e76fed91ebd..5053c530920 100644 --- a/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/OverlayAccess.java +++ b/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/OverlayAccess.java @@ -286,7 +286,7 @@ public void assertInitialConfiguration() { assertEnabled(SearchOptions.GLOBAL); assertEnabled(SearchOptions.REGEX); assertEnabled(SearchOptions.CASE_SENSITIVE); - if (getFindText().equals("") || findReplaceLogic.isWholeWordSearchAvailable(getFindText())) { + if (getFindText().equals("") || findReplaceLogic.isWholeWordSearchAvailable()) { assertEnabled(SearchOptions.WHOLE_WORD); } else { assertDisabled(SearchOptions.WHOLE_WORD); diff --git a/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/workbench/texteditor/tests/DialogAccess.java b/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/workbench/texteditor/tests/DialogAccess.java index 0eb7f60a73f..9e528504e73 100644 --- a/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/workbench/texteditor/tests/DialogAccess.java +++ b/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/workbench/texteditor/tests/DialogAccess.java @@ -196,6 +196,7 @@ public void setFindText(String text) { @Override public void setReplaceText(String text) { replaceCombo.setText(text); + replaceCombo.notifyListeners(SWT.Modify, null); } @Override From 08b804b7962e7e6dd130123b47c0be8f007032fe Mon Sep 17 00:00:00 2001 From: Heiko Klare Date: Sun, 18 Aug 2024 14:11:00 +0200 Subject: [PATCH 18/23] Find/replace: unify search options availability and activity retrieval The FindReplaceLogic provides different methods for retrieving the active/available state of different search options. This change unifies the state retrieval by providing an isAvailable() and isAvailableAndActive() method accepting a search option instance, such that the information can be retrieved for any search option of interest in a unform way. --- .../findandreplace/FindReplaceLogic.java | 53 +++++++++++-------- .../findandreplace/IFindReplaceLogic.java | 45 ++++++++-------- .../overlay/FindReplaceOverlay.java | 8 +-- .../ui/texteditor/FindReplaceDialog.java | 18 +++---- .../findandreplace/FindReplaceLogicTest.java | 2 +- .../findandreplace/overlay/OverlayAccess.java | 2 +- 6 files changed, 69 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 1288a2f5328..9938018e591 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 @@ -61,7 +61,7 @@ public class FindReplaceLogic implements IFindReplaceLogic { @Override public void setFindString(String findString) { this.findString = Objects.requireNonNull(findString); - if (isIncrementalSearchAvailable() && isActive(SearchOptions.INCREMENTAL)) { + if (isAvailableAndActive(SearchOptions.INCREMENTAL)) { performSearch(true); } } @@ -130,14 +130,28 @@ private void resetStatus() { } @Override - public boolean isIncrementalSearchAvailable() { - return !isRegExSearchAvailableAndActive(); + public boolean isAvailable(SearchOptions searchOption) { + switch (searchOption) { + case INCREMENTAL: + return !isAvailableAndActive(SearchOptions.REGEX); + case REGEX: + return isTargetSupportingRegEx; + case WHOLE_WORD: + return !isAvailableAndActive(SearchOptions.REGEX) && isWord(findString); + case CASE_SENSITIVE: + case FORWARD: + case GLOBAL: + case WRAP: + default: + return true; + } } @Override - public boolean isWholeWordSearchAvailable() { - return !isRegExSearchAvailableAndActive() && isWord(findString); + public boolean isAvailableAndActive(SearchOptions searchOption) { + return isAvailable(searchOption) && isActive(searchOption); } + /** * Tests whether each character in the given string is a letter. * @@ -148,11 +162,6 @@ private static boolean isWord(String str) { return str != null && str.chars().allMatch(Character::isJavaIdentifierPart); } - @Override - public boolean isRegExSearchAvailableAndActive() { - return isActive(SearchOptions.REGEX) && isTargetSupportingRegEx; - } - @Override public void resetIncrementalBaseLocation() { if (target != null && shouldInitIncrementalBaseLocation()) { @@ -185,7 +194,8 @@ private void initializeSearchScope() { Point lineSelection = extensionTarget.getLineSelection(); scope = new Region(lineSelection.x, lineSelection.y); - int offset = isActive(SearchOptions.FORWARD) ? scope.getOffset() : scope.getOffset() + scope.getLength(); + int offset = isAvailableAndActive(SearchOptions.FORWARD) ? scope.getOffset() + : scope.getOffset() + scope.getLength(); extensionTarget.setSelection(offset, 0); extensionTarget.setScope(scope); @@ -318,7 +328,7 @@ public boolean performSearch() { private boolean performSearch(boolean validateSearchOptions) { resetStatus(); - if (validateSearchOptions && (isActive(SearchOptions.INCREMENTAL) && !isIncrementalSearchAvailable())) { + if (validateSearchOptions && !isAvailable(SearchOptions.INCREMENTAL) && isActive(SearchOptions.INCREMENTAL)) { return false; // Do nothing if search options are not compatible } boolean somethingFound = false; @@ -417,7 +427,7 @@ private int selectAll() { */ private int findIndex(int startPosition) { int index = 0; - if (isActive(SearchOptions.FORWARD)) { + if (isAvailableAndActive(SearchOptions.FORWARD)) { index = findAndSelect(startPosition); } else { index = startPosition == 0 ? -1 @@ -426,7 +436,7 @@ private int findIndex(int startPosition) { if (index == -1) { - if (isActive(SearchOptions.WRAP)) { + if (isAvailableAndActive(SearchOptions.WRAP)) { statusLineMessage(FindReplaceMessages.FindReplace_Status_wrapped_label); status = new FindStatus(FindStatus.StatusCode.WRAPPED); index = findAndSelect(-1); @@ -439,10 +449,10 @@ private int findIndex(int startPosition) { @Override public int findAndSelect(int offset) { - boolean wholeWordSearch = isActive(SearchOptions.WHOLE_WORD) && isWholeWordSearchAvailable(); - boolean forwardSearch = isActive(SearchOptions.FORWARD); - boolean caseSensitiveSearch = isActive(SearchOptions.CASE_SENSITIVE); - boolean regexSearch = isActive(SearchOptions.REGEX); + boolean wholeWordSearch = isAvailableAndActive(SearchOptions.WHOLE_WORD); + boolean forwardSearch = isAvailableAndActive(SearchOptions.FORWARD); + boolean caseSensitiveSearch = isAvailableAndActive(SearchOptions.CASE_SENSITIVE); + boolean regexSearch = isAvailableAndActive(SearchOptions.REGEX); if (target instanceof IFindReplaceTargetExtension3 regexSupportingTarget) { return (regexSupportingTarget).findAndSelect(offset, findString, @@ -464,7 +474,8 @@ public int findAndSelect(int offset) { */ private Point replaceSelection() { if (target instanceof IFindReplaceTargetExtension3) - ((IFindReplaceTargetExtension3) target).replaceSelection(replaceString, isRegExSearchAvailableAndActive()); + ((IFindReplaceTargetExtension3) target).replaceSelection(replaceString, + isAvailableAndActive(SearchOptions.REGEX)); else target.replaceSelection(replaceString); @@ -553,9 +564,9 @@ public boolean performSelectAndReplace() { private boolean isFindStringSelected() { String selectedString = getCurrentSelection(); - if (isRegExSearchAvailableAndActive()) { + if (isAvailableAndActive(SearchOptions.REGEX)) { int patternFlags = 0; - if (!isActive(SearchOptions.CASE_SENSITIVE)) { + if (!isAvailableAndActive(SearchOptions.CASE_SENSITIVE)) { patternFlags |= Pattern.CASE_INSENSITIVE | Pattern.UNICODE_CASE; } Pattern pattern = Pattern.compile(findString, patternFlags); diff --git a/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/IFindReplaceLogic.java b/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/IFindReplaceLogic.java index d0eb24b9ce1..9cb68e1d1d7 100644 --- a/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/IFindReplaceLogic.java +++ b/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/IFindReplaceLogic.java @@ -60,29 +60,37 @@ public interface IFindReplaceLogic { public boolean isActive(SearchOptions searchOption); /** - * Returns the current status of FindReplaceLogic. The Status can inform about - * events such as an error happening, a warning happening (e.g.: the - * search-string wasn't found) and brings a method to retrieve a message that - * can directly be displayed to the user. + * Returns whether the given search options is currently available. This + * includes validation of whether the target supports a specific option (such as + * {@link SearchOptions#REGEX}) and the compatibility of search options (such as + * {@link SearchOptions#INCREMENTAL} not being available when + * {@link SearchOptions#REGEX} is active). * - * @return FindAndReplaceMessageStatus + * @param searchOption the search option to check for availability + * + * @return whether the search option is currently available */ - public IFindReplaceStatus getStatus(); + public boolean isAvailable(SearchOptions searchOption); /** - * RegEx-Search is not possible on every target. Hence, even after {code - * activate(SearchOptions.REGEX)}, we need to check, whether we may use - * RegEx-Search. + * Returns whether the given search options is currently available and active. + * Combines {@link #isActive(SearchOptions)} and + * {@link #isAvailable(SearchOptions)}. * - * @return whether RegEx search is currently used + * @param searchOption the search option to check + * @return whether the search option is currently available and active */ - public boolean isRegExSearchAvailableAndActive(); + public boolean isAvailableAndActive(SearchOptions searchOption); /** - * {@return whether incremental search may be performed by the - * find/replace-logic based on the currently active options} + * Returns the current status of FindReplaceLogic. The Status can inform about + * events such as an error happening, a warning happening (e.g.: the + * search-string wasn't found) and brings a method to retrieve a message that + * can directly be displayed to the user. + * + * @return FindAndReplaceMessageStatus */ - public boolean isIncrementalSearchAvailable(); + public IFindReplaceStatus getStatus(); /** * Replaces all occurrences of the current find string with the replace string. @@ -156,15 +164,6 @@ public interface IFindReplaceLogic { */ public IFindReplaceTarget getTarget(); - /** - * Returns true if searching can be restricted to entire words, - * false if not. Searching for whole words requires the given find - * string to be an entire word and the regex search option to be disabled. - * - * @return true if the search can be restricted to whole words - */ - public boolean isWholeWordSearchAvailable(); - /** * Initializes the anchor used as the starting point for incremental searching. * Subsequent incremental searches will start from the first letter of the diff --git a/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlay.java b/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlay.java index 2747b861dfd..d31554e4981 100644 --- a/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlay.java +++ b/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlay.java @@ -556,7 +556,7 @@ private void createRegexSearchButton() { .withToolTipText(FindReplaceMessages.FindReplaceOverlay_regexSearchButton_toolTip) .withOperation(() -> { activateInFindReplacerIf(SearchOptions.REGEX, regexSearchButton.getSelection()); - wholeWordSearchButton.setEnabled(findReplaceLogic.isWholeWordSearchAvailable()); + wholeWordSearchButton.setEnabled(findReplaceLogic.isAvailable(SearchOptions.WHOLE_WORD)); updateIncrementalSearch(); updateContentAssistAvailability(); }).withShortcuts(KeyboardShortcuts.OPTION_REGEX).build(); @@ -632,7 +632,7 @@ private void createSearchBar() { searchBar.forceFocus(); searchBar.selectAll(); searchBar.addModifyListener(e -> { - wholeWordSearchButton.setEnabled(findReplaceLogic.isWholeWordSearchAvailable()); + wholeWordSearchButton.setEnabled(findReplaceLogic.isAvailable(SearchOptions.WHOLE_WORD)); showUserFeedback(normalTextForegroundColor, true); findReplaceLogic.setFindString(searchBar.getText()); @@ -978,7 +978,7 @@ private void updateFromTargetSelection() { findReplaceLogic.deactivate(SearchOptions.GLOBAL); searchInSelectionButton.setSelection(true); } else if (!selectionText.isEmpty()) { - if (findReplaceLogic.isRegExSearchAvailableAndActive()) { + if (findReplaceLogic.isAvailable(SearchOptions.REGEX)) { selectionText = FindReplaceDocumentAdapter.escapeForRegExPattern(selectionText); } searchBar.setText(selectionText); @@ -1038,6 +1038,6 @@ private void setContentAssistsEnablement(boolean enable) { } private void updateContentAssistAvailability() { - setContentAssistsEnablement(findReplaceLogic.isRegExSearchAvailableAndActive()); + setContentAssistsEnablement(findReplaceLogic.isAvailableAndActive(SearchOptions.REGEX)); } } \ No newline at end of file diff --git a/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/texteditor/FindReplaceDialog.java b/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/texteditor/FindReplaceDialog.java index 157de2177b0..cfe2d46d7b3 100644 --- a/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/texteditor/FindReplaceDialog.java +++ b/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/texteditor/FindReplaceDialog.java @@ -755,18 +755,18 @@ public void widgetSelected(SelectionEvent e) { storeSettings(); updateButtonState(); setContentAssistsEnablement(newState); - fIncrementalCheckBox.setEnabled(findReplaceLogic.isIncrementalSearchAvailable()); + fIncrementalCheckBox.setEnabled(findReplaceLogic.isAvailable(SearchOptions.INCREMENTAL)); } }); storeButtonWithMnemonicInMap(fIsRegExCheckBox); - fWholeWordCheckBox.setEnabled(findReplaceLogic.isWholeWordSearchAvailable()); + fWholeWordCheckBox.setEnabled(findReplaceLogic.isAvailable(SearchOptions.WHOLE_WORD)); fWholeWordCheckBox.addSelectionListener(new SelectionAdapter() { @Override public void widgetSelected(SelectionEvent e) { updateButtonState(); } }); - fIncrementalCheckBox.setEnabled(findReplaceLogic.isIncrementalSearchAvailable()); + fIncrementalCheckBox.setEnabled(findReplaceLogic.isAvailable(SearchOptions.INCREMENTAL)); return panel; } @@ -957,7 +957,7 @@ private void initFindStringFromSelection() { String selection = getCurrentSelection(); String searchInput = getFirstLine(selection); boolean isSingleLineInput = searchInput.equals(selection); - if (findReplaceLogic.isRegExSearchAvailableAndActive()) { + if (findReplaceLogic.isAvailableAndActive(SearchOptions.REGEX)) { searchInput = FindReplaceDocumentAdapter.escapeForRegExPattern(selection); } fFindField.setText(searchInput); @@ -1088,11 +1088,11 @@ private void updateButtonState(boolean disableReplace) { boolean isFindStringSet = str != null && !str.isEmpty(); String selectionString = enable ? target.getSelection().toString() : ""; //$NON-NLS-1$ boolean isTargetEditable = enable ? target.isEditable() : false; - boolean isRegExSearchAvailableAndActive = findReplaceLogic.isRegExSearchAvailableAndActive(); + boolean isRegExSearchAvailableAndActive = findReplaceLogic.isAvailableAndActive(SearchOptions.REGEX); boolean isSelectionGoodForReplace = selectionString != "" //$NON-NLS-1$ || !isRegExSearchAvailableAndActive; - fWholeWordCheckBox.setEnabled(findReplaceLogic.isWholeWordSearchAvailable()); + fWholeWordCheckBox.setEnabled(findReplaceLogic.isAvailable(SearchOptions.WHOLE_WORD)); fFindNextButton.setEnabled(enable && isFindStringSet); fSelectAllButton.setEnabled(enable && isFindStringSet && (target instanceof IFindReplaceTargetExtension4)); fReplaceSelectionButton.setEnabled( @@ -1203,11 +1203,11 @@ public void updateTarget(IFindReplaceTarget target, boolean isTargetEditable, bo } if (okToUse(fWholeWordCheckBox)) { - fWholeWordCheckBox.setEnabled(findReplaceLogic.isWholeWordSearchAvailable()); + fWholeWordCheckBox.setEnabled(findReplaceLogic.isAvailable(SearchOptions.WHOLE_WORD)); } if (okToUse(fIncrementalCheckBox)) { - fIncrementalCheckBox.setEnabled(findReplaceLogic.isIncrementalSearchAvailable()); + fIncrementalCheckBox.setEnabled(findReplaceLogic.isAvailable(SearchOptions.INCREMENTAL)); } if (okToUse(fReplaceLabel)) { @@ -1222,7 +1222,7 @@ public void updateTarget(IFindReplaceTarget target, boolean isTargetEditable, bo updateButtonState(); - setContentAssistsEnablement(findReplaceLogic.isRegExSearchAvailableAndActive()); + setContentAssistsEnablement(findReplaceLogic.isAvailableAndActive(SearchOptions.REGEX)); } /** 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 430e7d415cf..b5e41cc324b 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 @@ -630,7 +630,7 @@ public void testWholeWordSearchAvailable() { Predicate isConsideredWholeWord= string -> { findReplaceLogic.setFindString(string); - return findReplaceLogic.isWholeWordSearchAvailable(); + return findReplaceLogic.isAvailable(SearchOptions.WHOLE_WORD); }; assertTrue(isConsideredWholeWord.test("oneword")); diff --git a/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/OverlayAccess.java b/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/OverlayAccess.java index 5053c530920..634fb6a42cf 100644 --- a/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/OverlayAccess.java +++ b/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/OverlayAccess.java @@ -286,7 +286,7 @@ public void assertInitialConfiguration() { assertEnabled(SearchOptions.GLOBAL); assertEnabled(SearchOptions.REGEX); assertEnabled(SearchOptions.CASE_SENSITIVE); - if (getFindText().equals("") || findReplaceLogic.isWholeWordSearchAvailable()) { + if (getFindText().equals("") || findReplaceLogic.isAvailable(SearchOptions.WHOLE_WORD)) { assertEnabled(SearchOptions.WHOLE_WORD); } else { assertDisabled(SearchOptions.WHOLE_WORD); From 60d839573590acd3b4d6a4a680410eb7dcd0f1be Mon Sep 17 00:00:00 2001 From: Heiko Klare Date: Sun, 18 Aug 2024 13:36:43 +0200 Subject: [PATCH 19/23] Find/replace: move handling of performing incremental search into logic Currently, consumers of the FindReplaceLogic are responsible for properly handling incremental searches by executing a search when the input text changes and by disabling incremental mode when an ordinary search shall be performed. With this change, incremental "search-as-you-type" is automatically performed by the FindReplaceLogic when the incremental search option is activated. Explicitly performing a search does require to deactivate the incremental mode anymore. --- .../findandreplace/FindReplaceLogic.java | 58 +++++++++---------- .../findandreplace/IFindReplaceLogic.java | 9 ++- .../overlay/FindReplaceOverlay.java | 5 +- .../ui/texteditor/FindReplaceDialog.java | 6 +- .../findandreplace/FindReplaceLogicTest.java | 44 ++++++++------ .../overlay/FindReplaceOverlayTest.java | 7 ++- 6 files changed, 67 insertions(+), 62 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 9938018e591..812787f7a86 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 @@ -322,26 +322,24 @@ private boolean prepareTargetForEditing() { @Override public boolean performSearch() { - return performSearch(true); + boolean result = performSearch(false); + resetIncrementalBaseLocation(); + return result; } - private boolean performSearch(boolean validateSearchOptions) { + private boolean performSearch(boolean updateFromIncrementalBaseLocation) { resetStatus(); - - if (validateSearchOptions && !isAvailable(SearchOptions.INCREMENTAL) && isActive(SearchOptions.INCREMENTAL)) { - return false; // Do nothing if search options are not compatible + if (findString.isEmpty()) { + return false; } - boolean somethingFound = false; - - if (findString != null && !findString.isEmpty()) { - try { - somethingFound = findNext(); - } catch (PatternSyntaxException ex) { - status = new InvalidRegExStatus(ex); - } catch (IllegalStateException ex) { - // we don't keep state in this dialog - } + boolean somethingFound = false; + try { + return somethingFound = findNext(updateFromIncrementalBaseLocation); + } catch (PatternSyntaxException ex) { + status = new InvalidRegExStatus(ex); + } catch (IllegalStateException ex) { + // we don't keep state in this dialog } return somethingFound; } @@ -482,21 +480,13 @@ private Point replaceSelection() { return target.getSelection(); } - /** - * Returns whether the specified search string can be found using the given - * options. - * - * @return true if the search string can be found using the given - * options - * - */ - private boolean findNext() { + private boolean findNext(boolean updateFromIncrementalBaseLocation) { if (target == null) { return false; } - int findReplacePosition = calculateFindBeginningOffset(); + int findReplacePosition = calculateFindBeginningOffset(updateFromIncrementalBaseLocation); int index = findIndex(findReplacePosition); @@ -515,19 +505,25 @@ private boolean findNext() { return true; } - private int calculateFindBeginningOffset() { + private int calculateFindBeginningOffset(boolean updateFromExistingBaseLocation) { Point r = null; - if (isActive(SearchOptions.INCREMENTAL)) { + if (updateFromExistingBaseLocation) { r = incrementalBaseLocation; } else { r = target.getSelection(); } int findReplacePosition = r.x; - if (isActive(SearchOptions.FORWARD) && !isActive(SearchOptions.INCREMENTAL) - || isActive(SearchOptions.INCREMENTAL) && !isActive(SearchOptions.FORWARD)) { + if (!isActive(SearchOptions.FORWARD)) { findReplacePosition += r.y; } + if (!updateFromExistingBaseLocation) { + if (isActive(SearchOptions.FORWARD)) { + findReplacePosition += r.y; + } else { + findReplacePosition -= r.y; + } + } return findReplacePosition; } @@ -535,7 +531,7 @@ private int calculateFindBeginningOffset() { public boolean performReplaceAndFind() { resetStatus(); if (performSelectAndReplace()) { - performSearch(false); + performSearch(); return true; } return false; @@ -545,7 +541,7 @@ public boolean performReplaceAndFind() { public boolean performSelectAndReplace() { resetStatus(); if (!isFindStringSelected()) { - performSearch(false); + performSearch(); } if (getStatus().wasSuccessful()) { if (!prepareTargetForEditing()) { diff --git a/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/IFindReplaceLogic.java b/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/IFindReplaceLogic.java index 9cb68e1d1d7..1d7af642f4d 100644 --- a/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/IFindReplaceLogic.java +++ b/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/IFindReplaceLogic.java @@ -26,9 +26,14 @@ public interface IFindReplaceLogic { /** - * Sets the string to be used for searching in find and replace operations. + * Sets the string to be used for searching in find and replace operations. In + * case the {@link SearchOptions#INCREMENTAL} is active, this also triggers an + * incremental search operation, starting from the current incremental base + * location. This also updates the search status accordingly. * * @param findString the find string to use, must not be null + * + * @see #getStatus() */ public void setFindString(String findString); @@ -107,8 +112,6 @@ public interface IFindReplaceLogic { * Locates the current find string in the target. If incremental search is * activated, the search will be performed starting from an incremental search * position, which can be reset using {@link #resetIncrementalBaseLocation()}. - * If incremental search is activated and RegEx search is activated, nothing - * happens. * * @return Whether the string was found in the target */ diff --git a/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlay.java b/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlay.java index d31554e4981..3ecc45e358d 100644 --- a/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlay.java +++ b/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlay.java @@ -635,7 +635,6 @@ private void createSearchBar() { wholeWordSearchButton.setEnabled(findReplaceLogic.isAvailable(SearchOptions.WHOLE_WORD)); showUserFeedback(normalTextForegroundColor, true); - findReplaceLogic.setFindString(searchBar.getText()); updateIncrementalSearch(); }); searchBar.addFocusListener(new FocusListener() { @@ -656,7 +655,7 @@ public void focusLost(FocusEvent e) { } private void updateIncrementalSearch() { - findReplaceLogic.performSearch(); + findReplaceLogic.setFindString(searchBar.getText()); evaluateFindReplaceStatus(); } @@ -964,10 +963,8 @@ private void performSingleReplace() { private void performSearch(boolean forward) { boolean oldForwardSearchSetting = findReplaceLogic.isActive(SearchOptions.FORWARD); activateInFindReplacerIf(SearchOptions.FORWARD, forward); - findReplaceLogic.deactivate(SearchOptions.INCREMENTAL); findReplaceLogic.performSearch(); activateInFindReplacerIf(SearchOptions.FORWARD, oldForwardSearchSetting); - findReplaceLogic.activate(SearchOptions.INCREMENTAL); evaluateFindReplaceStatus(); searchBar.storeHistory(); } diff --git a/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/texteditor/FindReplaceDialog.java b/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/texteditor/FindReplaceDialog.java index cfe2d46d7b3..456e51db8a7 100644 --- a/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/texteditor/FindReplaceDialog.java +++ b/bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/texteditor/FindReplaceDialog.java @@ -118,8 +118,7 @@ public void shellDeactivated(ShellEvent e) { } /** - * Modify listener to update the find logic with the current input and update - * the search result in case of incremental search. + * Modify listener to update the find logic with the current input. * * @since 2.0 */ @@ -639,9 +638,6 @@ private Composite createInputPanel(Composite parent) { fFindModifyListener = new InputModifyListener(() -> { if (okToUse(fFindField)) { findReplaceLogic.setFindString(fFindField.getText()); - if (findReplaceLogic.isActive(SearchOptions.INCREMENTAL)) { - findReplaceLogic.performSearch(); - } } }); fFindField.addModifyListener(fFindModifyListener); 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 b5e41cc324b..fd8c772c839 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 @@ -81,7 +81,6 @@ private TextViewer setupTextViewer(String contentText) { return textViewer; } - private void setFindAndReplaceString(IFindReplaceLogic findReplaceLogic, String findString, String replaceString) { findReplaceLogic.setFindString(findString); findReplaceLogic.setReplaceString(replaceString); @@ -430,7 +429,6 @@ public void testPerformSearchAndReplaceRegEx_incrementalActive() { findReplaceLogic.activate(SearchOptions.REGEX); findReplaceLogic.setFindString("text"); - findReplaceLogic.performSearch(); textViewer.setSelectedRange(0, 0); findReplaceLogic.setReplaceString(""); @@ -769,25 +767,38 @@ public void testResetIncrementalBaseLocation() { } @Test - public void testPerformIncrementalSearch() { + public void testSetFindString_incrementalInactive() { + TextViewer textViewer= setupTextViewer("Test Test Test Test"); + IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(textViewer); + findReplaceLogic.activate(SearchOptions.FORWARD); + + assertEquals(new Point(0, 0), findReplaceLogic.getTarget().getSelection()); + findReplaceLogic.setFindString("Test"); + assertEquals(new Point(0, 0), findReplaceLogic.getTarget().getSelection()); + } + + @Test + public void testSetFindString_incrementalActive() { TextViewer textViewer= setupTextViewer("Test Test Test Test"); IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(textViewer); findReplaceLogic.activate(SearchOptions.INCREMENTAL); findReplaceLogic.activate(SearchOptions.FORWARD); + assertEquals(new Point(0, 0), findReplaceLogic.getTarget().getSelection()); findReplaceLogic.setFindString("Test"); - findReplaceLogic.performSearch(); - assertThat(findReplaceLogic.getTarget().getSelection().x, is(0)); - assertThat(findReplaceLogic.getTarget().getSelection().y, is(4)); + assertEquals(new Point(0, 4), findReplaceLogic.getTarget().getSelection()); - findReplaceLogic.performSearch(); // incremental search is idempotent - assertThat(findReplaceLogic.getTarget().getSelection().x, is(0)); - assertThat(findReplaceLogic.getTarget().getSelection().y, is(4)); + findReplaceLogic.setFindString("Test"); // incremental search is idempotent + assertEquals(new Point(0, 4), findReplaceLogic.getTarget().getSelection()); - findReplaceLogic.setFindString(""); - findReplaceLogic.performSearch(); // this clears the incremental search, but the "old search" still remains active - assertThat(findReplaceLogic.getTarget().getSelection().x, is(0)); - assertThat(findReplaceLogic.getTarget().getSelection().y, is(4)); + findReplaceLogic.setFindString("T"); + assertEquals(new Point(0, 1), findReplaceLogic.getTarget().getSelection()); + + findReplaceLogic.setFindString("Te"); + 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()); } @Test @@ -798,7 +809,6 @@ public void testIncrementBaseLocationWithRegEx() { findReplaceLogic.activate(SearchOptions.FORWARD); findReplaceLogic.setFindString("Test"); - findReplaceLogic.performSearch(); assertThat(findReplaceLogic.getTarget().getSelection(), is(new Point(0, 4))); findReplaceLogic.activate(SearchOptions.REGEX); @@ -812,10 +822,10 @@ public void testIncrementBaseLocationWithRegEx() { assertThat(findReplaceLogic.getTarget().getSelection(), is(new Point(10, 4))); findReplaceLogic.deactivate(SearchOptions.REGEX); - findReplaceLogic.performSearch(); + findReplaceLogic.setFindString("Test"); assertThat(findReplaceLogic.getTarget().getSelection(), is(new Point(10, 4))); findReplaceLogic.performSearch(); - assertThat(findReplaceLogic.getTarget().getSelection(), is(new Point(10, 4))); + assertThat(findReplaceLogic.getTarget().getSelection(), is(new Point(15, 4))); } @Test @@ -827,7 +837,6 @@ public void testIncrementalSearchNoUpdateIfAlreadyOnWord() { findReplaceLogic.activate(SearchOptions.INCREMENTAL); textViewer.setSelectedRange(0, 0); findReplaceLogic.setFindString("hello"); - findReplaceLogic.performSearch(); assertThat(findReplaceLogic.getTarget().getSelection(), is(new Point(0, 5))); } @@ -840,7 +849,6 @@ public void testIncrementalSearchBackwardNoUpdateIfAlreadyOnWord() { findReplaceLogic.activate(SearchOptions.INCREMENTAL); textViewer.setSelectedRange(5, 0); findReplaceLogic.setFindString("hello"); - findReplaceLogic.performSearch(); assertThat(findReplaceLogic.getTarget().getSelection(), is(new Point(5, 5))); } diff --git a/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayTest.java b/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayTest.java index ca0badf64e0..414a3b371e0 100644 --- a/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayTest.java +++ b/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayTest.java @@ -20,6 +20,8 @@ import org.junit.Test; +import org.eclipse.swt.graphics.Point; + import org.eclipse.core.runtime.preferences.IEclipsePreferences; import org.eclipse.core.runtime.preferences.InstanceScope; @@ -88,10 +90,13 @@ public void testIncrementalSearchUpdatesAfterChangingOptions() { initializeTextViewerWithFindReplaceUI("alinee\naLinee\nline\nline"); OverlayAccess dialog= getDialog(); IFindReplaceTarget target= dialog.getTarget(); - dialog.setFindText("Line"); + assertThat(dialog.getTarget().getSelectionText(), is("line")); + assertEquals(new Point(1,4), dialog.getTarget().getSelection()); + dialog.select(SearchOptions.CASE_SENSITIVE); assertThat(dialog.getTarget().getSelectionText(), is("Line")); + assertEquals(new Point(8,4), dialog.getTarget().getSelection()); dialog.unselect(SearchOptions.CASE_SENSITIVE); assertEquals(1, (target.getSelection()).x); From e2090fd6a79efa9720804106ad8042cfc0a94e1a Mon Sep 17 00:00:00 2001 From: Heiko Klare Date: Sat, 7 Sep 2024 10:43:18 +0200 Subject: [PATCH 20/23] Version bump org.eclipse.ui.workbench.texteditor.tests for 4.34 stream --- .../META-INF/MANIFEST.MF | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/org.eclipse.ui.workbench.texteditor.tests/META-INF/MANIFEST.MF b/tests/org.eclipse.ui.workbench.texteditor.tests/META-INF/MANIFEST.MF index 407818d48bd..1a6ef95d92a 100644 --- a/tests/org.eclipse.ui.workbench.texteditor.tests/META-INF/MANIFEST.MF +++ b/tests/org.eclipse.ui.workbench.texteditor.tests/META-INF/MANIFEST.MF @@ -2,7 +2,7 @@ Manifest-Version: 1.0 Bundle-ManifestVersion: 2 Bundle-Name: %Plugin.name Bundle-SymbolicName: org.eclipse.ui.workbench.texteditor.tests -Bundle-Version: 3.14.500.qualifier +Bundle-Version: 3.14.600.qualifier Bundle-Vendor: %Plugin.providerName Bundle-Localization: plugin Export-Package: From 4adc99584f3238327a64b0f0296816fdbf86edf1 Mon Sep 17 00:00:00 2001 From: Heiko Klare Date: Wed, 14 Aug 2024 10:04:59 +0200 Subject: [PATCH 21/23] Replace custom array equals assertion methods with standard ones --- .../tests/databinding/scenarios/ComboScenarios.java | 1 + .../databinding/scenarios/ComboViewerScenario.java | 3 ++- .../databinding/scenarios/ListViewerScenario.java | 1 + .../databinding/scenarios/MasterDetailScenarios.java | 1 + .../databinding/scenarios/ScenariosTestCase.java | 8 -------- .../jface/tests/viewers/AbstractTreeViewerTest.java | 12 ++---------- .../viewers/MultipleEqualElementsTreeViewerTest.java | 7 ++++--- 7 files changed, 11 insertions(+), 22 deletions(-) diff --git a/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/jface/tests/databinding/scenarios/ComboScenarios.java b/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/jface/tests/databinding/scenarios/ComboScenarios.java index 12b0e2ea18c..18e4baba56b 100644 --- a/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/jface/tests/databinding/scenarios/ComboScenarios.java +++ b/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/jface/tests/databinding/scenarios/ComboScenarios.java @@ -15,6 +15,7 @@ *******************************************************************************/ package org.eclipse.jface.tests.databinding.scenarios; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import java.lang.reflect.InvocationTargetException; diff --git a/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/jface/tests/databinding/scenarios/ComboViewerScenario.java b/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/jface/tests/databinding/scenarios/ComboViewerScenario.java index ba1b0b7dada..dccbb695cb4 100644 --- a/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/jface/tests/databinding/scenarios/ComboViewerScenario.java +++ b/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/jface/tests/databinding/scenarios/ComboViewerScenario.java @@ -16,13 +16,14 @@ *******************************************************************************/ package org.eclipse.jface.tests.databinding.scenarios; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import org.eclipse.core.databinding.beans.typed.BeanProperties; import org.eclipse.core.databinding.observable.list.IObservableList; import org.eclipse.core.databinding.observable.value.IObservableValue; -import org.eclipse.jface.databinding.viewers.typed.ViewerProperties; import org.eclipse.jface.databinding.viewers.ViewerSupport; +import org.eclipse.jface.databinding.viewers.typed.ViewerProperties; import org.eclipse.jface.examples.databinding.model.Adventure; import org.eclipse.jface.examples.databinding.model.Catalog; import org.eclipse.jface.examples.databinding.model.Lodging; diff --git a/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/jface/tests/databinding/scenarios/ListViewerScenario.java b/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/jface/tests/databinding/scenarios/ListViewerScenario.java index 8ec098b0569..7c127b0a8d7 100644 --- a/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/jface/tests/databinding/scenarios/ListViewerScenario.java +++ b/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/jface/tests/databinding/scenarios/ListViewerScenario.java @@ -15,6 +15,7 @@ *******************************************************************************/ package org.eclipse.jface.tests.databinding.scenarios; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import org.eclipse.core.databinding.beans.typed.BeanProperties; diff --git a/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/jface/tests/databinding/scenarios/MasterDetailScenarios.java b/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/jface/tests/databinding/scenarios/MasterDetailScenarios.java index 9fa0a5f5050..a8d15130d10 100644 --- a/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/jface/tests/databinding/scenarios/MasterDetailScenarios.java +++ b/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/jface/tests/databinding/scenarios/MasterDetailScenarios.java @@ -15,6 +15,7 @@ *******************************************************************************/ package org.eclipse.jface.tests.databinding.scenarios; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; diff --git a/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/jface/tests/databinding/scenarios/ScenariosTestCase.java b/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/jface/tests/databinding/scenarios/ScenariosTestCase.java index 2f062e06c79..0d52955dff1 100644 --- a/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/jface/tests/databinding/scenarios/ScenariosTestCase.java +++ b/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/jface/tests/databinding/scenarios/ScenariosTestCase.java @@ -14,10 +14,6 @@ *******************************************************************************/ package org.eclipse.jface.tests.databinding.scenarios; -import static org.junit.Assert.assertEquals; - -import java.util.Arrays; - import org.eclipse.core.databinding.DataBindingContext; import org.eclipse.core.databinding.observable.Realm; import org.eclipse.jface.databinding.conformance.util.RealmTester; @@ -151,8 +147,4 @@ protected void enterText(Text text, String string) { text.notifyListeners(SWT.FocusOut, null); } - protected void assertArrayEquals(Object[] expected, Object[] actual) { - assertEquals(Arrays.asList(expected), Arrays.asList(actual)); - } - } diff --git a/tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/viewers/AbstractTreeViewerTest.java b/tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/viewers/AbstractTreeViewerTest.java index 2c06b5b1edb..62c76e31a02 100644 --- a/tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/viewers/AbstractTreeViewerTest.java +++ b/tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/viewers/AbstractTreeViewerTest.java @@ -13,6 +13,7 @@ *******************************************************************************/ package org.eclipse.jface.tests.viewers; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -35,15 +36,6 @@ public abstract class AbstractTreeViewerTest extends StructuredItemViewerTest { AbstractTreeViewer fTreeViewer; - protected void assertEqualsArray(String s, Object[] a1, Object[] a2) { - int s1 = a1.length; - int s2 = a2.length; - assertEquals(s, s1, s2); - for (int i = 0; i < s1; i++) { - assertEquals(s, a1[i], a2[i]); - } - } - @Override protected void assertSelectionEquals(String message, TestElement expected) { IStructuredSelection selection = fViewer.getStructuredSelection(); @@ -79,7 +71,7 @@ public void testBulkExpand() { Object[] list2 = fTreeViewer.getExpandedElements(); - assertEqualsArray("old and new expand state are the same", list1, list2); + assertArrayEquals("old and new expand state are the same", list1, list2); } @Test diff --git a/tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/viewers/MultipleEqualElementsTreeViewerTest.java b/tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/viewers/MultipleEqualElementsTreeViewerTest.java index eb6e2240edc..ac701cfc438 100644 --- a/tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/viewers/MultipleEqualElementsTreeViewerTest.java +++ b/tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/viewers/MultipleEqualElementsTreeViewerTest.java @@ -13,6 +13,7 @@ *******************************************************************************/ package org.eclipse.jface.tests.viewers; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; @@ -148,13 +149,13 @@ public void testSelection() { @Test public void testExpansion() { getTreeViewer().expandToLevel(treePath_1_21_212, 1); - assertEqualsArray("element expansion", new Object[] { element_1, element_2_1 }, + assertArrayEquals("element expansion", new Object[] { element_1, element_2_1 }, getTreeViewer().getExpandedElements()); - assertEqualsArray("path expansion", new Object[] { treePath_1, treePath_1_21 }, + assertArrayEquals("path expansion", new Object[] { treePath_1, treePath_1_21 }, getTreeViewer().getExpandedTreePaths()); getTreeViewer().setExpandedTreePaths( new TreePath[] { treePath_1, treePath_1_2, treePath_1_2_21, treePath_2, treePath_2_21 }); - assertEqualsArray("path expansion", + assertArrayEquals("path expansion", new Object[] { treePath_1, treePath_1_2, treePath_1_2_21, treePath_2, treePath_2_21 }, getTreeViewer().getExpandedTreePaths()); } From bc672de76091e9b8b1282b9a094d3e4225758cf2 Mon Sep 17 00:00:00 2001 From: Heiko Klare Date: Mon, 19 Aug 2024 17:22:44 +0200 Subject: [PATCH 22/23] Version bumps org.eclipse.jface.tests.* for 4.34 stream --- tests/org.eclipse.jface.tests.databinding/META-INF/MANIFEST.MF | 2 +- tests/org.eclipse.jface.tests/META-INF/MANIFEST.MF | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/org.eclipse.jface.tests.databinding/META-INF/MANIFEST.MF b/tests/org.eclipse.jface.tests.databinding/META-INF/MANIFEST.MF index b0ea3f51b9f..b3f8cae1914 100644 --- a/tests/org.eclipse.jface.tests.databinding/META-INF/MANIFEST.MF +++ b/tests/org.eclipse.jface.tests.databinding/META-INF/MANIFEST.MF @@ -2,7 +2,7 @@ Manifest-Version: 1.0 Bundle-ManifestVersion: 2 Bundle-Name: %pluginName Bundle-SymbolicName: org.eclipse.jface.tests.databinding -Bundle-Version: 1.12.400.qualifier +Bundle-Version: 1.12.500.qualifier Bundle-Vendor: %providerName Bundle-Localization: plugin Require-Bundle: org.eclipse.core.databinding;bundle-version="[1.3.0,2.0.0)", diff --git a/tests/org.eclipse.jface.tests/META-INF/MANIFEST.MF b/tests/org.eclipse.jface.tests/META-INF/MANIFEST.MF index 15b357fd9f7..48ffc6df3f9 100644 --- a/tests/org.eclipse.jface.tests/META-INF/MANIFEST.MF +++ b/tests/org.eclipse.jface.tests/META-INF/MANIFEST.MF @@ -2,7 +2,7 @@ Manifest-Version: 1.0 Bundle-ManifestVersion: 2 Bundle-Name: %Bundle-Name Bundle-SymbolicName: org.eclipse.jface.tests -Bundle-Version: 1.4.600.qualifier +Bundle-Version: 1.4.700.qualifier Automatic-Module-Name: org.eclipse.jface.tests Bundle-RequiredExecutionEnvironment: JavaSE-17 Require-Bundle: org.junit;bundle-version="4.12.0", From 5c7461925bbdc7eceac27202cd3f556d46fd02c8 Mon Sep 17 00:00:00 2001 From: Heiko Klare Date: Fri, 6 Sep 2024 18:16:13 +0200 Subject: [PATCH 23/23] Find/replace overlay: improve expressiveness of IFindReplaceUIAccess Improves the design/expressiveness of the IFindReplaceUIAccess to reduce dependency on implementation details (like using shells as the container for the dialog and the overlay). --- .../internal/findandreplace/FindReplaceUITest.java | 2 +- .../findandreplace/IFindReplaceUIAccess.java | 5 +++-- .../overlay/FindReplaceOverlayTest.java | 4 ++-- .../findandreplace/overlay/OverlayAccess.java | 13 +++++++++++-- .../ui/workbench/texteditor/tests/DialogAccess.java | 9 +++++++-- 5 files changed, 24 insertions(+), 9 deletions(-) diff --git a/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/FindReplaceUITest.java b/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/FindReplaceUITest.java index 30cfca379f6..a2f674af0ab 100644 --- a/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/FindReplaceUITest.java +++ b/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/FindReplaceUITest.java @@ -81,7 +81,7 @@ protected void reopenFindReplaceUIForTextViewer() { protected final void ensureHasFocusOnGTK() { if (Util.isGtk()) { runEventQueue(); - if (dialog.getActiveShell() == null) { + if (!dialog.hasFocus()) { String screenshotPath= ScreenshotTest.takeScreenshot(FindReplaceUITest.class, testName.getMethodName(), System.out); fail("this test does not work on GTK unless the runtime workbench has focus. Screenshot: " + screenshotPath); } diff --git a/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/IFindReplaceUIAccess.java b/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/IFindReplaceUIAccess.java index f7a1b503f9c..700bf25df04 100644 --- a/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/IFindReplaceUIAccess.java +++ b/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/IFindReplaceUIAccess.java @@ -10,7 +10,6 @@ *******************************************************************************/ package org.eclipse.ui.internal.findandreplace; -import org.eclipse.swt.widgets.Shell; import org.eclipse.swt.widgets.Widget; import org.eclipse.jface.text.IFindReplaceTarget; @@ -26,6 +25,8 @@ public interface IFindReplaceUIAccess { void close(); + boolean isShown(); + void unselect(SearchOptions option); void select(SearchOptions option); @@ -42,7 +43,7 @@ public interface IFindReplaceUIAccess { void setReplaceText(String text); - Shell getActiveShell(); + boolean hasFocus(); Widget getButtonForSearchOption(SearchOptions option); diff --git a/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayTest.java b/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayTest.java index 414a3b371e0..51beb527d4a 100644 --- a/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayTest.java +++ b/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayTest.java @@ -16,7 +16,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertFalse; import org.junit.Test; @@ -174,7 +174,7 @@ public void testDisableOverlayViaPreference() { boolean useOverlayPreference= preferences.getBoolean(USE_FIND_REPLACE_OVERLAY, true); try { preferences.putBoolean(USE_FIND_REPLACE_OVERLAY, false); - assertNull("dialog should be closed after changing preference", getDialog().getActiveShell()); + assertFalse("dialog should be closed after changing preference", getDialog().isShown()); } finally { preferences.putBoolean(USE_FIND_REPLACE_OVERLAY, useOverlayPreference); reopenFindReplaceUIForTextViewer(); diff --git a/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/OverlayAccess.java b/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/OverlayAccess.java index 634fb6a42cf..2d4896e9317 100644 --- a/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/OverlayAccess.java +++ b/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/OverlayAccess.java @@ -26,6 +26,7 @@ import org.eclipse.swt.SWT; import org.eclipse.swt.widgets.Button; +import org.eclipse.swt.widgets.Control; import org.eclipse.swt.widgets.Event; import org.eclipse.swt.widgets.Shell; import org.eclipse.swt.widgets.ToolItem; @@ -321,8 +322,16 @@ public void assertEnabled(SearchOptions option) { } @Override - public Shell getActiveShell() { - return shellRetriever.get(); + public boolean isShown() { + return shellRetriever.get() != null && shellRetriever.get().isVisible(); + } + + @Override + public boolean hasFocus() { + Shell overlayShell= shellRetriever.get(); + Control focusControl= overlayShell.getDisplay().getFocusControl(); + Shell focusControlShell= focusControl != null ? focusControl.getShell() : null; + return focusControlShell == overlayShell; } } diff --git a/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/workbench/texteditor/tests/DialogAccess.java b/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/workbench/texteditor/tests/DialogAccess.java index 9e528504e73..bbbd0b507bc 100644 --- a/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/workbench/texteditor/tests/DialogAccess.java +++ b/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/workbench/texteditor/tests/DialogAccess.java @@ -165,8 +165,8 @@ public void close() { } @Override - public Shell getActiveShell() { - return shellRetriever.get(); + public boolean hasFocus() { + return shellRetriever.get() != null; } @Override @@ -316,4 +316,9 @@ private Set getSelectedOptions() { .collect(Collectors.toSet()); } + @Override + public boolean isShown() { + return shellRetriever.get() != null; + } + }