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.

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

Cargo add displays either feature list or summarized count #12702

Merged
merged 2 commits into from
Sep 19, 2023
Merged

Cargo add displays either feature list or summarized count #12702

merged 2 commits into from
Sep 19, 2023

Conversation

Angelin01
Copy link
Contributor

What does this PR try to resolve?

Fixes #12617

Adjusts cargo add to display only the activated/deactivate feature list or the summary, not both.

How should we test and review this PR?

Simply run cargo add with some features enabled. The previous PR also has a few example commands.

Tests were adjusted and have sample output as well.

Additional Information

This first version does not consider a variable terminal size, as suggested in the issue.

There's also some minor code refactor, trying to improve readability. Please warn if the changes are undesirable

@rustbot
Copy link
Collaborator

rustbot commented Sep 19, 2023

r? @epage

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added Command-add S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 19, 2023
@Angelin01 Angelin01 changed the title Cargo add features displays either feature list or summarized count Cargo add displays either feature list or summarized count Sep 19, 2023
@Angelin01
Copy link
Contributor Author

@epage Regarding using terminal_size for a dynamic item threshold, terminal_size() simply returns a None, as expected because we use snapbox. I tried looking through snapbox code, but I don't think it's capable of emulating a tty or any kind of terminal size.

So, while we could implement the dynamic lines idea, it would be untested, and I'm not sure I'd like to do that. I'll mark the PR as ready-to-review, unless you have some other ideia how we could simulate the different terminal sizes in code.

@Angelin01 Angelin01 marked this pull request as ready for review September 19, 2023 16:31
@epage
Copy link
Contributor

epage commented Sep 19, 2023

Yes, terminal interactions don't get as much test coverage; thats understandable. terminal_size also isn't a blocker for this.

@epage
Copy link
Contributor

epage commented Sep 19, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Sep 19, 2023

📌 Commit 72dccec has been approved by epage

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 19, 2023
@bors
Copy link
Contributor

bors commented Sep 19, 2023

⌛ Testing commit 72dccec with merge 40b78fe...

@Angelin01
Copy link
Contributor Author

Thank you for the help and the reviews. Despite the confusion, I'll stand by what I said here, the whole process, from my end, has been quite painless and fruitful.

@bors
Copy link
Contributor

bors commented Sep 19, 2023

☀️ Test successful - checks-actions
Approved by: epage
Pushing 40b78fe to master...

@bors bors merged commit 40b78fe into rust-lang:master Sep 19, 2023
@Angelin01 Angelin01 deleted the refactor-cargo-add-features-display branch September 19, 2023 17:23
@weihanglo weihanglo mentioned this pull request Sep 22, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 22, 2023
@bors
Auto merge of rust-lang#116066 - weihanglo:update-cargo, r=weihanglo
e0e7361 
Update cargo

