-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Stabilize short error format #49546
Stabilize short error format #49546
Conversation
Does this have a tracking issue? |
No, it's just a format I added a long time ago. I think it's time to stabilize it now. |
@rfcbot fcp merge This is a simple addition that is very useful for simple IDEs without rls support or just getting a good overview of all errors without the "noise" of details. I don't know of any open questions about it, except that some errors display very badly (e.g. An output example can be found in https://github.com/GuillaumeGomez/rust/blob/3ac4f6d7a41caebdf612c079d0bf13b0f54bee74/src/test/ui/short-error-format.stderr |
Team member @oli-obk has proposed to merge this. The next step is review by the rest of the tagged teams:
Concerns:
Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
@rfcbot concern weight Is this pulling its weight? Are people and/or tools using it? Do we have reports that it is satisfying their needs properly and is not just a stop gap for them? |
@rfcbot concern details Many errors don't make sense without the details (the secondary diagnostics), or at least are not very clear. Is this a problem here? Would the short form be better if it included these, but excluded the code snippets and some other stuff? (Having registered both these concerns, my inclination is still to stabilise, however, I do want to check we're doing the right thing). Also cc @rust-lang/dev-tools |
I know a few who are using it yes (the ones who asked for it). They use it to avoid getting huge rustc output mainly, not for tooling. |
I tried it today and yesterday and I like it, in general. |
I have no idea what is being "stabilized" here. Surely not the text of the messages. Something about the format? If so, what? It'd be nice to see some documentation to that effect. |
(Personally, I feel this could have merited an RFC, also.) |
There's a vocal small group of people that really like this feature. The increase in complexity is fairly contained.
I'm yet to write down a proposal pointing out (been offline for two weeks), but I want to formalize the way all errors are written to make short errors always make sense. Labels and notes should provide extra context and guidance, not be mandatory for understanding, although for the type mismatch errors we might want to wait until we have a way to use "shorter" (local namespace) type names to avoid overly long messages. As to @nikomatsakis' comments, I feel that the improvement of the text to fit this format can be tackled progressively over time and the feature is small and obscure enough (I foresee few people to use it on a regular basis, and the ones that do are more likely to accept the growing pains until we improve the messages) to not be a problem to stabilize it as presented. |
Perhaps we can add a ~/.cargo/config key for this? You can already do this as a rustflags key but a dedicated key should be nice. IMO folks should not be interacting with Rust flags directly as much as possible. |
Ping from triage @rust-lang/compiler! How's the discussion going for this? |
I still do not know what we are stabilizing! Can somebody please write that up? All I see: I would like to see something like the report in this issue: It doesn't have to be long, just say what is being stabilized and not. =) |
Also, how it is tested =) |
Some more discussion on IRC here. Specifically, I would like to know:
Presumably we are not stabilizing the specific text of error messages, which I guess we would never do. Is there documentation, or a plan to document? Are there tests? |
Here's what i know about this feature. @GuillaumeGomez can fill in more specifics if i'm missing something.
If there are other implications to adding an error format, i haven't considered them. I personally think it's harmless to add, but i also didn't write it. 😁 EDIT: For an example of how this looks, there's a UI test that shows it off. |
So I would specifically like to know if we are committing to:
I think we should consider adopting one of the standard gnu formats, which I think we almost do, except that we should say |
@rfcbot resolved what-is-being-stabilized thanks @QuietMisdreavus ! |
@rfcbot concern gnu-compatible I think the output should be compatible with the standard gnu formats, which I think we almost do, except that we should say |
@rfcbot resolve what-is-being-stabilized |
@bors r+ |
📌 Commit 0be4cb9 has been approved by |
🔒 Merge conflict |
0be4cb9
to
426b63f
Compare
Didn't have merge conflict apparently but rebased just in case... |
@bors: r=oli-obk |
📌 Commit 426b63f has been approved by |
…r-format, r=oli-obk Stabilize short error format r? @oli-obk Added in rust-lang#44636
Rollup of 7 pull requests Successful merges: - #49546 (Stabilize short error format) - #51123 (Update build instructions) - #51146 (typeck: Do not pass the field check on field error) - #51193 (Fixes some style issues in rustdoc "implementations on Foreign types") - #51213 (fs: copy: Use File::set_permissions instead of fs::set_permissions) - #51227 (mod.rs isn't beautiful) - #51240 (Two minor parsing tweaks) Failed merges:
💥 Test timed out |
This is a 2nd time i see bors reporting a timeout error incorrectly.
…On Fri, Jun 1, 2018, 05:59 bors ***@***.***> wrote:
💥 Test timed out
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#49546 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AApc0ih9d--J6EfUUBcgjh6nM5z5gmU0ks5t4K33gaJpZM4TCoV5>
.
|
Iirc this happens when a previous stuck build eventually times out (on the
bors side -- usually it's died ages ago on Travis but bors doesn't notice
and keeps it around)
On Thu, May 31, 2018, 9:45 PM Simonas Kazlauskas <notifications@github.com>
wrote:
… This is a 2nd time i see bors reporting a timeout error incorrectly.
On Fri, Jun 1, 2018, 05:59 bors ***@***.***> wrote:
> 💥 Test timed out
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <#49546 (comment)>,
or mute
> the thread
> <
https://github.com/notifications/unsubscribe-auth/AApc0ih9d--J6EfUUBcgjh6nM5z5gmU0ks5t4K33gaJpZM4TCoV5
>
> .
>
—
You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub
<#49546 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ABivSInOHmQ1DDeHHDnNDyEUHAPYiK0Iks5t4MbqgaJpZM4TCoV5>
.
|
r? @oli-obk
Added in #44636