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

add code to handle new-style rustc errors #154

Merged
merged 1 commit into from
May 6, 2016

Conversation

nikomatsakis
Copy link
Contributor

These errors are available on nightly builds (or will be soon), but only (at the moment) when enabled via environment variable. They will become the default at some point in the future. In this commit we match on the -->, but after that we have to scroll the compilation window to make the error visible.

(One shortcoming of this commit is that the resulting colors could be better: emacs highlights the filename and line-number, but not the error message itself. I consider the result suboptimal. I considered adding more logic here to add a "bold" attribute to the error message lines, but haven't had time to try it out.)

r? @MicahChalmer

(point))))
(set-window-start (selected-window) start-of-error))))))

(add-hook 'next-error-hook 'rustc-scroll-down-after-next-error)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, I should probably move this into eval-after-load 'compile

These errors are available on nightly builds (or will be soon), but
only (at the moment) when enabled via environment variable. They will
become the default at some point in the future.

In this commit we match on the `-->`, but after that we have to scroll
the compilation window to make the error visible. One shortcoming is
that if you enter the window and click on the filename/line-number, then
the "next-error-hook" doesn't seem to run. (If you click at the start of
the line, it does.) It may be possible to tweak the "hyperlink" here to
make that work more smoothly, or perhaps add a hook somewhere else.
@MicahChalmer
Copy link
Contributor

Interesting discussion happening on https://internals.rust-lang.org/t/editor-compatibility-and-the-new-error-format/3435/21. I would hope that somehow there's a way to get it to work without the hack...but since the new style errors are in rustc nightly, better to put this in for now. The hack can be removed if it becomes unnecessary (as in your suggestion on rust-lang/rust#32486.)

The hook added here will scroll the compilation window so that the start of the error message is at the very top of it, even if that wasn't necessary to bring the error message into view. That is, it will scroll away anything above the error, even if the error was already visible before the next-error call. That may not be a bad thing (if the error is going to print out a big block of code, I'd even say it's a good thing) but it's different than the normal behavior for other compilations.

@MicahChalmer MicahChalmer merged commit 4fce178 into rust-lang:master May 6, 2016
(let ((file "\\([^\n]+\\)")
(start-line "\\([0-9]+\\)")
(start-col "\\([0-9]+\\)"))
(let ((re (concat "^ *--> " file ":" start-line ":" start-col ; --> 1:2:3
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nikomatsakis why didn't you "just" make this regexp match the pair of lines, rather than the hack of scrolling backwards via rustc-scroll-down-after-next-error ?

E.g. this code I think would do the trick:

    (let ((re-new (concat "^" error-or-warning "\\[E[0-9]*\\]: [^\n]*\n"
                          "   --> " file ":" start-line ":" start-col)))
      (cons re-new '(2 3 4 1)))

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm I infer from this comment #154 (comment) that you tried something like this and determined it does not work reliably.

Do we have errors messages that span multiple lines? I suppose I should not be surprised if we did...

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.

4 participants