-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Helpful error when the [package] table is missing #16501
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
base: master
Are you sure you want to change the base?
Conversation
|
r? @weihanglo rustbot has assigned @weihanglo. Use |
|
Thanks! |
9cd3143 to
806d798
Compare
| }; | ||
| let toml_table = format_args!("{table_open}{field}{table_close}"); | ||
| anyhow!( | ||
| "this virtual manifest does not have a `[package]` section, which is required when specifying the `{toml_table}` section" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, didn't notice the wording change before because of everything else that changed (which is one reason why splitting up changes into individual commits can be helpful).
You are right that the old message is bad, focusing on terminology that is unlikely to be meaningful to the user. Something both messages have in common though is they are indirect about the problem at hand
e.g. the old message should have said something like:
`{field}` section is invalid in virtual manifests
Looking at the new wording, one possible suggestion is:
`{toml_table}` requires a `[package]` section
or
`[package]` section is missing
note: required by `{toml_table}`
When we switch this to annotate snippets, we could move the note to an annotation on the {toml_table} in the source.
What does this PR try to resolve?
Makes an error message more helpful to novice users who may encounter it when writing
Cargo.tomlfor the first time. The previous wording expected users to know what a "virtual manifest" and did not provide any further hints.How to test and review this PR?
Updated unit tests demonstrate it.