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

cli: Allow specifying multiple component or target values in one argument #2239

Merged
merged 2 commits into from
Mar 17, 2020
Merged

cli: Allow specifying multiple component or target values in one argument #2239

merged 2 commits into from
Mar 17, 2020

Conversation

jplatte
Copy link
Contributor

@jplatte jplatte commented Feb 25, 2020

It has happened a few times to me now that I've tried to install a toolchain with rustup toolchain install <toolchain> -c a,b,c, only to get an error that the component "a,b,c" does not exist.

This PR changes the meaning of that command to the (IMO obvious) one of installing the toolchain <toolchain> with components a, b and c. Targets can also be specified in one -t argument when delimited with commas.

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.

Nominally this change is good, however if we want to guarantee to support this going into the future then we need a test to demonstrate the functionality. Could you please sort a test out (probably in cli-v2 or cli-misc) and push it to the PR?

@jplatte
Copy link
Contributor Author

jplatte commented Feb 27, 2020

I've added tests for rustup, should I create additional ones for rustup-init?

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, However as you note, there are changes in setup_mode.rs which do need verifying too.

Please try and a test to confirm the behaviour in the installer.

@kinnison kinnison added this to the 1.22.0 milestone Mar 15, 2020
@jplatte
Copy link
Contributor Author

jplatte commented Mar 15, 2020

@kinnison Done.

@kinnison kinnison merged commit 96bc015 into rust-lang:master Mar 17, 2020
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.

3 participants