[Find/Replace] 'Find Next' (Ctrl+K) command broken with the overlay

[sratz]

When the find/replace overlay is enabled, the 'Find Next (Ctrl+K)' (org.eclipse.ui.edit.findNext) command has no effect.

When the find/replace overlay is disabled, it works again.

When then re-enabling the overlay, Ctrl+K still works on the 'old' search pattern that was entered in the legacy dialog.

The findNext action org.eclipse.ui.texteditor.FindNextAction works on org.eclipse.ui.texteditor.FindNextAction.getDialogSettings() which uses

IDialogSettings settings = PlatformUI.getDialogSettingsProvider(FrameworkUtil.getBundle(FindNextAction.class)).getDialogSettings();

Apparently this dialog setting is not written anymore when using the new overlay.

I guess the overlay should also write to those dialog settings to keep the search histories in sync.

[jmccabe]

I was hoping to be the first to report this but, sadly, no, you got in before me 😢

Unless there's another key combination that can do the job, I'm going to have to revert to not using the overlay, even though it looks nice.

[HeikoKlare]

Thank you for reporting this. I agree on the expectation that those actions should behave according to the last search operation (no matter whether it was in the dialog or overlay).

Still, this might be a bit tricker to resolve (properly). Having a static value for the search pattern worked with the existing dialog, as there was only one for the whole workbench. Now that there is one overlay per editor, we need to ensure that the context is properly considered. Maybe it is easy to fix the behavior but I am not sure whether it may also require design change to not have it work "by accident" (as the last search operation always implicitly belongs to the editor/view of interest).

[Wittmaxi]

I am having trouble understanding what is the issue here. I just did a naive runthrough and could not reproduce any issues - this is my user-journey, everything works as (I) expected.

Can somebody please fill me in on what exactly is the issue here?

[Wittmaxi]

Either way, a lot of these "quick search" command reimplement logic which was abstracted into the FindReplaceLogic class. (Ctrl+I, Ctrl+J, Ctrl+K, ...). It would be a clear improvement in code quality to make these commands re-use the logic which is now very well tested and controlled

[HeikoKlare]

Can somebody please fill me in on what exactly is the issue here?

The search string for CTRL+K is not updated from the find/replace overlay. In your screencast, you set the search string via double-clicking the exact same search string as entered into the overlay, so it will be searched via CTRL+K. If you don't do that (just switch focus from the overlay to the editor), it will not behave as expected:

[Wittmaxi]

@HeikoKlare when refactoring the search history, I changed the name of the dialog-settings entry to "searchhistory" to fit the name to match our notion of "searching" in the overlay.

The FindNextAction still uses "findhistory" as search key.
FindNextAction; 364 vs FindReplaceOverlay; 631

This is the problem and we should make sure all three have a similar name

[HeikoKlare]

Great, can you provide a PR with a fix?

[Wittmaxi]

@HeikoKlare I am currently working on this