assistant2: Unify path rendering for file context (#27537)

Bennet Bo Fenner created 10 months ago

This ensures that we render path matches the same in both the file
context picker and the messag editor completion provider.

Release Notes:

- N/A

Change summary

crates/assistant2/src/context_picker/completion_provider.rs | 198 ++----
crates/assistant2/src/context_picker/file_context_picker.rs |  37 
2 files changed, 93 insertions(+), 142 deletions(-)

Detailed changes

crates/assistant2/src/context_picker/completion_provider.rs

@@ -1,6 +1,6 @@
 use std::cell::RefCell;
 use std::ops::Range;
-use std::path::{Path, PathBuf};
+use std::path::Path;
 use std::rc::Rc;
 use std::sync::atomic::AtomicBool;
 use std::sync::Arc;
@@ -70,18 +70,18 @@ impl ContextPickerCompletionProvider {
             .filter_map(|entry| match entry {
                 super::RecentEntry::File {
                     project_path,
-                    path_prefix: _,
-                } => Self::completion_for_path(
+                    path_prefix,
+                } => Some(Self::completion_for_path(
                     project_path.clone(),
+                    path_prefix,
                     true,
                     false,
                     excerpt_id,
                     source_range.clone(),
                     editor.clone(),
                     context_store.clone(),
-                    workspace.clone(),
                     cx,
-                ),
+                )),
                 super::RecentEntry::Thread(thread_context_entry) => {
                     let thread_store = thread_store
                         .as_ref()
@@ -120,56 +120,24 @@ impl ContextPickerCompletionProvider {
         completions
     }
 
-    fn full_path_for_entry(
-        worktree_id: WorktreeId,
-        path: &Path,
-        workspace: Entity<Workspace>,
-        cx: &App,
-    ) -> Option<PathBuf> {
-        let worktree = workspace
-            .read(cx)
-            .project()
-            .read(cx)
-            .worktree_for_id(worktree_id, cx)?
-            .read(cx);
-
-        let mut full_path = PathBuf::from(worktree.root_name());
-        full_path.push(path);
-        Some(full_path)
-    }
-
     fn build_code_label_for_full_path(
-        worktree_id: WorktreeId,
-        path: &Path,
-        workspace: Entity<Workspace>,
+        file_name: &str,
+        directory: Option<&str>,
         cx: &App,
-    ) -> Option<CodeLabel> {
+    ) -> CodeLabel {
         let comment_id = cx.theme().syntax().highlight_id("comment").map(HighlightId);
         let mut label = CodeLabel::default();
-        let worktree = workspace
-            .read(cx)
-            .project()
-            .read(cx)
-            .worktree_for_id(worktree_id, cx)?;
-
-        let entry = worktree.read(cx).entry_for_path(&path)?;
-        let file_name = path.file_name()?.to_string_lossy();
+
         label.push_str(&file_name, None);
-        if entry.is_dir() {
-            label.push_str("/ ", None);
-        } else {
-            label.push_str(" ", None);
-        };
+        label.push_str(" ", None);
 
-        let mut path_hint = PathBuf::from(worktree.read(cx).root_name());
-        if let Some(path_to_entry) = path.parent() {
-            path_hint.push(path_to_entry);
+        if let Some(directory) = directory {
+            label.push_str(&directory, comment_id);
         }
-        label.push_str(&path_hint.to_string_lossy(), comment_id);
 
         label.filter_range = 0..label.text().len();
 
-        Some(label)
+        label
     }
 
     fn completion_for_thread(
@@ -274,32 +242,36 @@ impl ContextPickerCompletionProvider {
 
     fn completion_for_path(
         project_path: ProjectPath,
+        path_prefix: &str,
         is_recent: bool,
         is_directory: bool,
         excerpt_id: ExcerptId,
         source_range: Range<Anchor>,
         editor: Entity<Editor>,
         context_store: Entity<ContextStore>,
-        workspace: Entity<Workspace>,
         cx: &App,
-    ) -> Option<Completion> {
-        let label = Self::build_code_label_for_full_path(
-            project_path.worktree_id,
-            &project_path.path,
-            workspace.clone(),
-            cx,
-        )?;
-        let full_path = Self::full_path_for_entry(
-            project_path.worktree_id,
+    ) -> Completion {
+        let (file_name, directory) = super::file_context_picker::extract_file_name_and_directory(
             &project_path.path,
-            workspace.clone(),
+            path_prefix,
+        );
+
+        let label = Self::build_code_label_for_full_path(
+            &file_name,
+            directory.as_ref().map(|s| s.as_ref()),
             cx,
-        )?;
+        );
+        let full_path = if let Some(directory) = directory {
+            format!("{}{}", directory, file_name)
+        } else {
+            file_name.to_string()
+        };
 
         let crease_icon_path = if is_directory {
             FileIcons::get_folder_icon(false, cx).unwrap_or_else(|| IconName::Folder.path().into())
         } else {
-            FileIcons::get_icon(&full_path, cx).unwrap_or_else(|| IconName::File.path().into())
+            FileIcons::get_icon(Path::new(&full_path), cx)
+                .unwrap_or_else(|| IconName::File.path().into())
         };
         let completion_icon_path = if is_recent {
             IconName::HistoryRerun.path().into()
@@ -307,15 +279,9 @@ impl ContextPickerCompletionProvider {
             crease_icon_path.clone()
         };
 
-        let crease_name = project_path
-            .path
-            .file_name()
-            .map(|file_name| file_name.to_string_lossy().to_string())
-            .unwrap_or_else(|| "untitled".to_string());
-
-        let new_text = format!("@file {}", full_path.to_string_lossy());
+        let new_text = format!("@file {}", full_path);
         let new_text_len = new_text.len();
-        Some(Completion {
+        Completion {
             old_range: source_range.clone(),
             new_text,
             label,
@@ -324,7 +290,7 @@ impl ContextPickerCompletionProvider {
             icon_path: Some(completion_icon_path),
             confirm: Some(confirm_completion_callback(
                 crease_icon_path,
-                crease_name.into(),
+                file_name,
                 excerpt_id,
                 source_range.start,
                 new_text_len,
@@ -340,7 +306,7 @@ impl ContextPickerCompletionProvider {
                     })
                 },
             )),
-        })
+        }
     }
 }
 
@@ -397,33 +363,34 @@ impl CompletionProvider for ContextPickerCompletionProvider {
                         .update(|cx| {
                             super::file_context_picker::search_paths(
                                 query,
-                                Arc::new(AtomicBool::default()),
+                                Arc::<AtomicBool>::default(),
                                 &workspace,
                                 cx,
                             )
                         })?
                         .await;
 
-                    completions.reserve(path_matches.len());
-                    cx.update(|cx| {
-                        completions.extend(path_matches.iter().filter_map(|mat| {
-                            let editor = editor.upgrade()?;
-                            Self::completion_for_path(
-                                ProjectPath {
-                                    worktree_id: WorktreeId::from_usize(mat.worktree_id),
-                                    path: mat.path.clone(),
-                                },
-                                false,
-                                mat.is_dir,
-                                excerpt_id,
-                                source_range.clone(),
-                                editor.clone(),
-                                context_store.clone(),
-                                workspace.clone(),
-                                cx,
-                            )
-                        }));
-                    })?;
+                    if let Some(editor) = editor.upgrade() {
+                        completions.reserve(path_matches.len());
+                        cx.update(|cx| {
+                            completions.extend(path_matches.iter().map(|mat| {
+                                Self::completion_for_path(
+                                    ProjectPath {
+                                        worktree_id: WorktreeId::from_usize(mat.worktree_id),
+                                        path: mat.path.clone(),
+                                    },
+                                    &mat.path_prefix,
+                                    false,
+                                    mat.is_dir,
+                                    excerpt_id,
+                                    source_range.clone(),
+                                    editor.clone(),
+                                    context_store.clone(),
+                                    cx,
+                                )
+                            }));
+                        })?;
+                    }
                 }
                 Some(ContextPickerMode::Fetch) => {
                     if let Some(editor) = editor.upgrade() {
@@ -771,7 +738,6 @@ mod tests {
                 .unwrap();
         }
 
-        //TODO: Construct the editor without an actual buffer that points to a file
         let item = workspace
             .update_in(&mut cx, |workspace, window, cx| {
                 workspace.open_path(
@@ -821,10 +787,10 @@ mod tests {
             assert_eq!(
                 current_completion_labels(editor),
                 &[
-                    format!("seven.txt {}", separator!("dir/b")).as_str(),
-                    format!("six.txt {}", separator!("dir/b")).as_str(),
-                    format!("five.txt {}", separator!("dir/b")).as_str(),
-                    format!("four.txt {}", separator!("dir/a")).as_str(),
+                    "seven.txt dir/b/",
+                    "six.txt dir/b/",
+                    "five.txt dir/b/",
+                    "four.txt dir/a/",
                     "Files & Directories",
                     "Fetch"
                 ]
@@ -853,10 +819,7 @@ mod tests {
         editor.update(&mut cx, |editor, cx| {
             assert_eq!(editor.text(cx), "Lorem @file one");
             assert!(editor.has_visible_completions_menu());
-            assert_eq!(
-                current_completion_labels(editor),
-                vec![format!("one.txt {}", separator!("dir/a")).as_str(),]
-            );
+            assert_eq!(current_completion_labels(editor), vec!["one.txt dir/a/"]);
         });
 
         editor.update_in(&mut cx, |editor, window, cx| {
@@ -865,10 +828,7 @@ mod tests {
         });
 
         editor.update(&mut cx, |editor, cx| {
-            assert_eq!(
-                editor.text(cx),
-                format!("Lorem @file {}", separator!("dir/a/one.txt"))
-            );
+            assert_eq!(editor.text(cx), "Lorem @file dir/a/one.txt",);
             assert!(!editor.has_visible_completions_menu());
             assert_eq!(
                 crease_ranges(editor, cx),
@@ -879,10 +839,7 @@ mod tests {
         cx.simulate_input(" ");
 
         editor.update(&mut cx, |editor, cx| {
-            assert_eq!(
-                editor.text(cx),
-                format!("Lorem @file {} ", separator!("dir/a/one.txt"))
-            );
+            assert_eq!(editor.text(cx), "Lorem @file dir/a/one.txt ",);
             assert!(!editor.has_visible_completions_menu());
             assert_eq!(
                 crease_ranges(editor, cx),
@@ -893,10 +850,7 @@ mod tests {
         cx.simulate_input("Ipsum ");
 
         editor.update(&mut cx, |editor, cx| {
-            assert_eq!(
-                editor.text(cx),
-                format!("Lorem @file {} Ipsum ", separator!("dir/a/one.txt"))
-            );
+            assert_eq!(editor.text(cx), "Lorem @file dir/a/one.txt Ipsum ",);
             assert!(!editor.has_visible_completions_menu());
             assert_eq!(
                 crease_ranges(editor, cx),
@@ -907,10 +861,7 @@ mod tests {
         cx.simulate_input("@file ");
 
         editor.update(&mut cx, |editor, cx| {
-            assert_eq!(
-                editor.text(cx),
-                format!("Lorem @file {} Ipsum @file ", separator!("dir/a/one.txt"))
-            );
+            assert_eq!(editor.text(cx), "Lorem @file dir/a/one.txt Ipsum @file ",);
             assert!(editor.has_visible_completions_menu());
             assert_eq!(
                 crease_ranges(editor, cx),
@@ -927,11 +878,7 @@ mod tests {
         editor.update(&mut cx, |editor, cx| {
             assert_eq!(
                 editor.text(cx),
-                format!(
-                    "Lorem @file {} Ipsum @file {}",
-                    separator!("dir/a/one.txt"),
-                    separator!("dir/b/seven.txt")
-                )
+                "Lorem @file dir/a/one.txt Ipsum @file dir/b/seven.txt"
             );
             assert!(!editor.has_visible_completions_menu());
             assert_eq!(
@@ -948,11 +895,7 @@ mod tests {
         editor.update(&mut cx, |editor, cx| {
             assert_eq!(
                 editor.text(cx),
-                format!(
-                    "Lorem @file {} Ipsum @file {}\n@",
-                    separator!("dir/a/one.txt"),
-                    separator!("dir/b/seven.txt")
-                )
+                "Lorem @file dir/a/one.txt Ipsum @file dir/b/seven.txt\n@"
             );
             assert!(editor.has_visible_completions_menu());
             assert_eq!(
@@ -973,12 +916,7 @@ mod tests {
         editor.update(&mut cx, |editor, cx| {
             assert_eq!(
                 editor.text(cx),
-                format!(
-                    "Lorem @file {} Ipsum @file {}\n@file {}",
-                    separator!("dir/a/one.txt"),
-                    separator!("dir/b/seven.txt"),
-                    separator!("dir/b/six.txt"),
-                )
+                "Lorem @file dir/a/one.txt Ipsum @file dir/b/seven.txt\n@file dir/b/six.txt"
             );
             assert!(!editor.has_visible_completions_menu());
             assert_eq!(

crates/assistant2/src/context_picker/file_context_picker.rs

@@ -273,17 +273,17 @@ pub(crate) fn search_paths(
     }
 }
 
-pub fn render_file_context_entry(
-    id: ElementId,
+pub fn extract_file_name_and_directory(
     path: &Path,
-    path_prefix: &Arc<str>,
-    is_directory: bool,
-    context_store: WeakEntity<ContextStore>,
-    cx: &App,
-) -> Stateful<Div> {
-    let (file_name, directory) = if path == Path::new("") {
+    path_prefix: &str,
+) -> (SharedString, Option<SharedString>) {
+    if path == Path::new("") {
         (
-            SharedString::from(path_prefix.trim_end_matches('/').to_string()),
+            SharedString::from(
+                path_prefix
+                    .trim_end_matches(std::path::MAIN_SEPARATOR)
+                    .to_string(),
+            ),
             None,
         )
     } else {
@@ -294,7 +294,9 @@ pub fn render_file_context_entry(
             .to_string()
             .into();
 
-        let mut directory = path_prefix.to_string();
+        let mut directory = path_prefix
+            .trim_end_matches(std::path::MAIN_SEPARATOR)
+            .to_string();
         if !directory.ends_with('/') {
             directory.push('/');
         }
@@ -303,8 +305,19 @@ pub fn render_file_context_entry(
             directory.push('/');
         }
 
-        (file_name, Some(directory))
-    };
+        (file_name, Some(directory.into()))
+    }
+}
+
+pub fn render_file_context_entry(
+    id: ElementId,
+    path: &Path,
+    path_prefix: &Arc<str>,
+    is_directory: bool,
+    context_store: WeakEntity<ContextStore>,
+    cx: &App,
+) -> Stateful<Div> {
+    let (file_name, directory) = extract_file_name_and_directory(path, path_prefix);
 
     let added = context_store.upgrade().and_then(|context_store| {
         if is_directory {