Track abs paths in history

Kirill Bulatov created

Change summary

crates/file_finder/src/file_finder.rs | 31 +++++++------
crates/workspace/src/pane.rs          | 39 +++++++++++++-----
crates/workspace/src/workspace.rs     | 62 ++++++++++++++++++++++++----
3 files changed, 97 insertions(+), 35 deletions(-)

Detailed changes

crates/file_finder/src/file_finder.rs 🔗

@@ -71,6 +71,12 @@ struct FoundPath {
     absolute: Option<PathBuf>,
 }
 
+impl FoundPath {
+    fn new(project: ProjectPath, absolute: Option<PathBuf>) -> Self {
+        Self { project, absolute }
+    }
+}
+
 actions!(file_finder, [Toggle]);
 
 pub fn init(cx: &mut AppContext) {
@@ -83,37 +89,34 @@ const MAX_RECENT_SELECTIONS: usize = 20;
 fn toggle_file_finder(workspace: &mut Workspace, _: &Toggle, cx: &mut ViewContext<Workspace>) {
     workspace.toggle_modal(cx, |workspace, cx| {
         let project = workspace.project().read(cx);
-        let project_to_found_path = |project_path: ProjectPath| FoundPath {
-            absolute: project
-                // TODO kb this will be called on every panel reopen, will the workspaec exist if all files are closed?
-                // might need to store these in the history instead
-                .worktree_for_id(project_path.worktree_id, cx)
-                .map(|worktree| worktree.read(cx).abs_path().join(&project_path.path)),
-            project: project_path,
-        };
 
         let currently_opened_path = workspace
             .active_item(cx)
             .and_then(|item| item.project_path(cx))
-            .map(project_to_found_path);
+            .map(|project_path| {
+                let abs_path = project
+                    .worktree_for_id(project_path.worktree_id, cx)
+                    .map(|worktree| worktree.read(cx).abs_path().join(&project_path.path));
+                FoundPath::new(project_path, abs_path)
+            });
 
         // if exists, bubble the currently opened path to the top
-        let history_items = currently_opened_path
-            .clone()
+        let history_items = dbg!(dbg!(currently_opened_path.clone())
             .into_iter()
             .chain(
                 workspace
+                    // TODO kb history contains empty paths
                     .recent_navigation_history(Some(MAX_RECENT_SELECTIONS), cx)
                     .into_iter()
-                    .filter(|history_path| {
+                    .filter(|(history_path, _)| {
                         Some(history_path)
                             != currently_opened_path
                                 .as_ref()
                                 .map(|found_path| &found_path.project)
                     })
-                    .map(project_to_found_path),
+                    .map(|(history_path, abs_path)| FoundPath::new(history_path, abs_path)),
             )
-            .collect();
+            .collect());
 
         let project = workspace.project().clone();
         let workspace = cx.handle().downgrade();

crates/workspace/src/pane.rs 🔗

@@ -31,7 +31,7 @@ use std::{
     any::Any,
     cell::RefCell,
     cmp, mem,
-    path::Path,
+    path::{Path, PathBuf},
     rc::Rc,
     sync::{
         atomic::{AtomicUsize, Ordering},
@@ -180,7 +180,7 @@ struct NavHistory {
     backward_stack: VecDeque<NavigationEntry>,
     forward_stack: VecDeque<NavigationEntry>,
     closed_stack: VecDeque<NavigationEntry>,
-    paths_by_item: HashMap<usize, ProjectPath>,
+    paths_by_item: HashMap<usize, (ProjectPath, Option<PathBuf>)>,
     pane: WeakViewHandle<Pane>,
     next_timestamp: Arc<AtomicUsize>,
 }
@@ -482,7 +482,7 @@ impl Pane {
                             .paths_by_item
                             .get(&entry.item.id())
                             .cloned()
-                            .map(|project_path| (project_path, entry));
+                            .map(|(project_path, _)| (project_path, entry));
                     }
                 }
             })
@@ -492,7 +492,7 @@ impl Pane {
 
         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 = workspace.load_path(project_path, cx);
+            let task = workspace.load_path(project_path.clone(), cx);
             cx.spawn(|workspace, mut cx| async move {
                 let task = task.await;
                 let mut navigated = false;
@@ -510,6 +510,7 @@ impl Pane {
                             workspace,
                             pane.clone(),
                             project_entry_id,
+                            &project_path,
                             true,
                             cx,
                             build_item,
@@ -546,6 +547,7 @@ impl Pane {
         workspace: &mut Workspace,
         pane: ViewHandle<Pane>,
         project_entry_id: ProjectEntryId,
+        project_path: &ProjectPath,
         focus_item: bool,
         cx: &mut ViewContext<Workspace>,
         build_item: impl FnOnce(&mut ViewContext<Pane>) -> Box<dyn ItemHandle>,
@@ -578,6 +580,15 @@ impl Pane {
                 None,
                 cx,
             );
+            {
+                let abs_path = workspace.absolute_path(project_path, cx);
+                pane.read(cx)
+                    .nav_history
+                    .borrow_mut()
+                    .paths_by_item
+                    .insert(new_item.id(), (project_path.clone(), abs_path));
+            }
+
             new_item
         }
     }
@@ -1003,10 +1014,14 @@ impl Pane {
             .set_mode(NavigationMode::Normal);
 
         if let Some(path) = item.project_path(cx) {
+            let abs_path = self
+                .workspace()
+                .upgrade(cx)
+                .and_then(|workspace| workspace.read(cx).absolute_path(&path, cx));
             self.nav_history
                 .borrow_mut()
                 .paths_by_item
-                .insert(item.id(), path);
+                .insert(item.id(), (path, abs_path));
         } else {
             self.nav_history
                 .borrow_mut()
@@ -1954,7 +1969,7 @@ impl PaneNavHistory {
     pub fn for_each_entry(
         &self,
         cx: &AppContext,
-        mut f: impl FnMut(&NavigationEntry, ProjectPath),
+        mut f: impl FnMut(&NavigationEntry, (ProjectPath, Option<PathBuf>)),
     ) {
         let borrowed_history = self.0.borrow();
         borrowed_history
@@ -1963,12 +1978,14 @@ impl PaneNavHistory {
             .chain(borrowed_history.backward_stack.iter())
             .chain(borrowed_history.closed_stack.iter())
             .for_each(|entry| {
-                if let Some(path) = borrowed_history.paths_by_item.get(&entry.item.id()) {
-                    f(entry, path.clone());
+                if let Some(project_and_abs_path) =
+                    borrowed_history.paths_by_item.get(&entry.item.id())
+                {
+                    f(entry, project_and_abs_path.clone());
                 } else if let Some(item) = entry.item.upgrade(cx) {
-                    let path = item.project_path(cx);
-                    if let Some(path) = path {
-                        f(entry, path);
+                    if let Some(path) = item.project_path(cx) {
+                        // TODO kb ??? this should be the full path
+                        f(entry, (path, None));
                     }
                 }
             })

crates/workspace/src/workspace.rs 🔗

@@ -946,31 +946,53 @@ impl Workspace {
         &self,
         limit: Option<usize>,
         cx: &AppContext,
-    ) -> Vec<ProjectPath> {
-        let mut history: HashMap<ProjectPath, usize> = HashMap::default();
+    ) -> Vec<(ProjectPath, Option<PathBuf>)> {
+        let mut abs_paths_opened: HashMap<PathBuf, HashSet<ProjectPath>> = HashMap::default();
+        let mut history: HashMap<ProjectPath, (Option<PathBuf>, usize)> = HashMap::default();
         for pane in &self.panes {
             let pane = pane.read(cx);
             pane.nav_history()
-                .for_each_entry(cx, |entry, project_path| {
+                .for_each_entry(cx, |entry, (project_path, fs_path)| {
+                    if let Some(fs_path) = &fs_path {
+                        abs_paths_opened
+                            .entry(fs_path.clone())
+                            .or_default()
+                            .insert(project_path.clone());
+                    }
                     let timestamp = entry.timestamp;
                     match history.entry(project_path) {
                         hash_map::Entry::Occupied(mut entry) => {
-                            if &timestamp > entry.get() {
-                                entry.insert(timestamp);
+                            let (old_fs_path, old_timestamp) = entry.get();
+                            if &timestamp > old_timestamp {
+                                assert_eq!(&fs_path, old_fs_path, "Inconsistent nav history");
+                                entry.insert((fs_path, timestamp));
                             }
                         }
                         hash_map::Entry::Vacant(entry) => {
-                            entry.insert(timestamp);
+                            entry.insert((fs_path, timestamp));
                         }
                     }
                 });
         }
 
+        let project = self.project.read(cx);
         history
             .into_iter()
-            .sorted_by_key(|(_, timestamp)| *timestamp)
-            .map(|(project_path, _)| project_path)
+            .sorted_by_key(|(_, (_, timestamp))| *timestamp)
+            .map(|(project_path, (fs_path, _))| (project_path, fs_path))
             .rev()
+            .filter(|(history_path, abs_path)| {
+                project
+                    .worktree_for_id(history_path.worktree_id, cx)
+                    .is_some()
+                    || abs_path
+                        .as_ref()
+                        .and_then(|abs_path| {
+                            let buffers_opened = abs_paths_opened.get(abs_path)?;
+                            Some(buffers_opened.len() < 2)
+                        })
+                        .unwrap_or(false)
+            })
             .take(limit.unwrap_or(usize::MAX))
             .collect()
     }
@@ -1283,6 +1305,17 @@ impl Workspace {
         })
     }
 
+    pub fn absolute_path(&self, project_path: &ProjectPath, cx: &AppContext) -> Option<PathBuf> {
+        Some(
+            self.project()
+                .read(cx)
+                .worktree_for_id(project_path.worktree_id, cx)?
+                .read(cx)
+                .abs_path()
+                .to_path_buf(),
+        )
+    }
+
     fn add_folder_to_project(&mut self, _: &AddFolderToProject, cx: &mut ViewContext<Self>) {
         let mut paths = cx.prompt_for_paths(PathPromptOptions {
             files: false,
@@ -1628,14 +1661,23 @@ impl Workspace {
             })
         });
 
-        let task = self.load_path(path.into(), cx);
+        let project_path = path.into();
+        let task = self.load_path(project_path.clone(), cx);
         cx.spawn(|this, mut cx| async move {
             let (project_entry_id, build_item) = task.await?;
             let pane = pane
                 .upgrade(&cx)
                 .ok_or_else(|| anyhow!("pane was closed"))?;
             this.update(&mut cx, |this, cx| {
-                Pane::open_item(this, pane, project_entry_id, focus_item, cx, build_item)
+                Pane::open_item(
+                    this,
+                    pane,
+                    project_entry_id,
+                    &project_path,
+                    focus_item,
+                    cx,
+                    build_item,
+                )
             })
         })
     }