Stream: t-compiler

Topic: json-rendered flag


nikomatsakis (Jul 08 2019 at 16:46, on Zulip):

Hey @oli -- you around?

nikomatsakis (Jul 08 2019 at 16:46, on Zulip):

(Also cc @Eric Huss, @Alex Crichton)

oli (Jul 08 2019 at 16:46, on Zulip):

Jop

nikomatsakis (Jul 08 2019 at 16:46, on Zulip):

I wanted to briefly discuss #60987

nikomatsakis (Jul 08 2019 at 16:46, on Zulip):

I think we should stabilize something, so I moved to stabilize the latest proposal.

That said, I have some concerns: I think this is mildly backwards incompatible and not necessarily the behavior that people would expect.

The original proposal, where --json-rendered is effectively a hidden flag that controls the details of JSON output, seems fine to me and in some ways superior. Moreover, it's something we could always deprecate in the future. So I kind of prefer the original proposal.

nikomatsakis (Jul 08 2019 at 16:47, on Zulip):

If I fcp cancel and move to merge the original proposal, are there strong objections?

oli (Jul 08 2019 at 16:47, on Zulip):

On a :boat: in the Mediterranean, so no promises on internet stability

nikomatsakis (Jul 08 2019 at 16:47, on Zulip):

I basically want to move pipelined compilation forward :)

nikomatsakis (Jul 08 2019 at 16:47, on Zulip):

Also, I'm jealous

oli (Jul 08 2019 at 16:48, on Zulip):

Reading the issue, one sec

Zoxc (Jul 08 2019 at 16:49, on Zulip):

I like the idea of a json flag which let's you request what you want printed with JSON, https://github.com/rust-lang/rust/issues/60419#issuecomment-508217889

Zoxc (Jul 08 2019 at 16:49, on Zulip):

Colored errors could be one thing on that list

oli (Jul 08 2019 at 16:51, on Zulip):

Yea, renaming json-rendered is probably a good idea

oli (Jul 08 2019 at 16:51, on Zulip):

Other than that, the original proposal is forwards compatible with passing a list of things

nikomatsakis (Jul 08 2019 at 16:51, on Zulip):

Yep. If it's just a question of the name, I think basically any name seems fine

nikomatsakis (Jul 08 2019 at 16:52, on Zulip):

Is it is --json vs --json-rendered?

oli (Jul 08 2019 at 16:52, on Zulip):

As long as we have some way to combine flags like color and shortness in a sensible way (dash separated is fine by me), I have no objections

nikomatsakis (Jul 08 2019 at 16:52, on Zulip):

I don't understand what you mean by that :)

nikomatsakis (Jul 08 2019 at 16:53, on Zulip):

I guess you mean --json-rendered=foo-bar or something?

oli (Jul 08 2019 at 16:53, on Zulip):

Yea

nikomatsakis (Jul 08 2019 at 16:53, on Zulip):

as opposed to ,?

nikomatsakis (Jul 08 2019 at 16:53, on Zulip):

anyway, that's for the future it seems

oli (Jul 08 2019 at 16:53, on Zulip):

No

oli (Jul 08 2019 at 16:53, on Zulip):

Comma is for multiple things

nikomatsakis (Jul 08 2019 at 16:53, on Zulip):

oh I see

nikomatsakis (Jul 08 2019 at 16:54, on Zulip):

so something like --json=color=foo,bar-shortness=true?

oli (Jul 08 2019 at 16:54, on Zulip):

Dash is for having short diagnostics but with color or short diagnostics without colora

oli (Jul 08 2019 at 16:54, on Zulip):

Hmmm that may be overkill

nikomatsakis (Jul 08 2019 at 16:54, on Zulip):

seems like overkill but also not an immediate concern :)

nikomatsakis (Jul 08 2019 at 16:54, on Zulip):

or at least, there are many plausible separators

oli (Jul 08 2019 at 16:54, on Zulip):

Just diagnostics-short-color

nikomatsakis (Jul 08 2019 at 16:55, on Zulip):

the current proposal accepts plain, ansi-colors, short, short-ansi-colors

oli (Jul 08 2019 at 16:55, on Zulip):

Yea I mean just to change the original proposal by Alex to be diagnostic-*

nikomatsakis (Jul 08 2019 at 16:55, on Zulip):

oh like change the values to diagnostic-plain, ...?

oli (Jul 08 2019 at 16:55, on Zulip):

In order to differentiate from zoxcs artifact example

