Fix cargo compilation regex #414
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously this regex attempted to match the entire cargo output including the name of the thread that panicked and the entire panic message. In addition it was required that there be whitespace before the start of the word "thread" in that output. I don't know if this was the previous format, but in current Rust compilers "thread" comes at the beginning of the first line. However, there were two problems with this approach. One is that making that leading whitespace mandatory caused the regex to not match correctly. The second problem is that multiline regex matching in emacs is a bit iffy, and even matching the start correctly, if the panic message contained enough characters the full multiline regex could not be matched and the same problem would occur. This commit changes and vastly simplifies the regex to search for the guaranteed portion of the cargo output, starting with the ending `', `. This does come with an increased risk of accidentally matching input that is not actually a panic message, but I think in practice this will be fairly rare.
It's more distracting than helpful.
|
Sorry for the delay but I'm extremely careful changing regexes, especially when I don't totally understand the consequences. It was years ago that I have touched code that is related to compilation regexes. But I think I'll give this a try since I think I remember this issue(I'm using the code of my rust-mode extension that I wrote 4 years ago). If anybody notices problems please open an issue. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently the regex that matches panic messages in cargo output does not correctly match the file name and line number. This causes attempts to jump to one of these failures with
next-error(or similar commands) brings up a prompt for a file name to search in and generally causes unnecessary friction in dealing with test failures or other explicit panics during tests.This change to the regex provides at least a partial fix for that issue by reducing the amount of text matched so that it won't be a multi-line match. The new method has a small chance of false-positive matches, but a really robust method would require substantially more code I think. I might work on that too though because I think I have an idea of how that might work.
The second commit reduces the amount of text that would be highlighted/underlined by compilation mode when that regex does match. This version only underlines the filename and line number rather than the entire panic message.