[Modern Find/Replace] Implemented Find-Replace Overlay

[Wittmaxi]

Modern Find-Replace Overlay

newly introduced Overlay in dark theme

newly introduced Overlay on the bottom of the screen, used to perform a replace-operation.

at the top of the screen, with some search-options enabled

What are we addressing (copied from #1090)

The current solution opens a Modal on keypress Control + F - prompting a user to enter a string for finding and optionally a string to replace the currently found string with.

The status quo also has multiple options for searching which are available by selecting the appropriate checkbox.

• The modal can be disturbing while programming
  • The modal requires a switch to a new window
  • The modal can hide important controls while open
  • It is not clear which View the Find/Replace-Dialog is bound to. This becomes an issue, when two editors are opened side-by-side.
• In many cases, the user "just want's to search", not needing too many options. I personally really like using vim's search feature for it's simplicity.
• Searching forward and backwards requires to select the correct radio button and then pressing "search" - which is not intuitive and hindering in many workflows

Showcase

https://github.com/eclipse-platform/eclipse.platform.ui/assets/16443184/baabe986-90fb-491f-829d-7eea758f4c91

[github-actions[bot]]

Test Results

 1 812 files   1 812 suites   1h 35m 0s :stopwatch:  7 634 tests  7 405 :white_check_mark: 228 :zzz: 1 :x: 24 063 runs  23 313 :white_check_mark: 749 :zzz: 1 :x:

For more details on these failures, see this check.

Results for commit 3a2e4137.

:recycle: This comment has been updated with latest results.

[jukzi]

I like the idea! But the implementation does not feel perfect yet:

1. The Minus Symbol is not at the same location as the Plus symbol. so expand/collapse does not feel right
2. The text "classic find/Replace" takes too much space. I would prefer a small button
3. i can't move the Popup around. It may hide some text in the editor. Should be either a) moveable (mouse drag) or b) outside the Editor content area. I would prefer to have it at the Bottom left - not as an overlay but like the statusbar.
4. i am missing the last searched/replaced words in a dropdown. I am using that a lot and its a real show stopper for me to not have access to the last searches.
5. The Match-Case Symbol does not feel intuitive. Better a capital letter next to an lower letter.
6. The colors feel a bit odd. i don't know what exactly is wrong. But most likely the grey border around the search input. Especially letters like "j","g" that have some drawing below the writing line do touch the border below, while Capital letters do not touch the upper border
7. after resizing the input field is sometimes distorted / text drawn to much left
8. it currently doe not work well in console view (wrong location, too small, +/- not working):
9. it should replace the search in the Javadoc search
10. error "Widget is disposed"

org.eclipse.swt.SWTException: Widget is disposed
	at org.eclipse.swt.SWT.error(SWT.java:4918)
	at org.eclipse.swt.SWT.error(SWT.java:4833)
	at org.eclipse.swt.SWT.error(SWT.java:4804)
	at org.eclipse.swt.widgets.Widget.error(Widget.java:450)
	at org.eclipse.swt.widgets.Widget.checkWidget(Widget.java:369)
	at org.eclipse.swt.widgets.Control.isFocusControl(Control.java:1899)
	at org.eclipse.ui.texteditor.FindReplaceOverlay$1.performEnterAction(FindReplaceOverlay.java:128)
	at org.eclipse.ui.texteditor.FindReplaceOverlay$1.keyPressed(FindReplaceOverlay.java:159)
	at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:171)

[Wittmaxi]

Thank you for your feedback, @jukzi.

In particular, 9) is related to this issue.

[jukzi]

Optically i like the "filter" bar in the Error Log View much more - below the editor it would be nice for searching:

[vogella]

Did we make any progress here? Would be a shame not to get this beautiful new UI into Eclipse.

[HeikoKlare]

@vogella Maximilian is still working on this. The first to merge is the refactoring in #1132. And then I hope that we can merge a first version of this new UI (probably to be optionally activated at that point in time) for 4.31 M1.

[Wittmaxi]

The layouting now happens in three stages:

1. Maximal width: Will stop growing at a certain width, such that it does not become too big (especially on large screens)

2. Compromise width The Toolbars are removed, making space for the inputs. Since all tools are still available through keybindings, we don't "lose" functionality per se, and you can still "quickly search" in your small editor. I made the assumption that users will not want to power-use the feature in an editor that was cast to the side and that users who want to perform harder operations that require the toolbar to be visible would also open the editor to long enough to display the entire find/replace-bar.