nikomatsakis (Jul 08 2019 at 16:56, on Zulip):

I see

nikomatsakis (Jul 08 2019 at 16:56, on Zulip):

/me shrugs

nikomatsakis (Jul 08 2019 at 16:57, on Zulip):

i.e., because these are all "diagnostic things", so we can group them

oli (Jul 08 2019 at 16:57, on Zulip):

We can then worry about what passing both diagnostic-short and diagnostic-short-color means in the futurr

oli (Jul 08 2019 at 16:57, on Zulip):

We can then worry about what passing both diagnostic-short and diagnostic-short-color means in the future

nikomatsakis (Jul 08 2019 at 16:57, on Zulip):

in case we have in the future some other kinds of json (e.g., artifacts)

oli (Jul 08 2019 at 16:57, on Zulip):

Or that

oli (Jul 08 2019 at 16:57, on Zulip):

Ok

nikomatsakis (Jul 08 2019 at 16:57, on Zulip):

I think any set of strings is fine :)

oli (Jul 08 2019 at 16:57, on Zulip):

Original proposal seems fine to me

oli (Jul 08 2019 at 16:58, on Zulip):

Maybe error out if --color is used together with --json-rendered

oli (Jul 08 2019 at 16:59, on Zulip):

I gotta run now, dinnertime, ruddering to the shore

nikomatsakis (Jul 08 2019 at 16:59, on Zulip):

ok

oli (Jul 08 2019 at 17:00, on Zulip):

So yea, going with Alex's original proposal is fine with me

pnkfelix (Jul 08 2019 at 19:59, on Zulip):

I have sympathy for @eddyb 's point that there may be a collection of related flags hiding here that deserve treatment analogous to -C

pnkfelix (Jul 08 2019 at 19:59, on Zulip):

can we grab -J (json) or -F (formatting) for this or something ?

pnkfelix (Jul 08 2019 at 20:00, on Zulip):

having said that, the time for such discussion was probably back when @eddyb first made the point. I doin't mind being told "we don't want to keep bike-shedding this, and such a flag-family could be added later backward-compatibly"

nikomatsakis (Jul 08 2019 at 20:28, on Zulip):

having said that, the time for such discussion was probably back when eddyb first made the point. I doin't mind being told "we don't want to keep bike-shedding this, and such a flag-family could be added later backward-compatibly"

roughly my feeling :P

nikomatsakis (Jul 08 2019 at 20:28, on Zulip):

keep in mind that this is a fairly "internal" flag

nikomatsakis (Jul 08 2019 at 20:29, on Zulip):

i.e., it's really just meant for cargo to communicate with rustc

nikomatsakis (Jul 08 2019 at 20:29, on Zulip):

(@Alex Crichton and I were debating about whether it would make sense to have a --cargo=foo=bar flag where the expected arguments are documented as potentially changing every rustc revision, so we could add stuff like this without stabilizing the core rustc-cargo interface.)

pnkfelix (Jul 09 2019 at 08:21, on Zulip):

I think --cargo=foo=bar is fine as long as we take care in how we "sell" it. We probably don't want to have features that are forever deployed solely for cargo (or at least not many such features): If its worth having for cargo, its probably worth providing to others.

pnkfelix (Jul 09 2019 at 08:21, on Zulip):

But if the intent is more: "This is how we are initially exposing such functionality, and in the longer term we may add access to these features via proper, stable flags", then that is easier for me to swallow.

eddyb (Jul 11 2019 at 08:18, on Zulip):

-Fjson=... would be fine with me

eddyb (Jul 11 2019 at 08:19, on Zulip):

(but I agree this should've been discussed back then)

nikomatsakis (Jul 11 2019 at 14:55, on Zulip):

But if the intent is more: "This is how we are initially exposing such functionality, and in the longer term we may add access to these features via proper, stable flags", then that is easier for me to swallow.

yeah, I agree with that @pnkfelix

eddyb (Jul 11 2019 at 14:56, on Zulip):

to quote myself:

IMO one thing I'd be fine with is just increasing the amount of detail in the JSON output, and expect any consumers to adapt
as an alternative to increasing amounts of configurability

matklad (Jul 17 2019 at 11:03, on Zulip):

I were debating about whether it would make sense to have a --cargo=foo=bar flag where the expected arguments are documented as potentially changing every rustc revision

That might come in handy for "out of process rustc for RLS": flag --dump-save-analysis should be usable on stable by RLS, but we don't want to stabilize it.

Last update: Nov 16 2019 at 01:10UTC