Skip to content

compilation regexps: Match line before, too. #294

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

Closed
wants to merge 1 commit into from

Conversation

ijackson
Copy link

@ijackson ijackson commented Jan 5, 2019

"New format" (since 2016) Rust error messages look like this:

error[E0308]: mismatched types
--> src/bin/e19b.rs:672:47
|
672 | ExprRef::new(0, Nt(NT{ args : others, ..x_nt })),
| ^^^^^^ expected struct ExprRef, found enum ExprCore
|
= note: expected type std::vec::Vec<ExprRef>
found type std::vec::Vec<ExprCore>

The regexp matches the line with the -->'. But of course next-error scrolls the window so the start of the regexp is at the top of the buffer. So we must match the line before, too. Rust won't ever start its messages with a -->' so we can unconditionally include the
previous line in the match.

Signed-off-by: Ian Jackson ijackson@chiark.greenend.org.uk

"New format" (since 2016) Rust error messages look like this:

  error[E0308]: mismatched types
     --> src/bin/e19b.rs:672:47
      |
  672 |                 ExprRef::new(0, Nt(NT{ args : others, ..x_nt })),
      |                                               ^^^^^^ expected struct `ExprRef`, found enum `ExprCore`
      |
      = note: expected type `std::vec::Vec<ExprRef>`
		 found type `std::vec::Vec<ExprCore>`

The regexp matches the line with the `-->'.  But of course next-error
scrolls the window so the start of the regexp is at the top of the
buffer.  So we must match the line before, too.  Rust won't ever start
its messages with a `-->' so we can unconditionally include the
previous line in the match.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @pnkfelix (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@ijackson
Copy link
Author

ijackson commented Jan 5, 2019

I filed a Debian bug about this too; that has a formal "steps to reproduce" and a patch against 0.3.0 (the forward/back-port has trivial conflicts). https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=918373

@pnkfelix
Copy link
Member

Is it possible to add a regression test for this to rust-mode-tests.el as well?

@nikomatsakis
Copy link
Contributor

There is a reason we did not use this approach: multi-line regex in compiler-mode were found to be unrealiable in practice. My theory (unverified) was that there was some kind of line-by-line buffering going on.

@nikomatsakis
Copy link
Contributor

(Also, this doesn't seem to eliminate the "scrolling up" code?)

@ijackson
Copy link
Author

ijackson commented May 12, 2019 via email

@ijackson
Copy link
Author

ijackson commented May 12, 2019 via email

@Dushistov
Copy link

There is a reason we did not use this approach: multi-line regex in compiler-mode were found to be
unrealiable in practice. My theory (unverified) was that there was some kind of line-by-line buffering going on.

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=25133

@jimblandy
Copy link
Contributor

This bug seems to have been fixed differently in #406. From the current rust-compile.el:

(defun rustc-scroll-down-after-next-error ()
  "In the new style error messages, the regular expression
matches on the file name (which appears after `-->`), but the
start of the error appears a few lines earlier.  This hook runs
after `next-error' (\\[next-error]); it simply scrolls down a few lines in
the compilation window until the top of the error is visible."

This seems to be addressing exactly what this PR is concerned with.

I tried out the steps to reproduce in the Debian bug, and next-error put the line starting with "error: " at the top of the window, so I think this is working.

If this isn't correct, please re-open this issue and provide more details.

@jimblandy jimblandy closed this Jan 8, 2023
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

Successfully merging this pull request may close these issues.

6 participants