3. Worst-case compromise Every tool-button is discarded and the Dialog is resized to take up 90% of the width of the parent. You can still perform all actions and enable/disable the "Replace"-Feature using Ctrl+R

[vogella]

@Wittmaxi can you continue with this enhancement now that the refactoring is done?

[HeikoKlare]

can you continue with this enhancement now that the refactoring is done?

He is working on it, but it may still take some time due to limited time budget. We optimistically aim for a contribtion as soon as possible after 2024-03 freeze, so we have quite some time for evaluation in the development cycle for 2024-06. We at least have to do these things for an initial contribution:

• Provide a configuration option via property (to select whether old dialog or new overlay shall be used)
• Fix a bug with overlay placement in different kinds of editors (current issue with the console view)
• Select / design proper icons and finalize the look&feel
• Conduct some intensive testing

[Wittmaxi]

I expect at least one Test to fail: The FindReplaceDialog-Test currently cannot test the default behavior since the "new Overlay" is opened by default. If this persists after I correctly parametrise the new Overlay, I might have overlooked a problem while merging.

[Wittmaxi]

@HeikoKlare @vogella very happy news!

I could fix another final milestone: the Overlay now binds to the constraints of Console's when opened on one (not yet perfectly...):

Here is a screenshot of the parameter used for selecting between Find/Replace-Methods:

The Find/Replace-Overlay is off by default (for now).

I have one doubt and need to check this: What happens when I add a Listener to a Control in a PageBookView and the current Page changes? How can I retrieve that listener? Is that even necessary or will it just be destroyed? Do I risk some nullpointer-errors?

This is the relevant code:

	private void unbindListeners() {
		if (targetPart != null && targetPart instanceof StatusTextEditor textEditor) {
			Control targetWidget = textEditor.getSourceViewer().getTextWidget();
			if (targetWidget != null) {
				targetWidget.getShell().removeControlListener(shellMovementListener);
				targetWidget.removePaintListener(widgetMovementListener);
				targetPart.getSite().getPage().removePartListener(partListener);
			}
		}
		if (targetPart != null && targetPart instanceof PageBookView consoleView) {
			Control targetWidget = consoleView.getCurrentPage().getControl();
			if (targetWidget != null) {
				targetWidget.getShell().removeControlListener(shellMovementListener);
				targetWidget.removePaintListener(widgetMovementListener);
				targetPart.getSite().getPage().removePartListener(partListener);
			}
		}
	}

	private void bindListeners() {
		if (targetPart instanceof StatusTextEditor textEditor) {
			Control targetWidget = textEditor.getSourceViewer().getTextWidget();

			targetWidget.getShell().addControlListener(shellMovementListener);
			targetWidget.addPaintListener(widgetMovementListener);
			targetPart.getSite().getPage().addPartListener(partListener);
		}

		if (targetPart != null && targetPart instanceof PageBookView consoleView) {
			Control targetWidget = consoleView.getCurrentPage().getControl();

			targetWidget.getShell().addControlListener(shellMovementListener);
			targetWidget.addPaintListener(widgetMovementListener);
			targetPart.getSite().getPage().addPartListener(partListener);
		}
	}

[HeikoKlare]

I have tested the current state of implementations and found some points that we need to address:

• Pressing escape should always close the overlay when the editor has focus, not only if the overlay has focus.
• Selected options are stored across "sessions" but not visualized accordingly. E.g., if you enable "match case", close the overlay and open it again, the option is still enabled (logically), but the toggle button in the overlay is disabled.
• Only presses of the "main" Enter key are handled. The numpad Enter key is ignored.
• Usually, performing a replace operation also performs a find, i.e., moves to the next match. Sometimes, this does not happen and the match stays the same, so that performing repeated replace operations always apply to the same match instead of moving to the next one (obviously this requires the find string to be a substring of the replace string). I was not able to figure out the exact pattern when this happens, but it is actually quite often, so should be easy to reproduce.
• The "forward search" button performs a backward search.
• When I perform a search, so that a match is highlighted, then close the overlay and open it again, it moves to the next match instead of keeping the current one. When closing and reopening the overaly again (without performing another search), the same match stays highlighted. This is inconsistent. I would expect that opening the overlay does not perform any search and does never change the highlighted match.
• Regular expressions do not work (activating the option does not seem to have any effect).
• There is no feedback if a performed operation does not provide any result. It is fine for me to drop the beep functionality of the existing FindReplaceDialog (but maybe someone has a different opinion, so that should be discussed), but we should give some visual feedback to the user instead. Maybe we could do something like changing the background color of the input box to signal that nothing was found/replaced?
• The same as above applies to all other kinds of status feedbacks. For example, the number of matches when performing a "select all" is only shown in the application's status bar, but I would expect that information somewhere near the place the user is interacting with (i.e., the find/replace overlay).
• The option for only searching in the selected line(s), as known from the FindReplaceDialog, is missing. I think this is a mandatory option that we should provide.

