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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

DJMcNab
Copy link

@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?

The existing tests seem to cover this quite well. I don't think that this feature warrants adding its own test.

@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
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
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 from a44b7f0 to 7b9140f Compare February 4, 2025 08:20
@DJMcNab DJMcNab force-pushed the missing-feature-error branch from 7b9140f to 3d30671 Compare February 4, 2025 08:48
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.

6 participants