Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incomplete use of "Invalid regular expression" error message #6043

Open
sasumner opened this issue Aug 13, 2019 · 7 comments
Open

Incomplete use of "Invalid regular expression" error message #6043

sasumner opened this issue Aug 13, 2019 · 7 comments

Comments

@sasumner
Copy link
Contributor

Description of the Issue

The error message Find: Invalid regular expression is used to alert the user to a syntax problem with their regular expression ONLY when a Find Next action is invoked. An invalid-regular-expression message needs to be shown on the Find window's status bar whenever ANY type of find/replace/count/mark operation is initiated and the user's regular expression is malformed. (Obviously this applies only when the user sets the Search mode to Regular expression.)

Steps to Reproduce the Issue

  1. Invoke the Find window (e.g. ctrl+f)
  2. Put abc[def in the Find what box. (This is a bad regex because the [ pattern metacharacter has no corresponding ]).
  3. Select Regular expression for the Search mode.
  4. Press the Find Next button. Observe the Find window's status bar indicates in red: Find: Invalid regular expression. (This is correct and desired behavior and is only shown here to contrast with the following incorrect behavior.)
  5. Press the Count button. Observe the Find window's status bar indicates in blue: Count: 0 matches. The status bar at this point should indicate that the regular expression is invalid.
  6. Try other search actions (besides Find Next) that support regular expressions without changing the Find what box data. Observe that none report on the invalidity of the regular expression, but rather indicate zero matches. [Indeed, a find-in-files operation in a big folder tree will work for a long time to report zero matches, when it should abort very quickly at the start to indicate that the regex is bad.]

Expected Behavior

I expect to be told that my regular expression is not correct, and other search activities to be aborted. Zero matches is misleading when the true problem is that a search parameter is incorrect.

Actual Behavior

See 5 and 6 above in Steps to Repro...

Debug Information

Notepad++ v7.7.1 (32-bit)
Build time : Jun 16 2019 - 21:14:50
Path : C:............\npp.7.7.1.bin\notepad++.exe
Admin mode : OFF
Local Conf mode : ON
OS : Windows 10 (64-bit)
Plugins : ColumnTools.dll Explorer.dll LocationNavigate.dll MarkdownViewerPlusPlus.dll mimeTools.dll NppConverter.dll NppExec.dll NppExport.dll PythonScript.dll

@sasumner
Copy link
Contributor Author

Some investigation in the code shows that for Replace All, Count and Mark All, there is an error message The regular expression to search is malformed. that should be shown, but can't be because the logic to get to it is faulty.

An example using Count with irrelevant stuff blotted out:

int nbCounted = processAll(ProcessCountAll, ...);
...
if (nbCounted < 0)
{
    result = TEXT("Count: The regular expression to search is malformed.");
}
...
setStatusbarMessage(result, FSMessage);

In running the code with the abc[def bad regex, the nbCounted returned is zero, so the error message is never shown.

@Ekopalypse
Copy link
Contributor

Ekopalypse commented Aug 13, 2019

Probably related with this?

@sasumner
Copy link
Contributor Author

Probably related with this

Not related exactly, but I uncovered the "complaint" of this issue when researching that one. ;)

My plan is to fix both, but this one first as it is easier.

@sasumner
Copy link
Contributor Author

Similar thing as above for Replace All, Count and Mark All noted for Replace All in All Opened Documents:

nbTotal += _findReplaceDlg.processAll(ProcessReplaceAll, ...);
...
if (nbTotal < 0)
{
	generic_string msg = _nativeLangSpeaker.getLocalizedStrFromID("find-status-replaceinfiles-re-malformed", TEXT("Replace in Opened Files: The regular expression is malformed."));

but nbTotal is equal to zero after the call(s) to .processAll(), not less than zero, so the msg is never actually filled with the "malformed" string.

@sasumner
Copy link
Contributor Author

An even worse result than reporting zero replacements done, however, is this one for a Replace in Files action with an invalid regex, yikes! :

image

@xylographe
Copy link
Contributor

Actual proof that NPP is a magical application. :)

@sasumner
Copy link
Contributor Author

A replacement I was trying to do was making me crazy for a bit:

Find: \R
Repl: "); //$43XX$0

Notepad++ would perform the replacement but I didn't get what I wanted. There were 2 problems:

  1. the unescaped )

  2. the $43

At first I thought that ) should not need escaping due to no prior use in the expression of (. But that was wrong; use of an unescaped ) without a prior ( immediately truncates the replacement string.

Next I thought that the unescaped $ (in $43) was fine because clearly there is no group 43 captured via the search expression. (I think) that was wrong as well.

It sure would be nice if Notepad++ could detect these problems in the replacement expression and alert the user...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants