minor: Read workspace root ratomls on startup#17661
Conversation
rustbot
added
the
S-waiting-on-review
alibektas
changed the title
| if !matches!(ws.kind, ProjectWorkspaceKind::DetachedFile { .. }) { | ||
| let ws_root = ws.workspace_root(); | ||
| file_set_roots.push(VfsPath::from(ws_root.to_owned())); | ||
| entries.push(ws_root.to_owned()); | ||
| register = true; | ||
| } |
There was a problem hiding this comment.
We should specifically push the rust-analyzer.toml file path here, not the workspace root itself. We only want to watch that one file there
There was a problem hiding this comment.
sorry ofc I meant to write that. 😞
crates/load-cargo/src/lib.rs
Outdated
| res.load.push(entry); | ||
| local_filesets.push(fsc.len() as u64); | ||
| fsc.add_file_set(file_set_roots) | ||
| if register { |
There was a problem hiding this comment.
register can be replaced by checking whether entries and/or file_set_roots is empty or not
Veykril
added
S-waiting-on-author
alibektas
added
S-waiting-on-review
|
@bors r+ |
|
☀️ Test successful - checks-actions |
|
Given #17246, it's still not being read early enough -- there is a pretty big chicken/egg problem with configs like this: [workspace.discoverConfig]
...We don't read rust-analyzer.toml until workspaces have already been discovered (to find the root), and we can't discover a workspace because haven't read it. I think we need to search upwards from the current working directory to find a rust-analyzer.toml to use on startup. For rust-analyzer.toml users, these configs will never be populated in src/bin/main.rs: rust-analyzer/crates/rust-analyzer/src/bin/main.rs Lines 268 to 273 in f46af90 |
|
Well, we should actually not allow We are in general planning a slight restructuring of config grouping as the current structure has some problems. |
|
Configuring a repo to discover non-cargo workspaces for anyone who opens it is one of the main difficulties I was hoping rust- analyzer.toml would solve, so I really hope we can break the chicken/egg somehow. |
|
We definitely want to have some solution there, but the current implementation isn't sufficient for that (its really just an MVP atm). |
|
That's a valid point to which I have no answer atm. But I am making a separate issue from this and I will find a way to address this |
5 participants
No description provided.