feat: TOML based config for rust-analyzer #17058

alibektas · 2024-04-13T11:05:44Z

TOML Based Config for RA

This PR ( fixes #13529 and this is a follow-up PR on #16639 ) makes rust-analyzer configurable by configuration files called rust-analyzer.toml. Files must be named rust-analyzer.toml. There is not a strict rule regarding where the files should be placed, but it is recommended to put them near a file that triggers server to start (i.e., Cargo.{toml,lock}, rust-project.json).

Configuration Types

Previous configuration keys are now split into three different classes.

Client keys: These keys only make sense when set by the client (e.g., by setting them in settings.json in VSCode). They are but a small portion of this list. One such example is rust_analyzer.files_watcher, based on which either the client or the server will be responsible for watching for changes made to project files.
Global keys: These keys apply to the entire workspace and can only be set on the very top layers of the hierarchy. The next section gives instructions on which layers these are.
Local keys: Keys that can be changed for each crate if desired.

How Am I Supposed To Know If A Config Is Gl/Loc/Cl ?

#17101

Configuration Hierarchy

There are 5 levels in the configuration hierarchy. When a key is searched for, it is searched in a bottom-up depth-first fashion.

Default Configuration

Scope: Global, Local, and Client

This is a hard-coded set of configurations. When a configuration key could not be found, then its default value applies.

User configuration

Scope: Global, Local

If you want your configurations to apply to every project you have, you can do so by setting them in your $CONFIG_DIR/rust-analyzer/rust-analyzer.toml file, where $CONFIG_DIR is :

Platform	Value	Example
Linux	`$XDG_CONFIG_HOME` or `$HOME`/.config	/home/alice/.config
macOS	`$HOME`/Library/Application Support	/Users/Alice/Library/Application Support
Windows	`{FOLDERID_RoamingAppData}`	C:\Users\Alice\AppData\Roaming

Client configuration

Scope: Global, Local, and Client

Previously, the only way to configure rust-analyzer was to configure it from the settings of the Client you are using. This level corresponds to that.

With this PR, you don't need to port anything to benefit from new features. You can continue to use your old settings as they are.

Workspace Root Configuration

Scope: Global, Local

Rust-analyzer already used the path of the workspace you opened in your Client. We used this information to create a configuration file that won't affect your other projects and define global level configurations at the same time.

Local Configuration

Scope: Local

You can also configure rust-analyzer on a crate level. Although it is not an error to define global ( or client ) level keys in such files, they won't be taken into consideration by the server. Defined local keys will affect the crate in which they are defined and crate's descendants. Internally, a Rust project is split into what we call SourceRoots. This, although with exceptions, is equal to splitting a project into crates.

You may choose to have more than one rust-analyzer.toml files within a SourceRoot, but among them, the one closer to the project root will be

davidbarsky

some nits; i'll try to review more later.

crates/rust-analyzer/tests/slow-tests/main.rs

crates/rust-analyzer/src/reload.rs

crates/rust-analyzer/src/mem_docs.rs

crates/rust-analyzer/src/main_loop.rs

crates/rust-analyzer/src/config.rs

davidbarsky · 2024-04-14T21:08:33Z

crates/rust-analyzer/src/config.rs

@@ -827,16 +1177,45 @@ impl Config {
        caps: ClientCapabilities,
        workspace_roots: Vec<AbsPathBuf>,
        visual_studio_code_version: Option<Version>,
+        user_config_path: Option<Utf8PathBuf>,


question: are there non-user config paths? i would just rename all usage here to config_path, IMO.

(i see a "root config path", but I'd remove the user qualifier entirely.)

the user here refers to the user's home dir I think

Yeah naming things is not my strength. I though of XDG_CONFIG_.. but that is also not very accurate either. As a matter of fact it was Veykril's suggestion to change it to user_config. In any case the name is still the same but I added comment.

davidbarsky · 2024-04-14T21:11:55Z

crates/rust-analyzer/src/config.rs

+            client_config: ClientConfig::default(),
+            user_config: None,
+            ratoml_files: FxHashMap::default(),
+            default_config: ConfigData::default(),
+            source_root_parent_map: Arc::new(FxHashMap::default()),
+            user_config_path,
+            root_ratoml: None,
+            root_ratoml_path,
+            should_update: false,


I'd do two things:

keep ConfigData named data instead of default_config.

I'd group all the configuration file-related values into their own wrapper struct.

davidbarsky · 2024-04-14T21:13:36Z

crates/rust-analyzer/src/config.rs

+        // FIXME @alibektas : This is the only config that is confusing. If it's a proper configuration
+        // why is it not among the others? If it's client only which I doubt it is current state should be alright


it's weird because the VS Code extension, by default, cares about what folder rust-analyzer is running in. it's debatable whether it makes sense in this context, but I'm inclined to think it does.

This is indeed a very odd config and only necessary (I hope) because we are lacking proper support for handling detached files currently.

Veykril

Also not a complete review yet, will give the previous PR another pass and merge that first (then adjust things as I see to speed up things, instead of doing more review rounds on that)

crates/rust-analyzer/src/cli/scip.rs

crates/rust-analyzer/src/global_state.rs

crates/rust-analyzer/src/mem_docs.rs

crates/rust-analyzer/src/global_state.rs

crates/rust-analyzer/src/config.rs

bors · 2024-04-16T08:07:24Z

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

bors · 2024-04-19T21:04:22Z

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

crates/paths/src/lib.rs

Veykril · 2024-04-21T07:29:11Z

crates/rust-analyzer/src/cli/scip.rs

+            change.change_client_config(json);
+            let mut error_sink = ConfigError::default();
+            config = config.apply_change(change, &mut error_sink);
+            // FIXME @alibektas : What happens to errors without logging?


They get eaten, we should probably set up our tracing logging to error by default in these binaries

I am holding onto this one as I couldn't quite understand what I need to do.

crates/rust-analyzer/tests/slow-tests/support.rs

crates/rust-analyzer/src/lib.rs

crates/rust-analyzer/src/main_loop.rs

crates/rust-analyzer/src/config.rs

crates/rust-analyzer/src/global_state.rs

bors · 2024-04-21T15:07:03Z

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

bors · 2024-04-29T08:16:16Z

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

Veykril

2 more things otherwise I think this is good to go

crates/rust-analyzer/tests/slow-tests/support.rs

crates/rust-analyzer/src/config.rs

Veykril · 2024-04-29T08:21:14Z

@bors delegate+

bors · 2024-04-29T08:21:17Z

✌️ @alibektas, you can now approve this pull request!

If @Veykril told you to "r=me" after making some further change, please make that change, then do @bors r=@Veykril

alibektas · 2024-04-29T20:34:09Z

My splitting the config keys into global/local/client wasn't really meticulous, because I was thinking that you would intervene at some point 😄. Are we planning to this in the near future or do you want to take a look before I actually merge this PR @Veykril ?

Veykril · 2024-04-29T20:51:38Z

I haven't given the split a look yet tbf, I can do that afterwards / once you've fixed up the rest here

cbiffle · 2024-05-02T17:38:57Z

Hi! I'm reading this with an eye toward enabling rust-analyzer in our complex cross-compilation builds in Hubris. I'm not very deep into it (and in particular need to look at your split of config keys into client/global/local) but one thing jumped out:

Although it is not an error to define global ( or client ) level keys in such files, they won't be taken into consideration by the server

It would be really nice for this to be at least a warning. Otherwise I foresee frustration where we set a key and it silently doesn't take effect.

cbiffle · 2024-05-02T17:53:05Z

Caveat: I am not familiar with this codebase and have no idea how big of a request I am making. I'm also not a VScode user, and some of the comments in the code seem VScode-specific, so it's possible I'm not understanding things correctly.

It would be nice for rust-embedded use cases if the following things were available as crate-local rather than workspace-level settings; from my read of the current code they are global:

Control of features, in particular whether or not to pass --all-features -- for Cargo reasons, a lot of projects in the rust-embedded space use features to control processor variants, and cannot build with --all-features set. (This is not Hubris-specific.) While I assume your workspace-level feature specification allows for the cratename/featurename style of specification (since workspace-level features aren't a thing), it would be nice to specify these in crates themselves to avoid maintaining a big list at the root.
Target triple. It's possible the target triple can be set here already -- rust-analyzer using the word "target" to mean both "a thing that can be built" and "the target triple" means it's difficult for me, not being super familiar with the codebase, to tell which case is referring to which. But since Cargo still has no stable way of indicating a preference to build a crate for a specific triple, generated rust-analyzer client configurations tend to fill in for this rn, and this PR could eliminate that if it were configurable at a crate level.

I assume that sufficiently flexible/configurable clients are able to override all of the settings? (This seems important if config files are checked in but you want to do something different in a specific editor.)

Veykril · 2024-05-03T08:19:44Z

It would be really nice for this to be at least a warning. Otherwise I foresee frustration where we set a key and it silently doesn't take effect.

We ought to have that indeed thats a good point to raise.

Regarding the preference for crate-local configs, the ones you raised are certainly targets for those but they will require some more follow up work to this. This PR is mainly bringing the infra up that allows us to have these config files for now.

bors · 2024-05-13T10:43:17Z

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

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 13, 2024

alibektas force-pushed the 13529/ratoml branch from e4b15eb to 12e154f Compare April 13, 2024 11:09

alibektas requested review from davidbarsky and Veykril April 13, 2024 11:42

alibektas changed the title ~~feature: TOML based config for rust-analyzer~~ feat: TOML based config for rust-analyzer Apr 13, 2024

davidbarsky reviewed Apr 14, 2024

View reviewed changes

Veykril reviewed Apr 15, 2024

View reviewed changes

Veykril added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 18, 2024

alibektas force-pushed the 13529/ratoml branch from 12e154f to 9cce938 Compare April 18, 2024 16:21

alibektas added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 18, 2024

alibektas mentioned this pull request Apr 18, 2024

Add tags local/global/client to config docs #17101

Open

alibektas force-pushed the 13529/ratoml branch from 9cce938 to b03ce39 Compare April 20, 2024 13:54

Veykril reviewed Apr 21, 2024

View reviewed changes

alibektas mentioned this pull request Apr 27, 2024

Unify DocumentFilters in rust-analyzer module #17154

Open

alibektas force-pushed the 13529/ratoml branch 2 times, most recently from 74e7863 to 2199f2d Compare April 28, 2024 11:27

Veykril reviewed Apr 29, 2024

View reviewed changes

crates/rust-analyzer/tests/slow-tests/support.rs Show resolved Hide resolved

crates/rust-analyzer/src/config.rs Outdated Show resolved Hide resolved

Veykril added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 29, 2024

alibektas force-pushed the 13529/ratoml branch from 2199f2d to 861713e Compare April 29, 2024 20:57

alibektas added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 29, 2024

Veykril self-assigned this May 2, 2024

alibektas and others added 4 commits May 3, 2024 09:06

Apply suggested changes

1e77115

Apply requested changes round 2

02adb3d

Apply requested changes round 3

cffba18

Shuffle around some of the configs between the levels

521b462

Veykril force-pushed the 13529/ratoml branch from 861713e to 7967b0d Compare May 3, 2024 08:17

Fix local configs allowing to contain global changes

424b012

Veykril force-pushed the 13529/ratoml branch from 7967b0d to 424b012 Compare May 3, 2024 08:20

Veykril added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: TOML based config for rust-analyzer #17058

feat: TOML based config for rust-analyzer #17058

alibektas commented Apr 13, 2024 •

edited

davidbarsky left a comment

davidbarsky Apr 14, 2024

Veykril Apr 15, 2024

alibektas Apr 18, 2024 •

edited

davidbarsky Apr 14, 2024

davidbarsky Apr 14, 2024

Veykril Apr 15, 2024

Veykril left a comment

bors commented Apr 16, 2024

bors commented Apr 19, 2024

Veykril Apr 21, 2024

alibektas Apr 27, 2024

bors commented Apr 21, 2024

bors commented Apr 29, 2024

Veykril left a comment

Veykril commented Apr 29, 2024

bors commented Apr 29, 2024

alibektas commented Apr 29, 2024 •

edited

Veykril commented Apr 29, 2024

cbiffle commented May 2, 2024

cbiffle commented May 2, 2024

Veykril commented May 3, 2024

bors commented May 13, 2024

		// FIXME @alibektas : This is the only config that is confusing. If it's a proper configuration
		// why is it not among the others? If it's client only which I doubt it is current state should be alright

feat: TOML based config for rust-analyzer #17058

Are you sure you want to change the base?

feat: TOML based config for rust-analyzer #17058

Conversation

alibektas commented Apr 13, 2024 • edited

TOML Based Config for RA

Configuration Types

How Am I Supposed To Know If A Config Is Gl/Loc/Cl ?

Configuration Hierarchy

Default Configuration

User configuration

Client configuration

Workspace Root Configuration

Local Configuration

davidbarsky left a comment

Choose a reason for hiding this comment

davidbarsky Apr 14, 2024

Choose a reason for hiding this comment

Veykril Apr 15, 2024

Choose a reason for hiding this comment

alibektas Apr 18, 2024 • edited

Choose a reason for hiding this comment

davidbarsky Apr 14, 2024

Choose a reason for hiding this comment

davidbarsky Apr 14, 2024

Choose a reason for hiding this comment

Veykril Apr 15, 2024

Choose a reason for hiding this comment

Veykril left a comment

Choose a reason for hiding this comment

bors commented Apr 16, 2024

bors commented Apr 19, 2024

Veykril Apr 21, 2024

Choose a reason for hiding this comment

alibektas Apr 27, 2024

Choose a reason for hiding this comment

bors commented Apr 21, 2024

bors commented Apr 29, 2024

Veykril left a comment

Choose a reason for hiding this comment

Veykril commented Apr 29, 2024

bors commented Apr 29, 2024

alibektas commented Apr 29, 2024 • edited

Veykril commented Apr 29, 2024

cbiffle commented May 2, 2024

cbiffle commented May 2, 2024

Veykril commented May 3, 2024

bors commented May 13, 2024

alibektas commented Apr 13, 2024 •

edited

alibektas Apr 18, 2024 •

edited

alibektas commented Apr 29, 2024 •

edited