Improve settings tree title and descriptions #20154
Conversation
- All settings are now phrased in the imperative form stating what the setting does rather than talking about what it controls. (E.g.: "Show `Debug` action." instead of "Whether to show `Debug` action" - Categories are now displayed in title case - Categories are now sorted lexicographically - General category is removed (and all the settings are moved to the top level) - Language for a few descriptions is made a bit less ambiguous
rustbot
added
the
S-waiting-on-review
There was a problem hiding this comment.
No manual changes made to this doc - purely generated by running cargo test
| "configuration": [ | ||
| { | ||
| "title": "general", | ||
| "title": "Rust Analyzer" | ||
| }, | ||
| { | ||
| "title": "Assist" | ||
| }, | ||
| { | ||
| "title": "Cache Priming" | ||
| }, | ||
| { | ||
| "title": "Cargo" |
There was a problem hiding this comment.
This part forces the categories to be ordered lexicographically. Otherwise they are ordered based the order which they appear in the file. It's possible that this was intentional, in which case I'd remove these lines.
| "title": "Workspace" | ||
| }, | ||
| { | ||
| "title": "rust-analyzer", |
There was a problem hiding this comment.
Change this category title from "general" to "rust-analyzer" makes this appear at the top level of the settings instead of having an unnecessary category.
| "markdownDescription": "Whether to restart the server automatically when certain settings that require a restart are changed.", | ||
| "description": "Restart the server automatically when settings that require a restart are changed.", |
There was a problem hiding this comment.
markdownDescription is only needed when there's markdown.
I left it there for the generated descriptions to avoid having to detect whether there's markdown or not
| "title": "assist", | ||
| "title": "Assist", |
There was a problem hiding this comment.
Below this point, all the changes are generated from config.rs. There are no manual changes to this file below here. The changes are generally:
- category name is title case
- description is imperative
| // Defines the server-side configuration of the rust-analyzer. We generate | ||
| // *parts* of VS Code's `package.json` config from this. Run `cargo test` to | ||
| // re-generate that file. | ||
| // Defines the server-side configuration of the rust-analyzer. We generate *parts* of VS Code's | ||
| // `package.json` config from this. Run `cargo test` to re-generate that file. |
There was a problem hiding this comment.
In general, Ive wrapped these at 100 chars as the changes were otherwise difficult to read at times with various wrap points across each item. I've also put a single blank line between each item instead of 0-3, this makes it easier to read as the comment is more obviously grouped with the following item rather than the previous.
I also removed the vertically aligned defaults ( = true etc.), as this formatting doesn't appear elsewhere in the source code and doesn't make the values easier to read.
| // To deprecate an option by replacing it with another name use `new_name` | `old_name` so that we keep | ||
| // parsing the old name. | ||
| // To deprecate an option by replacing it with another name use `new_name` | `old_name` so that we | ||
| // keep parsing the old name. | ||
| config_data! { |
There was a problem hiding this comment.
The changes in this are to:
- comments - wording etc.
- spacing - consistent wrapping etc.
The generated code doesn't care too much about single newlines, only double newlines like standard markdown.
| let category = name | ||
| .split_once(".") | ||
| .map(|(category, _name)| to_title_case(category)) | ||
| .unwrap_or("rust-analyzer".into()); |
There was a problem hiding this comment.
Defaut changed to put all non-grouped settings at the top level instead of under a "General" category. (Casing is important here, it has to match the case of the extension display name.)
| name.find('.').map(|end| String::from(&name[..end])).unwrap_or("general".into()); | ||
| let category = name | ||
| .split_once(".") | ||
| .map(|(category, _name)| to_title_case(category)) |
There was a problem hiding this comment.
Generated category names are now title case
| /// This likely should be in stdx (or just use heck instead), but it doesn't handle any edge cases | ||
| /// and is intentionally simple. | ||
| fn to_title_case(s: &str) -> String { |
There was a problem hiding this comment.
I'm inclined to leave this method here rather than extracting it to something more general. If someone wants to make it more useful, performant etc., then that's fine, but for now it's usefullness is this one use case.
I also don't really care too much about the impl here as it's dev time code. It's likely there's edge cases a more general function would have to catch.
setting does rather than talking about what it controls. (E.g.:
"Show
Debugaction." instead of "Whether to showDebugaction"level)
Before:
After: