Improve project name disambiguation

Richard Feldman created

- Optimize path_suffix to use reverse iteration instead of collecting all components
- Remove unused display_name method, rename display_name_from_suffixes to display_name
- Add doc comment warning about fixed-point requirement on compute_disambiguation_details
- Apply disambiguation to project picker (get_recent_projects and get_open_folders)

Change summary

crates/project/src/project.rs                 | 25 ++-------
crates/recent_projects/src/recent_projects.rs | 53 +++++++++++++++-----
crates/sidebar/src/sidebar.rs                 |  2 
crates/util/src/disambiguate.rs               |  5 +
4 files changed, 50 insertions(+), 35 deletions(-)

Detailed changes

crates/project/src/project.rs 🔗

@@ -6066,26 +6066,11 @@ impl ProjectGroupKey {
         Self { paths, host }
     }
 
-    pub fn display_name(&self) -> SharedString {
-        let mut names = Vec::with_capacity(self.paths.paths().len());
-        for abs_path in self.paths.paths() {
-            if let Some(name) = abs_path.file_name() {
-                names.push(name.to_string_lossy().to_string());
-            }
-        }
-        if names.is_empty() {
-            // TODO: Can we do something better in this case?
-            "Empty Workspace".into()
-        } else {
-            names.join(", ").into()
-        }
-    }
-
     pub fn path_list(&self) -> &PathList {
         &self.paths
     }
 
-    pub fn display_name_from_suffixes(
+    pub fn display_name(
         &self,
         path_detail_map: &std::collections::HashMap<PathBuf, usize>,
     ) -> SharedString {
@@ -6110,15 +6095,17 @@ impl ProjectGroupKey {
 }
 
 pub fn path_suffix(path: &Path, detail: usize) -> String {
-    let components: Vec<_> = path
+    let mut components: Vec<_> = path
         .components()
+        .rev()
         .filter_map(|component| match component {
             std::path::Component::Normal(s) => Some(s.to_string_lossy()),
             _ => None,
         })
+        .take(detail + 1)
         .collect();
-    let start = components.len().saturating_sub(detail + 1);
-    components[start..].join("/")
+    components.reverse();
+    components.join("/")
 }
 
 pub struct PathMatchCandidateSet {

crates/recent_projects/src/recent_projects.rs 🔗

@@ -100,27 +100,38 @@ pub async fn get_recent_projects(
         .await
         .unwrap_or_default();
 
-    let entries: Vec<RecentProjectEntry> = workspaces
+    let filtered: Vec<_> = workspaces
         .into_iter()
         .filter(|(id, _, _, _)| Some(*id) != current_workspace_id)
         .filter(|(_, location, _, _)| matches!(location, SerializedWorkspaceLocation::Local))
+        .collect();
+
+    let all_paths: Vec<PathBuf> = filtered
+        .iter()
+        .flat_map(|(_, _, path_list, _)| path_list.paths().iter().cloned())
+        .collect();
+    let path_details =
+        util::disambiguate::compute_disambiguation_details(&all_paths, |path, detail| {
+            project::path_suffix(path, detail)
+        });
+    let path_detail_map: std::collections::HashMap<PathBuf, usize> =
+        all_paths.into_iter().zip(path_details).collect();
+
+    let entries: Vec<RecentProjectEntry> = filtered
+        .into_iter()
         .map(|(workspace_id, _, path_list, timestamp)| {
             let paths: Vec<PathBuf> = path_list.paths().to_vec();
             let ordered_paths: Vec<&PathBuf> = path_list.ordered_paths().collect();
 
-            let name = if ordered_paths.len() == 1 {
-                ordered_paths[0]
-                    .file_name()
-                    .map(|n| n.to_string_lossy().to_string())
-                    .unwrap_or_else(|| ordered_paths[0].to_string_lossy().to_string())
-            } else {
-                ordered_paths
-                    .iter()
-                    .filter_map(|p| p.file_name())
-                    .map(|n| n.to_string_lossy().to_string())
-                    .collect::<Vec<_>>()
-                    .join(", ")
-            };
+            let name = ordered_paths
+                .iter()
+                .map(|p| {
+                    let detail = path_detail_map.get(*p).copied().unwrap_or(0);
+                    project::path_suffix(p, detail)
+                })
+                .filter(|s| !s.is_empty())
+                .collect::<Vec<_>>()
+                .join(", ");
 
             let full_path = ordered_paths
                 .iter()
@@ -173,6 +184,17 @@ fn get_open_folders(workspace: &Workspace, cx: &App) -> Vec<OpenFolderEntry> {
             .map(|wt| wt.read(cx).id())
     });
 
+    let all_paths: Vec<PathBuf> = visible_worktrees
+        .iter()
+        .map(|wt| wt.read(cx).abs_path().to_path_buf())
+        .collect();
+    let path_details =
+        util::disambiguate::compute_disambiguation_details(&all_paths, |path, detail| {
+            project::path_suffix(path, detail)
+        });
+    let path_detail_map: std::collections::HashMap<PathBuf, usize> =
+        all_paths.into_iter().zip(path_details).collect();
+
     let git_store = project.git_store().read(cx);
     let repositories: Vec<_> = git_store.repositories().values().cloned().collect();
 
@@ -181,8 +203,9 @@ fn get_open_folders(workspace: &Workspace, cx: &App) -> Vec<OpenFolderEntry> {
         .map(|worktree| {
             let worktree_ref = worktree.read(cx);
             let worktree_id = worktree_ref.id();
-            let name = SharedString::from(worktree_ref.root_name().as_unix_str().to_string());
             let path = worktree_ref.abs_path().to_path_buf();
+            let detail = path_detail_map.get(&path).copied().unwrap_or(0);
+            let name = SharedString::from(project::path_suffix(&path, detail));
             let branch = get_branch_for_worktree(worktree_ref, &repositories, cx);
             let is_active = active_worktree_id == Some(worktree_id);
             OpenFolderEntry {

crates/sidebar/src/sidebar.rs 🔗

@@ -817,7 +817,7 @@ impl Sidebar {
                 continue;
             }
 
-            let label = group_key.display_name_from_suffixes(&path_detail_map);
+            let label = group_key.display_name(&path_detail_map);
 
             let is_collapsed = self.collapsed_groups.contains(&path_list);
             let should_load_threads = !is_collapsed || !query.is_empty();

crates/util/src/disambiguate.rs 🔗

@@ -6,6 +6,11 @@ use std::hash::Hash;
 /// stay at 0; items that collide get their detail level incremented until either
 /// the collision is resolved or increasing the level no longer changes the
 /// description (preventing infinite loops for truly identical items).
+///
+/// The `get_description` closure must return a sequence that eventually reaches
+/// a "fixed point" where increasing `detail` no longer changes the output. If
+/// an item reaches its fixed point, it is assumed it will no longer change and
+/// will no longer be checked for collisions.
 pub fn compute_disambiguation_details<T, D>(
     items: &[T],
     get_description: impl Fn(&T, usize) -> D,