-
Notifications
You must be signed in to change notification settings - Fork 13.7k
bootstrap: emit hint if a config key is used in the wrong section #146611
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?
bootstrap: emit hint if a config key is used in the wrong section #146611
Conversation
This PR modifies If appropriate, please update |
e85f09f
to
2593762
Compare
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.
The implementation is so hacky I don't even want to look at it 😆 But since this is just a best effort hint, and doesn't affect the rest of the codebase, I'm fine with it. Left one nit. Thank you!
} | ||
|
||
fn find_correct_section_for_field(field_name: &str) -> Vec<&'static str> { | ||
let sections = ["<top level>", "build", "install", "llvm", "gcc", "rust", "dist"]; |
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.
We actually have a field named build
, so for that this heuristic wouldn't work, but that's not so important.
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.
The code could easily be generalized to detect when a section name is also valid as a key in a given section, but the problem is I have no idea what a good diagnostic to print in that case would be.
exit!(2); | ||
} | ||
|
||
fn find_correct_section_for_field(field_name: &str) -> Vec<&'static str> { |
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.
Could you please use an enum rather than a string reference here?
based on discussion on #146591
now, if the user puts
build.download-rustc
inbootstrap.toml
, they'll get a diagnostic:hint: try moving `download-rustc` to the `rust` section
and if they nest things too much (
rust.rust.download-rustc
):hint: section name `rust` used as a key within a section
if they specify a top-level key within a section (
rust.profile
):hint: try using `profile` as a top level key
r? @Kobzol