19 commits in b4ddf95ad9954118ac0dae835f2966394ad04c02..414d9e3a6d8096f3e276234ce220c868767a8792
2023-09-18 03:48:09 +0000 to 2023-09-22 07:03:57 +0000
- refactor(TomlManifest): fail when package_root is not a directory (rust-lang/cargo#12722)
- Better suggestion for unsupported mode in build command (rust-lang/cargo#12693)
- Update curl-sys to pull in curl 8.3.0 (rust-lang/cargo#12718)
- chore(ci): Ignore patch version in MSRV (rust-lang/cargo#12716)
- refactor: move cached crates.io SourceID to config module (rust-lang/cargo#12711)
- fix: typos in registry authentication documentation (rust-lang/cargo#12714)
- doc: mention unstable flag `-Z asymmetric-token` (rust-lang/cargo#12712)
- fix: copy PDBs for EFI targets (rust-lang/cargo#12688)
- infra: add auto-trigger rules for new labels (rust-lang/cargo#12713)
- fix: use channel-specific link for registry auth error (rust-lang/cargo#12709)
- Add some enhancements to `cargo clean` (rust-lang/cargo#12638)
- chore: Fix typos (rust-lang/cargo#12707)
- Shortest path (rust-lang/cargo#12678)
- doc/reference/manifest: Adjust `keywords` description (rust-lang/cargo#12705)
- Cargo add displays either feature list or summarized count (rust-lang/cargo#12702)
- SemVer: Update documentation about removing optional dependencies (rust-lang/cargo#12687)
- publish.py: Remove obsolete `sleep()` calls (rust-lang/cargo#12686)
- generalise suggestion on abiguous spec (rust-lang/cargo#12685)
- util/toml: Remove duplicate `serde(rename)` attributes (rust-lang/cargo#12682)

r? ghost
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 22, 2023
@bors
Auto merge of rust-lang#116066 - weihanglo:update-cargo, r=weihanglo
8c18582 
Update cargo

19 commits in b4ddf95ad9954118ac0dae835f2966394ad04c02..414d9e3a6d8096f3e276234ce220c868767a8792
2023-09-18 03:48:09 +0000 to 2023-09-22 07:03:57 +0000
- refactor(TomlManifest): fail when package_root is not a directory (rust-lang/cargo#12722)
- Better suggestion for unsupported mode in build command (rust-lang/cargo#12693)
- Update curl-sys to pull in curl 8.3.0 (rust-lang/cargo#12718)
- chore(ci): Ignore patch version in MSRV (rust-lang/cargo#12716)
- refactor: move cached crates.io SourceID to config module (rust-lang/cargo#12711)
- fix: typos in registry authentication documentation (rust-lang/cargo#12714)
- doc: mention unstable flag `-Z asymmetric-token` (rust-lang/cargo#12712)
- fix: copy PDBs for EFI targets (rust-lang/cargo#12688)
- infra: add auto-trigger rules for new labels (rust-lang/cargo#12713)
- fix: use channel-specific link for registry auth error (rust-lang/cargo#12709)
- Add some enhancements to `cargo clean` (rust-lang/cargo#12638)
- chore: Fix typos (rust-lang/cargo#12707)
- Shortest path (rust-lang/cargo#12678)
- doc/reference/manifest: Adjust `keywords` description (rust-lang/cargo#12705)
- Cargo add displays either feature list or summarized count (rust-lang/cargo#12702)
- SemVer: Update documentation about removing optional dependencies (rust-lang/cargo#12687)
- publish.py: Remove obsolete `sleep()` calls (rust-lang/cargo#12686)
- generalise suggestion on abiguous spec (rust-lang/cargo#12685)
- util/toml: Remove duplicate `serde(rename)` attributes (rust-lang/cargo#12682)

r? ghost
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 23, 2023
@bors
Auto merge of rust-lang#116066 - weihanglo:update-cargo, r=weihanglo
2d08657 
Update cargo

19 commits in b4ddf95ad9954118ac0dae835f2966394ad04c02..414d9e3a6d8096f3e276234ce220c868767a8792
2023-09-18 03:48:09 +0000 to 2023-09-22 07:03:57 +0000
- refactor(TomlManifest): fail when package_root is not a directory (rust-lang/cargo#12722)
- Better suggestion for unsupported mode in build command (rust-lang/cargo#12693)
- Update curl-sys to pull in curl 8.3.0 (rust-lang/cargo#12718)
- chore(ci): Ignore patch version in MSRV (rust-lang/cargo#12716)
- refactor: move cached crates.io SourceID to config module (rust-lang/cargo#12711)
- fix: typos in registry authentication documentation (rust-lang/cargo#12714)
- doc: mention unstable flag `-Z asymmetric-token` (rust-lang/cargo#12712)
- fix: copy PDBs for EFI targets (rust-lang/cargo#12688)
- infra: add auto-trigger rules for new labels (rust-lang/cargo#12713)
- fix: use channel-specific link for registry auth error (rust-lang/cargo#12709)
- Add some enhancements to `cargo clean` (rust-lang/cargo#12638)
- chore: Fix typos (rust-lang/cargo#12707)
- Shortest path (rust-lang/cargo#12678)
- doc/reference/manifest: Adjust `keywords` description (rust-lang/cargo#12705)
- Cargo add displays either feature list or summarized count (rust-lang/cargo#12702)
- SemVer: Update documentation about removing optional dependencies (rust-lang/cargo#12687)
- publish.py: Remove obsolete `sleep()` calls (rust-lang/cargo#12686)
- generalise suggestion on abiguous spec (rust-lang/cargo#12685)
- util/toml: Remove duplicate `serde(rename)` attributes (rust-lang/cargo#12682)

r? ghost
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Sep 23, 2023
@bors
Auto merge of #116066 - weihanglo:update-cargo, r=weihanglo
73b3c86 
Update cargo

19 commits in b4ddf95ad9954118ac0dae835f2966394ad04c02..414d9e3a6d8096f3e276234ce220c868767a8792
2023-09-18 03:48:09 +0000 to 2023-09-22 07:03:57 +0000
- refactor(TomlManifest): fail when package_root is not a directory (rust-lang/cargo#12722)
- Better suggestion for unsupported mode in build command (rust-lang/cargo#12693)
- Update curl-sys to pull in curl 8.3.0 (rust-lang/cargo#12718)
- chore(ci): Ignore patch version in MSRV (rust-lang/cargo#12716)
- refactor: move cached crates.io SourceID to config module (rust-lang/cargo#12711)
- fix: typos in registry authentication documentation (rust-lang/cargo#12714)
- doc: mention unstable flag `-Z asymmetric-token` (rust-lang/cargo#12712)
- fix: copy PDBs for EFI targets (rust-lang/cargo#12688)
- infra: add auto-trigger rules for new labels (rust-lang/cargo#12713)
- fix: use channel-specific link for registry auth error (rust-lang/cargo#12709)
- Add some enhancements to `cargo clean` (rust-lang/cargo#12638)
- chore: Fix typos (rust-lang/cargo#12707)
- Shortest path (rust-lang/cargo#12678)
- doc/reference/manifest: Adjust `keywords` description (rust-lang/cargo#12705)
- Cargo add displays either feature list or summarized count (rust-lang/cargo#12702)
- SemVer: Update documentation about removing optional dependencies (rust-lang/cargo#12687)
- publish.py: Remove obsolete `sleep()` calls (rust-lang/cargo#12686)
- generalise suggestion on abiguous spec (rust-lang/cargo#12685)
- util/toml: Remove duplicate `serde(rename)` attributes (rust-lang/cargo#12682)

r? ghost
@ehuss ehuss added this to the 1.74.0 milestone Sep 30, 2023
epage added a commit to epage/cargo-information that referenced this pull request Mar 22, 2024
@epage
fix: Don't overwhelm the user with features
ab0507a 
This was modeled after the work done in `cargo-add` in rust-lang/cargo#12702
@epage epage mentioned this pull request Mar 22, 2024
Merged
epage added a commit to epage/cargo-information that referenced this pull request Mar 27, 2024
@epage
fix: Don't overwhelm the user with features
e3b6fc8 
This was modeled after the work done in `cargo-add` in rust-lang/cargo#12702
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@epage epage epage approved these changes

Assignees

@epage epage

Labels
Command-add S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.74.0
Development

Successfully merging this pull request may close these issues.

Limit the number of crate features that are displayed as result of cargo add
5 participants
@Angelin01 @rustbot @epage @bors @ehuss