Note that this was not an in-depth test. I just stopped after I had documented these points and will test again once they have been addressed.

One additional observation: the option for (not) wrapping the search, known from the FindReplaceDialog, is missing in the overlay. For me, it would be okay to drop that option, but we should make that explicit so if someone thinks differently we can discuss that.

[Wittmaxi]

I agree with you on the need to address these points, I will keep track of my progress in this check list. It seems like at least some of these bugs were caused by the merge of the new FindReplaceLogic-class.

I DID Intend to drop the option to not wrap search. I (personally) do not see the need to include it and would not know when I would find such an option useful. I welcome any feedback on this that could help inform this decision.

• [x] Selected options are stored across "sessions" but not visualized accordingly. E.g., if you enable "match case", close the overlay and open it again, the option is still enabled (logically), but the toggle button in the overlay is disabled. fixed
• [x] Only presses of the "main" Enter key are handled. The numpad Enter key is ignored. fixed - I don't believe other key-combinations (eg. shift+enter) would need to be supported on Keypad, please tell me if I overlooked some.
• [x] The "forward search" button performs a backward search.
• [x] Usually, performing a replace operation also performs a find, i.e., moves to the next match. Sometimes, this does not happen and the match stays the same, so that performing repeated replace operations always apply to the same match instead of moving to the next one (obviously this requires the find string to be a substring of the replace string). I was not able to figure out the exact pattern when this happens, but it is actually quite often, so should be easy to reproduce. I'm confident that I fixed this. I will test more
• [x] When I perform a search, so that a match is highlighted, then close the overlay and open it again, it moves to the next match instead of keeping the current one. When closing and reopening the overaly again (without performing another search), the same match stays highlighted. This is inconsistent. I would expect that opening the overlay does not perform any search and does never change the highlighted match. fixed
• [x] Regular expressions do not work (activating the option does not seem to have any effect). --> I could not reproduce this issue. I struggled a bit with the RegEx "(a|b)" because the asterisk needs to be escaped to "(a|b)\". Do we want to escape those symbols for the user? In that case, how would a user enter an "unescaped" asterisk? The Find/Replace-Dialog has the same quirk.
• [x] There is no feedback if a performed operation does not provide any result. It is fine for me to drop the beep functionality of the existing FindReplaceDialog (but maybe someone has a different opinion, so that should be discussed), but we should give some visual feedback to the user instead. Maybe we could do something like changing the background color of the input box to signal that nothing was found/replaced?
• [x] The option for only searching in the selected line(s), as known from the FindReplaceDialog, is missing. I think this is a mandatory option that we should provide.

--

• Pressing escape should always close the overlay when the editor has focus, not only if the overlay has focus. I could not find a good mechanism to inject a listener into the target part. I agree with you on a functionality-level that it would be great to hit escape and get a "clear view" of the file. A good solution (as discussed in person) would be to create an Interface that allows for Find/Replace-Targets to broadcast a request for all overlaying tooling to close itself. It might be interesting to think of a global strategy for all possible overlays/dialogs to close on Escape.
• The same as above applies to all other kinds of status feedbacks. For example, the number of matches when performing a "select all" is only shown in the application's status bar, but I would expect that information somewhere near the place the user is interacting with (i.e., the find/replace overlay).

[Wittmaxi]

I have now introduced the feature for "search in selection" and I have added a very naive solution for showing feedback when no match was found to the find-string. Both of these still need to be styled:

• the icon for "search in selection" is still a default-icon
• the feedback for "no match" does not look good at all and is more of a proof-of-concept for discussion

I have introduced shared and separate unit-tests for both the Find/Replace-Dialog and the Find/Replace-Overlay. I introduced an API that (unfortunately has to) use reflection in order to unify the Unit-Tests for both UIs. I unit-tested my fixes to @HeikoKlare's review and I ensured parity between both User-Interfaces using the tests. In particular, I introduced tests for RegEx-search, for Replacing, for Replacing in Selected-areas, for keybindings (for example testing that Keypad-Enter is also valid when navigating the Interface)

