-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Providing an invalid regex produces a misformatted error message #395
Comments
Oh that's just wonderful. The error message is pretty printing the AST, and since the AST isn't a faithful AST, it ends up emitting an unreadable character class. (The AST stores the expanded form of There are various things that can fix this. One is building our own error message. Another is rewriting the regex parser to itself give better error messages (which is in progress). |
…urntSushi Remove sub-expressions from parsing error This PR skips printing sub-expressions in the error message. This should avoid situations where the error message contains an AST with unreadable characters, as shown [here](BurntSushi/ripgrep#395). <img width="1030" alt="unreadable-chars" src="https://user-images.githubusercontent.com/12599697/29924193-02147d0c-8e11-11e7-8b36-a97b63f802b4.png">
This update brings with it many bug fixes: * Better error messages are printed overall. We also include explicit call out for unsupported features like backreferences and look-around. * Regexes like `\s*{` no longer emit incomprehensible errors. * Unicode escape sequences, such as `\u{..}` are now supported. For the most part, this upgrade was done in a straight-forward way. We resist the urge to refactor the `grep` crate, in anticipation of it being rewritten anyway. Note that we removed the `--fixed-strings` suggestion whenever a regex syntax error occurs. In practice, I've found that it results in a lot of false positives, and I believe that its use is not as paramount now that regex parse errors are much more readable. Closes #268, Closes #395, Closes #702, Closes #853
This update brings with it many bug fixes: * Better error messages are printed overall. We also include explicit call out for unsupported features like backreferences and look-around. * Regexes like `\s*{` no longer emit incomprehensible errors. * Unicode escape sequences, such as `\u{..}` are now supported. For the most part, this upgrade was done in a straight-forward way. We resist the urge to refactor the `grep` crate, in anticipation of it being rewritten anyway. Note that we removed the `--fixed-strings` suggestion whenever a regex syntax error occurs. In practice, I've found that it results in a lot of false positives, and I believe that its use is not as paramount now that regex parse errors are much more readable. Closes #268, Closes #395, Closes #702, Closes #853
I tried to search for the regex
\s*{
, which is invalid—the{
needed to be backslash-escaped. The error that ripgrep produced was kind of bizarre though:I’m using ripgrep 0.4.0 with the built-in macOS Terminal application. The text encoding is set to UTF-8.
Here is a hex dump of ripgrep’s error message:
Other queries that end with the bad
{
produce fine-looking error messages:The text was updated successfully, but these errors were encountered: