Detailed changes
@@ -10,7 +10,7 @@
use collections::{HashMap, HashSet, vecmap::VecMap};
use std::{
- path::{Path, PathBuf},
+ path::{Component, Path, PathBuf},
sync::Arc,
};
@@ -28,16 +28,32 @@ pub struct ProjectGroupName {
path_list: PathList,
}
+pub(crate) fn path_suffix(path: &Path, detail: usize) -> String {
+ let components: Vec<_> = path
+ .components()
+ .filter_map(|c| match c {
+ Component::Normal(s) => Some(s.to_string_lossy()),
+ _ => None,
+ })
+ .collect();
+ let start = components.len().saturating_sub(detail + 1);
+ components[start..].join("/")
+}
+
impl ProjectGroupName {
- pub fn display_name(&self) -> SharedString {
+ pub fn display_name_from_suffixes(
+ &self,
+ path_detail_map: &HashMap<PathBuf, usize>,
+ ) -> SharedString {
let mut names = Vec::with_capacity(self.path_list.paths().len());
for abs_path in self.path_list.paths() {
- if let Some(name) = abs_path.file_name() {
- names.push(name.to_string_lossy().to_string());
+ let detail = path_detail_map.get(abs_path).copied().unwrap_or(0);
+ let suffix = path_suffix(abs_path, detail);
+ if !suffix.is_empty() {
+ names.push(suffix);
}
}
if names.is_empty() {
- // TODO: Can we do something better in this case?
"Empty Workspace".into()
} else {
names.join(", ").into()
@@ -297,6 +313,81 @@ mod tests {
});
}
+ fn group_name_from_paths(paths: &[&str]) -> ProjectGroupName {
+ ProjectGroupName {
+ path_list: PathList::new(paths),
+ }
+ }
+
+ #[test]
+ fn test_path_suffix_detail_zero() {
+ assert_eq!(path_suffix(Path::new("/a/b/c"), 0), "c");
+ }
+
+ #[test]
+ fn test_path_suffix_detail_one() {
+ assert_eq!(path_suffix(Path::new("/a/b/c"), 1), "b/c");
+ }
+
+ #[test]
+ fn test_path_suffix_detail_two() {
+ assert_eq!(path_suffix(Path::new("/a/b/c"), 2), "a/b/c");
+ }
+
+ #[test]
+ fn test_path_suffix_clamped() {
+ let result = path_suffix(Path::new("/a/b"), 5);
+ assert_eq!(result, "a/b");
+ }
+
+ #[test]
+ fn test_display_name_from_suffixes_single_path() {
+ let name = group_name_from_paths(&["/code/zed"]);
+ let map = HashMap::default();
+ assert_eq!(name.display_name_from_suffixes(&map).as_ref(), "zed");
+
+ let map = HashMap::from_iter([(PathBuf::from("/code/zed"), 1)]);
+ assert_eq!(name.display_name_from_suffixes(&map).as_ref(), "code/zed");
+ }
+
+ #[test]
+ fn test_display_name_from_suffixes_multiple_paths() {
+ let name = group_name_from_paths(&["/a/zed", "/b/bar"]);
+
+ let map = HashMap::default();
+ assert_eq!(
+ name.display_name_from_suffixes(&map).as_ref(),
+ "zed, bar",
+ "PathList sorts lexicographically, so /a/zed comes before /b/bar"
+ );
+
+ let map = HashMap::from_iter([(PathBuf::from("/a/zed"), 1), (PathBuf::from("/b/bar"), 0)]);
+ assert_eq!(name.display_name_from_suffixes(&map).as_ref(), "a/zed, bar");
+ }
+
+ #[test]
+ fn test_display_name_from_suffixes_empty() {
+ let name = group_name_from_paths(&[]);
+ let map = HashMap::default();
+ assert_eq!(
+ name.display_name_from_suffixes(&map).as_ref(),
+ "Empty Workspace"
+ );
+ }
+
+ #[test]
+ fn test_display_name_from_suffixes_per_path_detail() {
+ let name = group_name_from_paths(&["/code/zed", "/code/bar/zed"]);
+ let map = HashMap::from_iter([
+ (PathBuf::from("/code/zed"), 1),
+ (PathBuf::from("/code/bar/zed"), 1),
+ ]);
+ assert_eq!(
+ name.display_name_from_suffixes(&map).as_ref(),
+ "bar/zed, code/zed",
+ );
+ }
+
#[gpui::test]
async fn test_worktree_checkout_canonicalizes_to_main_repo(cx: &mut TestAppContext) {
init_test(cx);
@@ -779,13 +779,26 @@ impl Sidebar {
(icon, icon_from_external_svg)
};
- for (group_name, group) in project_groups.groups() {
+ let groups: Vec<_> = project_groups.groups().collect();
+
+ let all_paths: Vec<std::path::PathBuf> = groups
+ .iter()
+ .flat_map(|(name, _)| name.path_list().paths().iter().cloned())
+ .collect();
+ let path_details =
+ util::disambiguate::compute_disambiguation_details(&all_paths, |path, detail| {
+ crate::project_group_builder::path_suffix(path, detail)
+ });
+ let path_detail_map: collections::HashMap<std::path::PathBuf, usize> =
+ all_paths.into_iter().zip(path_details).collect();
+
+ for (group_name, group) in &groups {
let path_list = group_name.path_list().clone();
if path_list.paths().is_empty() {
continue;
}
- let label = group_name.display_name();
+ let label = group_name.display_name_from_suffixes(&path_detail_map);
let is_collapsed = self.collapsed_groups.contains(&path_list);
let should_load_threads = !is_collapsed || !query.is_empty();
@@ -0,0 +1,147 @@
+use std::collections::HashMap;
+use std::hash::Hash;
+
+/// Computes the minimum detail level needed for each item so that no two items
+/// share the same description. Items whose descriptions are unique at level 0
+/// 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).
+pub fn compute_disambiguation_details<T, D>(
+ items: &[T],
+ get_description: impl Fn(&T, usize) -> D,
+) -> Vec<usize>
+where
+ D: Eq + Hash + Clone,
+{
+ let mut details = vec![0usize; items.len()];
+ let mut descriptions: HashMap<D, Vec<usize>> = HashMap::default();
+ let mut current_descriptions: Vec<D> =
+ items.iter().map(|item| get_description(item, 0)).collect();
+
+ loop {
+ let mut any_collisions = false;
+
+ for (index, (item, &detail)) in items.iter().zip(&details).enumerate() {
+ if detail > 0 {
+ let new_description = get_description(item, detail);
+ if new_description == current_descriptions[index] {
+ continue;
+ }
+ current_descriptions[index] = new_description;
+ }
+ descriptions
+ .entry(current_descriptions[index].clone())
+ .or_insert_with(Vec::new)
+ .push(index);
+ }
+
+ for (_, indices) in descriptions.drain() {
+ if indices.len() > 1 {
+ any_collisions = true;
+ for index in indices {
+ details[index] += 1;
+ }
+ }
+ }
+
+ if !any_collisions {
+ break;
+ }
+ }
+
+ details
+}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+
+ #[test]
+ fn test_no_conflicts() {
+ let items = vec!["alpha", "beta", "gamma"];
+ let details = compute_disambiguation_details(&items, |item, _detail| item.to_string());
+ assert_eq!(details, vec![0, 0, 0]);
+ }
+
+ #[test]
+ fn test_simple_two_way_conflict() {
+ // Two items with the same base name but different parents.
+ let items = vec![("src/foo.rs", "foo.rs"), ("lib/foo.rs", "foo.rs")];
+ let details = compute_disambiguation_details(&items, |item, detail| match detail {
+ 0 => item.1.to_string(),
+ _ => item.0.to_string(),
+ });
+ assert_eq!(details, vec![1, 1]);
+ }
+
+ #[test]
+ fn test_three_way_conflict() {
+ let items = vec![
+ ("foo.rs", "a/foo.rs"),
+ ("foo.rs", "b/foo.rs"),
+ ("foo.rs", "c/foo.rs"),
+ ];
+ let details = compute_disambiguation_details(&items, |item, detail| match detail {
+ 0 => item.0.to_string(),
+ _ => item.1.to_string(),
+ });
+ assert_eq!(details, vec![1, 1, 1]);
+ }
+
+ #[test]
+ fn test_deeper_conflict() {
+ // At detail 0, all three show "file.rs".
+ // At detail 1, items 0 and 1 both show "src/file.rs", item 2 shows "lib/file.rs".
+ // At detail 2, item 0 shows "a/src/file.rs", item 1 shows "b/src/file.rs".
+ let items = vec![
+ vec!["file.rs", "src/file.rs", "a/src/file.rs"],
+ vec!["file.rs", "src/file.rs", "b/src/file.rs"],
+ vec!["file.rs", "lib/file.rs", "x/lib/file.rs"],
+ ];
+ let details = compute_disambiguation_details(&items, |item, detail| {
+ let clamped = detail.min(item.len() - 1);
+ item[clamped].to_string()
+ });
+ assert_eq!(details, vec![2, 2, 1]);
+ }
+
+ #[test]
+ fn test_mixed_conflicting_and_unique() {
+ let items = vec![
+ ("src/foo.rs", "foo.rs"),
+ ("lib/foo.rs", "foo.rs"),
+ ("src/bar.rs", "bar.rs"),
+ ];
+ let details = compute_disambiguation_details(&items, |item, detail| match detail {
+ 0 => item.1.to_string(),
+ _ => item.0.to_string(),
+ });
+ assert_eq!(details, vec![1, 1, 0]);
+ }
+
+ #[test]
+ fn test_identical_items_terminates() {
+ // All items return the same description at every detail level.
+ // The algorithm must terminate rather than looping forever.
+ let items = vec!["same", "same", "same"];
+ let details = compute_disambiguation_details(&items, |item, _detail| item.to_string());
+ // After bumping to 1, the description doesn't change from level 0,
+ // so the items are skipped and the loop terminates.
+ assert_eq!(details, vec![1, 1, 1]);
+ }
+
+ #[test]
+ fn test_single_item() {
+ let items = vec!["only"];
+ let details = compute_disambiguation_details(&items, |item, _detail| item.to_string());
+ assert_eq!(details, vec![0]);
+ }
+
+ #[test]
+ fn test_empty_input() {
+ let items: Vec<&str> = vec![];
+ let details = compute_disambiguation_details(&items, |item, _detail| item.to_string());
+ let expected: Vec<usize> = vec![];
+ assert_eq!(details, expected);
+ }
+}
@@ -1,5 +1,6 @@
pub mod archive;
pub mod command;
+pub mod disambiguate;
pub mod fs;
pub mod markdown;
pub mod path_list;
@@ -4841,36 +4841,9 @@ fn dirty_message_for(buffer_path: Option<ProjectPath>, path_style: PathStyle) ->
}
pub fn tab_details(items: &[Box<dyn ItemHandle>], _window: &Window, cx: &App) -> Vec<usize> {
- let mut tab_details = items.iter().map(|_| 0).collect::<Vec<_>>();
- let mut tab_descriptions = HashMap::default();
- let mut done = false;
- while !done {
- done = true;
-
- // Store item indices by their tab description.
- for (ix, (item, detail)) in items.iter().zip(&tab_details).enumerate() {
- let description = item.tab_content_text(*detail, cx);
- if *detail == 0 || description != item.tab_content_text(detail - 1, cx) {
- tab_descriptions
- .entry(description)
- .or_insert(Vec::new())
- .push(ix);
- }
- }
-
- // If two or more items have the same tab description, increase their level
- // of detail and try again.
- for (_, item_ixs) in tab_descriptions.drain() {
- if item_ixs.len() > 1 {
- done = false;
- for ix in item_ixs {
- tab_details[ix] += 1;
- }
- }
- }
- }
-
- tab_details
+ util::disambiguate::compute_disambiguation_details(items, |item, detail| {
+ item.tab_content_text(detail, cx)
+ })
}
pub fn render_item_indicator(item: Box<dyn ItemHandle>, cx: &App) -> Option<Indicator> {
@@ -568,6 +568,27 @@ fn run_visual_tests(project_path: PathBuf, update_baseline: bool) -> Result<()>
}
}
+ // Run Test: Sidebar with duplicate project names
+ println!("\n--- Test: sidebar_duplicate_names ---");
+ match run_sidebar_duplicate_project_names_visual_tests(
+ app_state.clone(),
+ &mut cx,
+ update_baseline,
+ ) {
+ Ok(TestResult::Passed) => {
+ println!("✓ sidebar_duplicate_names: PASSED");
+ passed += 1;
+ }
+ Ok(TestResult::BaselineUpdated(_)) => {
+ println!("✓ sidebar_duplicate_names: Baselines updated");
+ updated += 1;
+ }
+ Err(e) => {
+ eprintln!("✗ sidebar_duplicate_names: FAILED - {}", e);
+ failed += 1;
+ }
+ }
+
// Run Test 9: Tool Permissions Settings UI visual test
println!("\n--- Test 9: tool_permissions_settings ---");
match run_tool_permissions_visual_tests(app_state.clone(), &mut cx, update_baseline) {
@@ -3069,6 +3090,279 @@ fn run_git_command(args: &[&str], dir: &std::path::Path) -> Result<()> {
Ok(())
}
+#[cfg(target_os = "macos")]
+/// Helper to create a project, add a worktree at the given path, and return the project.
+fn create_project_with_worktree(
+ worktree_dir: &Path,
+ app_state: &Arc<AppState>,
+ cx: &mut VisualTestAppContext,
+) -> Result<Entity<Project>> {
+ let project = cx.update(|cx| {
+ project::Project::local(
+ app_state.client.clone(),
+ app_state.node_runtime.clone(),
+ app_state.user_store.clone(),
+ app_state.languages.clone(),
+ app_state.fs.clone(),
+ None,
+ project::LocalProjectFlags {
+ init_worktree_trust: false,
+ ..Default::default()
+ },
+ cx,
+ )
+ });
+
+ let add_task = cx.update(|cx| {
+ project.update(cx, |project, cx| {
+ project.find_or_create_worktree(worktree_dir, true, cx)
+ })
+ });
+
+ cx.background_executor.allow_parking();
+ cx.foreground_executor
+ .block_test(add_task)
+ .context("Failed to add worktree")?;
+ cx.background_executor.forbid_parking();
+
+ cx.run_until_parked();
+ Ok(project)
+}
+
+#[cfg(target_os = "macos")]
+fn open_sidebar_test_window(
+ projects: Vec<Entity<Project>>,
+ app_state: &Arc<AppState>,
+ cx: &mut VisualTestAppContext,
+) -> Result<WindowHandle<MultiWorkspace>> {
+ anyhow::ensure!(!projects.is_empty(), "need at least one project");
+
+ let window_size = size(px(400.0), px(600.0));
+ let bounds = Bounds {
+ origin: point(px(0.0), px(0.0)),
+ size: window_size,
+ };
+
+ let mut projects_iter = projects.into_iter();
+ let first_project = projects_iter
+ .next()
+ .ok_or_else(|| anyhow::anyhow!("need at least one project"))?;
+ let remaining: Vec<_> = projects_iter.collect();
+
+ let multi_workspace_window: WindowHandle<MultiWorkspace> = cx
+ .update(|cx| {
+ cx.open_window(
+ WindowOptions {
+ window_bounds: Some(WindowBounds::Windowed(bounds)),
+ focus: false,
+ show: false,
+ ..Default::default()
+ },
+ |window, cx| {
+ let first_ws = cx.new(|cx| {
+ Workspace::new(None, first_project.clone(), app_state.clone(), window, cx)
+ });
+ cx.new(|cx| {
+ let mut mw = MultiWorkspace::new(first_ws, window, cx);
+ for project in remaining {
+ let ws = cx.new(|cx| {
+ Workspace::new(None, project, app_state.clone(), window, cx)
+ });
+ mw.activate(ws, window, cx);
+ }
+ mw
+ })
+ },
+ )
+ })
+ .context("Failed to open MultiWorkspace window")?;
+
+ cx.run_until_parked();
+
+ // Create the sidebar outside the MultiWorkspace update to avoid a
+ // re-entrant read panic (Sidebar::new reads the MultiWorkspace).
+ let sidebar = cx
+ .update_window(multi_workspace_window.into(), |root_view, window, cx| {
+ let mw_handle: Entity<MultiWorkspace> = root_view
+ .downcast()
+ .map_err(|_| anyhow::anyhow!("Failed to downcast root view to MultiWorkspace"))?;
+ Ok(cx.new(|cx| sidebar::Sidebar::new(mw_handle, window, cx)))
+ })
+ .context("Failed to create sidebar")??;
+
+ multi_workspace_window
+ .update(cx, |mw, _window, cx| {
+ mw.register_sidebar(sidebar.clone(), cx);
+ })
+ .context("Failed to register sidebar")?;
+
+ cx.run_until_parked();
+
+ // Open the sidebar
+ multi_workspace_window
+ .update(cx, |mw, window, cx| {
+ mw.toggle_sidebar(window, cx);
+ })
+ .context("Failed to toggle sidebar")?;
+
+ // Let rendering settle
+ for _ in 0..10 {
+ cx.advance_clock(Duration::from_millis(100));
+ cx.run_until_parked();
+ }
+
+ // Refresh the window
+ cx.update_window(multi_workspace_window.into(), |_, window, _cx| {
+ window.refresh();
+ })?;
+
+ cx.run_until_parked();
+
+ Ok(multi_workspace_window)
+}
+
+#[cfg(target_os = "macos")]
+fn cleanup_sidebar_test_window(
+ window: WindowHandle<MultiWorkspace>,
+ cx: &mut VisualTestAppContext,
+) -> Result<()> {
+ window.update(cx, |mw, _window, cx| {
+ for workspace in mw.workspaces() {
+ let project = workspace.read(cx).project().clone();
+ project.update(cx, |project, cx| {
+ let ids: Vec<_> = project.worktrees(cx).map(|wt| wt.read(cx).id()).collect();
+ for id in ids {
+ project.remove_worktree(id, cx);
+ }
+ });
+ }
+ })?;
+
+ cx.run_until_parked();
+
+ cx.update_window(window.into(), |_, window, _cx| {
+ window.remove_window();
+ })?;
+
+ cx.run_until_parked();
+
+ for _ in 0..15 {
+ cx.advance_clock(Duration::from_millis(100));
+ cx.run_until_parked();
+ }
+
+ Ok(())
+}
+
+#[cfg(target_os = "macos")]
+fn run_sidebar_duplicate_project_names_visual_tests(
+ app_state: Arc<AppState>,
+ cx: &mut VisualTestAppContext,
+ update_baseline: bool,
+) -> Result<TestResult> {
+ let temp_dir = tempfile::tempdir()?;
+ let temp_path = temp_dir.keep();
+ let canonical_temp = temp_path.canonicalize()?;
+
+ // Create directory structure where every leaf directory is named "zed" but
+ // lives at a distinct path. This lets us test that the sidebar correctly
+ // disambiguates projects whose names would otherwise collide.
+ //
+ // code/zed/ — project1 (single worktree)
+ // code/foo/zed/ — project2 (single worktree)
+ // code/bar/zed/ — project3, first worktree
+ // code/baz/zed/ — project3, second worktree
+ //
+ // No two projects share a worktree path, so ProjectGroupBuilder will
+ // place each in its own group.
+ let code_zed = canonical_temp.join("code").join("zed");
+ let foo_zed = canonical_temp.join("code").join("foo").join("zed");
+ let bar_zed = canonical_temp.join("code").join("bar").join("zed");
+ let baz_zed = canonical_temp.join("code").join("baz").join("zed");
+ std::fs::create_dir_all(&code_zed)?;
+ std::fs::create_dir_all(&foo_zed)?;
+ std::fs::create_dir_all(&bar_zed)?;
+ std::fs::create_dir_all(&baz_zed)?;
+
+ cx.update(|cx| {
+ cx.update_flags(true, vec!["agent-v2".to_string()]);
+ });
+
+ let mut has_baseline_update = None;
+
+ // --- Test 1: Two single-worktree projects whose leaf name is "zed" ---
+ {
+ let project1 = create_project_with_worktree(&code_zed, &app_state, cx)?;
+ let project2 = create_project_with_worktree(&foo_zed, &app_state, cx)?;
+
+ let window = open_sidebar_test_window(vec![project1, project2], &app_state, cx)?;
+
+ let result = run_visual_test(
+ "sidebar_two_projects_same_leaf_name",
+ window.into(),
+ cx,
+ update_baseline,
+ );
+
+ cleanup_sidebar_test_window(window, cx)?;
+ match result? {
+ TestResult::Passed => {}
+ TestResult::BaselineUpdated(path) => {
+ has_baseline_update = Some(path);
+ }
+ }
+ }
+
+ // --- Test 2: Three projects, third has two worktrees (all leaf names "zed") ---
+ //
+ // project1: code/zed
+ // project2: code/foo/zed
+ // project3: code/bar/zed + code/baz/zed
+ //
+ // Each project has a unique set of worktree paths, so they form
+ // separate groups. The sidebar must disambiguate all three.
+ {
+ let project1 = create_project_with_worktree(&code_zed, &app_state, cx)?;
+ let project2 = create_project_with_worktree(&foo_zed, &app_state, cx)?;
+
+ let project3 = create_project_with_worktree(&bar_zed, &app_state, cx)?;
+ let add_second_worktree = cx.update(|cx| {
+ project3.update(cx, |project, cx| {
+ project.find_or_create_worktree(&baz_zed, true, cx)
+ })
+ });
+ cx.background_executor.allow_parking();
+ cx.foreground_executor
+ .block_test(add_second_worktree)
+ .context("Failed to add second worktree to project 3")?;
+ cx.background_executor.forbid_parking();
+ cx.run_until_parked();
+
+ let window = open_sidebar_test_window(vec![project1, project2, project3], &app_state, cx)?;
+
+ let result = run_visual_test(
+ "sidebar_three_projects_with_multi_worktree",
+ window.into(),
+ cx,
+ update_baseline,
+ );
+
+ cleanup_sidebar_test_window(window, cx)?;
+ match result? {
+ TestResult::Passed => {}
+ TestResult::BaselineUpdated(path) => {
+ has_baseline_update = Some(path);
+ }
+ }
+ }
+
+ if let Some(path) = has_baseline_update {
+ Ok(TestResult::BaselineUpdated(path))
+ } else {
+ Ok(TestResult::Passed)
+ }
+}
+
#[cfg(all(target_os = "macos", feature = "visual-tests"))]
fn run_start_thread_in_selector_visual_tests(
app_state: Arc<AppState>,