eclipse.platform.ui icon indicating copy to clipboard operation
eclipse.platform.ui copied to clipboard

[Find/Replace]: Classic Dialog doesn't handle RegEx-Errors well

Open Wittmaxi opened this issue 2 years ago • 14 comments

Description

Reg-Ex errors can be displayed in a weird, non-pragmatic, way

Reproduce

  1. Open Find/Replace (Ctrl+F)
  2. Setup Find/Replace-Dialog to Find: "[", Options: Regulax Expressions | Wrap Search
  3. Press Find
  4. See that the error message is not displayed well on the default size grafik

Misc

when resizing the Dialog, the error-message is displayed in a better format. grafik I count this as a bug because a user might not suspect that the error-message is incomplete and needs resizing of the textbox. A pragmatic approach could be either resizing the Dialog, or putting the Text into a scrollable container that shows that there is more to the error message. From code-analysis, I also believe that this error message is not localized - needs to be reproduced sometime else. I will probably not work on this

Wittmaxi avatar Oct 09 '23 12:10 Wittmaxi

Find_replace_error

The error is also displayed down in the workbench status bar .

lathapatil avatar Oct 11 '23 05:10 lathapatil

@lathapatil Wow, that is a bigger problem than expected, since the Error now cannot (at all!) be displayed in completeness. I might need to find a good solution as part of #1132

Wittmaxi avatar Oct 11 '23 07:10 Wittmaxi

I would propose to not present such an error at either of those places at all. The field in the dialog is supposed to present a result (or status) of the (executed) search, such as the number of matches, wrapping the search or the like. This error, however, is about a faulty input (an invalid regex). I propose to present this error in a completely different way to not confuse the user. In case of the existing dialog, I see two valid options:

  1. Simply open a message box that informs about invalid input
  2. Add a dedicated control to the dialog that shows information about invalid inputs (which is clearly separated from the currently used label that should only shown information about results).

A benefit of the second option would be that you can see the message while adapting the input.

HeikoKlare avatar Oct 12 '23 08:10 HeikoKlare

@HeikoKlare SWT offers a special control decoration for this:

https://help.eclipse.org/latest/index.jsp?topic=%2Forg.eclipse.platform.doc.isv%2Freference%2Fapi%2Forg%2Feclipse%2Fjface%2Ffieldassist%2FControlDecoration.html

laeubi avatar Oct 12 '23 08:10 laeubi

Note that while typing a pattern, one can often end up with an intermediate state that is not a correct pattern, so a message box is not a good option...

merks avatar Oct 12 '23 08:10 merks

@merks the error is only displayed when the Find/Replace-Logic has performed the "RegEx-Find"

Wittmaxi avatar Oct 12 '23 08:10 Wittmaxi

@HeikoKlare SWT offers a special control decoration for this:

https://help.eclipse.org/latest/index.jsp?topic=%2Forg.eclipse.platform.doc.isv%2Freference%2Fapi%2Forg%2Feclipse%2Fjface%2Ffieldassist%2FControlDecoration.html

Perfect, thanks for the hint! Then this should of course be the preferred option.

Note that while typing a pattern, one can often end up with an intermediate state that is not a correct pattern, so a message box is not a good option...

True. To be more precise: I would expect the validation to be performed only when executing a search (like it is done now).

HeikoKlare avatar Oct 12 '23 08:10 HeikoKlare

Sorry, I'm mixing it up with this dialog where the feedback is immediate:

image

merks avatar Oct 12 '23 08:10 merks

Given the similarities between these two things, one might expect more similarities. 😁

merks avatar Oct 12 '23 08:10 merks

Side-Note: @merks I proposed in a discussion with @HeikoKlare that we implement (at some point, much later!) a new view that does a bit what https://regexr.com/ does:

  1. Try out your regexes
  2. show RegEx-Error messages 2.1) especially for Dialogs that allow for RegEx-Input

Wittmaxi avatar Oct 12 '23 08:10 Wittmaxi

Given the similarities between these two things, one might expect more similarities. 😁

Fully agree 🙂 There seems to be no reuse between the find/replace dialog for a single text document and the Eclipse-wide (file) search functionality yet. Would be good in terms of both maintainability and user experience if these functions share at least parts of their implementation. That may become easier after extracting the bussines logic from the find replace dialog (for a single text document) in #1132.

HeikoKlare avatar Oct 12 '23 08:10 HeikoKlare

Hello all. I don't think this is a good first issue anymore since it probably touches parts of the code that are going to be extracted/improved/reused as part of the (preparation) work for https://github.com/vi-eclipse/Eclipse-Platform/issues/22.

May I suggest we remove the tag and provide an expected behavior? Something like:

Expected behavior

  1. The text field displaying the error should be big enough to show the complete error message, just like it is in the Search dialog that can be opened via the menu Search > Search... (Ctrl + H in Windows)
  2. The text might be multi-line (but it shouldn't cause the Find/Replace dialog to resize if it becomes too big)
  3. The Find/Replace dialog should have a minimum width allowed so that the error and the buttons can be comfortably displayed

This is just a proposal, based on my own personal taste :-)

BTW @BeckerWdf mentioned (in the Eclipse-IDE General room) that SAP's ABAP would like to offer their expertise in UX topics, maybe this is one qualifies? :-)

fedejeanne avatar Apr 12 '24 07:04 fedejeanne

May be this issue should be closed as the find is now managed with an overlay (since 2024-09) ?

https://eclipse.dev/eclipse/news/4.33/platform.php#find-replace-overlay

And regexp seems to work well inside :

image

opcoach avatar Sep 25 '24 15:09 opcoach

@opcoach If we close this issue, we should replace it with new issues

  1. the old Dialog is still an option and even though we slowly fade out support for it, we should still make the concious decision of whether we should fix this or not
  2. we might want to show regex issues in the overlay

What do you think?

Wittmaxi avatar Sep 26 '24 11:09 Wittmaxi