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

Config revamp #12010

Merged
merged 11 commits into from
May 10, 2022
Merged

Config revamp #12010

merged 11 commits into from
May 10, 2022

Conversation

Veykril
Copy link
Member

@Veykril Veykril commented Apr 16, 2022

Fixes #11790
Fixes #12115

This PR changes a lot of config names, and a few ones are being merged or split apart. The reason for this is that our configuration names currently are rather inconsistent and some where poorly chosen in regards to extensability. This PR plans to fix that.

We still allow the old config names by patching them to the new ones before deserializing to keep backwards compatability with other clients (the VSCode client will auto update the config) but ideally we will get rid of that layer in the future.

Here is a list of the changes:

These are simple renames old_name | alias1 | alias2 ... -> new_name (the vscode client will fix these up automagically):

assist_allowMergingIntoGlobImports -> imports_merge_glob
assist_exprFillDefault -> assist_expressionFillDefault
assist_importEnforceGranularity -> imports_granularity_enforce
assist_importGranularity | assist_importMergeBehavior | assist_importMergeBehaviour -> imports_granularity_group
assist_importGroup -> imports_group_enable
assist_importPrefix -> imports_prefix

cache_warmup -> cachePriming_enable

cargo_runBuildScripts | cargo_loadOutDirsFromCheck -> cargo_buildScripts_enable
cargo_runBuildScriptsCommand -> cargo_buildScripts_overrideCommand
cargo_useRustcWrapperForBuildScripts -> cargo_buildScripts_useRustcWrapper

completion_snippets -> completion_snippets_custom

diagnostics_enableExperimental -> diagnostics_experimental_enable

experimental_procAttrMacros -> procMacro_attributes_enable

highlighting_strings -> semanticHighlighting_strings_enable

highlightRelated_breakPoints -> semanticHighlighting_breakPoints_enable
highlightRelated_exitPoints -> semanticHighlighting_exitPoints_enable
highlightRelated_yieldPoints -> semanticHighlighting_yieldPoints_enable
highlightRelated_references -> semanticHighlighting_references_enable

hover_documentation -> hover_documentation_enable
hover_linksInHover | hoverActions_linksInHover -> hover_links_enable
hoverActions_debug -> hover_actions_debug_enable
hoverActions_enable -> hover_actions_enable_enable
hoverActions_gotoTypeDef -> hover_actions_gotoTypeDef_enable
hoverActions_implementations -> hover_actions_implementations_enable
hoverActions_references -> hover_actions_references_enable
hoverActions_run -> hover_actions_run_enable

inlayHints_chainingHints -> inlayHints_chainingHints_enable
inlayHints_closureReturnTypeHints -> inlayHints_closureReturnTypeHints_enable
inlayHints_hideNamedConstructorHints -> inlayHints_typeHints_hideNamedConstructorHints
inlayHints_parameterHints -> inlayHints_parameterHints_enable
inlayHints_reborrowHints -> inlayHints_reborrowHints_enable
inlayHints_typeHints -> inlayHints_typeHints_enable

lruCapacity -> lru_capacity

runnables_cargoExtraArgs -> runnables_extraArgs 
runnables_overrideCargo -> runnables_command 

rustcSource -> rustc_source

rustfmt_enableRangeFormatting -> rustfmt_rangeFormatting_enable

These are configs that have been merged or split apart, which have to be manually updated by the user:

callInfo_full -> signatureInfo_detail, signatureInfo_documentation_enable

cargo_allFeatures, cargo_features -> cargo_features
checkOnSave_allFeatures, checkOnSave_features -> checkOnSave_features
completion_addCallArgumentSnippets completion_addCallParenthesis -> completion_callable_snippets

@Veykril Veykril changed the title Initial config revamp Config revamp Apr 16, 2022
@Veykril Veykril force-pushed the r-a-config branch 3 times, most recently from 020f910 to 68af7ce Compare April 26, 2022 12:39
@Veykril Veykril marked this pull request as ready for review April 26, 2022 12:39
Copy link
Member Author

@Veykril Veykril left a comment

Choose a reason for hiding this comment

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

I'll create a diff of the config changes later so that we can point people to that/derive a tool from it that allows people to easily migrate their configurations.

The diff is hard to read as I have sorted the configurations now, so I'd like if y'all could take a look at the config after my changes and see if it feels consistent and whether there are some outstanding issues with them.

One thing to note is, most toggles are now suffixed with _enabled, for those that are toggles with intermediate states I added a small trick (see LifetimeElisionDef), that would allow us to also expand on those config keys in the future.

Note the general idea here is to make the config format consistent and extensible for the future such that we can avoid the need for awkward config keys due to mistakes we've made or having to break the users configs again.

crates/rust-analyzer/src/config.rs Show resolved Hide resolved
crates/rust-analyzer/src/config.rs Show resolved Hide resolved
crates/rust-analyzer/src/config.rs Show resolved Hide resolved
crates/rust-analyzer/src/config.rs Outdated Show resolved Hide resolved
crates/rust-analyzer/src/config.rs Outdated Show resolved Hide resolved
crates/rust-analyzer/src/config.rs Outdated Show resolved Hide resolved
@Veykril
Copy link
Member Author

Veykril commented Apr 26, 2022

Also as CI shows, will have to do another check on things that I didn't break stuff by accident 😅

/// Whether to show HoverActions in Rust files.
hoverActions_enable: bool = "true",
hover_actions_enable: bool = "true",
Copy link
Member Author

@Veykril Veykril Apr 27, 2022

Choose a reason for hiding this comment

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

Right, so some of these also have global enable toggles. I am unsure whether it makes sense to have them, part of me says part of me says no. Basically all they really allow is to easily turn everything off, but if one wants all enabled they have to have this enabled AND each thing separately as well.

