Finish migrating off Document to ArkFile#1267
Conversation
79aae40 to
0b199bf
Compare
| @@ -18,10 +17,10 @@ fn test_diagnostics_published_through_refresh_snapshot() { | |||
| // a `Document` plus its matching `oak_db::File`. | |||
| // TODO: this is a stopgap that goes away with `ArkFile`. Once handlers are | ||
| // pure Oak, they return `File`-keyed results (diagnostics, the edit targets | ||
| // in a `WorkspaceEdit`) and the wire URL gets attached at the transport | ||
| // boundary from a map of open editor URLs owned by the LSP layer. In that | ||
| // design the verbatim URL never travels through the analysis layer. |
| for document in state.open_files.values() { | ||
| if !ExtUrl::should_diagnose(&document.url) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Remove usage of ark_file() in here, you already have one!
There was a problem hiding this comment.
Could also change document to file
| pub(crate) fn get_document_mut(&mut self, path: &FilePath) -> anyhow::Result<&mut Document> { | ||
| if let Some(doc) = self.documents.get_mut(path) { | ||
| Ok(doc) | ||
| pub(crate) fn ark_file_mut(&mut self, uri: &Url) -> anyhow::Result<&mut ArkFile> { |
There was a problem hiding this comment.
Looks like we only still need this for updating DocumentConfig. I wonder what that will end up being a part of
| url: document.url.clone(), | ||
| encoding: self.config.position_encoding, | ||
| }) | ||
| Ok(ark_file.clone()) |
There was a problem hiding this comment.
I would generally disagree with this on principle but ill allow it since ark_file is supposed to go away anyways 😛
| pub(crate) fn workspace_uris(state: &WorldState) -> Vec<Url> { | ||
| state | ||
| .documents | ||
| .open_files |
There was a problem hiding this comment.
| pub(crate) fn workspace_uris(state: &WorldState) -> Vec<Url> { | |
| state | |
| .documents | |
| .open_files | |
| pub(crate) fn open_file_uris(state: &WorldState) -> Vec<Url> { | |
| state | |
| .open_files |
?
Seems nicer
There was a problem hiding this comment.
I imagine this helper may go away eventually too. I guess it depends on how we end up handling config in update_config(), which is currently the only consumer of this
| // origin = "https://github.com/rust-lang/rust-analyzer/blob/master/crates/rust-analyzer/src/lsp/utils.rs" | ||
| // --- | ||
| /// Apply a batch of LSP content changes to `contents`, returning the new text. | ||
| fn apply_content_changes( |
There was a problem hiding this comment.
I feeeel like maybe this and its newly added tests at the bottom should just go in its own standalone file. Maybe content.rs or changes.rs or something?
Branched from #1266
Progress towards #1212
With this PR everything now goes through Oak's DB.
Remove Ark's
Documentstructure, along with its copy of the tree-sitter tree, source code, and line index. These are now all accessed via Oak queries onFile.WorldState'sdocumentsfield becomesopen_files: HashMap<FilePath, ArkFile>. (Maybe we should rename toArkOpenFile?)