-
Notifications
You must be signed in to change notification settings - Fork 13.5k
chore: Improve how the other suggestions message gets rendered #143661
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
base: master
Are you sure you want to change the base?
Conversation
rustbot has assigned @compiler-errors. Use |
//@ revisions: ascii unicode | ||
//@[unicode] compile-flags: -Zunstable-options --error-format=human-unicode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussion: maybe use e.g. a copy of this test for diagnostics emitter testing? This const-generics test AFAICT is a regression test (for a previous ICE), I feel like it probably shouldn't be dual-serving as diagnostics emitter testing, as there are two different test intentions? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is a very good point. I had not considered why a test exists when choosing one. I agree that it shouldn't serve the role of a regression test and a diagnostic emitter test. Looking at the other available tests, tests/ui/suggestions/too-many-field-suggestions.stderr
looks to be the best candidate, but I would be happy to create a dedicated test if that is better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a dedicated test for this specific purpose is probably best, even if superficially it looks like an exact copy of another test, because even that test looks like it has a different test intention (of suggestions for field suggestions). But a test comment would clarify that so.
In that, for the intentions of this PR, the actual "error" doesn't matter as much as the suggestion format itself :)
97c03f1
to
2230273
Compare
2230273
to
0043854
Compare
Note: This change is part of my ongoing work to use
annotate-snippets
asrustc
's emitterThis change started as a way to remove some specialty code paths from
annotate-snippets
, by making the "and {} other candidates" message get rendered like a secondary message with no level, but turned into a fix for the message's Unicode output. Before this change, when using the Unicode output, the other suggestions message would get rendered outside of the main suggestion block, making it feel disconnected from what it was referring to. This change makes it so that the message is on the last line of the block, aligning its rendering with other secondary messages, and making it clear what the message is referring to.Before:
After: