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

Bring output of rustup show active-toolchain and rustup default into line with rest of rustup #1654

Merged
merged 4 commits into from
Feb 28, 2019

Conversation

das-sein
Copy link
Contributor

This resolves: #1406, and brings into line the fix for #1633

#1406 seems to just be the result of the rustup show active-toolchain command not producing the same output as rustup show, which includes the override reason. This has been changed so that if active-toolchain returns an override, it states the reason for the override. The change to rustup default is just to bring its output into line with the rest of rustup in using the "<toolchain> (<status>)" format.

Previous behavior of rustup show active-toolchain while nightly override is set:

canis@latrans:~/rustup.rs$ rustup show active-toolchain
nightly-x86_64-unknown-linux-gnu

New behavior:

canis@latrans:~/rustup.rs$ rustup show active-toolchain
nightly-x86_64-unknown-linux-gnu (directory override for '/home/canis/rustup.rs')

Copy link
Contributor

@kinnison kinnison left a comment

Choose a reason for hiding this comment

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

This looks good though I'd like to know why we don't have a test affected by half the patch?

Is there an opportunity to add a test for rustup default ?

@das-sein
Copy link
Contributor Author

@kinnison Good point! I did also think it odd, but didn't want to add an unnecessary test in case it was intentional. I'll work on some.

@das-sein
Copy link
Contributor Author

@kinnison Test for show-active with override added.

Copy link
Contributor

@kinnison kinnison left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the test.

@kinnison kinnison merged commit c39e791 into rust-lang:master Feb 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rustup show: add flag to show only active toolchain
2 participants