diff --git a/crates/sidebar/src/sidebar.rs b/crates/sidebar/src/sidebar.rs index ee329fc3b415b3268d772e3ba7171f76a2fe0c37..281a140931318be206de615e06ab78a5e499669d 100644 --- a/crates/sidebar/src/sidebar.rs +++ b/crates/sidebar/src/sidebar.rs @@ -158,6 +158,7 @@ enum ListEntry { path_list: PathList, workspace: Entity, is_active_draft: bool, + worktrees: Vec, }, } @@ -739,6 +740,7 @@ impl Sidebar { .collect(); let mut threads: Vec = Vec::new(); + let mut threadless_workspaces: Vec<(Entity, Vec)> = Vec::new(); let mut has_running_threads = false; let mut waiting_thread_count: usize = 0; @@ -749,8 +751,16 @@ impl Sidebar { // Load threads from each workspace in the group. for workspace in &group.workspaces { let ws_path_list = workspace_path_list(workspace, cx); - - for row in thread_store.read(cx).entries_for_path(&ws_path_list) { + let mut workspace_rows = thread_store + .read(cx) + .entries_for_path(&ws_path_list) + .peekable(); + if workspace_rows.peek().is_none() { + let worktrees = + worktree_info_from_thread_paths(&ws_path_list, &project_groups); + threadless_workspaces.push((workspace.clone(), worktrees)); + } + for row in workspace_rows { if !seen_session_ids.insert(row.session_id.clone()) { continue; } @@ -773,7 +783,7 @@ impl Sidebar { } } - // Load threads from linked git worktrees + // Load threads from linked git worktrees whose // canonical paths belong to this group. let linked_worktree_queries = group .workspaces @@ -929,14 +939,11 @@ impl Sidebar { entries.push(thread.into()); } } else { - let thread_count = threads.len(); let is_draft_for_workspace = self.agent_panel_visible && self.active_thread_is_draft && self.focused_thread.is_none() && is_active; - let show_new_thread_entry = thread_count == 0 || is_draft_for_workspace; - project_header_indices.push(entries.len()); entries.push(ListEntry::ProjectHeader { path_list: path_list.clone(), @@ -952,11 +959,29 @@ impl Sidebar { continue; } - if show_new_thread_entry { + // Emit "New Thread" entries for threadless workspaces + // and active drafts, right after the header. + for (workspace, worktrees) in &threadless_workspaces { + let is_draft = is_draft_for_workspace && workspace == representative_workspace; + entries.push(ListEntry::NewThread { + path_list: path_list.clone(), + workspace: workspace.clone(), + is_active_draft: is_draft, + worktrees: worktrees.clone(), + }); + } + if is_draft_for_workspace + && !threadless_workspaces + .iter() + .any(|(ws, _)| ws == representative_workspace) + { + let ws_path_list = workspace_path_list(representative_workspace, cx); + let worktrees = worktree_info_from_thread_paths(&ws_path_list, &project_groups); entries.push(ListEntry::NewThread { path_list: path_list.clone(), workspace: representative_workspace.clone(), - is_active_draft: is_draft_for_workspace, + is_active_draft: true, + worktrees, }); } @@ -1114,9 +1139,16 @@ impl Sidebar { path_list, workspace, is_active_draft, - } => { - self.render_new_thread(ix, path_list, workspace, *is_active_draft, is_selected, cx) - } + worktrees, + } => self.render_new_thread( + ix, + path_list, + workspace, + *is_active_draft, + worktrees, + is_selected, + cx, + ), }; if is_group_header_after_first { @@ -2906,6 +2938,7 @@ impl Sidebar { _path_list: &PathList, workspace: &Entity, is_active_draft: bool, + worktrees: &[WorktreeInfo], is_selected: bool, cx: &mut Context, ) -> AnyElement { @@ -2924,6 +2957,16 @@ impl Sidebar { let thread_item = ThreadItem::new(id, label) .icon(IconName::Plus) .icon_color(Color::Custom(cx.theme().colors().icon_muted.opacity(0.8))) + .worktrees( + worktrees + .iter() + .map(|wt| ThreadItemWorktreeInfo { + name: wt.name.clone(), + full_path: wt.full_path.clone(), + highlight_positions: wt.highlight_positions.clone(), + }) + .collect(), + ) .selected(is_active) .focused(is_selected) .when(!is_active, |this| { diff --git a/crates/sidebar/src/sidebar_tests.rs b/crates/sidebar/src/sidebar_tests.rs index ecc3ce37600d9b903facd019408c08765dd5d094..8efea2970aab9529443107b71c395d16d484c8cc 100644 --- a/crates/sidebar/src/sidebar_tests.rs +++ b/crates/sidebar/src/sidebar_tests.rs @@ -222,8 +222,21 @@ fn visible_entries_as_strings( format!(" + View More{}", selected) } } - ListEntry::NewThread { .. } => { - format!(" [+ New Thread]{}", selected) + ListEntry::NewThread { worktrees, .. } => { + let worktree = if worktrees.is_empty() { + String::new() + } else { + let mut seen = Vec::new(); + let mut chips = Vec::new(); + for wt in worktrees { + if !seen.contains(&wt.name) { + seen.push(wt.name.clone()); + chips.push(format!("{{{}}}", wt.name)); + } + } + format!(" {}", chips.join(", ")) + }; + format!(" [+ New Thread{}]{}", worktree, selected) } } }) @@ -2342,7 +2355,11 @@ async fn test_cmd_n_shows_new_thread_entry_in_absorbed_worktree(cx: &mut TestApp assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [project]", " Hello {wt-feature-a} *"] + vec![ + "v [project]", + " [+ New Thread]", + " Hello {wt-feature-a} *" + ] ); // Simulate Cmd-N in the worktree workspace. @@ -2359,6 +2376,7 @@ async fn test_cmd_n_shows_new_thread_entry_in_absorbed_worktree(cx: &mut TestApp vec![ "v [project]", " [+ New Thread]", + " [+ New Thread {wt-feature-a}]", " Hello {wt-feature-a} *" ], "After Cmd-N in an absorbed worktree, the sidebar should show \ @@ -2475,7 +2493,11 @@ async fn test_git_worktree_added_live_updates_sidebar(cx: &mut TestAppContext) { assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [project]", " Worktree Thread {rosewood}",] + vec![ + "v [project]", + " [+ New Thread]", + " Worktree Thread {rosewood}", + ] ); } @@ -2589,12 +2611,108 @@ async fn test_two_worktree_workspaces_absorbed_when_main_added(cx: &mut TestAppC visible_entries_as_strings(&sidebar, cx), vec![ "v [project]", + " [+ New Thread]", " Thread A {wt-feature-a}", " Thread B {wt-feature-b}", ] ); } +#[gpui::test] +async fn test_threadless_workspace_shows_new_thread_with_worktree_chip(cx: &mut TestAppContext) { + // When a group has two workspaces — one with threads and one + // without — the threadless workspace should appear as a + // "New Thread" button with its worktree chip. + init_test(cx); + let fs = FakeFs::new(cx.executor()); + + // Main repo with two linked worktrees. + fs.insert_tree( + "/project", + serde_json::json!({ + ".git": { + "worktrees": { + "feature-a": { + "commondir": "../../", + "HEAD": "ref: refs/heads/feature-a", + }, + "feature-b": { + "commondir": "../../", + "HEAD": "ref: refs/heads/feature-b", + }, + }, + }, + "src": {}, + }), + ) + .await; + fs.insert_tree( + "/wt-feature-a", + serde_json::json!({ + ".git": "gitdir: /project/.git/worktrees/feature-a", + "src": {}, + }), + ) + .await; + fs.insert_tree( + "/wt-feature-b", + serde_json::json!({ + ".git": "gitdir: /project/.git/worktrees/feature-b", + "src": {}, + }), + ) + .await; + + fs.with_git_state(std::path::Path::new("/project/.git"), false, |state| { + state.worktrees.push(git::repository::Worktree { + path: std::path::PathBuf::from("/wt-feature-a"), + ref_name: Some("refs/heads/feature-a".into()), + sha: "aaa".into(), + }); + state.worktrees.push(git::repository::Worktree { + path: std::path::PathBuf::from("/wt-feature-b"), + ref_name: Some("refs/heads/feature-b".into()), + sha: "bbb".into(), + }); + }) + .unwrap(); + + cx.update(|cx| ::set_global(fs.clone(), cx)); + + // Workspace A: worktree feature-a (has threads). + let project_a = project::Project::test(fs.clone(), ["/wt-feature-a".as_ref()], cx).await; + project_a.update(cx, |p, cx| p.git_scans_complete(cx)).await; + + // Workspace B: worktree feature-b (no threads). + let project_b = project::Project::test(fs.clone(), ["/wt-feature-b".as_ref()], cx).await; + project_b.update(cx, |p, cx| p.git_scans_complete(cx)).await; + + let (multi_workspace, cx) = + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project_a.clone(), window, cx)); + multi_workspace.update_in(cx, |mw, window, cx| { + mw.test_add_workspace(project_b.clone(), window, cx); + }); + let sidebar = setup_sidebar(&multi_workspace, cx); + + // Only save a thread for workspace A. + let paths_a = PathList::new(&[std::path::PathBuf::from("/wt-feature-a")]); + save_named_thread_metadata("thread-a", "Thread A", &paths_a, cx).await; + + multi_workspace.update_in(cx, |_, _window, cx| cx.notify()); + cx.run_until_parked(); + + // Workspace A's thread appears normally. Workspace B (threadless) + // appears as a "New Thread" button with its worktree chip. + assert_eq!( + visible_entries_as_strings(&sidebar, cx), + vec![ + "v [project]", + " [+ New Thread {wt-feature-b}]", + " Thread A {wt-feature-a}", + ] + ); +} + #[gpui::test] async fn test_multi_worktree_thread_shows_multiple_chips(cx: &mut TestAppContext) { // A thread created in a workspace with roots from different git @@ -2926,7 +3044,11 @@ async fn test_absorbed_worktree_running_thread_shows_live_status(cx: &mut TestAp let entries = visible_entries_as_strings(&sidebar, cx); assert_eq!( entries, - vec!["v [project]", " Hello {wt-feature-a} * (running)",] + vec![ + "v [project]", + " [+ New Thread]", + " Hello {wt-feature-a} * (running)", + ] ); } @@ -3024,7 +3146,11 @@ async fn test_absorbed_worktree_completion_triggers_notification(cx: &mut TestAp assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [project]", " Hello {wt-feature-a} * (running)",] + vec![ + "v [project]", + " [+ New Thread]", + " Hello {wt-feature-a} * (running)", + ] ); connection.end_turn(session_id, acp::StopReason::EndTurn); @@ -3032,7 +3158,11 @@ async fn test_absorbed_worktree_completion_triggers_notification(cx: &mut TestAp assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [project]", " Hello {wt-feature-a} * (!)",] + vec![ + "v [project]", + " [+ New Thread]", + " Hello {wt-feature-a} * (!)", + ] ); } @@ -3097,7 +3227,11 @@ async fn test_clicking_worktree_thread_opens_workspace_when_none_exists(cx: &mut // Thread should appear under the main repo with a worktree chip. assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [project]", " WT Thread {wt-feature-a}"], + vec![ + "v [project]", + " [+ New Thread]", + " WT Thread {wt-feature-a}" + ], ); // Only 1 workspace should exist. @@ -3109,7 +3243,7 @@ async fn test_clicking_worktree_thread_opens_workspace_when_none_exists(cx: &mut // Focus the sidebar and select the worktree thread. open_and_focus_sidebar(&sidebar, cx); sidebar.update_in(cx, |sidebar, _window, _cx| { - sidebar.selection = Some(1); // index 0 is header, 1 is the thread + sidebar.selection = Some(2); // index 0 is header, 1 is new thread, 2 is the thread }); // Confirm to open the worktree thread. @@ -3195,12 +3329,16 @@ async fn test_clicking_worktree_thread_does_not_briefly_render_as_separate_proje assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [project]", " WT Thread {wt-feature-a}"], + vec![ + "v [project]", + " [+ New Thread]", + " WT Thread {wt-feature-a}" + ], ); open_and_focus_sidebar(&sidebar, cx); sidebar.update_in(cx, |sidebar, _window, _cx| { - sidebar.selection = Some(1); + sidebar.selection = Some(2); }); let assert_sidebar_state = |sidebar: &mut Sidebar, _cx: &mut Context| { @@ -3256,9 +3394,7 @@ async fn test_clicking_worktree_thread_does_not_briefly_render_as_separate_proje ListEntry::ViewMore { .. } => { panic!("unexpected `View More` entry while opening linked worktree thread"); } - ListEntry::NewThread { .. } => { - panic!("unexpected `New Thread` entry while opening linked worktree thread"); - } + ListEntry::NewThread { .. } => {} } } @@ -4124,6 +4260,7 @@ async fn test_linked_worktree_threads_not_duplicated_across_groups(cx: &mut Test visible_entries_as_strings(&sidebar, cx), vec![ "v [project]", + " [+ New Thread]", " Worktree Thread {wt-feature-a}", "v [other, project]", " [+ New Thread]", @@ -4909,41 +5046,10 @@ mod property_test { let workspaces = multi_workspace.read(cx).workspaces().to_vec(); - // For each workspace, collect the set of canonical repo paths - // (original_repo_abs_path) from its root repositories. Two - // workspaces that share a canonical repo path are in the same - // linked-worktree group. - let canonical_repos = |ws: &Entity| -> HashSet { - root_repository_snapshots(ws, cx) - .map(|snapshot| snapshot.original_repo_abs_path.to_path_buf()) - .collect::>() - }; - - // Build a map from canonical repo path → set of workspace - // EntityIds that share that repo. - let mut repo_to_workspaces: HashMap> = HashMap::new(); - for ws in &workspaces { - for repo_path in canonical_repos(ws) { - repo_to_workspaces - .entry(repo_path) - .or_default() - .insert(ws.entity_id()); - } - } - - // A workspace participates in a linked-worktree group when it - // shares a canonical repo path with at least one other workspace. - let in_linked_worktree_group = |ws: &Entity| -> bool { - canonical_repos(ws).iter().any(|repo_path| { - repo_to_workspaces - .get(repo_path) - .is_some_and(|members| members.len() > 1) - }) - }; - - // TODO - // Carve-out 1: workspaces with no root paths are not shown - // because the sidebar skips empty path lists. + // Workspaces with no root paths are not shown because the + // sidebar skips empty path lists. All other workspaces should + // appear — either via a Thread entry or a NewThread entry for + // threadless workspaces. let expected_workspaces: HashSet = workspaces .iter() .filter(|ws| !workspace_path_list(ws, cx).paths().is_empty()) @@ -4957,33 +5063,17 @@ mod property_test { .filter_map(|entry| entry.workspace().map(|ws| ws.entity_id())) .collect(); - // Check every mismatch between the two sets. Each one must be - // explainable by a known carve-out. let missing = &expected_workspaces - &sidebar_workspaces; let stray = &sidebar_workspaces - &expected_workspaces; - for entity_id in missing.iter().chain(stray.iter()) { - let Some(workspace) = workspaces.iter().find(|ws| ws.entity_id() == *entity_id) else { - anyhow::bail!("workspace {entity_id:?} not found in multi-workspace"); - }; - - // TODO - // Carve-out 2: when multiple workspaces share a linked- - // worktree group, only one representative is shown. Either - // side of the relationship (parent or linked worktree) may - // be the representative, so both can appear in the diff. - anyhow::ensure!( - in_linked_worktree_group(workspace), - "workspace {:?} (paths {:?}) is in the mismatch but does not \ - participate in a linked-worktree group.\n\ - Only in sidebar (stray): {:?}\n\ - Only in multi-workspace (missing): {:?}", - entity_id, - workspace_path_list(workspace, cx).paths(), - stray, - missing, - ); - } + anyhow::ensure!( + missing.is_empty() && stray.is_empty(), + "sidebar workspaces don't match multi-workspace.\n\ + Only in multi-workspace (missing): {:?}\n\ + Only in sidebar (stray): {:?}", + missing, + stray, + ); Ok(()) } diff --git a/crates/ui/src/components/ai/thread_item.rs b/crates/ui/src/components/ai/thread_item.rs index 18f54936853151001308f6476741772d2f8bda16..aebfbffce6926741c7ec1faa393750b3a1b35ebd 100644 --- a/crates/ui/src/components/ai/thread_item.rs +++ b/crates/ui/src/components/ai/thread_item.rs @@ -73,6 +73,7 @@ impl ThreadItem { hovered: false, added: None, removed: None, + project_paths: None, worktrees: Vec::new(), on_click: None, @@ -391,102 +392,110 @@ impl RenderOnce for ThreadItem { }) }), ) - .when(has_worktree || has_diff_stats || has_timestamp, |this| { - // Collect all full paths for the shared tooltip. - let worktree_tooltip: SharedString = self - .worktrees - .iter() - .map(|wt| wt.full_path.as_ref()) - .collect::>() - .join("\n") - .into(); - let worktree_tooltip_title = if self.worktrees.len() > 1 { - "Thread Running in Local Git Worktrees" - } else { - "Thread Running in a Local Git Worktree" - }; - - // Deduplicate chips by name — e.g. two paths both named - // "olivetti" produce a single chip. Highlight positions - // come from the first occurrence. - let mut seen_names: Vec = Vec::new(); - let mut worktree_chips: Vec = Vec::new(); - for wt in self.worktrees { - if seen_names.contains(&wt.name) { - continue; - } - let chip_index = seen_names.len(); - seen_names.push(wt.name.clone()); - let label = if wt.highlight_positions.is_empty() { - Label::new(wt.name) - .size(LabelSize::Small) - .color(Color::Muted) - .into_any_element() + .when( + has_project_paths || has_worktree || has_diff_stats || has_timestamp, + |this| { + // Collect all full paths for the shared tooltip. + let worktree_tooltip: SharedString = self + .worktrees + .iter() + .map(|wt| wt.full_path.as_ref()) + .collect::>() + .join("\n") + .into(); + let worktree_tooltip_title = if self.worktrees.len() > 1 { + "Thread Running in Local Git Worktrees" } else { - HighlightedLabel::new(wt.name, wt.highlight_positions) - .size(LabelSize::Small) - .color(Color::Muted) - .into_any_element() + "Thread Running in a Local Git Worktree" }; - let tooltip_title = worktree_tooltip_title; - let tooltip_meta = worktree_tooltip.clone(); - worktree_chips.push( + + // Deduplicate chips by name — e.g. two paths both named + // "olivetti" produce a single chip. Highlight positions + // come from the first occurrence. + let mut seen_names: Vec = Vec::new(); + let mut worktree_chips: Vec = Vec::new(); + for wt in self.worktrees { + if seen_names.contains(&wt.name) { + continue; + } + let chip_index = seen_names.len(); + seen_names.push(wt.name.clone()); + let label = if wt.highlight_positions.is_empty() { + Label::new(wt.name) + .size(LabelSize::Small) + .color(Color::Muted) + .into_any_element() + } else { + HighlightedLabel::new(wt.name, wt.highlight_positions) + .size(LabelSize::Small) + .color(Color::Muted) + .into_any_element() + }; + let tooltip_title = worktree_tooltip_title; + let tooltip_meta = worktree_tooltip.clone(); + worktree_chips.push( + h_flex() + .id(format!("{}-worktree-{chip_index}", self.id.clone())) + .gap_0p5() + .child( + Icon::new(IconName::GitWorktree) + .size(IconSize::XSmall) + .color(Color::Muted), + ) + .child(label) + .tooltip(move |_, cx| { + Tooltip::with_meta( + tooltip_title, + None, + tooltip_meta.clone(), + cx, + ) + }) + .into_any_element(), + ); + } + + this.child( h_flex() - .id(format!("{}-worktree-{chip_index}", self.id.clone())) - .gap_0p5() - .child( - Icon::new(IconName::GitWorktree) - .size(IconSize::XSmall) - .color(Color::Muted), - ) - .child(label) - .tooltip(move |_, cx| { - Tooltip::with_meta(tooltip_title, None, tooltip_meta.clone(), cx) + .min_w_0() + .gap_1p5() + .child(icon_container()) // Icon Spacing + .when_some(project_paths, |this, paths| { + this.child( + Label::new(paths) + .size(LabelSize::Small) + .color(Color::Muted) + .into_any_element(), + ) }) - .into_any_element(), - ); - } - - this.child( - h_flex() - .min_w_0() - .gap_1p5() - .child(icon_container()) // Icon Spacing - .when_some(project_paths, |this, paths| { - this.child( - Label::new(paths) - .size(LabelSize::Small) - .color(Color::Muted) - .into_any_element(), - ) - }) - .when(has_project_paths && has_worktree, |this| { - this.child(dot_separator()) - }) - .children(worktree_chips) - .when( - (has_project_paths || has_worktree) - && (has_diff_stats || has_timestamp), - |this| this.child(dot_separator()), - ) - .when(has_diff_stats, |this| { - this.child( - DiffStat::new(diff_stat_id, added_count, removed_count) - .tooltip("Unreviewed changes"), - ) - }) - .when(has_diff_stats && has_timestamp, |this| { - this.child(dot_separator()) - }) - .when(has_timestamp, |this| { - this.child( - Label::new(timestamp.clone()) - .size(LabelSize::Small) - .color(Color::Muted), + .when(has_project_paths && has_worktree, |this| { + this.child(dot_separator()) + }) + .children(worktree_chips) + .when( + (has_project_paths || has_worktree) + && (has_diff_stats || has_timestamp), + |this| this.child(dot_separator()), ) - }), - ) - }) + .when(has_diff_stats, |this| { + this.child( + DiffStat::new(diff_stat_id, added_count, removed_count) + .tooltip("Unreviewed changes"), + ) + }) + .when(has_diff_stats && has_timestamp, |this| { + this.child(dot_separator()) + }) + .when(has_timestamp, |this| { + this.child( + Label::new(timestamp.clone()) + .size(LabelSize::Small) + .color(Color::Muted), + ) + }), + ) + }, + ) .when_some(self.on_click, |this, on_click| this.on_click(on_click)) } }