Skip to content

Conversation

jjwest
Copy link
Contributor

@jjwest jjwest commented Feb 4, 2019

This updates the compilation mode regex so that warnings are colored properly,
making them easier to differentiate from errors.

Before:
https://i.imgur.com/mChGm3k.png

After:
https://i.imgur.com/hBXjGk0.png

@rust-highfive
Copy link

r? @pnkfelix

(rust_highfive has picked a reviewer for you, use r? to override)

@pnkfelix
Copy link
Contributor

Hmm I've played with something like this in the past but I don't think I had gotten the updates to the numbers right when I tried it.

Let me give it a whirl to double-check that the overall UX (namely for jumping from error to error) remains the same -- a big problem we want to avoid is forcing a user to step through tons of warnings when the intention was to step directly from the last diagnostic to the next true error.

@pnkfelix
Copy link
Contributor

pnkfelix commented Mar 4, 2019

(I'm sorry that I haven't yet actually followed through on my promise to give this a whirl. Its still on my todo list.)

@pnkfelix
Copy link
Contributor

pnkfelix commented Mar 4, 2019

I gave this a quick whirl locally. (No, I didn't. I thought I was reloading my rust-mode.el but I forgot I need to actually restart emacs; reloading the lisp code does not suffice.)

Outdated incorrect commentary follows in details block.

I would have thought that if I get both warnings and errors during a compilation (e.g. because I'm compiling multiple crates), I would see distinct increments to compilation-num-errors-found versus compilation-num-warnings-found.

~~But I am not seeing that here.

(Maybe that is for some reason not what you desired?)~~

I did see that we are at least flagging the warnings in the diagnostic output, and M-x compilation-next-error (often mapped to TAB) is jumping between them. But I'm still not sure what is best.

I will admit that in some (many?) projects, one often does not see warnings mixed with errors, because the compiler will only emit warnings for a given input crate if no errors were issued (right?)

So its possible the approach used here is still a net win and should be merged.

But I don't want to do so without first double-checking if there is someway to get emacs to account for the warning/error distinction (as I think should be reflected in the two aforementioned variables).

nikomatsakis do you have any thoughts here? Would you prefer to just merge this as is?

@pnkfelix
Copy link
Contributor

pnkfelix commented Mar 4, 2019

(Or is the situation I just described where we already were at, and this PR is truly just an improvement on the status quo... hmm sorry if I misunderstood where we are in rust-mode.el at the moment...)

@pnkfelix
Copy link
Contributor

pnkfelix commented Mar 4, 2019

Yes okay looking at the linked images I am realizing that something must be wrong with my own (screen terminal-based) setup, since I am not seeing quite the same set of colors that are shown there.

@pnkfelix
Copy link
Contributor

pnkfelix commented Mar 4, 2019

So I'm going to go ahead and merge since this does seem to be a net win.

@pnkfelix pnkfelix merged commit a871d10 into rust-lang:master Mar 4, 2019
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.

3 participants