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

Filtering of component list by available, required, and installed #1635

Closed
wants to merge 1 commit into from

Conversation

das-sein
Copy link
Contributor

@das-sein das-sein commented Feb 4, 2019

Allows filtering of rustup component list command.

Example output:

canis@latrans$ rustup component add clippy
canis@latrans$ rustup component list --filter installed
cargo-x86_64-unknown-linux-gnu (default)
clippy-x86_64-unknown-linux-gnu (installed)
rust-docs-x86_64-unknown-linux-gnu (default)
rust-std-x86_64-unknown-linux-gnu (default)
rustc-x86_64-unknown-linux-gnu (default)
canis@latrans$ rustup component list --filter required
cargo-x86_64-unknown-linux-gnu (default)
rust-docs-x86_64-unknown-linux-gnu (default)
rust-std-x86_64-unknown-linux-gnu (default)
rustc-x86_64-unknown-linux-gnu (default)
canis@latrans$ rustup component list --filter available
llvm-tools-preview-x86_64-unknown-linux-gnu
rls-x86_64-unknown-linux-gnu
rust-analysis-x86_64-unknown-linux-gnu
rust-src
...

@kinnison
Copy link
Contributor

kinnison commented Feb 4, 2019

Hi @lazorgator,

Thanks for your contribution to Rustup. Could you please confirm if this is just a wishlist feature from you, or is there an issue already filed which you think you're addressing with this commit? If there's the latter then I'd really like to see the commit mention it.

Also, thank you for including a test for the feature, but it'd also be helpful if you could write a small section for README.md which details the filters, and also what it means for a component to be required or not.

If you aren't comfortable writing that then let me know, I wouldn't block the feature purely on that, but it'd be super-nice to have.

@das-sein
Copy link
Contributor Author

das-sein commented Feb 5, 2019

Hi there @kinnison ,

I actually noticed this wasn't implemented from hacking on rustup for the past couple days and implemented it as a feature off-hand. If that's not okay due to concerns about feature creep, it's fine if it's not merged. I can work on the README entry for it, most certainly.

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.

Slight wording change to point the user at --toolchain and I think this will be golden.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@das-sein das-sein force-pushed the component-list-filter branch from 42b417e to 9c4de11 Compare February 5, 2019 23:28
@das-sein
Copy link
Contributor Author

das-sein commented Feb 5, 2019

@kinnison Added mention of --toolchain flag for rustup component add and removed unrelated whitespace hunk.

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.

r? @dwijnand

@bors
Copy link
Contributor

bors commented Feb 10, 2019

☔ The latest upstream changes (presumably #1643) made this pull request unmergeable. Please resolve the merge conflicts.

@das-sein das-sein force-pushed the component-list-filter branch 3 times, most recently from 9c4de11 to 26ed8d8 Compare February 10, 2019 08:40
@das-sein
Copy link
Contributor Author

Messed up the PR. README changes overwritten. I'll close this now in favor of a new PR on top of the 2018 Edition changes :<

@das-sein das-sein closed this Feb 10, 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.

3 participants