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

Extract LSP state in own object #358

Merged
merged 2 commits into from
May 29, 2024
Merged

Extract LSP state in own object #358

merged 2 commits into from
May 29, 2024

Conversation

lionel-
Copy link
Contributor

@lionel- lionel- commented May 17, 2024

Branched from #357.

This PR starts the work of gathering all LSP inputs to a new WorldState struct. This data structure will eventually be a pure value without any interior mutability (e.g. no DashMap of Documents). The goal is that if you take a clone of it, you can send it off to a long-running background task without running the risk of invalidated assumptions due to input mutation after an LSP update.

Progress towards posit-dev/positron#3181 (Incremental and cancellable computations with salsa): In the longer term this could serve as the source of root Salsa inputs for incremental computation. Querying an outdated Salsa input would cancel the background task with a special unwinding panic.

Comment on lines 16 to 24
pub struct Workspace {
pub folders: Vec<Url>,
}

impl Default for Workspace {
fn default() -> Self {
Self {
folders: Default::default(),
}
Copy link
Contributor

Choose a reason for hiding this comment

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

While we are here, any preference for default() vs new() for Workspace? I probably would have implemented a new() method that set folders: Vec::new(). No super strong opinion, but sometimes I have disagreed with what default() implementations of the underlying components ended up being, so now I guess I prefer new()

Copy link
Contributor Author

@lionel- lionel- May 29, 2024

Choose a reason for hiding this comment

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

I think it makes sense to start with an empty workspace.

Regarding new vs default, a nice property of the former is that it allows algorithms based on mem::take() (not that we are likely to need this here). It also makes it easier to wrap around a constructor and override only some of the fields.

I agree that it's a bit ambiguous when to use one versus the other.

Base automatically changed from upkeep/open-editor-via-jupyter to main May 29, 2024 09:19
@lionel- lionel- force-pushed the feature/lsp-state branch from 6e31619 to ed7d32f Compare May 29, 2024 09:22
@lionel- lionel- merged commit 6db7c99 into main May 29, 2024
1 check passed
@lionel- lionel- deleted the feature/lsp-state branch May 29, 2024 09:25
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.

3 participants