diff --git a/crates/sidebar/src/project_group_builder.rs b/crates/sidebar/src/project_group_builder.rs index 0b8e56ac99565218dd827048afdee71e896f2667..221a68aea97736133f8563354d41dfc4495f59e8 100644 --- a/crates/sidebar/src/project_group_builder.rs +++ b/crates/sidebar/src/project_group_builder.rs @@ -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, + ) -> 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); diff --git a/crates/sidebar/src/sidebar.rs b/crates/sidebar/src/sidebar.rs index e09ee3e8809417924b1b1b43f25cee75834568a1..616535b65a5f7ad6a9e622b0165fb67e93b7e28f 100644 --- a/crates/sidebar/src/sidebar.rs +++ b/crates/sidebar/src/sidebar.rs @@ -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 = 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 = + 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(); diff --git a/crates/util/src/disambiguate.rs b/crates/util/src/disambiguate.rs new file mode 100644 index 0000000000000000000000000000000000000000..08580b467f67bcd2e22f59935ff867f298413005 --- /dev/null +++ b/crates/util/src/disambiguate.rs @@ -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( + items: &[T], + get_description: impl Fn(&T, usize) -> D, +) -> Vec +where + D: Eq + Hash + Clone, +{ + let mut details = vec![0usize; items.len()]; + let mut descriptions: HashMap> = HashMap::default(); + let mut current_descriptions: Vec = + 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 = vec![]; + assert_eq!(details, expected); + } +} diff --git a/crates/util/src/util.rs b/crates/util/src/util.rs index bd8ab4e2d4d99864c5e0dc228410904f3338d7c6..3b704e50a531c5302024e215754cb9a866f0036b 100644 --- a/crates/util/src/util.rs +++ b/crates/util/src/util.rs @@ -1,5 +1,6 @@ pub mod archive; pub mod command; +pub mod disambiguate; pub mod fs; pub mod markdown; pub mod path_list; diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index deb7e1efef37acff992d8f5be5825741e887b979..356e17e3ce51c8e13f412e90b7cf815a7b3bd260 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -4841,36 +4841,9 @@ fn dirty_message_for(buffer_path: Option, path_style: PathStyle) -> } pub fn tab_details(items: &[Box], _window: &Window, cx: &App) -> Vec { - let mut tab_details = items.iter().map(|_| 0).collect::>(); - 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, cx: &App) -> Option { diff --git a/crates/zed/src/visual_test_runner.rs b/crates/zed/src/visual_test_runner.rs index 27776eba84a10c5314c5e28fab430ad61e65bbf2..dcd2ec81ee44a091c9b444e79177145cf5cfceb7 100644 --- a/crates/zed/src/visual_test_runner.rs +++ b/crates/zed/src/visual_test_runner.rs @@ -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, + cx: &mut VisualTestAppContext, +) -> Result> { + 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>, + app_state: &Arc, + cx: &mut VisualTestAppContext, +) -> Result> { + 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 = 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 = 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, + 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, + cx: &mut VisualTestAppContext, + update_baseline: bool, +) -> Result { + 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,