[Wittmaxi]

There have been UI-Improvements. The "Find in selected Area"-option has been added (the icon will definitely change, for now I just re-used another icon). I removed the big borders around the input-bars, using different shadings instead.

The interface also turns red (for a short time) when no match was found or a replace-operation couldn't be performed

added some shortcuts to access all options to parametrize the Overlay:

Keybinding	What it does
Ctrl + shift + P	Toggle RegEx-Search
Ctrl + shift + F	Close Find/Replace-Dialog
Ctrl + shift + R	Toggle Replace-Pane
Ctrl + shift + W	Toggle whole-word-search
Ctrl + shift + A	Toggle searching in selected Area
Ctrl + shift + C	Toggle case-sensitive search
Enter (in Find-Input)	Perform search
Enter (in Replace-Input)	Perform single replace
Shift + Enter (in Find-Input)	Perform search backwards
Ctrl + Enter (in Replace-Input)	Replace all
Ctrl + Enter (in Find-Input)	Highlight all

[Wittmaxi]

1. instead of changing the background, I now change the font. I don't use hard-coded colors anymore and use JFaceColors.getErrorText() instead. The font-color stays red until a valid search was entered.
2. I don't hard-code the background-colors anymore. Instead I rely on a hack to get the background-color of the Text(..., SWT.SINGLE | SWT.SEARCH)-component.
3. When opening the find/replace-overlay while having selected a word, the word that get's picked up is also automatically selected, giving the user feedback

[jukzi]

@Wittmaxi i took a quick look at the current implementation. Findings:

• modern search should be enabled by default until release, so that all testers are forced to use it.
• "only find in selected area" currently does exacty the opposite.
• the ringtone when no search result is found is â̵̧̢͖̲̐̑ň̵̺̰̣̣̈n̴̨͋o̸͓̊y̶̡̥̓ị̷̺͔͌ṋ̵̝͓͎͐̄͝g̷̬̑
• the icon for regexp does look like a "options" symbol. not intuitive
• icons for "only find in selected area" and "only find whole words" are the same - should be distinct.
• i don't find the "replace" dialog anymore
• NPE on crtl+shift+r:

java.lang.NullPointerException: Cannot invoke "org.eclipse.swt.widgets.Button.getSelection()" because "this.this$0.replaceToggle" is null
	at org.eclipse.ui.texteditor.FindReplaceOverlay$2.keyPressed(FindReplaceOverlay.java:186)
	at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:184)
	at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:91)
	at org.eclipse.swt.widgets.Display.sendEvent(Display.java:4274)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1156)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1180)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1165)
	at org.eclipse.swt.widgets.Widget.sendKeyEvent(Widget.java:1207)

[HeikoKlare]

Icons

I took a look at options for the icons to be used in the view-integrated find/replace overlay as I needed to produce a preview screenshot for a presentation anyway. I propose to slightly change the order of the provided search options (whole word, case sensitive, regex, selected area). Most importantly, I propose to put the "selected area" option at the end and have the "whole word", "case sensitive" and "regex" options first. I could image to use icons as follows:

I have used the following (CC-licensed) icons:

• Whole Word: https://www.iconbolt.com/iconsets/codicons/whole-word
• Case sensitive: https://www.iconbolt.com/iconsets/codicons/case-sensitive
• Regex: https://www.iconbolt.com/iconsets/bootstrap-icons/regex
• Expand: https://www.iconbolt.com/iconsets/darkwing-free/right
• Collapse: https://www.iconbolt.com/iconsets/darkwing-free/up
• Search all: custom icon
  • Normal:
  • 200%:

[BeckerWdf]

I have used the following (CC-licensed) icons:

• Whole Word: https://www.iconbolt.com/iconsets/codicons/whole-word
• Case sensitive: https://www.iconbolt.com/iconsets/codicons/case-sensitive
• Regex: https://www.iconbolt.com/iconsets/bootstrap-icons/regex
• Expand: https://www.iconbolt.com/iconsets/darkwing-free/right
• Collapse: https://www.iconbolt.com/iconsets/darkwing-free/up
• Search all: custom icon

I you need help with creation of new icons. Just ping me.

[Wittmaxi]

@jukzi A possible explanation for the missing "replace"-expansion is that you were currently inside of a read-only file - can you please check if the bug still persists in an editable java-document?

