Skip to content

fix: Updating settings should not clobber discovered projects #18059

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

Merged
merged 1 commit into from
Sep 6, 2024

Conversation

Wilfred
Copy link
Contributor

@Wilfred Wilfred commented Sep 5, 2024

linkedProjects is owned by the user's configuration, so when users update this setting, linkedProjects is reset. This is problematic when linkedProjects also contains projects discovered with discoverCommand.

The buggy behaviour occurred when:

(1) The user configures discoverCommand and loads a Rust project.

(2) The user changes any setting in VS Code, so rust-analyzer receives workspace/didChangeConfiguration.

(3) handle_did_change_configuration ultimately calls Client::apply_change_with_sink(), which updates
config.user_config and discards any items we added in linkedProjects.

Instead, separate out discovered_projects_from_filesystem and discovered_projects_from_command from user configuration, so user settings cannot affect any type of discovered project.

This fixes the subtle issue mentioned here: #17246 (comment)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 5, 2024
@Wilfred Wilfred marked this pull request as ready for review September 5, 2024 22:43
`linkedProjects` is owned by the user's configuration, so when users
update this setting, `linkedProjects` is reset. This is problematic when
`linkedProjects` also contains projects discovered with `discoverCommand`.

The buggy behaviour occurred when:

(1) The user configures `discoverCommand` and loads a Rust project.

(2) The user changes any setting in VS Code, so rust-analyzer receives
`workspace/didChangeConfiguration`.

(3) `handle_did_change_configuration` ultimately calls
`Client::apply_change_with_sink()`, which updates `config.user_config`
and discards any items we added in `linkedProjects`.

Instead, separate out `discovered_projects_from_filesystem` and
`discovered_projects_from_command` from user configuration, so user
settings cannot affect any type of discovered project.

This fixes the subtle issue mentioned here:
rust-lang#17246 (comment)
@Veykril
Copy link
Member

Veykril commented Sep 6, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Sep 6, 2024

📌 Commit 3cf28f1 has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Sep 6, 2024

⌛ Testing commit 3cf28f1 with merge 315b66f...

@bors
Copy link
Contributor

bors commented Sep 6, 2024

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing 315b66f to master...

@bors bors merged commit 315b66f into rust-lang:master Sep 6, 2024
11 checks passed
@Wilfred Wilfred deleted the config_cleanups branch September 6, 2024 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants