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

Error count display is different when there's only one error left, making it difficult to scan output #12390

Closed
carols10cents opened this issue Jul 21, 2023 · 11 comments · Fixed by #12484
Assignees
Labels
A-diagnostics Area: Error and warning messages generated by Cargo itself. E-easy Experience: Easy S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review

Comments

@carols10cents
Copy link
Member

carols10cents commented Jul 21, 2023

Code

Take any code that doesn't compile with multiple errors, then fix them one at a time, running the compiler between each fix.

Current output

When there are 3 errors left, the compiler says:

error: could not compile `crate` (lib test) due to 3 previous errors

When there are 2 errors left, the compiler says:

error: could not compile `crate` (lib test) due to 2 previous errors

When there is 1 error left, the compiler says:

error: could not compile `crate` (lib test) due to previous error

Desired output

When there's only 1 error left, I wish the number 1 appeared in the output so that it scans the same as the output when there's more than 1 error, so:

error: could not compile `crate` (lib test) due to 1 previous error

Rationale and extra context

When I'm in a cycle of quickly going between edit -> compile -> see how many errors there are to ensure I'm making forward progress and see how much work is left -> edit, my brain goes to the due to previous error message looking for a digit. My brain also sees that there's clearly still error output, so it's saying "where's the count?!??! how many are left?!? what's going on?!?!" until my brain's flow is broken and it goes "oh right, this error message is different, there's 1 left".

It's a minor paper cut. Feel free to prioritize accordingly.

I'm using Rust stable 1.71.0.

@carols10cents
Copy link
Member Author

@rustbot label +D-papercut

@chenyukang
Copy link
Member

sounds reasonable, if there is N in the message, make it more explicit:
image

@chenyukang chenyukang added the E-easy Experience: Easy label Jul 22, 2023
@ehuss ehuss transferred this issue from rust-lang/rust Jul 22, 2023
@ehuss
Copy link
Contributor

ehuss commented Jul 22, 2023

Transferred to the cargo repo, since these error messages are generated via cargo. Although rustc generates a similar message ("aborting due to previous error"), cargo intercepts that message and does not display it, and instead displays its own summary. I'm assuming @carols10cents is interested in the message when using cargo (although I'd guess that changing rustc's error to have a similar form could also be done).

@ehuss ehuss added A-diagnostics Area: Error and warning messages generated by Cargo itself. S-needs-team-input Status: Needs input from team on whether/how to proceed. labels Jul 22, 2023
@carols10cents
Copy link
Member Author

Oh I had no idea this was cargo's doing! Thank you for taking care of the move :) Yes, I was using cargo.

@weihanglo
Copy link
Member

Not sure about other team members' opinions. The intention sounds pretty valid to me. The relevant code is here:

let errors = match output_options.errors_seen {
0 => String::new(),
1 => " due to previous error".to_string(),
count => format!(" due to {} previous errors", count),
};

@adrianEffe
Copy link
Contributor

Would it make sense to do the same for warnings if we go ahead with this?

@weihanglo
Copy link
Member

Anyone willing to have a look at how rustc handles this diagnostic? Would be great if rustc and Cargo are more consistent in the format.

@chenyukang
Copy link
Member

I can take a look for the rustc part.
It's here:
https://github.com/rust-lang/rust/blob/64ad036307727d21070a46f1a5e323cd442229da/compiler/rustc_errors/src/lib.rs#L1443

If cargo fixed I may create PR to fix it and update cargo.

@adrianEffe
Copy link
Contributor

Happy to open a PR for this if it gets labelled as accepted 🙏

@weihanglo
Copy link
Member

weihanglo commented Aug 7, 2023

Since it's a revertible change, I'll go ahead and accept this change. Just a tiny advice — open the PR in both rust-lang/rust and rust-lang/cargo at the same time, so that maintainers of each repo can have a reference of whether they want to proceed.

If anyone objects to this, please call out here. Thank you :)


@rustbot label +S-accepted -S-needs-team-input

@rustbot rustbot added S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review and removed S-needs-team-input Status: Needs input from team on whether/how to proceed. labels Aug 7, 2023
@adrianEffe
Copy link
Contributor

great will do

@rustbot claim

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Error and warning messages generated by Cargo itself. E-easy Experience: Easy S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants