Skip to content

fix(cli): Adjust messages to match rustc #15928

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Sep 8, 2025
Merged

fix(cli): Adjust messages to match rustc #15928

merged 3 commits into from
Sep 8, 2025
+2,759 −2,781

Conversation

epage
Copy link
Contributor

@epage epage commented Sep 4, 2025 •
edited
Loading

What does this PR try to resolve?

Fixes #12740

This is also prep for #15917, #15922

Changes:

  • note changes from Cyan to Bright Green
    • cargo info had to be changed because the header and context colors became the same
  • Switch to Bright colors which is usually redundant with Bold but that is theme dependent
    • warning only changes to Bright on Windows which is to work around a shell issue on some Windows versions
    • except for PLACEHOLDER to tell it apart from LITERAL
  • the : in note: is no longer bolded to match rustc

How to test and review this PR?

Notes

While annotate-snippets does not have a style for every one of our styles, I updated our manual styles to be similar to annotate snippets.

For annotate snippets color definitions, see https://github.com/rust-lang/annotate-snippets-rs/blob/d38b08b81d7d574369ecfd30124195a970b37fd3/src/renderer/mod.rs#L41-L78

@rustbot
Copy link
Collaborator

rustbot commented Sep 4, 2025

r? @weihanglo

rustbot has assigned @weihanglo.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 4, 2025
@epage epage mentioned this pull request Sep 4, 2025
Draft
@epage epage force-pushed the annotate-consistent branch from 00766f9 to ce7397a Compare September 4, 2025 21:43
weihanglo
weihanglo approved these changes Sep 4, 2025
View reviewed changes
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@epage epage force-pushed the annotate-consistent branch 2 times, most recently from 9815a5e to f09e09c Compare September 4, 2025 23:59
@rustbot rustbot added A-interacts-with-crates.io Area: interaction with registries Command-info labels Sep 5, 2025
epage
epage commented Sep 5, 2025
View reviewed changes
@epage epage force-pushed the annotate-consistent branch from f09e09c to dafdb3f Compare September 5, 2025 00:10
@epage epage force-pushed the annotate-consistent branch 2 times, most recently from 269280f to 17fc092 Compare September 5, 2025 00:14
epage
epage commented Sep 5, 2025
View reviewed changes
tests/testsuite/cargo_info/specify_version_within_ws_and_match_with_lockfile/stderr.term.svg
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am a little disappointed that our note color is the same as our headers. However, unsure if we have the plethora of options to do much about it wholly on our end

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should make sure this is merged after beta branch so we have extra time to tweak things (e.g. turning some notes into helps)

epage
epage commented Sep 5, 2025
View reviewed changes
tests/testsuite/cargo_add/default_features/stderr.term.svg
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside: strange that the deleted version had Adding in cyan despite files had it being green and the code isn't conditional

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really strange. It happened not only once, so probably some bug in term_svg in snapbox?

cargo/tests/testsuite/cargo_add/default_features/stderr.term.svg

Line 31 in 3b379fc

<tspan x="10px" y="100px"><tspan class="fg-cyan bold"> Adding</tspan><tspan> my-package2 v0.4.1+my-package </tspan><tspan class="fg-yellow bold">(available: v99999.0.0+my-package)</tspan>

I have produced it, but not sure why. All Adding status with a (available:) extra messages seem to have the issue.
image

@epage epage force-pushed the annotate-consistent branch 2 times, most recently from 74ece70 to 46d3e9b Compare September 5, 2025 14:04
@rustbot rustbot added the A-console-output Area: Terminal output, colors, progress bar, etc. label Sep 5, 2025
@epage epage changed the title fix(cli): Adjust terminal styles to match rustc fix(cli): Adjust messages to match rustc Sep 5, 2025
@epage epage force-pushed the annotate-consistent branch 4 times, most recently from f41116e to eac1ad1 Compare September 5, 2025 19:51
weihanglo
weihanglo approved these changes Sep 8, 2025
View reviewed changes
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good for matching rustc style. Thank you!

tests/testsuite/cargo_add/default_features/stderr.term.svg
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really strange. It happened not only once, so probably some bug in term_svg in snapbox?

cargo/tests/testsuite/cargo_add/default_features/stderr.term.svg

Line 31 in 3b379fc

<tspan x="10px" y="100px"><tspan class="fg-cyan bold"> Adding</tspan><tspan> my-package2 v0.4.1+my-package </tspan><tspan class="fg-yellow bold">(available: v99999.0.0+my-package)</tspan>

I have produced it, but not sure why. All Adding status with a (available:) extra messages seem to have the issue.
image

tests/testsuite/cargo_info/within_ws/stderr.term.svg
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit prefer to the previous version. note is more outstanding.

@weihanglo weihanglo added this pull request to the merge queue Sep 8, 2025
Merged via the queue into rust-lang:master with commit 3ceb2cb Sep 8, 2025
27 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 8, 2025
@epage epage deleted the annotate-consistent branch September 8, 2025 11:40
@weihanglo weihanglo mentioned this pull request Sep 9, 2025
Merged
@epage epage mentioned this pull request Sep 10, 2025
Open
10 tasks
bors added a commit to rust-lang/rust that referenced this pull request Sep 10, 2025
@bors
Auto merge of #146386 - weihanglo:update-cargo, r=weihanglo
a312b65 
Update cargo submodule

10 commits in 761c4658d0079d607e6d33cf0c060e61a617cad3..98402ac7a41dd0f564d3e56d58180d325d0417a0
2025-09-04 01:25:01 +0000 to 2025-09-09 20:19:39 +0000
- fix(flock): check if they are marked unsupported in libstd (rust-lang/cargo#15941)
- test(manifest): Fix test output order (rust-lang/cargo#15940)
- refactor(shell): Simplify some code (rust-lang/cargo#15937)
- fix(manifest): Report script manifest errors for the right line number (rust-lang/cargo#15927)
- refactor: replace flock with std flock (rust-lang/cargo#15935)
- fix(cli): Adjust messages to match rustc  (rust-lang/cargo#15928)
- fix: Switch from --nocapture to --no-capture (rust-lang/cargo#15930)
- Render individual compilation sections in `--timings` pipeline graph (rust-lang/cargo#15923)
- test(credential): Switch more expected results to snapshots (rust-lang/cargo#15929)
- refactor(cli): Pull out error chain iteration (rust-lang/cargo#15926)

r? ghost
@epage epage mentioned this pull request Sep 10, 2025
Merged
github-merge-queue bot pushed a commit that referenced this pull request Sep 10, 2025
@weihanglo
test(help): Ensure consistent behavior regardless of rustup use (#15949)
5d151a9 
### What does this PR try to resolve?

#15928 removed the fix from #13488 which breaks rust-lang/rust. This
fixes that and makes the test less brittle for the future.

### How to test and review this PR?
bors added a commit to rust-lang/rust that referenced this pull request Sep 11, 2025
@bors
Auto merge of #146386 - weihanglo:update-cargo, r=weihanglo
5eda692 
Update cargo submodule

13 commits in 761c4658d0079d607e6d33cf0c060e61a617cad3..24bb93c388fb8c211a37986539f24a819dc669d3
2025-09-04 01:25:01 +0000 to 2025-09-10 23:16:07 +0000
- Bump miow to 0.60.1 (rust-lang/cargo#15950)
- test(help): Ensure consistent behavior regardless of rustup use (rust-lang/cargo#15949)
- docs(changelog): Clarify how manifest paths are used (rust-lang/cargo#15946)
- fix(flock): check if they are marked unsupported in libstd (rust-lang/cargo#15941)
- test(manifest): Fix test output order (rust-lang/cargo#15940)
- refactor(shell): Simplify some code (rust-lang/cargo#15937)
- fix(manifest): Report script manifest errors for the right line number (rust-lang/cargo#15927)
- refactor: replace flock with std flock (rust-lang/cargo#15935)
- fix(cli): Adjust messages to match rustc  (rust-lang/cargo#15928)
- fix: Switch from --nocapture to --no-capture (rust-lang/cargo#15930)
- Render individual compilation sections in `--timings` pipeline graph (rust-lang/cargo#15923)
- test(credential): Switch more expected results to snapshots (rust-lang/cargo#15929)
- refactor(cli): Pull out error chain iteration (rust-lang/cargo#15926)
@rustbot rustbot added this to the 1.91.0 milestone Sep 11, 2025
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Sep 12, 2025
@bors
Auto merge of #146386 - weihanglo:update-cargo, r=weihanglo
fda6d68 
Update cargo submodule

13 commits in 761c4658d0079d607e6d33cf0c060e61a617cad3..24bb93c388fb8c211a37986539f24a819dc669d3
2025-09-04 01:25:01 +0000 to 2025-09-10 23:16:07 +0000
- Bump miow to 0.60.1 (rust-lang/cargo#15950)
- test(help): Ensure consistent behavior regardless of rustup use (rust-lang/cargo#15949)
- docs(changelog): Clarify how manifest paths are used (rust-lang/cargo#15946)
- fix(flock): check if they are marked unsupported in libstd (rust-lang/cargo#15941)
- test(manifest): Fix test output order (rust-lang/cargo#15940)
- refactor(shell): Simplify some code (rust-lang/cargo#15937)
- fix(manifest): Report script manifest errors for the right line number (rust-lang/cargo#15927)
- refactor: replace flock with std flock (rust-lang/cargo#15935)
- fix(cli): Adjust messages to match rustc  (rust-lang/cargo#15928)
- fix: Switch from --nocapture to --no-capture (rust-lang/cargo#15930)
- Render individual compilation sections in `--timings` pipeline graph (rust-lang/cargo#15923)
- test(credential): Switch more expected results to snapshots (rust-lang/cargo#15929)
- refactor(cli): Pull out error chain iteration (rust-lang/cargo#15926)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@weihanglo weihanglo weihanglo approved these changes

Assignees

@weihanglo weihanglo

Labels
A-cli Area: Command-line interface, option parsing, etc. A-console-output Area: Terminal output, colors, progress bar, etc. A-interacts-with-crates.io Area: interaction with registries Command-add Command-bench Command-build Command-check Command-clean Command-doc Command-fetch Command-fix Command-generate-lockfile Command-info Command-init Command-install Command-locate-project Command-login Command-logout Command-metadata Command-new Command-owner Command-package Command-pkgid Command-publish Command-read-manifest Command-remove Command-report Command-run Command-rustc Command-rustdoc Command-search Command-test Command-tree Command-uninstall Command-update Command-vendor Command-version Command-yank
Projects
None yet
Milestone
1.91.0
Development

Successfully merging this pull request may close these issues.

Cargo and rustc are using different tones of red for errors on the terminal
3 participants
@epage @rustbot @weihanglo