So the question is, is it beneficial to have an easy way to disable everything in a category or not

@Veykril Veykril force-pushed the r-a-config branch 5 times, most recently from 1f5ae35 to d49aaa8 Compare April 29, 2022 08:18

/// Show function name and docs in parameter hints.
callInfo_full: bool = "true",
assist_expressionFillDefault: ExprFillDefaultDef = "\"todo\"",
Copy link
Member Author

Choose a reason for hiding this comment

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

Using assist here feels wrong to me, this isn't solely used by assists, similar to how the import stuff wasn't assist specific.

@Veykril
Copy link
Member Author

Veykril commented Apr 29, 2022

Alright, the client extension now updates all the simple renames. So the user is unfortunately required to manual update the following ones:

callInfo_full -> signatureInfo_detail, signatureInfo_documentation_enable

cargo_allFeatures, cargo_features -> cargo_features
checkOnSave_allFeatures, checkOnSave_features -> checkOnSave_features
completion_addCallArgumentSnippets completion_addCallParenthesis -> completion_callable_snippets

Simple reason for this being that updating these is a bit more wonky and I'd rather not have the extension touch more than necessary.

@Veykril Veykril force-pushed the r-a-config branch 2 times, most recently from 2258cdf to 24f35e6 Compare April 29, 2022 11:42
@Veykril Veykril force-pushed the r-a-config branch 2 times, most recently from 7d2f4e6 to 67e95cb Compare April 29, 2022 13:28
@Veykril
Copy link
Member Author

Veykril commented Apr 29, 2022

Okay this is finished, as proposed in Zulip I'll merge this after the release on monday unless something comes up.

@bors
Copy link
Contributor

bors commented May 1, 2022

☔ The latest upstream changes (presumably #12118) made this pull request unmergeable. Please resolve the merge conflicts.

@Veykril
Copy link
Member Author

Veykril commented May 9, 2022

So I am inclined to merge this now (or tomorrow rather), any objections?

@Veykril
Copy link
Member Author

Veykril commented May 10, 2022

@bors r+

@bors
Copy link
Contributor

bors commented May 10, 2022

📌 Commit 73df43f has been approved by Veykril

@bors
Copy link
Contributor

bors commented May 10, 2022

⌛ Testing commit 73df43f with merge 460e389...

@bors
Copy link
Contributor

bors commented May 10, 2022

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing 460e389 to master...

@bors bors merged commit 460e389 into rust-lang:master May 10, 2022
@Veykril Veykril deleted the r-a-config branch May 10, 2022 12:50
bors added a commit that referenced this pull request Jul 13, 2022
…Veykril

Fix obsolete config keys

The config keys were drastically reorganized by #12010, but the docs don't reflect the updates, causing inconsistency and confusion.  I checked for such obsolete configuration keys and updated to the new one.  For reproducibility, I attach a small shell script that I used to examine the old keys.  Now the script only detects `cargoExtraArgs` and `overrideCargo`, which originates from other type definition in the code but not from the configuration.

<details><summary>script</summary>

```bash
echo "allowMergingIntoGlobImports
exprFillDefault
importEnforceGranularity
importGranularity
importMergeBehavior
importMergeBehaviour
importGroup
importPrefix
warmup
loadOutDirsFromCheck
runBuildScripts
runBuildScriptsCommand
useRustcWrapperForBuildScripts
enableExperimental
procAttrMacros
breakPoints
exitPoints
yieldPoints
linksInHover
linksInHover
gotoTypeDef
chainingHints
closureReturnTypeHints
hideNamedConstructorHints
parameterHints
reborrowHints
typeHints
lruCapacity
cargoExtraArgs
overrideCargo
rustcSource
enableRangeFormatting
assist\.allowMergingIntoGlobImports
assist\.exprFillDefault
assist\.importEnforceGranularity
assist\.importGranularity
assist\.importMergeBehavior
assist\.importMergeBehaviour
assist\.importGroup
assist\.importPrefix
primeCaches\.enable
cache\.warmup
cargo\.loadOutDirsFromCheck
cargo\.runBuildScripts
cargo\.runBuildScriptsCommand
cargo\.useRustcWrapperForBuildScripts
completion\.snippets
diagnostics\.enableExperimental
experimental\.procAttrMacros
highlighting\.strings
highlightRelated\.breakPoints
highlightRelated\.exitPoints
highlightRelated\.yieldPoints
highlightRelated\.references
hover\.documentation
hover\.linksInHover
hoverActions\.linksInHover
hoverActions\.debug
hoverActions\.enable
hoverActions\.gotoTypeDef
hoverActions\.implementations
hoverActions\.references
hoverActions\.run
inlayHints\.chainingHints
inlayHints\.closureReturnTypeHints
inlayHints\.hideNamedConstructorHints
inlayHints\.parameterHints
inlayHints\.reborrowHints
inlayHints\.typeHints
lruCapacity
runnables\.cargoExtraArgs
runnables\.overrideCargo
rustcSource
rustfmt\.enableRangeFormatting
allFeatures
addCallArgumentSnippets
addCallParenthesis
callInfo\.full
cargo\.allFeatures
checkOnSave\.allFeatures
completion\.addCallArgumentSnippets
completion\.addCallParenthesis" | while read -r pattern
do
  rg '\b'$pattern'\b([^.]|$)' . -g "!crates/rust-analyzer/src/config/patch_old_style.rs" -g "!editors/code/src/config.ts" -g "!a.sh" --no-heading --color=always --line-number
done

exit

excluded
# debug
# enable
# run
# implementations
# references
# documentation
# references
# snippets
# strings
# full
```

</details>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unhelpful documentation for runBuildScriptsCommand Fix our config format to be more consistent
3 participants