@HeikoKlare and I agree with you on the ringtone being annoying. We adopted the behaviour from the old overlay - if there are no accessibility-concerns, I would be more than happy to just get rid of the ringtone alltogether.

I have adapted the icons according to Heiko's comment. I will push that PR after dealing with the merge issues.

[vogella]

+1 for removing the ringtone in general (also in the existing functional)

[Wittmaxi]

I have removed the Ringtone, added the newly proposed icons (any feedback?) and cleaned up the repository for a possible upcoming review.

Since we cannot different icon sets based on themes (or can we?), I will find a hue for the icons that works in both themes reasonably well - like the custom "Select All".

[HeikoKlare]

You can try the following icons for use with light and dark theme. I have recolored the proposed icons (using the original vector graphics and Inkscape) with the color of the "search all" icon. I have also create new replace icons based on the color gradient of the forward/backward search icons: findreplaceicons.zip

I you need help with creation of new icons. Just ping me.

Thank you for offering help, @BeckerWdf! We greatly appreciate any kind of help, in particular on the icon. We had the following essential points in our discussions:

• We want to have icons that fit into the existing Eclipse "theme" (discussions on maybe changing the theme should be done independently). In particular, this means to keep using the 3D icon styling with color gradients. We think that the icons for the forward search and backward search button fit quite well. I have created icons for the replace actions based on the color gradient of the forward/backward search icons.
• My proposal for the search option icons (whole word, case sensitive, regex) may be considered a kind of style break, as they are almost text-only icons (single-colored and flat). However, we cannot think of any way of visually presenting these options that better fits into the current theme and is still comprehensive enough.

Maybe you have an opinion on these points or the proposed icons themselves? In case you have feedback to the user experience in general, that would also be great, as we now have the chance to properly design this new UI element.

+1 for removing the ringtone in general (also in the existing functional)

Fully agree. Potentially removing it from the existing functionality should be something we do in a separate PR.

[BeckerWdf]

Since we cannot different icon sets based on themes (or can we?)

No we can't. So they have to fit for both light and dark background.

[BeckerWdf]

Maybe you have an opinion on these points or the proposed icons themselves? In case you have feedback to the user experience in general, that would also be great, as we now have the chance to properly design this new UI element.

I like the icons. I only have one remark.

This is the replace icon:

and this the replace all icon:

The replace all icon is a variation of the replace icon but makes to difference.

1. There multiple boxes
2. There are more letter

Do you think both is needed? Wouldn't it be sufficient to only do the "multiple boxes" think because the "all" refers to multiple occurrences and not multiple letters.

[HeikoKlare]

Do you think both is needed? Wouldn't it be sufficient to only do the "multiple boxes" think because the "all" refers to multiple occurrences and not multiple letters.

Makes total sense. I used the icons from an icon theme as a template without challenging the way the icons are built. Here is the "replace all" icon with only the multiple boxes: simplified_replace_all.zip

One reason for the icons in the theme being defined that way could be that the 100% version of the icons is so small that only one element of distinction may be hard to grasp at a first glance. For me it is still quite easy to distinguish them, in particular if they appear next to each other. But w.r.t. accessibility, it may be different for visually impaired people?

[BeckerWdf]

Do you think both is needed? Wouldn't it be sufficient to only do the "multiple boxes" think because the "all" refers to multiple occurrences and not multiple letters.

Makes total sense. I used the icons from an icon theme as a template without challenging the way the icons are built. Here is the "replace all" icon with only the multiple boxes: simplified_replace_all.zip

One reason for the icons in the theme being defined that way could be that the 100% version of the icons is so small that only one element of distinction may be hard to grasp at a first glance. For me it is still quite easy to distinguish them, in particular if they appear next to each other. But w.r.t. accessibility, it may be different for visually impaired people?

Looks good.

[Wittmaxi]

I have used this icon as a basis to create this new icon for the "search in area"-option.

With that in place, the new Overlay looks like this:

Quite a drastic improvement, IMHO! Thank you very much @HeikoKlare @BeckerWdf @vogella for your help :)

[HeikoKlare]

With that in place, the new Overlay looks like this:

Looks good! I'd like to proposal one final change: we now almost have a visual separation of search options (uni-colored icons) and search actions (gradient-colored icons). The only derivation is the icon for the search all action, which is uni-colored as well. Maybe we could align that with the forward/backward search an replace icons? colored_search_all.zip

This should make it is easy for users to visually distinguish between search options and search actions.

[BeckerWdf]

Maybe we could align that with the forward/backward search an replace icons?

Good idea.