Add env flags RUSTC_COLOR and RUSTC_ERROR_FORMAT

[estebank]

• Rework flag RUSTC_COLOR.
• Add flag RUSTC_ERROR_FORMAT.

r? @nikomatsakis

[estebank]

I have a separate commit awaiting for this and #45752 to be merged to make the output configurable:

[nikomatsakis]

Some thoughts:

1. I don't see any tests here.
2. Can we feature-gate this?
   • (For example, we might require -Z custom-color or something before we check for the presence of the environment variable.)
3. I think we want to have an RFC here. It feels like the choice of environment variables and their legal values ought to be more widely discussed (and advertised).

[nikomatsakis]

cc @rust-lang/compiler @rust-lang/dev-tools

This PR modifies the compiler to consider various environment variables and tweak its output accordingly (for example, permitting "shorter" or "longer" output, or more or less colors). Such a step may be inevitable, though I think we should endeavor not to wind up supporting too many options.

It seems like something we ought to discuss in an RFC -- perhaps a dev-tools RFC? I'm not really sure who "controls" the compiler's command-line interface, it's kind of a grey area.

There is also a sort of backwards compatibility aspect here: examining an environment variable we previously ignored could break people's tooling. Do we care about this?

[alexcrichton]

There's an old PR for a similar feature (that was closed) as well

[GuillaumeGomez]

I was hoping for something even more powerful, like LS_COLORS (to change output colors through an environment variable). I'm not sure this is such a good idea to pass this option as an environment variable as well...

[nikomatsakis]

I would think that we would want to put more complex configuration into some form of file, no?

[estebank]

@nikomatsakis @GuillaumeGomez That was my original thinking, adding a configuration file that could have as many flags as possible so that anyone could fine tweak the diagnostic output to their liking, but getting serde to work in rustc means foregoing serde_derive, which makes it slightly more annoying than I'd like.

[nikomatsakis]

@estebank

Argh, that is frustrating.

Of course, in the limit, we could write some custom code for parsing the particular TOML that the compiler wants (which would presumably be very simple -- at least for now).

Alternatively, we could perhaps run a "pre-process" step to generate the glue (and check it in).

All suboptimal, of course, but it would offer us a way forward. We ought to look at how to support serde-derive though.

[nikomatsakis]

@estebank what do you think about landing this as is but with some kind of -Z flag required, just for now? I'd like to make progress on the color etc.

[nrc]

I'm not sure whether I prefer a file or env vars - the latter is certainly easier for tools which call rustc. I guess I don't really know what the use cases are here, and in that context I think an RFC, or maybe an internals thread making a plan for the error display evolution would be good.

[nikomatsakis]

I think I agree with the idea of an RFC -- what team do you think ought to make the final decision? Sometimes I feel like we need a "compiler UI" team that is focused on error display and formatting.

[estebank]

I'll draft up an RFC over the weekend.

[nrc]

what team do you think ought to make the final decision?

It works pretty well to have multiple teams on an RFC, so dev-tools + compiler seems good

[nikomatsakis]

@estebank my review queue is ridiculous and I'd like to clear it out some. Do you think we can either land this gated (pending an RFC) or else close it? I'm good with either one, really.

[BubbaSheen]

Close it in my personal opinion.

…

On Jan 19, 2018 6:39 PM, "Niko Matsakis" ***@***.***> wrote: @estebank <https://github.com/estebank> my review queue is ridiculous and I'd like to clear it out some. Do you think we can either land this gated (pending an RFC) or else close it? I'm good with either one, really. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#46961 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AhXLZtb93khPXnFnFmuJmZccgTXJT-36ks5tMSfEgaJpZM4RLiqv> .

[estebank]

Closing it in favor of gating on -Zteach.