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

make unknown features on cargo add more discoverable #11098

Merged
merged 2 commits into from
Sep 21, 2022

Conversation

Emilgardis
Copy link
Contributor

@Emilgardis Emilgardis commented Sep 16, 2022

When adding an unknown feature via cargo add -F krate/feat, it can be easy to miss the fact that the change failed.

This fixes that by showing the following output on fail

image

@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ehuss (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 16, 2022
@epage
Copy link
Contributor

epage commented Sep 16, 2022

It seems like there is more to discuss on the usability of the error message and how we should handle it; can we move the discussion to an issue as recommended in our contributing information?

src/cargo/ops/cargo_add/mod.rs Outdated Show resolved Hide resolved
src/cargo/ops/cargo_add/mod.rs Show resolved Hide resolved
src/cargo/ops/cargo_add/mod.rs Show resolved Hide resolved
src/cargo/ops/cargo_add/mod.rs Outdated Show resolved Hide resolved
also makes gathering of features a function
@Emilgardis Emilgardis force-pushed the early-fail branch 2 times, most recently from baee4cb to 1e9ec41 Compare September 19, 2022 21:58
@ehuss
Copy link
Contributor

ehuss commented Sep 21, 2022

r? epage

@ehuss
Copy link
Contributor

ehuss commented Sep 21, 2022

r? @epage

@rust-highfive rust-highfive assigned epage and unassigned ehuss Sep 21, 2022
@Emilgardis
Copy link
Contributor Author

I think theres one more thing to do here, should probably clearly signal that the command failed, and that nothing was changed for the crate in the manifest

@epage
Copy link
Contributor

epage commented Sep 21, 2022

@bors r+

I think theres one more thing to do here, should probably clearly signal that the command failed, and that nothing was changed for the crate in the manifest

I think the PR is sufficient for now and we can always iterate further on it.

@bors
Copy link
Collaborator

bors commented Sep 21, 2022

📌 Commit 1cc21b6 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 21, 2022
@bors
Copy link
Collaborator

bors commented Sep 21, 2022

⌛ Testing commit 1cc21b6 with merge bee9c89...

@bors
Copy link
Collaborator

bors commented Sep 21, 2022

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

@bors bors merged commit bee9c89 into rust-lang:master Sep 21, 2022
@Emilgardis Emilgardis deleted the early-fail branch September 25, 2022 13:03
bors pushed a commit to rust-lang-ci/rust that referenced this pull request Sep 27, 2022
22 commits in 73ba3f35e0205844418260722c11602113179c4a..f5fed93ba24607980647962c59863bbabb03ce14
2022-09-18 06:38:16 +0000 to 2022-09-27 12:03:57 +0000

- build-scripts.md: Use em dash consistently. (rust-lang/cargo#11150)
- Indicate how Cargo locates the manifest (rust-lang/cargo#10770)
- Reduce references to `[project]` within cargo (rust-lang/cargo#11135)
- Iteratively construct target cfg (rust-lang/cargo#11114)
- update comment about `CARGO_BIN_EXE_` (rust-lang/cargo#11146)
- Call out that not all config values can be set via env vars (rust-lang/cargo#11139)
- Bump to 0.67.0, update changelog (rust-lang/cargo#11137)
- ci: update toolchain for building api doc (rust-lang/cargo#11134)
- Http publish not noop (rust-lang/cargo#11111)
- Improve errors for TOML fields that support workspace inheritance (rust-lang/cargo#11113)
- switch to `std::task::ready!()` where possible (rust-lang/cargo#11130)
- Report cmd aliasing failure with more contexts (rust-lang/cargo#11087)
- minor: remove unused mut (rust-lang/cargo#11127)
- fix(cli): Forward non-UTF8 arguments to external subcommands (rust-lang/cargo#11118)
- This change adds an example to the authors attribute in the manifest. (rust-lang/cargo#10938)
- Add support for relative git submodule paths (rust-lang/cargo#11106)
- make unknown features on `cargo add` more discoverable (rust-lang/cargo#11098)
- Unlink old final artifacts before compilation (rust-lang/cargo#11122)
- refactor(cli): Prepare for clap v4 (rust-lang/cargo#11116)
- fix(cli): Error trailing args rather than ignore (rust-lang/cargo#11119)
- Add a minor clarification (rust-lang/cargo#11093)
- doc(changelog): mention CVE fixes (rust-lang/cargo#11104)
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 27, 2022
Update cargo

22 commits in 73ba3f35e0205844418260722c11602113179c4a..f5fed93ba24607980647962c59863bbabb03ce14 2022-09-18 06:38:16 +0000 to 2022-09-27 12:03:57 +0000

- build-scripts.md: Use em dash consistently. (rust-lang/cargo#11150)
- Indicate how Cargo locates the manifest (rust-lang/cargo#10770)
- Reduce references to `[project]` within cargo (rust-lang/cargo#11135)
- Iteratively construct target cfg (rust-lang/cargo#11114)
- update comment about `CARGO_BIN_EXE_` (rust-lang/cargo#11146)
- Call out that not all config values can be set via env vars (rust-lang/cargo#11139)
- Bump to 0.67.0, update changelog (rust-lang/cargo#11137)
- ci: update toolchain for building api doc (rust-lang/cargo#11134)
- Http publish not noop (rust-lang/cargo#11111)
- Improve errors for TOML fields that support workspace inheritance (rust-lang/cargo#11113)
- switch to `std::task::ready!()` where possible (rust-lang/cargo#11130)
- Report cmd aliasing failure with more contexts (rust-lang/cargo#11087)
- minor: remove unused mut (rust-lang/cargo#11127)
- fix(cli): Forward non-UTF8 arguments to external subcommands (rust-lang/cargo#11118)
- This change adds an example to the authors attribute in the manifest. (rust-lang/cargo#10938)
- Add support for relative git submodule paths (rust-lang/cargo#11106)
- make unknown features on `cargo add` more discoverable (rust-lang/cargo#11098)
- Unlink old final artifacts before compilation (rust-lang/cargo#11122)
- refactor(cli): Prepare for clap v4 (rust-lang/cargo#11116)
- fix(cli): Error trailing args rather than ignore (rust-lang/cargo#11119)
- Add a minor clarification (rust-lang/cargo#11093)
- doc(changelog): mention CVE fixes (rust-lang/cargo#11104)
@ehuss ehuss added this to the 1.66.0 milestone Oct 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants