From 54fd7ea699c99b6d2d5e5158a8a6cf5ca4da48a5 Mon Sep 17 00:00:00 2001 From: Danilo Leal <67129314+danilo-leal@users.noreply.github.com> Date: Tue, 28 Oct 2025 16:15:53 -0300 Subject: [PATCH] agent_ui: Don't show root project in path prefix in @-mention menu (#41372) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR hides the worktree root name from the path prefix displayed when you @-mention a file or directory in the agent panel. Given the tight UI real state we have, I believe that having the project name in there is redundant—the project you're in is already displayed in the title bar. Not only it was the very first word you'd see after the file's name, but it also made the path longer than it needs to. A bit of a design clean up here :) (PS: We still show the project name if there are more than one in the same workspace.) Release Notes: - N/A --- .../agent_ui/src/acp/completion_provider.rs | 21 +- crates/agent_ui/src/acp/message_editor.rs | 22 +- crates/agent_ui/src/context_picker.rs | 14 +- .../src/context_picker/completion_provider.rs | 250 ++++++++++++++++-- .../src/context_picker/file_context_picker.rs | 39 ++- 5 files changed, 294 insertions(+), 52 deletions(-) diff --git a/crates/agent_ui/src/acp/completion_provider.rs b/crates/agent_ui/src/acp/completion_provider.rs index 2dac9fc563..583d8070d9 100644 --- a/crates/agent_ui/src/acp/completion_provider.rs +++ b/crates/agent_ui/src/acp/completion_provider.rs @@ -575,6 +575,7 @@ impl ContextPickerCompletionProvider { .unwrap_or_default(); let workspace = workspace.read(cx); let project = workspace.project().read(cx); + let include_root_name = workspace.visible_worktrees(cx).count() > 1; if let Some(agent_panel) = workspace.panel::(cx) && let Some(thread) = agent_panel.read(cx).active_agent_thread(cx) @@ -601,7 +602,11 @@ impl ContextPickerCompletionProvider { project .worktree_for_id(project_path.worktree_id, cx) .map(|worktree| { - let path_prefix = worktree.read(cx).root_name().into(); + let path_prefix = if include_root_name { + worktree.read(cx).root_name().into() + } else { + RelPath::empty().into() + }; Match::File(FileMatch { mat: fuzzy::PathMatch { score: 1., @@ -828,9 +833,21 @@ impl CompletionProvider for ContextPickerCompletionProvider { path: mat.path.clone(), }; + // If path is empty, this means we're matching with the root directory itself + // so we use the path_prefix as the name + let path_prefix = if mat.path.is_empty() { + project + .read(cx) + .worktree_for_id(project_path.worktree_id, cx) + .map(|wt| wt.read(cx).root_name().into()) + .unwrap_or_else(|| mat.path_prefix.clone()) + } else { + mat.path_prefix.clone() + }; + Self::completion_for_path( project_path, - &mat.path_prefix, + &path_prefix, is_recent, mat.is_dir, source_range.clone(), diff --git a/crates/agent_ui/src/acp/message_editor.rs b/crates/agent_ui/src/acp/message_editor.rs index 08bdfa2b49..5fe591caca 100644 --- a/crates/agent_ui/src/acp/message_editor.rs +++ b/crates/agent_ui/src/acp/message_editor.rs @@ -2165,10 +2165,10 @@ mod tests { assert_eq!( current_completion_labels(editor), &[ - format!("eight.txt dir{slash}b{slash}"), - format!("seven.txt dir{slash}b{slash}"), - format!("six.txt dir{slash}b{slash}"), - format!("five.txt dir{slash}b{slash}"), + format!("eight.txt b{slash}"), + format!("seven.txt b{slash}"), + format!("six.txt b{slash}"), + format!("five.txt b{slash}"), ] ); editor.set_text("", window, cx); @@ -2196,10 +2196,10 @@ mod tests { assert_eq!( current_completion_labels(editor), &[ - format!("eight.txt dir{slash}b{slash}"), - format!("seven.txt dir{slash}b{slash}"), - format!("six.txt dir{slash}b{slash}"), - format!("five.txt dir{slash}b{slash}"), + format!("eight.txt b{slash}"), + format!("seven.txt b{slash}"), + format!("six.txt b{slash}"), + format!("five.txt b{slash}"), "Files & Directories".into(), "Symbols".into(), "Threads".into(), @@ -2232,7 +2232,7 @@ mod tests { assert!(editor.has_visible_completions_menu()); assert_eq!( current_completion_labels(editor), - vec![format!("one.txt dir{slash}a{slash}")] + vec![format!("one.txt a{slash}")] ); }); @@ -2511,7 +2511,7 @@ mod tests { format!("Lorem [@one.txt]({url_one}) Ipsum [@eight.txt]({url_eight}) [@MySymbol]({}) @file x.png", symbol.to_uri()) ); assert!(editor.has_visible_completions_menu()); - assert_eq!(current_completion_labels(editor), &[format!("x.png dir{slash}")]); + assert_eq!(current_completion_labels(editor), &["x.png "]); }); editor.update_in(&mut cx, |editor, window, cx| { @@ -2553,7 +2553,7 @@ mod tests { format!("Lorem [@one.txt]({url_one}) Ipsum [@eight.txt]({url_eight}) [@MySymbol]({}) @file x.png", symbol.to_uri()) ); assert!(editor.has_visible_completions_menu()); - assert_eq!(current_completion_labels(editor), &[format!("x.png dir{slash}")]); + assert_eq!(current_completion_labels(editor), &["x.png "]); }); editor.update_in(&mut cx, |editor, window, cx| { diff --git a/crates/agent_ui/src/context_picker.rs b/crates/agent_ui/src/context_picker.rs index caffb31521..0a6e811673 100644 --- a/crates/agent_ui/src/context_picker.rs +++ b/crates/agent_ui/src/context_picker.rs @@ -662,6 +662,7 @@ pub(crate) fn recent_context_picker_entries( let mut recent = Vec::with_capacity(6); let workspace = workspace.read(cx); let project = workspace.project().read(cx); + let include_root_name = workspace.visible_worktrees(cx).count() > 1; recent.extend( workspace @@ -675,9 +676,16 @@ pub(crate) fn recent_context_picker_entries( .filter_map(|(project_path, _)| { project .worktree_for_id(project_path.worktree_id, cx) - .map(|worktree| RecentEntry::File { - project_path, - path_prefix: worktree.read(cx).root_name().into(), + .map(|worktree| { + let path_prefix = if include_root_name { + worktree.read(cx).root_name().into() + } else { + RelPath::empty().into() + }; + RecentEntry::File { + project_path, + path_prefix, + } }) }), ); diff --git a/crates/agent_ui/src/context_picker/completion_provider.rs b/crates/agent_ui/src/context_picker/completion_provider.rs index 56444141f1..3a3ea45c80 100644 --- a/crates/agent_ui/src/context_picker/completion_provider.rs +++ b/crates/agent_ui/src/context_picker/completion_provider.rs @@ -655,13 +655,12 @@ impl ContextPickerCompletionProvider { let SymbolLocation::InProject(symbol_path) = &symbol.path else { return None; }; - let path_prefix = workspace + let _path_prefix = workspace .read(cx) .project() .read(cx) - .worktree_for_id(symbol_path.worktree_id, cx)? - .read(cx) - .root_name(); + .worktree_for_id(symbol_path.worktree_id, cx)?; + let path_prefix = RelPath::empty(); let (file_name, directory) = super::file_context_picker::extract_file_name_and_directory( &symbol_path.path, @@ -818,9 +817,21 @@ impl CompletionProvider for ContextPickerCompletionProvider { return None; } + // If path is empty, this means we're matching with the root directory itself + // so we use the path_prefix as the name + let path_prefix = if mat.path.is_empty() { + project + .read(cx) + .worktree_for_id(project_path.worktree_id, cx) + .map(|wt| wt.read(cx).root_name().into()) + .unwrap_or_else(|| mat.path_prefix.clone()) + } else { + mat.path_prefix.clone() + }; + Some(Self::completion_for_path( project_path, - &mat.path_prefix, + &path_prefix, is_recent, mat.is_dir, excerpt_id, @@ -1309,10 +1320,10 @@ mod tests { assert_eq!( current_completion_labels(editor), &[ - format!("seven.txt dir{slash}b{slash}"), - format!("six.txt dir{slash}b{slash}"), - format!("five.txt dir{slash}b{slash}"), - format!("four.txt dir{slash}a{slash}"), + format!("seven.txt b{slash}"), + format!("six.txt b{slash}"), + format!("five.txt b{slash}"), + format!("four.txt a{slash}"), "Files & Directories".into(), "Symbols".into(), "Fetch".into() @@ -1344,7 +1355,7 @@ mod tests { assert!(editor.has_visible_completions_menu()); assert_eq!( current_completion_labels(editor), - vec![format!("one.txt dir{slash}a{slash}")] + vec![format!("one.txt a{slash}")] ); }); @@ -1356,12 +1367,12 @@ mod tests { editor.update(&mut cx, |editor, cx| { assert_eq!( editor.text(cx), - format!("Lorem [@one.txt](@file:dir{slash}a{slash}one.txt) ") + format!("Lorem [@one.txt](@file:a{slash}one.txt) ") ); assert!(!editor.has_visible_completions_menu()); assert_eq!( fold_ranges(editor, cx), - vec![Point::new(0, 6)..Point::new(0, 37)] + vec![Point::new(0, 6)..Point::new(0, 33)] ); }); @@ -1370,12 +1381,12 @@ mod tests { editor.update(&mut cx, |editor, cx| { assert_eq!( editor.text(cx), - format!("Lorem [@one.txt](@file:dir{slash}a{slash}one.txt) ") + format!("Lorem [@one.txt](@file:a{slash}one.txt) ") ); assert!(!editor.has_visible_completions_menu()); assert_eq!( fold_ranges(editor, cx), - vec![Point::new(0, 6)..Point::new(0, 37)] + vec![Point::new(0, 6)..Point::new(0, 33)] ); }); @@ -1384,12 +1395,12 @@ mod tests { editor.update(&mut cx, |editor, cx| { assert_eq!( editor.text(cx), - format!("Lorem [@one.txt](@file:dir{slash}a{slash}one.txt) Ipsum "), + format!("Lorem [@one.txt](@file:a{slash}one.txt) Ipsum "), ); assert!(!editor.has_visible_completions_menu()); assert_eq!( fold_ranges(editor, cx), - vec![Point::new(0, 6)..Point::new(0, 37)] + vec![Point::new(0, 6)..Point::new(0, 33)] ); }); @@ -1398,12 +1409,12 @@ mod tests { editor.update(&mut cx, |editor, cx| { assert_eq!( editor.text(cx), - format!("Lorem [@one.txt](@file:dir{slash}a{slash}one.txt) Ipsum @file "), + format!("Lorem [@one.txt](@file:a{slash}one.txt) Ipsum @file "), ); assert!(editor.has_visible_completions_menu()); assert_eq!( fold_ranges(editor, cx), - vec![Point::new(0, 6)..Point::new(0, 37)] + vec![Point::new(0, 6)..Point::new(0, 33)] ); }); @@ -1416,14 +1427,14 @@ mod tests { editor.update(&mut cx, |editor, cx| { assert_eq!( editor.text(cx), - format!("Lorem [@one.txt](@file:dir{slash}a{slash}one.txt) Ipsum [@seven.txt](@file:dir{slash}b{slash}seven.txt) ") + format!("Lorem [@one.txt](@file:a{slash}one.txt) Ipsum [@seven.txt](@file:b{slash}seven.txt) ") ); assert!(!editor.has_visible_completions_menu()); assert_eq!( fold_ranges(editor, cx), vec![ - Point::new(0, 6)..Point::new(0, 37), - Point::new(0, 45)..Point::new(0, 80) + Point::new(0, 6)..Point::new(0, 33), + Point::new(0, 41)..Point::new(0, 72) ] ); }); @@ -1433,14 +1444,14 @@ mod tests { editor.update(&mut cx, |editor, cx| { assert_eq!( editor.text(cx), - format!("Lorem [@one.txt](@file:dir{slash}a{slash}one.txt) Ipsum [@seven.txt](@file:dir{slash}b{slash}seven.txt) \n@") + format!("Lorem [@one.txt](@file:a{slash}one.txt) Ipsum [@seven.txt](@file:b{slash}seven.txt) \n@") ); assert!(editor.has_visible_completions_menu()); assert_eq!( fold_ranges(editor, cx), vec![ - Point::new(0, 6)..Point::new(0, 37), - Point::new(0, 45)..Point::new(0, 80) + Point::new(0, 6)..Point::new(0, 33), + Point::new(0, 41)..Point::new(0, 72) ] ); }); @@ -1454,20 +1465,203 @@ mod tests { editor.update(&mut cx, |editor, cx| { assert_eq!( editor.text(cx), - format!("Lorem [@one.txt](@file:dir{slash}a{slash}one.txt) Ipsum [@seven.txt](@file:dir{slash}b{slash}seven.txt) \n[@six.txt](@file:dir{slash}b{slash}six.txt) ") + format!("Lorem [@one.txt](@file:a{slash}one.txt) Ipsum [@seven.txt](@file:b{slash}seven.txt) \n[@six.txt](@file:b{slash}six.txt) ") ); assert!(!editor.has_visible_completions_menu()); assert_eq!( fold_ranges(editor, cx), vec![ - Point::new(0, 6)..Point::new(0, 37), - Point::new(0, 45)..Point::new(0, 80), - Point::new(1, 0)..Point::new(1, 31) + Point::new(0, 6)..Point::new(0, 33), + Point::new(0, 41)..Point::new(0, 72), + Point::new(1, 0)..Point::new(1, 27) ] ); }); } + #[gpui::test] + async fn test_context_completion_provider_multiple_worktrees(cx: &mut TestAppContext) { + init_test(cx); + + let app_state = cx.update(AppState::test); + + cx.update(|cx| { + language::init(cx); + editor::init(cx); + workspace::init(app_state.clone(), cx); + Project::init_settings(cx); + }); + + app_state + .fs + .as_fake() + .insert_tree( + path!("/project1"), + json!({ + "a": { + "one.txt": "", + "two.txt": "", + } + }), + ) + .await; + + app_state + .fs + .as_fake() + .insert_tree( + path!("/project2"), + json!({ + "b": { + "three.txt": "", + "four.txt": "", + } + }), + ) + .await; + + let project = Project::test( + app_state.fs.clone(), + [path!("/project1").as_ref(), path!("/project2").as_ref()], + cx, + ) + .await; + let window = cx.add_window(|window, cx| Workspace::test_new(project.clone(), window, cx)); + let workspace = window.root(cx).unwrap(); + + let worktrees = project.update(cx, |project, cx| { + let worktrees = project.worktrees(cx).collect::>(); + assert_eq!(worktrees.len(), 2); + worktrees + }); + + let mut cx = VisualTestContext::from_window(*window.deref(), cx); + let slash = PathStyle::local().separator(); + + for (worktree_idx, paths) in [ + vec![rel_path("a/one.txt"), rel_path("a/two.txt")], + vec![rel_path("b/three.txt"), rel_path("b/four.txt")], + ] + .iter() + .enumerate() + { + let worktree_id = worktrees[worktree_idx].read_with(&cx, |wt, _| wt.id()); + for path in paths { + workspace + .update_in(&mut cx, |workspace, window, cx| { + workspace.open_path( + ProjectPath { + worktree_id, + path: (*path).into(), + }, + None, + false, + window, + cx, + ) + }) + .await + .unwrap(); + } + } + + let editor = workspace.update_in(&mut cx, |workspace, window, cx| { + let editor = cx.new(|cx| { + Editor::new( + editor::EditorMode::full(), + multi_buffer::MultiBuffer::build_simple("", cx), + None, + window, + cx, + ) + }); + workspace.active_pane().update(cx, |pane, cx| { + pane.add_item( + Box::new(cx.new(|_| AtMentionEditor(editor.clone()))), + true, + true, + None, + window, + cx, + ); + }); + editor + }); + + let context_store = cx.new(|_| ContextStore::new(project.downgrade())); + + let editor_entity = editor.downgrade(); + editor.update_in(&mut cx, |editor, window, cx| { + window.focus(&editor.focus_handle(cx)); + editor.set_completion_provider(Some(Rc::new(ContextPickerCompletionProvider::new( + workspace.downgrade(), + context_store.downgrade(), + None, + None, + editor_entity, + None, + )))); + }); + + cx.simulate_input("@"); + + // With multiple worktrees, we should see the project name as prefix + editor.update(&mut cx, |editor, cx| { + assert_eq!(editor.text(cx), "@"); + assert!(editor.has_visible_completions_menu()); + let labels = current_completion_labels(editor); + + assert!( + labels.contains(&format!("four.txt project2{slash}b{slash}")), + "Expected 'four.txt project2{slash}b{slash}' in labels: {:?}", + labels + ); + assert!( + labels.contains(&format!("three.txt project2{slash}b{slash}")), + "Expected 'three.txt project2{slash}b{slash}' in labels: {:?}", + labels + ); + }); + + editor.update_in(&mut cx, |editor, window, cx| { + editor.context_menu_next(&editor::actions::ContextMenuNext, window, cx); + editor.context_menu_next(&editor::actions::ContextMenuNext, window, cx); + editor.context_menu_next(&editor::actions::ContextMenuNext, window, cx); + editor.context_menu_next(&editor::actions::ContextMenuNext, window, cx); + editor.confirm_completion(&editor::actions::ConfirmCompletion::default(), window, cx); + }); + + cx.run_until_parked(); + + editor.update(&mut cx, |editor, cx| { + assert_eq!(editor.text(cx), "@file "); + assert!(editor.has_visible_completions_menu()); + }); + + cx.simulate_input("one"); + + editor.update(&mut cx, |editor, cx| { + assert_eq!(editor.text(cx), "@file one"); + assert!(editor.has_visible_completions_menu()); + assert_eq!( + current_completion_labels(editor), + vec![format!("one.txt project1{slash}a{slash}")] + ); + }); + + editor.update_in(&mut cx, |editor, window, cx| { + editor.confirm_completion(&editor::actions::ConfirmCompletion::default(), window, cx); + }); + + editor.update(&mut cx, |editor, cx| { + assert_eq!( + editor.text(cx), + format!("[@one.txt](@file:project1{slash}a{slash}one.txt) ") + ); + assert!(!editor.has_visible_completions_menu()); + }); + } + fn fold_ranges(editor: &Editor, cx: &mut App) -> Vec> { let snapshot = editor.buffer().read(cx).snapshot(cx); editor.display_map.update(cx, |display_map, cx| { diff --git a/crates/agent_ui/src/context_picker/file_context_picker.rs b/crates/agent_ui/src/context_picker/file_context_picker.rs index 8d1e5cb46d..ded24caa92 100644 --- a/crates/agent_ui/src/context_picker/file_context_picker.rs +++ b/crates/agent_ui/src/context_picker/file_context_picker.rs @@ -197,34 +197,50 @@ pub(crate) fn search_files( if query.is_empty() { let workspace = workspace.read(cx); let project = workspace.project().read(cx); + let visible_worktrees = workspace.visible_worktrees(cx).collect::>(); + let include_root_name = visible_worktrees.len() > 1; + let recent_matches = workspace .recent_navigation_history(Some(10), cx) .into_iter() - .filter_map(|(project_path, _)| { - let worktree = project.worktree_for_id(project_path.worktree_id, cx)?; - Some(FileMatch { + .map(|(project_path, _)| { + let path_prefix = if include_root_name { + project + .worktree_for_id(project_path.worktree_id, cx) + .map(|wt| wt.read(cx).root_name().into()) + .unwrap_or_else(|| RelPath::empty().into()) + } else { + RelPath::empty().into() + }; + + FileMatch { mat: PathMatch { score: 0., positions: Vec::new(), worktree_id: project_path.worktree_id.to_usize(), path: project_path.path, - path_prefix: worktree.read(cx).root_name().into(), + path_prefix, distance_to_relative_ancestor: 0, is_dir: false, }, is_recent: true, - }) + } }); - let file_matches = project.worktrees(cx).flat_map(|worktree| { + let file_matches = visible_worktrees.into_iter().flat_map(|worktree| { let worktree = worktree.read(cx); + let path_prefix: Arc = if include_root_name { + worktree.root_name().into() + } else { + RelPath::empty().into() + }; worktree.entries(false, 0).map(move |entry| FileMatch { mat: PathMatch { score: 0., positions: Vec::new(), worktree_id: worktree.id().to_usize(), path: entry.path.clone(), - path_prefix: worktree.root_name().into(), + path_prefix: path_prefix.clone(), distance_to_relative_ancestor: 0, is_dir: entry.is_dir(), }, @@ -235,6 +251,7 @@ pub(crate) fn search_files( Task::ready(recent_matches.chain(file_matches).collect()) } else { let worktrees = workspace.read(cx).visible_worktrees(cx).collect::>(); + let include_root_name = worktrees.len() > 1; let candidate_sets = worktrees .into_iter() .map(|worktree| { @@ -243,7 +260,7 @@ pub(crate) fn search_files( PathMatchCandidateSet { snapshot: worktree.snapshot(), include_ignored: worktree.root_entry().is_some_and(|entry| entry.is_ignored), - include_root_name: true, + include_root_name, candidates: project::Candidates::Entries, } }) @@ -276,6 +293,12 @@ pub fn extract_file_name_and_directory( path_prefix: &RelPath, path_style: PathStyle, ) -> (SharedString, Option) { + // If path is empty, this means we're matching with the root directory itself + // so we use the path_prefix as the name + if path.is_empty() && !path_prefix.is_empty() { + return (path_prefix.display(path_style).to_string().into(), None); + } + let full_path = path_prefix.join(path); let file_name = full_path.file_name().unwrap_or_default(); let display_path = full_path.display(path_style);