diff --git a/crates/agent/src/active_thread.rs b/crates/agent/src/active_thread.rs index ad4cb63261..0a356e2fc2 100644 --- a/crates/agent/src/active_thread.rs +++ b/crates/agent/src/active_thread.rs @@ -2840,10 +2840,10 @@ pub(crate) fn open_context( } } AssistantContext::Directory(directory_context) => { - let path = directory_context.project_path.clone(); + let project_path = directory_context.project_path(cx); workspace.update(cx, |workspace, cx| { workspace.project().update(cx, |project, cx| { - if let Some(entry) = project.entry_for_path(&path, cx) { + if let Some(entry) = project.entry_for_path(&project_path, cx) { cx.emit(project::Event::RevealInProjectPanel(entry.id)); } }) diff --git a/crates/agent/src/context.rs b/crates/agent/src/context.rs index 9133ca3ffa..c2ad806145 100644 --- a/crates/agent/src/context.rs +++ b/crates/agent/src/context.rs @@ -1,9 +1,9 @@ -use std::{ops::Range, sync::Arc}; +use std::{ops::Range, path::Path, sync::Arc}; use gpui::{App, Entity, SharedString}; use language::{Buffer, File}; use language_model::LanguageModelRequestMessage; -use project::ProjectPath; +use project::{ProjectPath, Worktree}; use serde::{Deserialize, Serialize}; use text::{Anchor, BufferId}; use ui::IconName; @@ -69,10 +69,21 @@ pub struct FileContext { #[derive(Debug, Clone)] pub struct DirectoryContext { pub id: ContextId, - pub project_path: ProjectPath, + pub worktree: Entity, + pub path: Arc, + /// Buffers of the files within the directory. pub context_buffers: Vec, } +impl DirectoryContext { + pub fn project_path(&self, cx: &App) -> ProjectPath { + ProjectPath { + worktree_id: self.worktree.read(cx).id(), + path: self.path.clone(), + } + } +} + #[derive(Debug, Clone)] pub struct SymbolContext { pub id: ContextId, @@ -86,12 +97,11 @@ pub struct FetchedUrlContext { pub text: SharedString, } -// TODO: Model holds onto the thread even if the thread is deleted. Can either handle this -// explicitly or have a WeakModel and remove during snapshot. - #[derive(Debug, Clone)] pub struct ThreadContext { pub id: ContextId, + // TODO: Entity holds onto the thread even if the thread is deleted. Should probably be + // a WeakEntity and handle removal from the UI when it has dropped. pub thread: Entity, pub text: SharedString, } @@ -105,12 +115,11 @@ impl ThreadContext { } } -// TODO: Model holds onto the buffer even if the file is deleted and closed. Should remove -// the context from the message editor in this case. - #[derive(Clone)] pub struct ContextBuffer { pub id: BufferId, + // TODO: Entity holds onto the thread even if the thread is deleted. Should probably be + // a WeakEntity and handle removal from the UI when it has dropped. pub buffer: Entity, pub file: Arc, pub version: clock::Global, diff --git a/crates/agent/src/context_picker.rs b/crates/agent/src/context_picker.rs index 36c0dc58be..a370112d0d 100644 --- a/crates/agent/src/context_picker.rs +++ b/crates/agent/src/context_picker.rs @@ -289,12 +289,14 @@ impl ContextPicker { path_prefix, } => { let context_store = self.context_store.clone(); + let worktree_id = project_path.worktree_id; let path = project_path.path.clone(); ContextMenuItem::custom_entry( move |_window, cx| { render_file_context_entry( ElementId::NamedInteger("ctx-recent".into(), ix), + worktree_id, &path, &path_prefix, false, @@ -466,7 +468,7 @@ fn recent_context_picker_entries( recent.extend( workspace .recent_navigation_history_iter(cx) - .filter(|(path, _)| !current_files.contains(&path.path.to_path_buf())) + .filter(|(path, _)| !current_files.contains(path)) .take(4) .filter_map(|(project_path, _)| { project diff --git a/crates/agent/src/context_picker/file_context_picker.rs b/crates/agent/src/context_picker/file_context_picker.rs index 7ea4e8f639..965f4a530e 100644 --- a/crates/agent/src/context_picker/file_context_picker.rs +++ b/crates/agent/src/context_picker/file_context_picker.rs @@ -189,6 +189,7 @@ impl PickerDelegate for FileContextPickerDelegate { .toggle_state(selected) .child(render_file_context_entry( ElementId::NamedInteger("file-ctx-picker".into(), ix), + WorktreeId::from_usize(mat.worktree_id), &mat.path, &mat.path_prefix, mat.is_dir, @@ -328,19 +329,26 @@ pub fn extract_file_name_and_directory( pub fn render_file_context_entry( id: ElementId, - path: &Path, + worktree_id: WorktreeId, + path: &Arc, path_prefix: &Arc, is_directory: bool, context_store: WeakEntity, cx: &App, ) -> Stateful
{ - let (file_name, directory) = extract_file_name_and_directory(path, path_prefix); + let (file_name, directory) = extract_file_name_and_directory(&path, path_prefix); let added = context_store.upgrade().and_then(|context_store| { + let project_path = ProjectPath { + worktree_id, + path: path.clone(), + }; if is_directory { - context_store.read(cx).includes_directory(path) + context_store.read(cx).includes_directory(&project_path) } else { - context_store.read(cx).will_include_file_path(path, cx) + context_store + .read(cx) + .will_include_file_path(&project_path, cx) } }); @@ -380,8 +388,9 @@ pub fn render_file_context_entry( ) .child(Label::new("Added").size(LabelSize::Small)), ), - FileInclusion::InDirectory(dir_name) => { - let dir_name = dir_name.to_string_lossy().into_owned(); + FileInclusion::InDirectory(directory_project_path) => { + // TODO: Consider using worktree full_path to include worktree name. + let directory_path = directory_project_path.path.to_string_lossy().into_owned(); el.child( h_flex() @@ -395,7 +404,7 @@ pub fn render_file_context_entry( ) .child(Label::new("Included").size(LabelSize::Small)), ) - .tooltip(Tooltip::text(format!("in {dir_name}"))) + .tooltip(Tooltip::text(format!("in {directory_path}"))) } }) } diff --git a/crates/agent/src/context_store.rs b/crates/agent/src/context_store.rs index 22b7c70a6b..fcea841685 100644 --- a/crates/agent/src/context_store.rs +++ b/crates/agent/src/context_store.rs @@ -1,5 +1,5 @@ use std::ops::Range; -use std::path::{Path, PathBuf}; +use std::path::Path; use std::sync::Arc; use anyhow::{Context as _, Result, anyhow}; @@ -28,7 +28,7 @@ pub struct ContextStore { // TODO: If an EntityId is used for all context types (like BufferId), can remove ContextId. next_context_id: ContextId, files: BTreeMap, - directories: HashMap, + directories: HashMap, symbols: HashMap, symbol_buffers: HashMap>, symbols_by_path: HashMap>, @@ -93,7 +93,7 @@ impl ContextStore { let buffer_id = this.update(cx, |_, cx| buffer.read(cx).remote_id())?; let already_included = this.update(cx, |this, cx| { - match this.will_include_buffer(buffer_id, &project_path.path) { + match this.will_include_buffer(buffer_id, &project_path) { Some(FileInclusion::Direct(context_id)) => { if remove_if_exists { this.remove_context(context_id, cx); @@ -159,7 +159,7 @@ impl ContextStore { return Task::ready(Err(anyhow!("failed to read project"))); }; - let already_included = match self.includes_directory(&project_path.path) { + let already_included = match self.includes_directory(&project_path) { Some(FileInclusion::Direct(context_id)) => { if remove_if_exists { self.remove_context(context_id, cx); @@ -223,14 +223,12 @@ impl ContextStore { .collect::>(); if context_buffers.is_empty() { - return Err(anyhow!( - "No text files found in {}", - &project_path.path.display() - )); + let full_path = cx.update(|cx| worktree.read(cx).full_path(&project_path.path))?; + return Err(anyhow!("No text files found in {}", &full_path.display())); } this.update(cx, |this, cx| { - this.insert_directory(project_path, context_buffers, cx); + this.insert_directory(worktree, project_path, context_buffers, cx); })?; anyhow::Ok(()) @@ -239,17 +237,20 @@ impl ContextStore { fn insert_directory( &mut self, + worktree: Entity, project_path: ProjectPath, context_buffers: Vec, cx: &mut Context, ) { let id = self.next_context_id.post_inc(); - self.directories.insert(project_path.path.to_path_buf(), id); + let path = project_path.path.clone(); + self.directories.insert(project_path, id); self.context .push(AssistantContext::Directory(DirectoryContext { id, - project_path, + worktree, + path, context_buffers, })); cx.notify(); @@ -478,23 +479,31 @@ impl ContextStore { /// Returns whether the buffer is already included directly in the context, or if it will be /// included in the context via a directory. Directory inclusion is based on paths rather than /// buffer IDs as the directory will be re-scanned. - pub fn will_include_buffer(&self, buffer_id: BufferId, path: &Path) -> Option { + pub fn will_include_buffer( + &self, + buffer_id: BufferId, + project_path: &ProjectPath, + ) -> Option { if let Some(context_id) = self.files.get(&buffer_id) { return Some(FileInclusion::Direct(*context_id)); } - self.will_include_file_path_via_directory(path) + self.will_include_file_path_via_directory(project_path) } /// Returns whether this file path is already included directly in the context, or if it will be /// included in the context via a directory. - pub fn will_include_file_path(&self, path: &Path, cx: &App) -> Option { + pub fn will_include_file_path( + &self, + project_path: &ProjectPath, + cx: &App, + ) -> Option { if !self.files.is_empty() { let found_file_context = self.context.iter().find(|context| match &context { AssistantContext::File(file_context) => { let buffer = file_context.context_buffer.buffer.read(cx); - if let Some(file_path) = buffer_path_log_err(buffer, cx) { - *file_path == *path + if let Some(context_path) = buffer.project_path(cx) { + &context_path == project_path } else { false } @@ -506,31 +515,40 @@ impl ContextStore { } } - self.will_include_file_path_via_directory(path) + self.will_include_file_path_via_directory(project_path) } - fn will_include_file_path_via_directory(&self, path: &Path) -> Option { + fn will_include_file_path_via_directory( + &self, + project_path: &ProjectPath, + ) -> Option { if self.directories.is_empty() { return None; } - let mut buf = path.to_path_buf(); + let mut path_buf = project_path.path.to_path_buf(); - while buf.pop() { - if let Some(_) = self.directories.get(&buf) { - return Some(FileInclusion::InDirectory(buf)); + while path_buf.pop() { + // TODO: This isn't very efficient. Consider using a better representation of the + // directories map. + let directory_project_path = ProjectPath { + worktree_id: project_path.worktree_id, + path: path_buf.clone().into(), + }; + if let Some(_) = self.directories.get(&directory_project_path) { + return Some(FileInclusion::InDirectory(directory_project_path)); } } None } - pub fn includes_directory(&self, path: &Path) -> Option { - if let Some(context_id) = self.directories.get(path) { + pub fn includes_directory(&self, project_path: &ProjectPath) -> Option { + if let Some(context_id) = self.directories.get(project_path) { return Some(FileInclusion::Direct(*context_id)); } - self.will_include_file_path_via_directory(path) + self.will_include_file_path_via_directory(project_path) } pub fn included_symbol(&self, symbol_id: &ContextSymbolId) -> Option { @@ -564,13 +582,13 @@ impl ContextStore { } } - pub fn file_paths(&self, cx: &App) -> HashSet { + pub fn file_paths(&self, cx: &App) -> HashSet { self.context .iter() .filter_map(|context| match context { AssistantContext::File(file) => { let buffer = file.context_buffer.buffer.read(cx); - buffer_path_log_err(buffer, cx).map(|p| p.to_path_buf()) + buffer.project_path(cx) } AssistantContext::Directory(_) | AssistantContext::Symbol(_) @@ -587,7 +605,7 @@ impl ContextStore { pub enum FileInclusion { Direct(ContextId), - InDirectory(PathBuf), + InDirectory(ProjectPath), } // ContextBuffer without text. @@ -654,19 +672,6 @@ fn collect_buffer_info_and_text( Ok((buffer_info, text_task)) } -pub fn buffer_path_log_err(buffer: &Buffer, cx: &App) -> Option> { - if let Some(file) = buffer.file() { - let mut path = file.path().clone(); - if path.as_os_str().is_empty() { - path = file.full_path(cx).into(); - } - Some(path) - } else { - log::error!("Buffer that had a path unexpectedly no longer has a path."); - None - } -} - fn to_fenced_codeblock(path: &Path, content: Rope) -> SharedString { let path_extension = path.extension().and_then(|ext| ext.to_str()); let path_string = path.to_string_lossy(); @@ -742,13 +747,13 @@ pub fn refresh_context_store_text( } } AssistantContext::Directory(directory_context) => { + let directory_path = directory_context.project_path(cx); let should_refresh = changed_buffers.is_empty() || changed_buffers.iter().any(|buffer| { - let buffer = buffer.read(cx); - - buffer_path_log_err(&buffer, cx).map_or(false, |path| { - path.starts_with(&directory_context.project_path.path) - }) + let Some(buffer_path) = buffer.read(cx).project_path(cx) else { + return false; + }; + buffer_path.starts_with(&directory_path) }); if should_refresh { @@ -835,14 +840,16 @@ fn refresh_directory_text( let context_buffers = future::join_all(futures); let id = directory_context.id; - let project_path = directory_context.project_path.clone(); + let worktree = directory_context.worktree.clone(); + let path = directory_context.path.clone(); Some(cx.spawn(async move |cx| { let context_buffers = context_buffers.await; context_store .update(cx, |context_store, _| { let new_directory_context = DirectoryContext { id, - project_path, + worktree, + path, context_buffers, }; context_store.replace_context(AssistantContext::Directory(new_directory_context)); diff --git a/crates/agent/src/context_strip.rs b/crates/agent/src/context_strip.rs index fd56dcf678..afc61f46ce 100644 --- a/crates/agent/src/context_strip.rs +++ b/crates/agent/src/context_strip.rs @@ -1,3 +1,4 @@ +use std::path::Path; use std::rc::Rc; use collections::HashSet; @@ -9,6 +10,7 @@ use gpui::{ }; use itertools::Itertools; use language::Buffer; +use project::ProjectItem; use ui::{KeyBinding, PopoverMenu, PopoverMenuHandle, Tooltip, prelude::*}; use workspace::{Workspace, notifications::NotifyResultExt}; @@ -93,26 +95,23 @@ impl ContextStrip { let active_buffer_entity = editor.buffer().read(cx).as_singleton()?; let active_buffer = active_buffer_entity.read(cx); - let path = active_buffer.file()?.full_path(cx); + let project_path = active_buffer.project_path(cx)?; if self .context_store .read(cx) - .will_include_buffer(active_buffer.remote_id(), &path) + .will_include_buffer(active_buffer.remote_id(), &project_path) .is_some() { return None; } - let name = match path.file_name() { - Some(name) => name.to_string_lossy().into_owned().into(), - None => path.to_string_lossy().into_owned().into(), - }; + let file_name = active_buffer.file()?.file_name(cx); - let icon_path = FileIcons::get_icon(&path, cx); + let icon_path = FileIcons::get_icon(&Path::new(&file_name), cx); Some(SuggestedContext::File { - name, + name: file_name.to_string_lossy().into_owned().into(), buffer: active_buffer_entity.downgrade(), icon_path, }) diff --git a/crates/agent/src/ui/context_pill.rs b/crates/agent/src/ui/context_pill.rs index 6ca80cc27e..a235444323 100644 --- a/crates/agent/src/ui/context_pill.rs +++ b/crates/agent/src/ui/context_pill.rs @@ -280,9 +280,10 @@ impl AddedContext { } AssistantContext::Directory(directory_context) => { - // TODO: handle worktree disambiguation. Maybe by storing an `Arc` to also - // handle renames? - let full_path = &directory_context.project_path.path; + let full_path = directory_context + .worktree + .read(cx) + .full_path(&directory_context.path); let full_path_string: SharedString = full_path.to_string_lossy().into_owned().into(); let name = full_path diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index e5bb6b3291..7ce0d92379 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -334,6 +334,10 @@ impl ProjectPath { path: Path::new("").into(), } } + + pub fn starts_with(&self, other: &ProjectPath) -> bool { + self.worktree_id == other.worktree_id && self.path.starts_with(&other.path) + } } #[derive(Debug, Default)] diff --git a/crates/worktree/src/worktree.rs b/crates/worktree/src/worktree.rs index 5deaaa4267..920b89770b 100644 --- a/crates/worktree/src/worktree.rs +++ b/crates/worktree/src/worktree.rs @@ -1172,6 +1172,31 @@ impl Worktree { pub fn is_single_file(&self) -> bool { self.root_dir().is_none() } + + /// For visible worktrees, returns the path with the worktree name as the first component. + /// Otherwise, returns an absolute path. + pub fn full_path(&self, worktree_relative_path: &Path) -> PathBuf { + let mut full_path = PathBuf::new(); + + if self.is_visible() { + full_path.push(self.root_name()); + } else { + let path = self.abs_path(); + + if self.is_local() && path.starts_with(home_dir().as_path()) { + full_path.push("~"); + full_path.push(path.strip_prefix(home_dir().as_path()).unwrap()); + } else { + full_path.push(path) + } + } + + if worktree_relative_path.components().next().is_some() { + full_path.push(&worktree_relative_path); + } + + full_path + } } impl LocalWorktree { @@ -3229,27 +3254,7 @@ impl language::File for File { } fn full_path(&self, cx: &App) -> PathBuf { - let mut full_path = PathBuf::new(); - let worktree = self.worktree.read(cx); - - if worktree.is_visible() { - full_path.push(worktree.root_name()); - } else { - let path = worktree.abs_path(); - - if worktree.is_local() && path.starts_with(home_dir().as_path()) { - full_path.push("~"); - full_path.push(path.strip_prefix(home_dir().as_path()).unwrap()); - } else { - full_path.push(path) - } - } - - if self.path.components().next().is_some() { - full_path.push(&self.path); - } - - full_path + self.worktree.read(cx).full_path(&self.path) } /// Returns the last component of this handle's absolute path. If this handle refers to the root