From b85af0e533767804c8393eae7ae37a8efe83c590 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Wed, 23 Oct 2024 17:34:23 +0300 Subject: [PATCH] Fall back to handling the abs path for external worktree entries (#19612) Certain files like Rust stdlib ones can be opened by cmd-clicking on terminal, editor contents, etc. Those files will not belong to the current worktree, so a fake worktree, with a single file, invisible (i.e. its dir(s) will not be shown in the UI such as project panel), will be created on the file opening. When the file is closed, the worktree is closed and removed along the way, so those worktrees are considered ephemeral and their ids are not stored in the database. This causes issues on reopening such files when they are closed. The PR makes Zed to fall back to opening the file by abs path when it's not in the project metadata, but has the abs path stored in history or in the opened items DB data. Release Notes: - Handle external worktree entries [re]open better --- crates/editor/src/items.rs | 109 ++++++++++++++++++------------ crates/editor/src/persistence.rs | 17 ++--- crates/project/src/lsp_store.rs | 11 +-- crates/workspace/src/workspace.rs | 88 ++++++++++++++++-------- 4 files changed, 141 insertions(+), 84 deletions(-) diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index 2574aa00a464f4100dd5030c87f696a291de18dc..cde1b33d369519da2f420b4f46df28d658a469cc 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -936,7 +936,7 @@ impl SerializableItem for Editor { fn deserialize( project: Model, - _workspace: WeakView, + workspace: WeakView, workspace_id: workspace::WorkspaceId, item_id: ItemId, cx: &mut ViewContext, @@ -953,7 +953,7 @@ impl SerializableItem for Editor { serialized_editor } else { SerializedEditor { - path: serialized_editor.path, + abs_path: serialized_editor.abs_path, contents: None, language: None, mtime: None, @@ -968,13 +968,13 @@ impl SerializableItem for Editor { } }; - let buffer_task = match serialized_editor { + match serialized_editor { SerializedEditor { - path: None, + abs_path: None, contents: Some(contents), language, .. - } => cx.spawn(|_, mut cx| { + } => cx.spawn(|pane, mut cx| { let project = project.clone(); async move { let language = if let Some(language_name) = language { @@ -1001,30 +1001,34 @@ impl SerializableItem for Editor { buffer.set_text(contents, cx); })?; - anyhow::Ok(buffer) + pane.update(&mut cx, |_, cx| { + cx.new_view(|cx| { + let mut editor = Editor::for_buffer(buffer, Some(project), cx); + + editor.read_scroll_position_from_db(item_id, workspace_id, cx); + editor + }) + }) } }), SerializedEditor { - path: Some(path), + abs_path: Some(abs_path), contents, mtime, .. } => { let project_item = project.update(cx, |project, cx| { - let (worktree, path) = project - .find_worktree(&path, cx) - .with_context(|| format!("No worktree for path: {path:?}"))?; + let (worktree, path) = project.find_worktree(&abs_path, cx)?; let project_path = ProjectPath { worktree_id: worktree.read(cx).id(), path: path.into(), }; - - Ok(project.open_path(project_path, cx)) + Some(project.open_path(project_path, cx)) }); - project_item - .map(|project_item| { - cx.spawn(|_, mut cx| async move { + match project_item { + Some(project_item) => { + cx.spawn(|pane, mut cx| async move { let (_, project_item) = project_item.await?; let buffer = project_item.downcast::().map_err(|_| { anyhow!("Project item at stored path was not a buffer") @@ -1051,26 +1055,36 @@ impl SerializableItem for Editor { })?; } - Ok(buffer) + pane.update(&mut cx, |_, cx| { + cx.new_view(|cx| { + let mut editor = Editor::for_buffer(buffer, Some(project), cx); + + editor.read_scroll_position_from_db(item_id, workspace_id, cx); + editor + }) + }) }) - }) - .unwrap_or_else(|error| Task::ready(Err(error))) + } + None => { + let open_by_abs_path = workspace.update(cx, |workspace, cx| { + workspace.open_abs_path(abs_path.clone(), false, cx) + }); + cx.spawn(|_, mut cx| async move { + let editor = open_by_abs_path?.await?.downcast::().with_context(|| format!("Failed to downcast to Editor after opening abs path {abs_path:?}"))?; + editor.update(&mut cx, |editor, cx| { + editor.read_scroll_position_from_db(item_id, workspace_id, cx); + })?; + Ok(editor) + }) + } + } } - _ => return Task::ready(Err(anyhow!("No path or contents found for buffer"))), - }; - - cx.spawn(|pane, mut cx| async move { - let buffer = buffer_task.await?; - - pane.update(&mut cx, |_, cx| { - cx.new_view(|cx| { - let mut editor = Editor::for_buffer(buffer, Some(project), cx); - - editor.read_scroll_position_from_db(item_id, workspace_id, cx); - editor - }) - }) - }) + SerializedEditor { + abs_path: None, + contents: None, + .. + } => Task::ready(Err(anyhow!("No path or contents found for buffer"))), + } } fn serialize( @@ -1096,12 +1110,19 @@ impl SerializableItem for Editor { let workspace_id = workspace.database_id()?; let buffer = self.buffer().read(cx).as_singleton()?; - let path = buffer - .read(cx) - .file() - .map(|file| file.full_path(cx)) - .and_then(|full_path| project.read(cx).find_project_path(&full_path, cx)) - .and_then(|project_path| project.read(cx).absolute_path(&project_path, cx)); + + let abs_path = buffer.read(cx).file().and_then(|file| { + let worktree_id = file.worktree_id(cx); + project + .read(cx) + .worktree_for_id(worktree_id, cx) + .and_then(|worktree| worktree.read(cx).absolutize(&file.path()).ok()) + .or_else(|| { + let full_path = file.full_path(cx); + let project_path = project.read(cx).find_project_path(&full_path, cx)?; + project.read(cx).absolute_path(&project_path, cx) + }) + }); let is_dirty = buffer.read(cx).is_dirty(); let mtime = buffer.read(cx).saved_mtime(); @@ -1120,7 +1141,7 @@ impl SerializableItem for Editor { }; let editor = SerializedEditor { - path, + abs_path, contents, language, mtime, @@ -1633,7 +1654,7 @@ mod tests { let item_id = 1234 as ItemId; let serialized_editor = SerializedEditor { - path: Some(PathBuf::from("/file.rs")), + abs_path: Some(PathBuf::from("/file.rs")), contents: Some("fn main() {}".to_string()), language: Some("Rust".to_string()), mtime: Some(now), @@ -1664,7 +1685,7 @@ mod tests { let item_id = 5678 as ItemId; let serialized_editor = SerializedEditor { - path: Some(PathBuf::from("/file.rs")), + abs_path: Some(PathBuf::from("/file.rs")), contents: None, language: None, mtime: None, @@ -1699,7 +1720,7 @@ mod tests { let item_id = 9012 as ItemId; let serialized_editor = SerializedEditor { - path: None, + abs_path: None, contents: Some("hello".to_string()), language: Some("Rust".to_string()), mtime: None, @@ -1737,7 +1758,7 @@ mod tests { .checked_sub(std::time::Duration::from_secs(60 * 60 * 24)) .unwrap(); let serialized_editor = SerializedEditor { - path: Some(PathBuf::from("/file.rs")), + abs_path: Some(PathBuf::from("/file.rs")), contents: Some("fn main() {}".to_string()), language: Some("Rust".to_string()), mtime: Some(old_mtime), diff --git a/crates/editor/src/persistence.rs b/crates/editor/src/persistence.rs index 9be1fbdd80ff871f560b4001f4fa3cf22c8472f4..a52fb60543004ab6d257a7b61abd94eb8f9256d6 100644 --- a/crates/editor/src/persistence.rs +++ b/crates/editor/src/persistence.rs @@ -11,7 +11,7 @@ use workspace::{ItemId, WorkspaceDb, WorkspaceId}; #[derive(Clone, Debug, PartialEq, Default)] pub(crate) struct SerializedEditor { - pub(crate) path: Option, + pub(crate) abs_path: Option, pub(crate) contents: Option, pub(crate) language: Option, pub(crate) mtime: Option, @@ -25,7 +25,7 @@ impl StaticColumnCount for SerializedEditor { impl Bind for SerializedEditor { fn bind(&self, statement: &Statement, start_index: i32) -> Result { - let start_index = statement.bind(&self.path, start_index)?; + let start_index = statement.bind(&self.abs_path, start_index)?; let start_index = statement.bind(&self.contents, start_index)?; let start_index = statement.bind(&self.language, start_index)?; @@ -51,7 +51,8 @@ impl Bind for SerializedEditor { impl Column for SerializedEditor { fn column(statement: &mut Statement, start_index: i32) -> Result<(Self, i32)> { - let (path, start_index): (Option, i32) = Column::column(statement, start_index)?; + let (abs_path, start_index): (Option, i32) = + Column::column(statement, start_index)?; let (contents, start_index): (Option, i32) = Column::column(statement, start_index)?; let (language, start_index): (Option, i32) = @@ -66,7 +67,7 @@ impl Column for SerializedEditor { .map(|(seconds, nanos)| UNIX_EPOCH + Duration::new(seconds as u64, nanos as u32)); let editor = Self { - path, + abs_path, contents, language, mtime, @@ -226,7 +227,7 @@ mod tests { let workspace_id = workspace::WORKSPACE_DB.next_id().await.unwrap(); let serialized_editor = SerializedEditor { - path: Some(PathBuf::from("testing.txt")), + abs_path: Some(PathBuf::from("testing.txt")), contents: None, language: None, mtime: None, @@ -244,7 +245,7 @@ mod tests { // Now update contents and language let serialized_editor = SerializedEditor { - path: Some(PathBuf::from("testing.txt")), + abs_path: Some(PathBuf::from("testing.txt")), contents: Some("Test".to_owned()), language: Some("Go".to_owned()), mtime: None, @@ -262,7 +263,7 @@ mod tests { // Now set all the fields to NULL let serialized_editor = SerializedEditor { - path: None, + abs_path: None, contents: None, language: None, mtime: None, @@ -281,7 +282,7 @@ mod tests { // Storing and retrieving mtime let now = SystemTime::now(); let serialized_editor = SerializedEditor { - path: None, + abs_path: None, contents: None, language: None, mtime: Some(now), diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index a2e1b7b0f8a1d891e3634e8e64c8856214160338..8152ddb3c0fbc361e7d9337f35d0a86b0bb2fb41 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -3356,11 +3356,12 @@ impl LspStore { diagnostics: Vec>>, cx: &mut ModelContext, ) -> Result<(), anyhow::Error> { - let (worktree, relative_path) = - self.worktree_store - .read(cx) - .find_worktree(&abs_path, cx) - .ok_or_else(|| anyhow!("no worktree found for diagnostics path {abs_path:?}"))?; + let Some((worktree, relative_path)) = + self.worktree_store.read(cx).find_worktree(&abs_path, cx) + else { + log::warn!("skipping diagnostics update, no worktree found for path {abs_path:?}"); + return Ok(()); + }; let project_path = ProjectPath { worktree_id: worktree.read(cx).id(), diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index cfe666b38e462301fbf5de6e354426088b576565..adc3e687413a51e8c4101102c18e6200b58da524 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -1362,14 +1362,13 @@ impl Workspace { if navigated { break None; } - } - // If the item is no longer present in this pane, then retrieve its - // project path in order to reopen it. - else { + } else { + // If the item is no longer present in this pane, then retrieve its + // path info in order to reopen it. break pane .nav_history() .path_for_item(entry.item.id()) - .map(|(project_path, _)| (project_path, entry)); + .map(|(project_path, abs_path)| (project_path, abs_path, entry)); } } }) @@ -1377,32 +1376,67 @@ impl Workspace { None }; - if let Some((project_path, entry)) = to_load { - // If the item was no longer present, then load it again from its previous path. - let task = self.load_path(project_path, cx); + if let Some((project_path, abs_path, entry)) = to_load { + // If the item was no longer present, then load it again from its previous path, first try the local path + let open_by_project_path = self.load_path(project_path.clone(), cx); + cx.spawn(|workspace, mut cx| async move { - let task = task.await; + let open_by_project_path = open_by_project_path.await; let mut navigated = false; - if let Some((project_entry_id, build_item)) = task.log_err() { - let prev_active_item_id = pane.update(&mut cx, |pane, _| { - pane.nav_history_mut().set_mode(mode); - pane.active_item().map(|p| p.item_id()) - })?; + match open_by_project_path + .with_context(|| format!("Navigating to {project_path:?}")) + { + Ok((project_entry_id, build_item)) => { + let prev_active_item_id = pane.update(&mut cx, |pane, _| { + pane.nav_history_mut().set_mode(mode); + pane.active_item().map(|p| p.item_id()) + })?; - pane.update(&mut cx, |pane, cx| { - let item = pane.open_item( - project_entry_id, - true, - entry.is_preview, - cx, - build_item, - ); - navigated |= Some(item.item_id()) != prev_active_item_id; - pane.nav_history_mut().set_mode(NavigationMode::Normal); - if let Some(data) = entry.data { - navigated |= item.navigate(data, cx); + pane.update(&mut cx, |pane, cx| { + let item = pane.open_item( + project_entry_id, + true, + entry.is_preview, + cx, + build_item, + ); + navigated |= Some(item.item_id()) != prev_active_item_id; + pane.nav_history_mut().set_mode(NavigationMode::Normal); + if let Some(data) = entry.data { + navigated |= item.navigate(data, cx); + } + })?; + } + Err(open_by_project_path_e) => { + // Fall back to opening by abs path, in case an external file was opened and closed, + // and its worktree is now dropped + if let Some(abs_path) = abs_path { + let prev_active_item_id = pane.update(&mut cx, |pane, _| { + pane.nav_history_mut().set_mode(mode); + pane.active_item().map(|p| p.item_id()) + })?; + let open_by_abs_path = workspace.update(&mut cx, |workspace, cx| { + workspace.open_abs_path(abs_path.clone(), false, cx) + })?; + match open_by_abs_path + .await + .with_context(|| format!("Navigating to {abs_path:?}")) + { + Ok(item) => { + pane.update(&mut cx, |pane, cx| { + navigated |= Some(item.item_id()) != prev_active_item_id; + pane.nav_history_mut().set_mode(NavigationMode::Normal); + if let Some(data) = entry.data { + navigated |= item.navigate(data, cx); + } + })?; + } + Err(open_by_abs_path_e) => { + log::error!("Failed to navigate history: {open_by_project_path_e:#} and {open_by_abs_path_e:#}"); + } + } } - })?; + } } if !navigated {