Find/Replace overlay: offer it in any part that can host it

bundles/org.eclipse.ui.editors/src/org/eclipse/ui/internal/editors/text/TextEditorDefaultsPreferencePage.java

@@ -94,6 +94,9 @@
  */
 public class TextEditorDefaultsPreferencePage extends PreferencePage implements IWorkbenchPreferencePage {
 
+	/** Horizontal indentation of a control that depends on the one above it. */
+	private static final int INDENTATION= 20;
+
 	private static abstract class Initializer {
 
 		protected final Preference fPreference;
@@ -759,6 +762,7 @@ private OverlayPreferenceStore createOverlayStore() {
 		overlayKeys.add(new OverlayPreferenceStore.OverlayKey(OverlayPreferenceStore.STRING, AbstractDecoratedTextEditorPreferenceConstants.EDITOR_CURRENT_LINE_COLOR));
 		overlayKeys.add(new OverlayPreferenceStore.OverlayKey(OverlayPreferenceStore.BOOLEAN, AbstractDecoratedTextEditorPreferenceConstants.EDITOR_USE_FIND_REPLACE_OVERLAY));
 		overlayKeys.add(new OverlayPreferenceStore.OverlayKey(OverlayPreferenceStore.BOOLEAN, AbstractDecoratedTextEditorPreferenceConstants.EDITOR_FIND_REPLACE_OVERLAY_AT_BOTTOM));
+		overlayKeys.add(new OverlayPreferenceStore.OverlayKey(OverlayPreferenceStore.BOOLEAN, AbstractDecoratedTextEditorPreferenceConstants.EDITOR_RESTRICT_FIND_REPLACE_OVERLAY_TO_TEXT_EDITORS));
 		overlayKeys.add(new OverlayPreferenceStore.OverlayKey(OverlayPreferenceStore.BOOLEAN, AbstractDecoratedTextEditorPreferenceConstants.EDITOR_CURRENT_LINE));
 
 		overlayKeys.add(new OverlayPreferenceStore.OverlayKey(OverlayPreferenceStore.INT, AbstractDecoratedTextEditorPreferenceConstants.EDITOR_TAB_WIDTH));
@@ -901,7 +905,14 @@ public void widgetSelected(SelectionEvent e) {
 		label= TextEditorMessages.TextEditorPreferencePage_showFindReplaceOverlayAtBottom;
 		Preference findReplaceOverlayAtBottom= new Preference(AbstractDecoratedTextEditorPreferenceConstants.EDITOR_FIND_REPLACE_OVERLAY_AT_BOTTOM, label, null);
 		final Button overlayAtBottom= addCheckBox(appearanceComposite, findReplaceOverlayAtBottom, new BooleanDomain(), 0);
-		createDependency(useOverlay, useFindReplaceOverlay, new Control[] { overlayAtBottom });
+
+		label= TextEditorMessages.TextEditorPreferencePage_restrictFindReplaceOverlayToTextEditors;
+		Preference restrictOverlayToTextEditors= new Preference(AbstractDecoratedTextEditorPreferenceConstants.EDITOR_RESTRICT_FIND_REPLACE_OVERLAY_TO_TEXT_EDITORS, label, null);
+		// Indented here rather than by the dependency below, which indents its first
+		// control only and would place this one next to that one instead of beneath it.
+		final Button overlayInTextEditorsOnly= addCheckBox(appearanceComposite, restrictOverlayToTextEditors, new BooleanDomain(), INDENTATION);
+
+		createDependency(useOverlay, useFindReplaceOverlay, new Control[] { overlayAtBottom, overlayInTextEditorsOnly });
 
 		label= TextEditorMessages.TextEditorPreferencePage_enableWordWrap;
 		Preference enableWordWrap= new Preference(AbstractTextEditor.PREFERENCE_WORD_WRAP_ENABLED, label, null);
@@ -1531,7 +1542,7 @@ private ColorEntry getSelectedAppearanceColorOption() {
 
 	private static void indent(Control control) {
 		GridData gridData= new GridData();
-		gridData.horizontalIndent= 20;
+		gridData.horizontalIndent= INDENTATION;
 		control.setLayoutData(gridData);
 	}
 

bundles/org.eclipse.ui.editors/src/org/eclipse/ui/internal/editors/text/TextEditorMessages.java

@@ -36,6 +36,7 @@ private TextEditorMessages() {
 	public static String TextEditorPreferencePage_displayedTabWidth;
 	public static String TextEditorPreferencePage_lineSpacing;
 	public static String TextEditorPreferencePage_useFindReplaceOverlay;
+	public static String TextEditorPreferencePage_restrictFindReplaceOverlayToTextEditors;
 	public static String TextEditorPreferencePage_showFindReplaceOverlayAtBottom;
 	public static String TextEditorPreferencePage_enableWordWrap;
 	public static String TextEditorPreferencePage_convertTabsToSpaces;

bundles/org.eclipse.ui.editors/src/org/eclipse/ui/internal/editors/text/TextEditorMessages.properties

@@ -19,6 +19,7 @@ TextEditorPreferencePage_displayedTabWidth=Displayed &tab width:
 TextEditorPreferencePage_lineSpacing=Line &spacing (extra % of font height):
 TextEditorPreferencePage_useFindReplaceOverlay=Use find/replace overla&y
 TextEditorPreferencePage_showFindReplaceOverlayAtBottom=&Display find/replace overlay at bottom of editor
+TextEditorPreferencePage_restrictFindReplaceOverlayToTextEditors=&Restrict find/replace overlay to text editors
 TextEditorPreferencePage_enableWordWrap=&Enable word wrap when opening an editor
 TextEditorPreferencePage_convertTabsToSpaces=&Insert spaces for tabs
 TextEditorPreferencePage_undoHistorySize=&Undo history size:

bundles/org.eclipse.ui.editors/src/org/eclipse/ui/texteditor/AbstractDecoratedTextEditorPreferenceConstants.java

@@ -252,6 +252,18 @@ private AbstractDecoratedTextEditorPreferenceConstants() {
 	 */
 	public final static String EDITOR_FIND_REPLACE_OVERLAY_AT_BOTTOM= "findReplaceOverlayAtBottom"; //$NON-NLS-1$
 
+	/**
+	 * A named preference that controls whether the find/replace overlay is restricted to text
+	 * editors, instead of being used in every part that can host it.
+	 *
+	 * <p>
+	 * The preference value is of type <code>Boolean</code>
+	 * </p>
+	 *
+	 * @since 3.23
+	 */
+	public final static String EDITOR_RESTRICT_FIND_REPLACE_OVERLAY_TO_TEXT_EDITORS= "restrictFindReplaceOverlayToTextEditors"; //$NON-NLS-1$
+
 	/**
 	 * A named preference that controls if the caret offset is shown in the status line.
 	 * <p>
@@ -784,6 +796,9 @@ private AbstractDecoratedTextEditorPreferenceConstants() {
 	public static void initializeDefaultValues(IPreferenceStore store) {
 		store.setDefault(AbstractDecoratedTextEditorPreferenceConstants.EDITOR_USE_FIND_REPLACE_OVERLAY, true);
 		store.setDefault(AbstractDecoratedTextEditorPreferenceConstants.EDITOR_FIND_REPLACE_OVERLAY_AT_BOTTOM, false);
+		store.setDefault(
+				AbstractDecoratedTextEditorPreferenceConstants.EDITOR_RESTRICT_FIND_REPLACE_OVERLAY_TO_TEXT_EDITORS,
+				false);
 		store.setDefault(AbstractDecoratedTextEditorPreferenceConstants.USE_ANNOTATIONS_PREFERENCE_PAGE, false);
 		store.setDefault(AbstractDecoratedTextEditorPreferenceConstants.USE_QUICK_DIFF_PREFERENCE_PAGE, false);
 

bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlay.java

@@ -40,6 +40,8 @@
 import org.eclipse.swt.widgets.ToolItem;
 import org.eclipse.swt.widgets.Widget;
 
+import org.eclipse.core.runtime.Assert;
+
 import org.eclipse.jface.dialogs.IDialogSettings;
 import org.eclipse.jface.fieldassist.ControlDecoration;
 import org.eclipse.jface.fieldassist.TextContentAdapter;
@@ -201,19 +203,38 @@ void install() {
 	}
 
 	public FindReplaceOverlay(IWorkbenchPart part, IFindReplaceTarget target) {
+		Assert.isLegal(canOpenInPart(part), "the overlay cannot be opened in the given part"); //$NON-NLS-1$
 		targetPart = part;
 		commandSupport = new FindReplaceOverlayCommandSupport(targetPart);
 		targetControl = getTargetControl(part);
 		findReplaceLogic = createFindReplaceLogic(target);
 		createContainerAndSearchControls();
 		containerControl.addDisposeListener(__ -> commandSupport.dispose());
+		// The overlay is a sibling of the target control, and in parts whose parent
+		// composite outlives that control it would otherwise stay behind as a widget
+		// nobody disposes, holding listeners on a disposed target.
+		targetControl.addDisposeListener(__ -> containerControl.dispose());
 		customFocusOrder.install();
 		updateReplaceVisibility(false);
 		containerControl.setVisible(false);
 		PlatformUI.getWorkbench().getHelpSystem().setHelp(containerControl,
 				IAbstractTextEditorHelpContextIds.FIND_REPLACE_OVERLAY);
 	}
 
+	/**
+	 * Tells whether the overlay can be opened in the given part, which is the
+	 * precondition of
+	 * {@link #FindReplaceOverlay(IWorkbenchPart, IFindReplaceTarget)}. It requires a
+	 * text control to place itself on and to take its colors from, which a part
+	 * showing its text through something else cannot provide.
+	 *
+	 * @param part the part to open the overlay in, may be {@code null}
+	 * @return whether the overlay can be opened in the given part
+	 */
+	public static boolean canOpenInPart(IWorkbenchPart part) {
+		return getTargetControl(part) != null;
+	}
+
 	private static StyledText getTargetControl(IWorkbenchPart targetPart) {
 		if (targetPart == null) {
 			return null;

bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/texteditor/FindReplaceAction.java

@@ -66,10 +66,22 @@ public class FindReplaceAction extends ResourceAction implements IUpdate {
 
 	private static final String FIND_REPLACE_OVERLAY_AT_BOTTOM = "findReplaceOverlayAtBottom"; //$NON-NLS-1$
 
+	private static final String RESTRICT_FIND_REPLACE_OVERLAY_TO_TEXT_EDITORS = "restrictFindReplaceOverlayToTextEditors"; //$NON-NLS-1$
+
+	// Whether the overlay can be opened in the part is only known to the overlay
+	// itself, whether it shall be is a matter of configuration.
 	private boolean shouldUseOverlay() {
+		if (!FindReplaceOverlay.canOpenInPart(fWorkbenchPart)) {
+			return false;
+		}
 		IPreferencesService preferences = Platform.getPreferencesService();
-		boolean overlayPreference = preferences.getBoolean(INSTANCE_SCOPE_NODE_NAME, USE_FIND_REPLACE_OVERLAY, true, null);
-		return overlayPreference && fWorkbenchPart instanceof StatusTextEditor;
+		if (!preferences.getBoolean(INSTANCE_SCOPE_NODE_NAME, USE_FIND_REPLACE_OVERLAY, true, null)) {
+			return false;
+		}
+		// Restores the former behavior of offering the overlay for text editors only.
+		boolean restrictedToTextEditors = preferences.getBoolean(INSTANCE_SCOPE_NODE_NAME,
+				RESTRICT_FIND_REPLACE_OVERLAY_TO_TEXT_EDITORS, false, null);
+		return !restrictedToTextEditors || fWorkbenchPart instanceof StatusTextEditor;
 	}
 
 	private static boolean shouldPositionOverlayOnTop() {
@@ -82,7 +94,9 @@ private static boolean shouldPositionOverlayOnTop() {
 		if (this.overlay == null) {
 			return;
 		}
-		if (event.getKey().equals(USE_FIND_REPLACE_OVERLAY)) {
+		if (event.getKey().equals(USE_FIND_REPLACE_OVERLAY)
+				|| (event.getKey().equals(RESTRICT_FIND_REPLACE_OVERLAY_TO_TEXT_EDITORS) && !shouldUseOverlay())) {
+			// The next invocation decides anew, so the open overlay must not outlive the change.
 			this.overlay.close();
 		} else if (event.getKey().equals(FIND_REPLACE_OVERLAY_AT_BOTTOM)) {
 			this.overlay.setPositionToTop(shouldPositionOverlayOnTop());
@@ -419,13 +433,19 @@ private void showDialog() {
 	private void showOverlayInEditor() {
 		if (overlay == null) {
 			overlay = new FindReplaceOverlay(fWorkbenchPart, fTarget);
+			// The overlay goes down with the control it is placed on, which may happen
+			// while the part is still alive, so the next invocation has to build a new one
+			// instead of reusing disposed controls.
+			overlay.getContainerControl().addDisposeListener(__ -> {
+				removeDialogPreferenceListener();
+				overlay = null;
+			});
 		}
 
 		overlay.open();
 		overlay.setPositionToTop(shouldPositionOverlayOnTop());
 
 		hookDialogPreferenceListener();
-		overlay.getContainerControl().addDisposeListener(__ -> removeDialogPreferenceListener());
 	}
 
 	@Override

tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayInEditorTest.java

@@ -14,6 +14,7 @@
 import static org.eclipse.ui.internal.findandreplace.FindReplaceTestUtil.processPendingEvents;
 import static org.eclipse.ui.internal.findandreplace.FindReplaceTestUtil.runEventQueue;
 import static org.eclipse.ui.internal.findandreplace.FindReplaceTestUtil.waitForFocus;
+import static org.eclipse.ui.internal.findandreplace.overlay.OverlayAccess.RESTRICT_FIND_REPLACE_OVERLAY_TO_TEXT_EDITORS_PREFERENCE;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
@@ -33,6 +34,9 @@
 import org.eclipse.swt.widgets.Display;
 import org.eclipse.swt.widgets.Text;
 
+import org.eclipse.core.runtime.preferences.IEclipsePreferences;
+import org.eclipse.core.runtime.preferences.InstanceScope;
+
 import org.eclipse.jface.bindings.TriggerSequence;
 import org.eclipse.jface.bindings.keys.KeySequence;
 import org.eclipse.jface.bindings.keys.KeyStroke;
@@ -81,8 +85,12 @@ public class FindReplaceOverlayInEditorTest {
 
 	private static final String REPLACE_FIELD = "replaceInput"; //$NON-NLS-1$
 
+	private static final String INSTANCE_SCOPE_NODE_NAME = "org.eclipse.ui.editors"; //$NON-NLS-1$
+
 	private StatusTextEditor editor;
 
+	private FindReplaceAction findReplaceAction;
+
 	private Text searchField;
 
 	private String testName;
@@ -97,8 +105,10 @@ void openEditorWithOverlay(TestInfo testInfo) throws PartInitException {
 
 		// Opening through the action rather than through its internals also asserts
 		// that an editor of this kind gets the overlay rather than the dialog.
-		new FindReplaceAction(ResourceBundle.getBundle("org.eclipse.ui.texteditor.ConstructedEditorMessages"), //$NON-NLS-1$
-				"Editor.FindReplace.", editor).run(); //$NON-NLS-1$
+		findReplaceAction = new FindReplaceAction(
+				ResourceBundle.getBundle("org.eclipse.ui.texteditor.ConstructedEditorMessages"), //$NON-NLS-1$
+				"Editor.FindReplace.", editor); //$NON-NLS-1$
+		findReplaceAction.run();
 
 		searchField = focusedInputField(SEARCH_FIELD);
 	}
@@ -132,13 +142,35 @@ static void closeWelcomePage() {
 
 	@AfterEach
 	void closeEditor() {
+		findReplaceAction = null;
 		searchField = null;
 		if (editor != null) {
 			editor.getSite().getPage().closeEditor(editor, false);
 			editor = null;
 		}
 	}
 
+	/**
+	 * User can restrict the overlay to text editors in case they have trouble with it elsewhere, so
+	 * an editor must keep getting the overlay rather than the dialog once they do.
+	 * <p>
+	 * The preference is meant as a temporary fallback for the overlay's generalization to parts
+	 * other than text editors. Remove this test together with it.
+	 */
+	@Test
+	public void testTextEditorKeepsOverlayWhenItIsRestrictedToTextEditors() {
+		IEclipsePreferences preferences= InstanceScope.INSTANCE.getNode(INSTANCE_SCOPE_NODE_NAME);
+		boolean wasRestricted= preferences.getBoolean(RESTRICT_FIND_REPLACE_OVERLAY_TO_TEXT_EDITORS_PREFERENCE, false);
+		try {
+			// Restricting it closes the overlay that is open, so the action decides anew.
+			preferences.putBoolean(RESTRICT_FIND_REPLACE_OVERLAY_TO_TEXT_EDITORS_PREFERENCE, true);
+			processPendingEvents();
+			assertTrue(searchField.isVisible(), "the overlay should stay open"); //$NON-NLS-1$
+		} finally {
+			preferences.putBoolean(RESTRICT_FIND_REPLACE_OVERLAY_TO_TEXT_EDITORS_PREFERENCE, wasRestricted);
+		}
+	}
+
 	/**
 	 * A key typed into an input field belongs to that field: it must never be
 	 * carried out on the editor's document instead.

tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayTest.java

@@ -14,20 +14,26 @@
 package org.eclipse.ui.internal.findandreplace.overlay;
 
 import static org.eclipse.ui.internal.findandreplace.FindReplaceTestUtil.waitForFocus;
+import static org.eclipse.ui.internal.findandreplace.overlay.OverlayAccess.RESTRICT_FIND_REPLACE_OVERLAY_TO_TEXT_EDITORS_PREFERENCE;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.Mockito.when;
 
+import java.util.Arrays;
 import java.util.ResourceBundle;
+import java.util.Set;
 
 import org.junit.jupiter.api.Test;
 import org.mockito.Mockito;
 
 import org.eclipse.swt.SWT;
 import org.eclipse.swt.graphics.Point;
 import org.eclipse.swt.graphics.Rectangle;
+import org.eclipse.swt.widgets.Display;
+import org.eclipse.swt.widgets.Shell;
 
 import org.eclipse.core.runtime.preferences.IEclipsePreferences;
 import org.eclipse.core.runtime.preferences.InstanceScope;
@@ -89,6 +95,22 @@ private static IWorkbenchPart createHostPart(TextViewer viewer) {
 		return part;
 	}
 
+	@Test
+	public void testOverlayCanOnlyBeOpenedInPartExposingTextViewerWidget() {
+		openTextViewer("line");
+
+		assertTrue(FindReplaceOverlay.canOpenInPart(createHostPart(getTextViewer())));
+		assertFalse(FindReplaceOverlay.canOpenInPart(Mockito.mock(IWorkbenchPart.class)));
+		assertFalse(FindReplaceOverlay.canOpenInPart(null));
+	}
+
+	@Test
+	public void testOverlayRejectsPartItCannotBeOpenedIn() {
+		IWorkbenchPart partWithoutTextViewer= Mockito.mock(IWorkbenchPart.class);
+
+		assertThrows(IllegalArgumentException.class, () -> new FindReplaceOverlay(partWithoutTextViewer, null));
+	}
+
 	@Test
 	public void testShortcutHintReflectsFocusedInputField() {
 		initializeTextViewerWithFindReplaceUI("line");
@@ -242,6 +264,52 @@ public void testDisableOverlayViaPreference() {
 		}
 	}
 
+	@Test
+	public void testOverlayIsOpenedByActionForPartExposingTextViewer() {
+		// The part hosting the viewer is no text editor, so this also covers that the
+		// action goes by what the part exposes rather than by its type.
+		initializeTextViewerWithFindReplaceUI("");
+		getDialog().close();
+		assertFalse(getDialog().isShown());
+
+		getFindReplaceAction().run();
+
+		assertTrue(getDialog().isShown(), "the action should have opened the overlay");
+	}
+
+	/**
+	 * User can restrict the overlay to text editors in case they have trouble with it elsewhere, so
+	 * we need to ensure that for an editor that is not a StatusTextEditor, the dialog is opened.
+	 * <p>
+	 * The preference is meant as a temporary fallback for the overlay's generalization to parts
+	 * other than text editors. Remove this test together with it.
+	 */
+	@Test
+	public void testOverlayIsNotOpenedForNonTextEditorWhenRestrictedToTextEditors() {
+		IEclipsePreferences preferences= InstanceScope.INSTANCE.getNode(INSTANCE_SCOPE_NODE_NAME);
+		boolean wasRestricted= preferences.getBoolean(RESTRICT_FIND_REPLACE_OVERLAY_TO_TEXT_EDITORS_PREFERENCE, false);
+		Display display= Display.getCurrent();
+		Set<Shell> shellsBeforeAction= Set.of(display.getShells());
+		try {
+			preferences.putBoolean(RESTRICT_FIND_REPLACE_OVERLAY_TO_TEXT_EDITORS_PREFERENCE, true);
+			initializeTextViewerWithFindReplaceUI("");
+			getDialog().close();
+			assertFalse(getDialog().isShown());
+			getFindReplaceAction().run();
+			assertFalse(getDialog().isShown(), "the action should have opened the dialog (so no overlay should be shown)");
+			assertTrue(Arrays.stream(display.getShells()).anyMatch(shell -> !shellsBeforeAction.contains(shell) && shell.isVisible()),
+					"the action should have opened the dialog");
+		} finally {
+			// The dialog is unknown to the overlay test infrastructure, so close every shell the action opened
+			for (Shell shell : display.getShells()) {
+				if (!shellsBeforeAction.contains(shell)) {
+					shell.close();
+				}
+			}
+			preferences.putBoolean(RESTRICT_FIND_REPLACE_OVERLAY_TO_TEXT_EDITORS_PREFERENCE, wasRestricted);
+		}
+	}
+
 	@Test
 	public void testSearchTermStoredInHistoryAfterSearchForward() {
 		// After a forward search, the term must be retrievable from history so that

tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/OverlayAccess.java

@@ -36,6 +36,9 @@
 import org.eclipse.ui.internal.findandreplace.WidgetExtractor;
 
 class OverlayAccess implements IFindReplaceUIAccess {
+
+	static final String RESTRICT_FIND_REPLACE_OVERLAY_TO_TEXT_EDITORS_PREFERENCE= "restrictFindReplaceOverlayToTextEditors"; //$NON-NLS-1$
+
 	private final IFindReplaceTarget findReplaceTarget;
 
 	private final HistoryTextWrapper find;