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

Suggest similar feature names on CLI #15133

Merged
merged 2 commits into from
Feb 4, 2025

Conversation

DJMcNab
Copy link
Contributor

@DJMcNab DJMcNab commented Feb 3, 2025

What does this PR try to resolve?

When you typo a feature name on the CLI, the error message isn't very helpful. Concretely, I was testing a PR which adds a feature called cosmic_text to enable a cosmic-text dependency, and got a correct but unhelpful error message:

error: Package `scenes v0.0.0 ([ELIDED]/linebender/vello/examples/scenes)` does not have feature `cosmic-text`. It has an optional dependency with that name, but that dependency uses the "dep:" syntax in the features table, so it does not have an implicit feature with that name.

I had to dig into the Cargo.lock file to find out how to fix this.

How should we test and review this PR?

Observe the new test cases

@rustbot
Copy link
Collaborator

rustbot commented Feb 3, 2025

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @weihanglo (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added A-dependency-resolution Area: dependency resolution and the resolver S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 3, 2025
@DJMcNab
Copy link
Contributor Author

DJMcNab commented Feb 3, 2025

@rustbot label A-diagnostics -A-dependency-resolution

@rustbot rustbot added A-diagnostics Area: Error and warning messages generated by Cargo itself. and removed A-dependency-resolution Area: dependency resolution and the resolver labels Feb 3, 2025
@epage
Copy link
Contributor

epage commented Feb 3, 2025

Thanks for adding this!

@rustbot rustbot added the A-dependency-resolution Area: dependency resolution and the resolver label Feb 3, 2025
@DJMcNab
Copy link
Contributor Author

DJMcNab commented Feb 3, 2025

Does rustbot know something I don't? If it insists on that label, I'm not going to fight it.

@arlosi
Copy link
Contributor

arlosi commented Feb 3, 2025

Does rustbot know something I don't? If it insists on that label, I'm not going to fight it.

It's because you're editing files within the resolver. Don't worry about it.

@epage
Copy link
Contributor

epage commented Feb 3, 2025

Can you restructure your PR

  • A commit that extends the tests to cover more cases (should pass)
  • A commit for behavior changes (tests should be updated to pass)

@DJMcNab DJMcNab force-pushed the missing-feature-error branch 2 times, most recently from 7b9140f to 3d30671 Compare February 4, 2025 08:48
@epage
Copy link
Contributor

epage commented Feb 4, 2025

Could you clean up the commits for how you'd want them merged?

@epage
Copy link
Contributor

epage commented Feb 4, 2025

FYI you'll need to rebase on top of #15138 (this PR inspired it but I didn't expect it to get in first)

@DJMcNab DJMcNab force-pushed the missing-feature-error branch from 3948ad7 to c5444d0 Compare February 4, 2025 18:30
 The left-aligned error message is there to
workaround rustfmt refusing to format files
which contain string literals which are too
wide. I have not found a consistent way to
fix this behaviour, but left-aligning does
resolve it in this case. I believe that this
should have an explanatory comment, but code
review determined that to be "noise" and so
I removed it.
@DJMcNab DJMcNab force-pushed the missing-feature-error branch from 32ea0cd to 378f021 Compare February 4, 2025 18:33
Copy link
Contributor

@epage epage left a comment

Choose a reason for hiding this comment

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

Thanks! This is a great improvement to our error reporting! Thank you for your patience through the code review process (and the semantic merge conflicts)

@epage epage added this pull request to the merge queue Feb 4, 2025
Merged via the queue into rust-lang:master with commit 83c11ee Feb 4, 2025
21 checks passed
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 8, 2025
Update cargo

14 commits in 0e3d73849ab8cbbab3ec5c65cbd555586cb21339..2928e32734b04925ee51e1ae88bea9a83d2fd451
2025-02-01 20:14:40 +0000 to 2025-02-07 16:50:22 +0000
- Simplify backtrack (rust-lang/cargo#15150)
- Don't use on Solaris libc::LOCK_* which were removed from libc in ver… (rust-lang/cargo#15143)
- feat: emit error if package not found within workspace (rust-lang/cargo#15071)
- Make cache tracking resilient to unexpected files (rust-lang/cargo#15147)
- Small resolver cleanups (rust-lang/cargo#15040)
- feat: add `cargo pkgid` support for cargo-script (rust-lang/cargo#14961)
- Suggest similar feature names on CLI (rust-lang/cargo#15133)
- fix: Don't use "did you mean" in errors (rust-lang/cargo#15138)
- Fix changelog link (rust-lang/cargo#15142)
- chore(deps): update rust crate rand to 0.9.0 (rust-lang/cargo#15129)
- Remove the original changelog (rust-lang/cargo#15123)
- chore(deps): update rust crate gix to 0.70.0 (rust-lang/cargo#15128)
- allow windows reserved names in CI (rust-lang/cargo#15135)
- removed a word that was repeated (rust-lang/cargo#15136)
@rustbot rustbot added this to the 1.86.0 milestone Feb 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-dependency-resolution Area: dependency resolution and the resolver A-diagnostics Area: Error and warning messages generated by Cargo itself. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants