From f78f6da255afe353fa2b726addca578dbcfd78c8 Mon Sep 17 00:00:00 2001 From: MartinYe1234 <52641447+MartinYe1234@users.noreply.github.com> Date: Thu, 21 May 2026 15:04:32 -0700 Subject: [PATCH] Make file paths in backticks clickable in agent panel (#57303) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the agent mentions a file path inside `backticks` (e.g. `` `src/main.rs` `` or `` `src/main.rs:42` ``), the rendered code span now becomes a clickable link in the agent panel. Clicking opens the referenced file in the workspace, jumping to the right line and column when present. ## How it works - **Shared path resolution.** Extracted `OpenTarget` and the workspace/worktree resolution logic out of `terminal_view::terminal_path_like_target` into a new `workspace::path_link` module so both the terminal and the agent panel can use the same code. Includes a `sanitize_path_text` helper ported from the terminal's URL/punctuation handling. Pure refactor — terminal behavior is unchanged. - **`markdown` crate hook.** Added `MarkdownElement::on_code_span_link(callback)`. When the callback returns `Some(url)` for a given code span's contents, the existing `push_link` machinery wires up cmd-hover, hit testing, and the existing `on_url_click` callback. When it returns `None`, the code span renders as before. The hook is opt-in, so `markdown` stays workspace-agnostic. - **Agent panel wiring.** `render_agent_markdown` constructs an `AgentCodeSpanResolver` that snapshots the project's visible worktree entries plus their file extensions. `try_resolve` does a cheap synchronous heuristic check (path must contain `/`/`\` or end in an extension present in the workspace, can't be a URL, can't be all digits, etc.) and then looks the candidate up in the per-worktree `HashSet>`. On a hit it returns a `MentionUri::File` or `MentionUri::Selection` URI, which the existing `thread_view::open_link` already knows how to open at the right line. ## Edge cases handled - Code spans inside fenced code blocks stay plain (gated on `builder.code_block_stack.is_empty()`, matching how regular markdown links behave). - Trailing prose punctuation (`` `src/main.rs.` ``) is stripped before lookup. - Identifiers like `` `String` ``, `` `await` ``, `` `npm run dev` `` stay plain — they don't pass the path-like heuristic. - Cross-platform path separators handled via the per-worktree `PathStyle`. ## Tests - `crates/markdown` — unit test asserting code spans become links when the callback returns `Some`, and stay plain when it doesn't. - `crates/agent_ui` — unit test for `AgentCodeSpanResolver::try_resolve` covering hits with and without a `:line` suffix, misses, identifiers, and trailing punctuation. - Existing `terminal_view` tests cover the moved resolution code (unchanged behavior). ## Notes - There's currently a temporary `log::info!` in `AgentCodeSpanResolver::try_resolve` that reports per-call worktree-walk timing and a cumulative total. Kept in for now to verify the feature isn't being called excessively during streaming renders. Can be removed before merge. - Resolution is sync-only against worktree entries; absolute paths outside the workspace are not resolved (would require an async re-render path). Closes AI-277 Release Notes: - Made file paths in `backticks` clickable in the agent panel; clicking opens the referenced file at the given line when present. --- Cargo.lock | 2 + Cargo.toml | 1 + crates/acp_thread/src/mention.rs | 77 +++- crates/agent_ui/Cargo.toml | 1 + crates/agent_ui/src/agent_ui.rs | 41 +- crates/agent_ui/src/conversation_view.rs | 299 ++++++++++++- .../src/conversation_view/thread_view.rs | 78 ++-- crates/agent_ui/src/mention_set.rs | 3 + crates/agent_ui/src/message_editor.rs | 5 + crates/agent_ui/src/ui/mention_crease.rs | 62 +-- crates/markdown/src/markdown.rs | 150 ++++++- .../src/terminal_path_like_target.rs | 402 ++--------------- crates/workspace/Cargo.toml | 1 + crates/workspace/src/path_link.rs | 422 ++++++++++++++++++ crates/workspace/src/workspace.rs | 1 + 15 files changed, 1096 insertions(+), 449 deletions(-) create mode 100644 crates/workspace/src/path_link.rs diff --git a/Cargo.lock b/Cargo.lock index 2ccdca4d6cc..43c2b961b86 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -407,6 +407,7 @@ dependencies = [ "language_models", "languages", "log", + "lru", "lsp", "markdown", "menu", @@ -22018,6 +22019,7 @@ dependencies = [ "collections", "component", "db", + "dirs", "fs", "futures 0.3.32", "futures-lite 1.13.0", diff --git a/Cargo.toml b/Cargo.toml index e936bea3d71..6db9f292287 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -622,6 +622,7 @@ linkify = "0.10.0" libwebrtc = "0.3.26" livekit = { version = "0.7.32", features = ["tokio", "rustls-tls-native-roots"] } log = { version = "0.4.16", features = ["kv_unstable_serde", "serde"] } +lru = "0.16" lsp-types = { git = "https://github.com/zed-industries/lsp-types", rev = "f4dfa89a21ca35cd929b70354b1583fabae325f8" } mach2 = "0.5" markup5ever_rcdom = "0.3.0" diff --git a/crates/acp_thread/src/mention.rs b/crates/acp_thread/src/mention.rs index 12827acc833..cb96de34813 100644 --- a/crates/acp_thread/src/mention.rs +++ b/crates/acp_thread/src/mention.rs @@ -51,6 +51,8 @@ pub enum MentionUri { #[serde(default, skip_serializing_if = "Option::is_none")] abs_path: Option, line_range: RangeInclusive, + #[serde(default, skip_serializing_if = "Option::is_none")] + column: Option, }, Fetch { url: Url, @@ -105,6 +107,17 @@ impl MentionUri { Ok(start_line..=end_line) } + let parse_column = + |input: Option| -> Option { input?.parse::().ok()?.checked_sub(1) }; + let validate_query_params = |url: &Url, allowed: &[&str]| -> Result<()> { + for (key, _) in url.query_pairs() { + if !allowed.contains(&key.as_ref()) { + bail!("invalid query parameter") + } + } + Ok(()) + }; + let parse_absolute_path = |input: &str| -> Result { let (path_input, fragment) = input .split_once('#') @@ -114,6 +127,7 @@ impl MentionUri { return Ok(MentionUri::Selection { abs_path: Some(path_input.into()), line_range: fragment, + column: None, }); } @@ -123,10 +137,12 @@ impl MentionUri { let line = row .checked_sub(1) .context("Line numbers should be 1-based")?; - // TODO: Preserve column info too. Ok(MentionUri::Selection { abs_path: Some(abs_path), line_range: line..=line, + column: path_with_position + .column + .map(|column| column.saturating_sub(1)), }) } else { Ok(MentionUri::File { abs_path }) @@ -156,8 +172,10 @@ impl MentionUri { let path = normalized.as_ref(); if let Some(fragment) = url.fragment() { + validate_query_params(&url, &["symbol", "column"])?; let line_range = parse_line_range(fragment).log_err().unwrap_or(1..=1); - if let Some(name) = single_query_param(&url, "symbol")? { + let column = parse_column(query_param(&url, "column")); + if let Some(name) = query_param(&url, "symbol") { Ok(Self::Symbol { name, abs_path: path.into(), @@ -167,6 +185,7 @@ impl MentionUri { Ok(Self::Selection { abs_path: Some(path.into()), line_range, + column, }) } } else if input.ends_with("/") { @@ -216,9 +235,11 @@ impl MentionUri { .fragment() .context("Missing fragment for untitled buffer selection")?; let line_range = parse_line_range(fragment)?; + validate_query_params(&url, &["column"])?; Ok(Self::Selection { abs_path: None, line_range, + column: parse_column(query_param(&url, "column")), }) } else if let Some(name) = path.strip_prefix("/agent/symbol/") { let fragment = url @@ -245,13 +266,15 @@ impl MentionUri { abs_path: path.into(), }) } else if path.starts_with("/agent/selection") { + validate_query_params(&url, &["path", "column"])?; let fragment = url.fragment().context("Missing fragment for selection")?; let line_range = parse_line_range(fragment)?; - let path = - single_query_param(&url, "path")?.context("Missing path for selection")?; + let column = parse_column(query_param(&url, "column")); + let path = query_param(&url, "path").context("Missing path for selection")?; Ok(Self::Selection { abs_path: Some(path.into()), line_range, + column, }) } else if path.starts_with("/agent/terminal-selection") { let line_count = single_query_param(&url, "lines")? @@ -460,6 +483,7 @@ impl MentionUri { abs_path, name, line_range, + .. } => { let mut url = Url::parse("file:///").unwrap(); url.set_path(&abs_path.to_string_lossy()); @@ -474,6 +498,7 @@ impl MentionUri { MentionUri::Selection { abs_path, line_range, + column, } => { let mut url = if let Some(path) = abs_path { let mut url = Url::parse("file:///").unwrap(); @@ -484,6 +509,10 @@ impl MentionUri { url.set_path("/agent/untitled-buffer"); url }; + if let Some(column) = column { + url.query_pairs_mut() + .append_pair("column", &(column + 1).to_string()); + } url.set_fragment(Some(&format!( "L{}:{}", line_range.start() + 1, @@ -564,6 +593,11 @@ fn default_include_errors() -> bool { true } +fn query_param(url: &Url, name: &'static str) -> Option { + url.query_pairs() + .find_map(|(key, value)| (key == name).then(|| value.to_string())) +} + fn single_query_param(url: &Url, name: &'static str) -> Result> { let pairs = url.query_pairs().collect::>(); match pairs.as_slice() { @@ -698,6 +732,7 @@ mod tests { abs_path: path, name, line_range, + .. } => { assert_eq!(path, Path::new(path!("/path/to/file.rs"))); assert_eq!(name, "MySymbol"); @@ -717,6 +752,7 @@ mod tests { MentionUri::Selection { abs_path: path, line_range, + .. } => { assert_eq!(path.as_ref().unwrap(), Path::new(path!("/path/to/file.rs"))); assert_eq!(line_range.start(), &4); @@ -748,6 +784,7 @@ mod tests { MentionUri::Selection { abs_path: None, line_range, + .. } => { assert_eq!(line_range.start(), &0); assert_eq!(line_range.end(), &9); @@ -895,6 +932,7 @@ mod tests { MentionUri::Selection { abs_path: path, line_range, + .. } => { assert_eq!(path.as_ref().unwrap(), Path::new("/path/to/file.rs")); assert_eq!(line_range.start(), &41); @@ -904,6 +942,29 @@ mod tests { } } + #[test] + fn test_parse_absolute_file_path_with_row_and_column() { + let file_path = "/path/to/file.rs:42:5"; + let parsed = MentionUri::parse(file_path, PathStyle::Posix).unwrap(); + match &parsed { + MentionUri::Selection { + abs_path: path, + line_range, + column, + } => { + assert_eq!(path.as_ref().unwrap(), Path::new("/path/to/file.rs")); + assert_eq!(line_range.start(), &41); + assert_eq!(line_range.end(), &41); + assert_eq!(column, &Some(4)); + + let parsed_again = MentionUri::parse(parsed.to_uri().as_ref(), PathStyle::Posix) + .expect("selection URI with column should parse"); + assert_eq!(parsed_again, parsed.clone()); + } + _ => panic!("Expected Selection variant"), + } + } + #[test] fn test_parse_absolute_file_path_with_fragment_line() { let file_path = "/path/to/file.rs#L42"; @@ -912,6 +973,7 @@ mod tests { MentionUri::Selection { abs_path: path, line_range, + .. } => { assert_eq!(path.as_ref().unwrap(), Path::new("/path/to/file.rs")); assert_eq!(line_range.start(), &41); @@ -941,6 +1003,7 @@ mod tests { MentionUri::Selection { abs_path: path, line_range, + .. } => { assert_eq!( path.as_ref().unwrap(), @@ -961,6 +1024,7 @@ mod tests { MentionUri::Selection { abs_path: path, line_range, + .. } => { assert_eq!( path.as_ref().unwrap(), @@ -993,6 +1057,7 @@ mod tests { MentionUri::Selection { abs_path: path, line_range, + .. } => { assert_eq!(path.as_ref().unwrap(), Path::new("/path/to/file.rs")); assert_eq!(line_range.start(), &41); @@ -1010,6 +1075,7 @@ mod tests { MentionUri::Selection { abs_path: path, line_range, + .. } => { assert_eq!( path.as_ref().unwrap(), @@ -1031,6 +1097,7 @@ mod tests { MentionUri::Selection { abs_path: path, line_range, + .. } => { assert_eq!(path.as_ref().unwrap(), Path::new(path!("/path/to/file.rs"))); assert_eq!(line_range.start(), &1871); @@ -1048,6 +1115,7 @@ mod tests { MentionUri::Selection { abs_path: path, line_range, + .. } => { assert_eq!(path.as_ref().unwrap(), Path::new(path!("/path/to/file.rs"))); assert_eq!(line_range.start(), &9); @@ -1063,6 +1131,7 @@ mod tests { MentionUri::Selection { abs_path: path, line_range, + .. } => { assert_eq!(path.as_ref().unwrap(), Path::new(path!("/path/to/file.rs"))); assert_eq!(line_range.start(), &9); diff --git a/crates/agent_ui/Cargo.toml b/crates/agent_ui/Cargo.toml index 91c0f21a51f..68b0b5faa41 100644 --- a/crates/agent_ui/Cargo.toml +++ b/crates/agent_ui/Cargo.toml @@ -69,6 +69,7 @@ language.workspace = true language_model.workspace = true language_models.workspace = true log.workspace = true +lru.workspace = true lsp.workspace = true markdown.workspace = true menu.workspace = true diff --git a/crates/agent_ui/src/agent_ui.rs b/crates/agent_ui/src/agent_ui.rs index c9063378d89..209976c090a 100644 --- a/crates/agent_ui/src/agent_ui.rs +++ b/crates/agent_ui/src/agent_ui.rs @@ -43,10 +43,12 @@ use ::ui::IconName; use agent_client_protocol::schema as acp; use agent_settings::{AgentProfileId, AgentSettings}; use command_palette_hooks::CommandPaletteFilter; +use editor::{Editor, SelectionEffects, scroll::Autoscroll}; use feature_flags::FeatureFlagAppExt as _; use fs::Fs; use gpui::{ - Action, App, Context, Entity, ImageSource, Resource, SharedString, SharedUri, Window, actions, + Action, App, Context, Entity, ImageSource, Resource, SharedString, SharedUri, TaskExt, Window, + actions, }; use language::{ LanguageRegistry, @@ -57,6 +59,7 @@ use language_model::{ }; use project::{AgentId, DisableAiSettings}; use prompt_store::{PromptBuilder, rules_to_skills_migration}; +use rope::Point; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use settings::{LanguageModelSelection, Settings as _, SettingsStore, SidebarSide}; @@ -112,6 +115,42 @@ pub(crate) fn resolve_agent_image( None } +pub(crate) fn open_abs_path_at_point( + workspace: &mut Workspace, + abs_path: PathBuf, + point: Point, + window: &mut Window, + cx: &mut Context, +) -> bool { + let project = workspace.project(); + let Some(path) = project.update(cx, |project, cx| project.find_project_path(abs_path, cx)) + else { + return false; + }; + + let item = workspace.open_path(path, None, true, window, cx); + window + .spawn(cx, async move |cx| { + let Some(editor) = item.await?.downcast::() else { + return Ok(()); + }; + let range = point..point; + editor + .update_in(cx, |editor, window, cx| { + editor.change_selections( + SelectionEffects::scroll(Autoscroll::center()), + window, + cx, + |selections| selections.select_ranges([range]), + ); + }) + .ok(); + anyhow::Ok(()) + }) + .detach_and_log_err(cx); + true +} + pub const DEFAULT_THREAD_TITLE: &str = "New Agent Thread"; const PARALLEL_AGENT_LAYOUT_BACKFILL_KEY: &str = "parallel_agent_layout_backfilled"; diff --git a/crates/agent_ui/src/conversation_view.rs b/crates/agent_ui/src/conversation_view.rs index b8e72840be6..3ef7e111fe6 100644 --- a/crates/agent_ui/src/conversation_view.rs +++ b/crates/agent_ui/src/conversation_view.rs @@ -40,14 +40,16 @@ use language_model::{LanguageModelCompletionError, LanguageModelRegistry}; use markdown::{ CodeBlockRenderer, CopyButtonVisibility, Markdown, MarkdownElement, MarkdownFont, MarkdownStyle, }; -use parking_lot::RwLock; -use project::{AgentId, AgentServerStore, Project, ProjectEntryId}; +use parking_lot::{Mutex, RwLock}; +use project::{AgentId, AgentServerStore, Project, ProjectEntryId, ProjectPath}; use prompt_store::{PromptId, PromptStore}; use crate::message_editor::SessionCapabilities; use crate::{AgentThreadSource, DEFAULT_THREAD_TITLE, resolve_agent_image}; +use lru::LruCache; use rope::Point; use settings::{NotifyWhenAgentWaiting, Settings as _, SettingsStore, ThinkingBlockDisplay}; +use std::num::NonZeroUsize; use std::path::{Path, PathBuf}; use std::sync::Arc; use std::time::Instant; @@ -61,11 +63,17 @@ use ui::{ KeyBinding, PopoverMenu, PopoverMenuHandle, TintColor, Tooltip, WithScrollbar, prelude::*, right_click_menu, }; -use util::{ResultExt, size::format_file_size, time::duration_alt_display}; -use util::{debug_panic, defer}; +use util::{ + ResultExt, debug_panic, defer, + paths::{PathStyle, PathWithPosition}, + rel_path::RelPath, + size::format_file_size, + time::duration_alt_display, +}; use workspace::PathList; use workspace::{ CollaboratorId, MultiWorkspace, NewTerminal, Toast, Workspace, notifications::NotificationId, + path_link::sanitize_path_text, }; use zed_actions::agent::{Chat, ToggleModelSelector}; use zed_actions::assistant::OpenRulesLibrary; @@ -509,6 +517,9 @@ pub struct ConversationView { /// causes mermaid diagrams to re-render). last_theme_id: Option, draft_prompt_persist_task: Option>, + /// Cache + worktree snapshot for resolving paths in markdown code spans. + /// Shared with the child [`ThreadView`] when one is constructed. + pub(crate) code_span_resolver: AgentCodeSpanResolver, _subscriptions: Vec, } @@ -707,7 +718,8 @@ impl ConversationView { cx: &mut Context, ) -> Self { let agent_server_store = project.read(cx).agent_server_store().clone(); - let subscriptions = vec![ + let code_span_resolver = AgentCodeSpanResolver::new(&project.downgrade(), cx); + let mut subscriptions = vec![ cx.observe_global_in::(window, Self::agent_ui_font_size_changed), cx.observe_global_in::(window, Self::invalidate_mermaid_caches), cx.observe_global_in::(window, Self::agent_ui_font_size_changed), @@ -718,6 +730,20 @@ impl ConversationView { Self::handle_agent_servers_updated, ), ]; + subscriptions.push(cx.subscribe(&project, { + let resolver = code_span_resolver.clone(); + move |_this: &mut Self, _project, event: &project::Event, cx| { + if matches!( + event, + project::Event::WorktreeAdded(_) + | project::Event::WorktreeRemoved(_) + | project::Event::WorktreeUpdatedEntries(_, _) + ) { + resolver.clear_cache(); + cx.notify(); + } + } + })); cx.on_release(|this, cx| { if let Some(connected) = this.as_connected() { @@ -764,6 +790,7 @@ impl ConversationView { auth_task: None, last_theme_id: Some(cx.theme().id.clone()), draft_prompt_persist_task: None, + code_span_resolver, _subscriptions: subscriptions, focus_handle: cx.focus_handle(), } @@ -1218,6 +1245,7 @@ impl ConversationView { session_capabilities, resumed_without_history, self.project.downgrade(), + self.code_span_resolver.clone(), self.thread_store.clone(), self.prompt_store.clone(), initial_content, @@ -2511,7 +2539,7 @@ impl ConversationView { markdown, style, &self.workspace, - &self.project.downgrade(), + &self.code_span_resolver, cx, ) } @@ -3118,20 +3146,12 @@ fn render_agent_markdown( markdown: Entity, style: MarkdownStyle, workspace: &WeakEntity, - project: &WeakEntity, + code_span_resolver: &AgentCodeSpanResolver, cx: &App, ) -> MarkdownElement { let workspace = workspace.clone(); - let worktree_roots: Vec = project - .upgrade() - .map(|project| { - project - .read(cx) - .visible_worktrees(cx) - .map(|worktree| worktree.read(cx).abs_path().to_path_buf()) - .collect() - }) - .unwrap_or_default(); + let worktree_roots = code_span_resolver.worktree_roots(cx); + let resolver = code_span_resolver.clone(); MarkdownElement::new(markdown, style) .code_block_renderer(markdown::CodeBlockRenderer::Default { copy_button_visibility: markdown::CopyButtonVisibility::VisibleOnHover, @@ -3142,6 +3162,175 @@ fn render_agent_markdown( .on_url_click(move |text, window, cx| { thread_view::open_link(text, &workspace, window, cx); }) + .on_code_span_link(move |text, cx| resolver.try_resolve(text, cx)) +} + +/// Shared, cloneable handle for resolving inline markdown code spans like +/// `` `src/main.rs:42` `` to clickable workspace file links. +#[derive(Clone)] +pub(crate) struct AgentCodeSpanResolver { + inner: Arc, +} + +/// Maximum number of memoized code-span resolutions kept in the cache. +const CODE_SPAN_CACHE_CAPACITY: NonZeroUsize = match NonZeroUsize::new(2048) { + Some(n) => n, + None => unreachable!(), +}; + +struct AgentCodeSpanResolverInner { + project: WeakEntity, + cache: Mutex, Option>>, +} + +impl AgentCodeSpanResolver { + pub(crate) fn new(project: &WeakEntity, _cx: &App) -> Self { + Self { + inner: Arc::new(AgentCodeSpanResolverInner { + project: project.clone(), + cache: Mutex::new(LruCache::new(CODE_SPAN_CACHE_CAPACITY)), + }), + } + } + + pub(crate) fn clear_cache(&self) { + self.inner.cache.lock().clear(); + } + + /// Absolute paths of every current worktree. + /// Used by the markdown image resolver, which needs the same set of roots. + fn worktree_roots(&self, cx: &App) -> Vec { + self.inner + .project + .upgrade() + .map(|project| { + project + .read(cx) + .visible_worktrees(cx) + .map(|worktree| worktree.read(cx).abs_path().to_path_buf()) + .collect() + }) + .unwrap_or_default() + } + + fn try_resolve(&self, text: &str, cx: &App) -> Option { + let trimmed = sanitize_path_text(text.trim()); + if !Self::is_path_like(trimmed) { + return None; + } + + if let Some(cached) = self.inner.cache.lock().get(trimmed).cloned() { + return cached; + } + + let resolved = self.resolve_uncached(trimmed, cx); + self.inner + .cache + .lock() + .push(Arc::from(trimmed), resolved.clone()); + resolved + } + + fn resolve_uncached(&self, trimmed: &str, cx: &App) -> Option { + let path_with_position = PathWithPosition::parse_str(trimmed); + let candidate_path = &path_with_position.path; + if candidate_path.as_os_str().is_empty() { + return None; + } + + let project = self.inner.project.upgrade()?; + let project = project.read(cx); + for worktree in project.visible_worktrees(cx) { + let worktree = worktree.read(cx); + for relative_path in Self::candidate_relative_paths( + candidate_path, + &worktree.abs_path(), + worktree.path_style(), + ) { + let project_path = ProjectPath { + worktree_id: worktree.id(), + path: relative_path.clone(), + }; + let Some(entry) = project.entry_for_path(&project_path, cx) else { + continue; + }; + if !entry.is_file() { + continue; + } + + let abs_path = worktree.absolutize(&relative_path); + let mention = match path_with_position.row.and_then(|row| row.checked_sub(1)) { + Some(line) => MentionUri::Selection { + abs_path: Some(abs_path), + line_range: line..=line, + column: path_with_position + .column + .map(|column| column.saturating_sub(1)), + }, + None => MentionUri::File { abs_path }, + }; + + return Some(mention.to_uri().to_string().into()); + } + } + + None + } + + fn candidate_relative_paths( + path: &Path, + worktree_abs_path: &Path, + path_style: PathStyle, + ) -> Vec> { + let path_text = path.to_string_lossy(); + let relative_path: Option> = + if util::paths::is_absolute(path_text.as_ref(), path_style) { + path_style + .strip_prefix(path, worktree_abs_path) + .map(std::borrow::Cow::into_owned) + .map(Into::into) + } else { + RelPath::new(path, path_style) + .ok() + .map(std::borrow::Cow::into_owned) + .map(Into::into) + }; + + let Some(relative_path) = relative_path else { + return Vec::new(); + }; + + let mut paths = vec![relative_path.clone()]; + if let Some(root_name) = worktree_abs_path.file_name().and_then(|name| name.to_str()) + && let Ok(root_name) = RelPath::new(Path::new(root_name), path_style) + && let Ok(stripped) = relative_path.strip_prefix(root_name.as_ref()) + && !stripped.is_empty() + { + paths.push(Arc::from(stripped)); + } + paths + } + + fn is_path_like(text: &str) -> bool { + if text.len() < 3 + || text.contains("://") + || text.contains('|') + || text.chars().any(char::is_control) + || text.chars().all(|character| character.is_ascii_digit()) + { + return false; + } + + let path = PathWithPosition::parse_str(text).path; + let path_text = path.to_string_lossy(); + if path_text.contains('/') || path_text.contains('\\') { + return true; + } + + path.extension() + .and_then(|extension| extension.to_str()) + .is_some_and(|extension| !extension.is_empty()) + } } fn plan_label_markdown_style( @@ -3256,6 +3445,82 @@ pub(crate) mod tests { }); } + #[gpui::test] + async fn test_agent_code_span_resolver_resolves_worktree_paths(cx: &mut TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + util::path!("/project"), + json!({ + "src": { + "main.rs": "" + }, + "README.md": "" + }), + ) + .await; + + let project = Project::test(fs, [Path::new(util::path!("/project"))], cx).await; + let resolver = cx.update(|cx| AgentCodeSpanResolver::new(&project.downgrade(), cx)); + + let uri = cx + .update(|cx| resolver.try_resolve("src/main.rs:10", cx)) + .expect("expected worktree-relative file path to resolve"); + assert_eq!( + MentionUri::parse(&uri, PathStyle::local()).unwrap(), + MentionUri::Selection { + abs_path: Some(PathBuf::from(util::path!("/project/src/main.rs"))), + line_range: 9..=9, + column: None, + } + ); + + let uri = cx + .update(|cx| resolver.try_resolve("src/main.rs:10:5", cx)) + .expect("expected worktree-relative file path with row and column to resolve"); + assert_eq!( + MentionUri::parse(&uri, PathStyle::local()).unwrap(), + MentionUri::Selection { + abs_path: Some(PathBuf::from(util::path!("/project/src/main.rs"))), + line_range: 9..=9, + column: Some(4), + } + ); + + let uri = cx + .update(|cx| resolver.try_resolve("src/main.rs:0", cx)) + .expect("`:0` should fall back to a file mention instead of returning None"); + assert_eq!( + MentionUri::parse(&uri, PathStyle::local()).unwrap(), + MentionUri::File { + abs_path: PathBuf::from(util::path!("/project/src/main.rs")), + } + ); + + assert!(cx.update(|cx| resolver.try_resolve("String", cx)).is_none()); + assert!( + cx.update(|cx| resolver.try_resolve("does/not/exist.rs", cx)) + .is_none() + ); + assert!( + cx.update(|cx| resolver.try_resolve("src/main.rs.", cx)) + .is_some() + ); + + let uri = cx + .update(|cx| resolver.try_resolve("project/src/main.rs:10", cx)) + .expect("expected root-prefixed worktree path to resolve"); + assert_eq!( + MentionUri::parse(&uri, PathStyle::local()).unwrap(), + MentionUri::Selection { + abs_path: Some(PathBuf::from(util::path!("/project/src/main.rs"))), + line_range: 9..=9, + column: None, + } + ); + } + #[gpui::test] async fn test_notification_for_stop_event(cx: &mut TestAppContext) { init_test(cx); diff --git a/crates/agent_ui/src/conversation_view/thread_view.rs b/crates/agent_ui/src/conversation_view/thread_view.rs index 3b0c0dd37cc..43d850f8688 100644 --- a/crates/agent_ui/src/conversation_view/thread_view.rs +++ b/crates/agent_ui/src/conversation_view/thread_view.rs @@ -1,6 +1,7 @@ use crate::{ DEFAULT_THREAD_TITLE, SelectPermissionGranularity, agent_configuration::configure_context_server_modal::default_markdown_style, + open_abs_path_at_point, thread_metadata_store::{ThreadId, ThreadMetadataStore}, }; use agent_client_protocol::schema as acp; @@ -330,6 +331,10 @@ pub struct ThreadView { pub add_context_menu_handle: PopoverMenuHandle, pub thinking_effort_menu_handle: PopoverMenuHandle, pub project: WeakEntity, + /// Cache + worktree snapshot for resolving paths in markdown code spans. + /// Cloned from the parent `ConversationView` so the cache is shared and the + /// snapshot stays in sync via the parent's project-event subscription. + pub(crate) code_span_resolver: AgentCodeSpanResolver, pub show_external_source_prompt_warning: bool, pub show_codex_windows_warning: bool, pub multi_root_callout_dismissed: bool, @@ -382,6 +387,7 @@ impl ThreadView { session_capabilities: SharedSessionCapabilities, resumed_without_history: bool, project: WeakEntity, + code_span_resolver: AgentCodeSpanResolver, thread_store: Option>, prompt_store: Option>, initial_content: Option, @@ -449,6 +455,23 @@ impl ThreadView { && project.upgrade().is_some_and(|p| p.read(cx).is_local()) && agent_id.as_ref() == "Codex"; + if let Some(project) = project.upgrade() { + subscriptions.push(cx.subscribe(&project, { + let resolver = code_span_resolver.clone(); + move |_this: &mut Self, _project, event: &project::Event, cx| { + if matches!( + event, + project::Event::WorktreeAdded(_) + | project::Event::WorktreeRemoved(_) + | project::Event::WorktreeUpdatedEntries(_, _) + ) { + resolver.clear_cache(); + cx.notify(); + } + } + })); + } + let title_editor = { let metadata = ThreadMetadataStore::try_global(cx) .and_then(|store| store.read(cx).entry(root_thread_id).cloned()); @@ -601,6 +624,7 @@ impl ThreadView { add_context_menu_handle: PopoverMenuHandle::default(), thinking_effort_menu_handle: PopoverMenuHandle::default(), project, + code_span_resolver, show_external_source_prompt_warning, show_codex_windows_warning, multi_root_callout_dismissed: false, @@ -8701,7 +8725,13 @@ impl ThreadView { style: MarkdownStyle, cx: &App, ) -> MarkdownElement { - render_agent_markdown(markdown, style, &self.workspace, &self.project, cx) + render_agent_markdown( + markdown, + style, + &self.workspace, + &self.code_span_resolver, + cx, + ) } fn create_copy_button(&self, message: impl Into) -> impl IntoElement { @@ -9372,39 +9402,27 @@ pub(crate) fn open_link( abs_path: path, line_range, .. + } => { + open_abs_path_at_point( + workspace, + path, + Point::new(*line_range.start(), 0), + window, + cx, + ); } - | MentionUri::Selection { + MentionUri::Selection { abs_path: Some(path), line_range, + column, } => { - let project = workspace.project(); - let Some(path) = - project.update(cx, |project, cx| project.find_project_path(path, cx)) - else { - return; - }; - - let item = workspace.open_path(path, None, true, window, cx); - window - .spawn(cx, async move |cx| { - let Some(editor) = item.await?.downcast::() else { - return Ok(()); - }; - let range = - Point::new(*line_range.start(), 0)..Point::new(*line_range.start(), 0); - editor - .update_in(cx, |editor, window, cx| { - editor.change_selections( - SelectionEffects::scroll(Autoscroll::center()), - window, - cx, - |s| s.select_ranges(vec![range]), - ); - }) - .ok(); - anyhow::Ok(()) - }) - .detach_and_log_err(cx); + open_abs_path_at_point( + workspace, + path, + Point::new(*line_range.start(), column.unwrap_or(0)), + window, + cx, + ); } MentionUri::Selection { abs_path: None, .. } => {} MentionUri::Thread { id, name } => { diff --git a/crates/agent_ui/src/mention_set.rs b/crates/agent_ui/src/mention_set.rs index d1335d31811..beecb840f08 100644 --- a/crates/agent_ui/src/mention_set.rs +++ b/crates/agent_ui/src/mention_set.rs @@ -167,6 +167,7 @@ impl MentionSet { MentionUri::Selection { abs_path: Some(abs_path), line_range, + .. } => self.confirm_mention_for_symbol(abs_path, line_range, cx), MentionUri::Selection { abs_path: None, .. } => Task::ready(Err(anyhow!( "Untitled buffer selection mentions are not supported for paste" @@ -570,6 +571,7 @@ impl MentionSet { let uri = MentionUri::Selection { abs_path: abs_path.clone(), line_range: line_range.clone(), + column: None, }; let crease = crease_for_mention( selection_name(abs_path.as_deref(), &line_range).into(), @@ -805,6 +807,7 @@ mod tests { MentionUri::Selection { abs_path: Some(path!("/project/file.rs").into()), line_range: 1..=2, + column: None, }, false, http_client, diff --git a/crates/agent_ui/src/message_editor.rs b/crates/agent_ui/src/message_editor.rs index 90cbdffb6db..5b6b8671413 100644 --- a/crates/agent_ui/src/message_editor.rs +++ b/crates/agent_ui/src/message_editor.rs @@ -1119,6 +1119,7 @@ impl MessageEditor { let mention_uri = MentionUri::Selection { abs_path: Some(file_path.clone()), line_range: line_range.clone(), + column: None, }; let mention_text = mention_uri.as_link().to_string(); @@ -4397,10 +4398,12 @@ mod tests { let first_uri = MentionUri::Selection { abs_path: Some(path!("/project/file.rs").into()), line_range: 0..=1, + column: None, }; let second_uri = MentionUri::Selection { abs_path: Some(path!("/project/file.rs").into()), line_range: 2..=3, + column: None, }; source_message_editor.update_in(&mut cx, |message_editor, window, cx| { @@ -4558,10 +4561,12 @@ mod tests { let first_uri = MentionUri::Selection { abs_path: Some(path!("/project/file.rs").into()), line_range: 0..=1, + column: None, }; let second_uri = MentionUri::Selection { abs_path: Some(path!("/project/file.rs").into()), line_range: 2..=3, + column: None, }; let buffer_len = message_editor.update_in(&mut cx, |message_editor, window, cx| { diff --git a/crates/agent_ui/src/ui/mention_crease.rs b/crates/agent_ui/src/ui/mention_crease.rs index 41e3d5cef5d..6ce10698b5f 100644 --- a/crates/agent_ui/src/ui/mention_crease.rs +++ b/crates/agent_ui/src/ui/mention_crease.rs @@ -1,8 +1,8 @@ -use std::{ops::RangeInclusive, path::PathBuf, time::Duration}; +use std::{path::PathBuf, time::Duration}; use acp_thread::MentionUri; use agent_client_protocol::schema as acp; -use editor::{Editor, SelectionEffects, scroll::Autoscroll}; +use editor::Editor; use gpui::{ Animation, AnimationExt, AnyView, Context, IntoElement, TaskExt, WeakEntity, Window, pulsating_between, @@ -15,6 +15,8 @@ use theme_settings::ThemeSettings; use ui::{ButtonLike, TintColor, Tooltip, prelude::*}; use workspace::{OpenOptions, Workspace}; +use crate::open_abs_path_at_point; + #[derive(IntoElement)] pub struct MentionCrease { id: ElementId, @@ -165,12 +167,27 @@ fn open_mention_uri( abs_path, line_range, .. + } => { + open_file( + workspace, + abs_path, + Some(Point::new(*line_range.start(), 0)), + window, + cx, + ); } - | MentionUri::Selection { + MentionUri::Selection { abs_path: Some(abs_path), line_range, + column, } => { - open_file(workspace, abs_path, Some(line_range), window, cx); + open_file( + workspace, + abs_path, + Some(Point::new(*line_range.start(), column.unwrap_or(0))), + window, + cx, + ); } MentionUri::Directory { abs_path } => { reveal_in_project_panel(workspace, abs_path, cx); @@ -260,40 +277,23 @@ fn open_skill_file( fn open_file( workspace: &mut Workspace, abs_path: PathBuf, - line_range: Option>, + point: Option, window: &mut Window, cx: &mut Context, ) { - let project = workspace.project(); + if let Some(point) = point { + if open_abs_path_at_point(workspace, abs_path.clone(), point, window, cx) { + return; + } + } + let project = workspace.project(); if let Some(project_path) = project.update(cx, |project, cx| project.find_project_path(&abs_path, cx)) { - let item = workspace.open_path(project_path, None, true, window, cx); - if let Some(line_range) = line_range { - window - .spawn(cx, async move |cx| { - let Some(editor) = item.await?.downcast::() else { - return Ok(()); - }; - editor - .update_in(cx, |editor, window, cx| { - let range = Point::new(*line_range.start(), 0) - ..Point::new(*line_range.start(), 0); - editor.change_selections( - SelectionEffects::scroll(Autoscroll::center()), - window, - cx, - |selections| selections.select_ranges(vec![range]), - ); - }) - .ok(); - anyhow::Ok(()) - }) - .detach_and_log_err(cx); - } else { - item.detach_and_log_err(cx); - } + workspace + .open_path(project_path, None, true, window, cx) + .detach_and_log_err(cx); } else if abs_path.exists() { workspace .open_abs_path( diff --git a/crates/markdown/src/markdown.rs b/crates/markdown/src/markdown.rs index f8ca35d7225..4abce8a878f 100644 --- a/crates/markdown/src/markdown.rs +++ b/crates/markdown/src/markdown.rs @@ -53,6 +53,7 @@ use crate::parser::CodeBlockKind; /// A callback function that can be used to customize the style of links based on the destination URL. /// If the callback returns `None`, the default link style will be used. type LinkStyleCallback = Rc Option>; +pub type CodeSpanLinkCallback = Arc Option + 'static>; type SourceClickCallback = Box bool>; type CheckboxToggleCallback = Rc, bool, &mut Window, &mut App)>; @@ -1079,6 +1080,7 @@ pub struct MarkdownElement { style: MarkdownStyle, code_block_renderer: CodeBlockRenderer, on_url_click: Option>, + code_span_link: Option, on_source_click: Option, on_checkbox_toggle: Option, image_resolver: Option Option>>, @@ -1097,6 +1099,7 @@ impl MarkdownElement { border: false, }, on_url_click: None, + code_span_link: None, on_source_click: None, on_checkbox_toggle: None, image_resolver: None, @@ -1139,6 +1142,14 @@ impl MarkdownElement { self } + pub fn on_code_span_link( + mut self, + callback: impl Fn(&str, &App) -> Option + 'static, + ) -> Self { + self.code_span_link = Some(Arc::new(callback)); + self + } + pub fn on_source_click( mut self, handler: impl Fn(usize, usize, &mut Window, &mut App) -> bool + 'static, @@ -1173,6 +1184,41 @@ impl MarkdownElement { self } + fn push_markdown_code_span( + &self, + builder: &mut MarkdownElementBuilder, + text: &str, + range: Range, + cx: &App, + ) { + let link_url = if builder.code_block_stack.is_empty() && builder.link_depth == 0 { + self.code_span_link + .as_ref() + .and_then(|callback| callback(text, cx)) + } else { + None + }; + + if let Some(url) = link_url { + builder.push_link(url.clone(), range.clone()); + let link_style = self + .style + .link_callback + .as_ref() + .and_then(|callback| callback(url.as_ref(), cx)) + .unwrap_or_else(|| self.style.link.clone()); + builder.push_text_style(self.style.inline_code.clone()); + builder.push_text_style(link_style); + builder.push_text(text, range); + builder.pop_text_style(); + builder.pop_text_style(); + } else { + builder.push_text_style(self.style.inline_code.clone()); + builder.push_text(text, range); + builder.pop_text_style(); + } + } + fn push_markdown_image( &self, builder: &mut MarkdownElementBuilder, @@ -2013,6 +2059,7 @@ impl Element for MarkdownElement { } MarkdownTag::Link { dest_url, .. } => { if builder.code_block_stack.is_empty() { + builder.link_depth += 1; builder.push_link(dest_url.clone(), range.clone()); let style = self .style @@ -2239,6 +2286,7 @@ impl Element for MarkdownElement { MarkdownTagEnd::Strikethrough => builder.pop_text_style(), MarkdownTagEnd::Link => { if builder.code_block_stack.is_empty() { + builder.link_depth = builder.link_depth.saturating_sub(1); builder.pop_text_style() } } @@ -2273,9 +2321,12 @@ impl Element for MarkdownElement { builder.push_text(text, range.clone()); } MarkdownEvent::Code => { - builder.push_text_style(self.style.inline_code.clone()); - builder.push_text(&parsed_markdown.source[range.clone()], range.clone()); - builder.pop_text_style(); + self.push_markdown_code_span( + &mut builder, + &parsed_markdown.source[range.clone()], + range.clone(), + cx, + ); } MarkdownEvent::Html => { let html = &parsed_markdown.source[range.clone()]; @@ -2293,6 +2344,19 @@ impl Element for MarkdownElement { } MarkdownEvent::InlineHtml => { let html = &parsed_markdown.source[range.clone()]; + if let Some(code) = html + .strip_prefix("") + .and_then(|html| html.strip_suffix("")) + { + let code_start = range.start + "".len(); + self.push_markdown_code_span( + &mut builder, + code, + code_start..code_start + code.len(), + cx, + ); + continue; + } if html.starts_with("") { builder.push_text_style(self.style.inline_code.clone()); continue; @@ -2653,6 +2717,7 @@ struct MarkdownElementBuilder { base_text_style: TextStyle, text_style_stack: Vec, code_block_stack: Vec>>, + link_depth: usize, list_stack: Vec, table: TableState, syntax_theme: Arc, @@ -2691,6 +2756,7 @@ impl MarkdownElementBuilder { base_text_style, text_style_stack: Vec::new(), code_block_stack: Vec::new(), + link_depth: 0, list_stack: Vec::new(), table: TableState::default(), syntax_theme, @@ -3470,6 +3536,40 @@ mod tests { render_markdown_with_language_registry(markdown, None, cx) } + fn render_markdown_with_code_span_link( + markdown: &str, + callback: impl Fn(&str, &App) -> Option + 'static, + cx: &mut TestAppContext, + ) -> RenderedText { + struct TestWindow; + + impl Render for TestWindow { + fn render(&mut self, _: &mut Window, _: &mut Context) -> impl IntoElement { + div() + } + } + + ensure_theme_initialized(cx); + + let (_, cx) = cx.add_window_view(|_, _| TestWindow); + let markdown = cx.new(|cx| Markdown::new(markdown.to_string().into(), None, None, cx)); + cx.run_until_parked(); + let (rendered, _) = cx.draw( + Default::default(), + size(px(600.0), px(600.0)), + |_window, _cx| { + MarkdownElement::new(markdown, MarkdownStyle::default()) + .on_code_span_link(callback) + .code_block_renderer(CodeBlockRenderer::Default { + copy_button_visibility: CopyButtonVisibility::Hidden, + wrap_button_visibility: WrapButtonVisibility::Hidden, + border: false, + }) + }, + ); + rendered.text + } + fn render_markdown_with_language_registry( markdown: &str, language_registry: Option>, @@ -4105,6 +4205,50 @@ mod tests { assert!(rendered.link_for_source_index(5).is_none()); } + #[gpui::test] + fn test_code_span_link_detected_for_source_index(cx: &mut TestAppContext) { + let source = "see `foo.rs` for details"; + let rendered = render_markdown_with_code_span_link( + source, + |text, _cx| (text == "foo.rs").then(|| "file:///tmp/foo.rs".into()), + cx, + ); + + assert_eq!(rendered.links.len(), 1); + assert_eq!(rendered.links[0].destination_url, "file:///tmp/foo.rs"); + + let code_index = source.find("foo.rs").unwrap(); + let link = rendered.link_for_source_index(code_index); + assert!(link.is_some()); + assert_eq!(link.unwrap().destination_url, "file:///tmp/foo.rs"); + + assert!( + rendered + .link_for_source_index(source.find("see").unwrap()) + .is_none() + ); + } + + #[gpui::test] + fn test_code_span_link_ignores_code_without_callback(cx: &mut TestAppContext) { + let rendered = render_markdown("see `foo.rs` for details", cx); + + assert!(rendered.links.is_empty()); + } + + #[gpui::test] + fn test_code_span_link_ignores_code_inside_markdown_link(cx: &mut TestAppContext) { + let source = "see [`foo.rs`](https://example.com) for details"; + let rendered = render_markdown_with_code_span_link( + source, + |text, _cx| (text == "foo.rs").then(|| "file:///tmp/foo.rs".into()), + cx, + ); + + assert_eq!(rendered.links.len(), 1); + assert_eq!(rendered.links[0].destination_url, "https://example.com"); + } + #[gpui::test] fn test_context_menu_link_initial_state(cx: &mut TestAppContext) { struct TestWindow; diff --git a/crates/terminal_view/src/terminal_path_like_target.rs b/crates/terminal_view/src/terminal_path_like_target.rs index fb3abf41db7..e6e6e94bffc 100644 --- a/crates/terminal_view/src/terminal_path_like_target.rs +++ b/crates/terminal_view/src/terminal_path_like_target.rs @@ -1,71 +1,17 @@ use super::{HoverTarget, HoveredWord, TerminalView}; use anyhow::{Context as _, Result}; use editor::Editor; -use gpui::{App, AppContext, Context, Task, TaskExt, WeakEntity, Window}; -use itertools::Itertools; -use project::{Entry, Metadata}; +use gpui::{Context, Task, TaskExt, WeakEntity, Window}; use std::path::PathBuf; use terminal::PathLikeTarget; -use util::{ - ResultExt, debug_panic, - paths::{PathStyle, PathWithPosition, normalize_lexically}, - rel_path::RelPath, +use util::{ResultExt, debug_panic}; +#[cfg(not(test))] +use workspace::path_link::possible_open_target; +#[cfg(test)] +use workspace::path_link::{ + BackgroundFsChecks, OpenTargetFoundBy, possible_open_target_with_fs_checks, }; -use workspace::{OpenOptions, OpenVisible, Workspace}; - -/// The way we found the open target. This is important to have for test assertions. -/// For example, remote projects never look in the file system. -#[cfg(test)] -#[derive(Debug, Clone, Copy, Eq, PartialEq)] -enum OpenTargetFoundBy { - WorktreeExact, - WorktreeScan, - FileSystemBackground, -} - -#[cfg(test)] -#[derive(Debug, Clone, Copy, Eq, PartialEq)] -enum BackgroundFsChecks { - Enabled, - Disabled, -} - -#[derive(Debug, Clone)] -enum OpenTarget { - Worktree(PathWithPosition, Entry, #[cfg(test)] OpenTargetFoundBy), - File(PathWithPosition, Metadata), -} - -impl OpenTarget { - fn is_file(&self) -> bool { - match self { - OpenTarget::Worktree(_, entry, ..) => entry.is_file(), - OpenTarget::File(_, metadata) => !metadata.is_dir, - } - } - - fn is_dir(&self) -> bool { - match self { - OpenTarget::Worktree(_, entry, ..) => entry.is_dir(), - OpenTarget::File(_, metadata) => metadata.is_dir, - } - } - - fn path(&self) -> &PathWithPosition { - match self { - OpenTarget::Worktree(path, ..) => path, - OpenTarget::File(path, _) => path, - } - } - - #[cfg(test)] - fn found_by(&self) -> OpenTargetFoundBy { - match self { - OpenTarget::Worktree(.., found_by) => *found_by, - OpenTarget::File(..) => OpenTargetFoundBy::FileSystemBackground, - } - } -} +use workspace::{OpenOptions, OpenVisible, Workspace, path_link::OpenTarget}; pub(super) fn hover_path_like_target( workspace: &WeakEntity, @@ -96,11 +42,19 @@ fn possible_hover_target( cx: &mut Context, #[cfg(test)] background_fs_checks: BackgroundFsChecks, ) -> Task<()> { + #[cfg(not(test))] let file_to_open_task = possible_open_target( workspace, - path_like_target, + &path_like_target.maybe_path, + path_like_target.terminal_dir.as_deref(), + cx, + ); + #[cfg(test)] + let file_to_open_task = possible_open_target_with_fs_checks( + workspace, + &path_like_target.maybe_path, + path_like_target.terminal_dir.as_deref(), cx, - #[cfg(test)] background_fs_checks, ); cx.spawn(async move |terminal_view, cx| { @@ -122,297 +76,6 @@ fn possible_hover_target( }) } -fn possible_open_target( - workspace: &WeakEntity, - path_like_target: &PathLikeTarget, - cx: &App, - #[cfg(test)] background_fs_checks: BackgroundFsChecks, -) -> Task> { - let Some(workspace) = workspace.upgrade() else { - return Task::ready(None); - }; - // We have to check for both paths, as on Unix, certain paths with positions are valid file paths too. - // We can be on FS remote part, without real FS, so cannot canonicalize or check for existence the path right away. - let mut potential_paths = Vec::new(); - let cwd = path_like_target.terminal_dir.as_ref(); - let maybe_path = &path_like_target.maybe_path; - let original_path = PathWithPosition::from_path(PathBuf::from(maybe_path)); - let path_with_position = PathWithPosition::parse_str(maybe_path); - let worktree_candidates = workspace - .read(cx) - .worktrees(cx) - .sorted_by_key(|worktree| { - let worktree_root = worktree.read(cx).abs_path(); - match cwd.and_then(|cwd| worktree_root.strip_prefix(cwd).ok()) { - Some(cwd_child) => cwd_child.components().count(), - None => usize::MAX, - } - }) - .collect::>(); - // Since we do not check paths via FS and joining, we need to strip off potential `./`, `a/`, `b/` prefixes out of it. - const GIT_DIFF_PATH_PREFIXES: &[&str] = &["a", "b"]; - for prefix_str in GIT_DIFF_PATH_PREFIXES.iter().chain(std::iter::once(&".")) { - if let Some(stripped) = original_path.path.strip_prefix(prefix_str).ok() { - potential_paths.push(PathWithPosition { - path: stripped.to_owned(), - row: original_path.row, - column: original_path.column, - }); - } - if let Some(stripped) = path_with_position.path.strip_prefix(prefix_str).ok() { - potential_paths.push(PathWithPosition { - path: stripped.to_owned(), - row: path_with_position.row, - column: path_with_position.column, - }); - } - } - - let insert_both_paths = original_path != path_with_position; - potential_paths.insert(0, original_path); - if insert_both_paths { - potential_paths.insert(1, path_with_position); - } - - // If we won't find paths "easily", we can traverse the entire worktree to look what ends with the potential path suffix. - // That will be slow, though, so do the fast checks first. - let mut worktree_paths_to_check = Vec::new(); - let mut is_cwd_in_worktree = false; - let mut open_target = None; - 'worktree_loop: for worktree in &worktree_candidates { - let worktree_root = worktree.read(cx).abs_path(); - let mut paths_to_check = Vec::with_capacity(potential_paths.len()); - let relative_cwd = cwd - .and_then(|cwd| cwd.strip_prefix(&worktree_root).ok()) - .and_then(|cwd| RelPath::new(cwd, PathStyle::local()).ok()) - .and_then(|cwd_stripped| { - (cwd_stripped.as_ref() != RelPath::empty()).then(|| { - is_cwd_in_worktree = true; - cwd_stripped - }) - }); - - for path_with_position in &potential_paths { - let path_to_check = if worktree_root.ends_with(&path_with_position.path) { - let root_path_with_position = PathWithPosition { - path: worktree_root.to_path_buf(), - row: path_with_position.row, - column: path_with_position.column, - }; - match worktree.read(cx).root_entry() { - Some(root_entry) => { - open_target = Some(OpenTarget::Worktree( - root_path_with_position, - root_entry.clone(), - #[cfg(test)] - OpenTargetFoundBy::WorktreeExact, - )); - break 'worktree_loop; - } - None => root_path_with_position, - } - } else { - PathWithPosition { - path: path_with_position - .path - .strip_prefix(&worktree_root) - .unwrap_or(&path_with_position.path) - .to_owned(), - row: path_with_position.row, - column: path_with_position.column, - } - }; - - // Normalize the path by joining with cwd if available (handles `.` and `..` segments) - let normalized_path = if path_to_check.path.is_relative() { - relative_cwd.as_ref().and_then(|relative_cwd| { - let joined = relative_cwd - .as_ref() - .as_std_path() - .join(&path_to_check.path); - normalize_lexically(&joined).ok().and_then(|p| { - RelPath::new(&p, PathStyle::local()) - .ok() - .map(std::borrow::Cow::into_owned) - }) - }) - } else { - None - }; - let original_path = RelPath::new(&path_to_check.path, PathStyle::local()).ok(); - - if !worktree.read(cx).is_single_file() - && let Some(entry) = normalized_path - .as_ref() - .and_then(|p| worktree.read(cx).entry_for_path(p)) - .or_else(|| { - original_path - .as_ref() - .and_then(|p| worktree.read(cx).entry_for_path(p.as_ref())) - }) - { - open_target = Some(OpenTarget::Worktree( - PathWithPosition { - path: worktree.read(cx).absolutize(&entry.path), - row: path_to_check.row, - column: path_to_check.column, - }, - entry.clone(), - #[cfg(test)] - OpenTargetFoundBy::WorktreeExact, - )); - break 'worktree_loop; - } - - paths_to_check.push(path_to_check); - } - - if !paths_to_check.is_empty() { - worktree_paths_to_check.push((worktree.clone(), paths_to_check)); - } - } - - #[cfg(not(test))] - let enable_background_fs_checks = workspace.read(cx).project().read(cx).is_local(); - #[cfg(test)] - let enable_background_fs_checks = background_fs_checks == BackgroundFsChecks::Enabled; - - if open_target.is_some() { - // We we want to prefer open targets found via background fs checks over worktree matches, - // however we can return early if either: - // - This is a remote project, or - // - If the terminal working directory is inside of at least one worktree - if !enable_background_fs_checks || is_cwd_in_worktree { - return Task::ready(open_target); - } - } - - // Before entire worktree traversal(s), make an attempt to do FS checks if available. - let fs_paths_to_check = - if enable_background_fs_checks { - let fs_cwd_paths_to_check = cwd - .iter() - .flat_map(|cwd| { - let mut paths_to_check = Vec::new(); - for path_to_check in &potential_paths { - let maybe_path = &path_to_check.path; - if path_to_check.path.is_relative() { - paths_to_check.push(PathWithPosition { - path: cwd.join(&maybe_path), - row: path_to_check.row, - column: path_to_check.column, - }); - } - } - paths_to_check - }) - .collect::>(); - fs_cwd_paths_to_check - .into_iter() - .chain( - potential_paths - .into_iter() - .flat_map(|path_to_check| { - let mut paths_to_check = Vec::new(); - let maybe_path = &path_to_check.path; - if maybe_path.starts_with("~") { - if let Some(home_path) = maybe_path.strip_prefix("~").ok().and_then( - |stripped_maybe_path| { - Some(dirs::home_dir()?.join(stripped_maybe_path)) - }, - ) { - paths_to_check.push(PathWithPosition { - path: home_path, - row: path_to_check.row, - column: path_to_check.column, - }); - } - } else { - paths_to_check.push(PathWithPosition { - path: maybe_path.clone(), - row: path_to_check.row, - column: path_to_check.column, - }); - if maybe_path.is_relative() { - for worktree in &worktree_candidates { - if !worktree.read(cx).is_single_file() { - paths_to_check.push(PathWithPosition { - path: worktree.read(cx).abs_path().join(maybe_path), - row: path_to_check.row, - column: path_to_check.column, - }); - } - } - } - } - paths_to_check - }) - .collect::>(), - ) - .collect() - } else { - Vec::new() - }; - - let fs = workspace.read(cx).project().read(cx).fs().clone(); - let background_fs_checks_task = cx.background_spawn(async move { - for mut path_to_check in fs_paths_to_check { - if let Some(fs_path_to_check) = fs.canonicalize(&path_to_check.path).await.ok() - && let Some(metadata) = fs.metadata(&fs_path_to_check).await.ok().flatten() - { - if open_target - .as_ref() - .map(|open_target| open_target.path().path != fs_path_to_check) - .unwrap_or(true) - { - path_to_check.path = fs_path_to_check; - return Some(OpenTarget::File(path_to_check, metadata)); - } - - break; - } - } - - open_target - }); - - cx.spawn(async move |cx| { - background_fs_checks_task.await.or_else(|| { - for (worktree, worktree_paths_to_check) in worktree_paths_to_check { - if let Some(found_entry) = - worktree.update(cx, |worktree, _| -> Option { - let traversal = - worktree.traverse_from_path(true, true, false, RelPath::empty()); - for entry in traversal { - if let Some(path_in_worktree) = - worktree_paths_to_check.iter().find(|path_to_check| { - RelPath::new(&path_to_check.path, PathStyle::local()) - .is_ok_and(|path| entry.path.ends_with(&path)) - }) - { - return Some(OpenTarget::Worktree( - PathWithPosition { - path: worktree.absolutize(&entry.path), - row: path_in_worktree.row, - column: path_in_worktree.column, - }, - entry.clone(), - #[cfg(test)] - OpenTargetFoundBy::WorktreeScan, - )); - } - } - None - }) - { - return Some(found_entry); - } - } - None - }) - }) -} - pub(super) fn open_path_like_target( workspace: &WeakEntity, terminal_view: &mut TerminalView, @@ -455,13 +118,25 @@ fn possibly_open_target( cx.spawn_in(window, async move |terminal_view, cx| { let Some(open_target) = terminal_view .update(cx, |_, cx| { - possible_open_target( - &workspace, - &path_like_target, - cx, - #[cfg(test)] - background_fs_checks, - ) + #[cfg(not(test))] + { + possible_open_target( + &workspace, + &path_like_target.maybe_path, + path_like_target.terminal_dir.as_deref(), + cx, + ) + } + #[cfg(test)] + { + possible_open_target_with_fs_checks( + &workspace, + &path_like_target.maybe_path, + path_like_target.terminal_dir.as_deref(), + cx, + background_fs_checks, + ) + } })? .await else { @@ -530,7 +205,7 @@ fn possibly_open_target( #[cfg(test)] mod tests { use super::*; - use gpui::TestAppContext; + use gpui::{AppContext as _, TestAppContext}; use project::Project; use serde_json::json; use std::path::{Path, PathBuf}; @@ -540,6 +215,7 @@ mod tests { terminal_settings::{AlternateScroll, CursorShape}, }; use util::path; + use util::paths::PathStyle; use workspace::{AppState, MultiWorkspace}; async fn init_test( diff --git a/crates/workspace/Cargo.toml b/crates/workspace/Cargo.toml index 0200f1b7d57..4bee4ba8c08 100644 --- a/crates/workspace/Cargo.toml +++ b/crates/workspace/Cargo.toml @@ -36,6 +36,7 @@ clock.workspace = true collections.workspace = true component.workspace = true db.workspace = true +dirs.workspace = true futures-lite.workspace = true fs.workspace = true futures.workspace = true diff --git a/crates/workspace/src/path_link.rs b/crates/workspace/src/path_link.rs new file mode 100644 index 00000000000..e707a7de603 --- /dev/null +++ b/crates/workspace/src/path_link.rs @@ -0,0 +1,422 @@ +use crate::Workspace; +use gpui::{App, AppContext, Task, WeakEntity}; +use itertools::Itertools; +use project::{Entry, Metadata}; +use std::path::{Path, PathBuf}; +use util::{ + paths::{PathStyle, PathWithPosition, normalize_lexically}, + rel_path::RelPath, +}; + +#[cfg(any(test, feature = "test-support"))] +#[derive(Debug, Clone, Copy, Eq, PartialEq)] +pub enum OpenTargetFoundBy { + WorktreeExact, + WorktreeScan, + FileSystemBackground, +} + +#[derive(Debug, Clone, Copy, Eq, PartialEq)] +pub enum BackgroundFsChecks { + Enabled, + Disabled, +} + +#[derive(Debug, Clone)] +pub enum OpenTarget { + Worktree( + PathWithPosition, + Entry, + #[cfg(any(test, feature = "test-support"))] OpenTargetFoundBy, + ), + File(PathWithPosition, Metadata), +} + +impl OpenTarget { + pub fn is_file(&self) -> bool { + match self { + OpenTarget::Worktree(_, entry, ..) => entry.is_file(), + OpenTarget::File(_, metadata) => !metadata.is_dir, + } + } + + pub fn is_dir(&self) -> bool { + match self { + OpenTarget::Worktree(_, entry, ..) => entry.is_dir(), + OpenTarget::File(_, metadata) => metadata.is_dir, + } + } + + pub fn path(&self) -> &PathWithPosition { + match self { + OpenTarget::Worktree(path, ..) => path, + OpenTarget::File(path, _) => path, + } + } + + #[cfg(any(test, feature = "test-support"))] + pub fn found_by(&self) -> OpenTargetFoundBy { + match self { + OpenTarget::Worktree(.., found_by) => *found_by, + OpenTarget::File(..) => OpenTargetFoundBy::FileSystemBackground, + } + } +} + +pub fn sanitize_path_text(text: &str) -> &str { + let start = first_unbalanced_open_paren(text).unwrap_or(0); + let mut sanitized = &text[start..]; + let (open_parens, mut close_parens) = + sanitized + .chars() + .fold((0, 0), |(opens, closes), character| match character { + '(' => (opens + 1, closes), + ')' => (opens, closes + 1), + _ => (opens, closes), + }); + + while let Some(last_char) = sanitized.chars().last() { + let should_remove = match last_char { + '.' | ',' | ':' | ';' => true, + '(' => true, + ')' if close_parens > open_parens => { + close_parens -= 1; + true + } + _ => false, + }; + + if should_remove { + sanitized = &sanitized[..sanitized.len() - last_char.len_utf8()]; + } else { + break; + } + } + + sanitized +} + +/// Returns the byte offset just past the first unbalanced `(` in `text`, or +/// `None` if all parentheses are balanced. +pub fn first_unbalanced_open_paren(text: &str) -> Option { + let mut balance: i32 = 0; + let mut first_unmatched = None; + for (index, character) in text.char_indices() { + match character { + '(' => { + if balance == 0 { + first_unmatched = Some(index + character.len_utf8()); + } + balance += 1; + } + ')' => { + balance -= 1; + if balance <= 0 { + balance = 0; + first_unmatched = None; + } + } + _ => {} + } + } + first_unmatched.filter(|_| balance > 0) +} + +pub fn possible_open_target( + workspace: &WeakEntity, + maybe_path: &str, + cwd: Option<&Path>, + cx: &App, +) -> Task> { + possible_open_target_internal(workspace, maybe_path, cwd, cx, None) +} + +#[cfg(any(test, feature = "test-support"))] +pub fn possible_open_target_with_fs_checks( + workspace: &WeakEntity, + maybe_path: &str, + cwd: Option<&Path>, + cx: &App, + background_fs_checks: BackgroundFsChecks, +) -> Task> { + possible_open_target_internal(workspace, maybe_path, cwd, cx, Some(background_fs_checks)) +} + +fn possible_open_target_internal( + workspace: &WeakEntity, + maybe_path: &str, + cwd: Option<&Path>, + cx: &App, + background_fs_checks: Option, +) -> Task> { + let Some(workspace) = workspace.upgrade() else { + return Task::ready(None); + }; + + let mut potential_paths = Vec::new(); + let original_path = PathWithPosition::from_path(PathBuf::from(maybe_path)); + let path_with_position = PathWithPosition::parse_str(maybe_path); + let worktree_candidates = workspace + .read(cx) + .worktrees(cx) + .sorted_by_key(|worktree| { + let worktree_root = worktree.read(cx).abs_path(); + match cwd.and_then(|cwd| worktree_root.strip_prefix(cwd).ok()) { + Some(cwd_child) => cwd_child.components().count(), + None => usize::MAX, + } + }) + .collect::>(); + + const GIT_DIFF_PATH_PREFIXES: &[&str] = &["a", "b"]; + for prefix_str in GIT_DIFF_PATH_PREFIXES.iter().chain(std::iter::once(&".")) { + if let Some(stripped) = original_path.path.strip_prefix(prefix_str).ok() { + potential_paths.push(PathWithPosition { + path: stripped.to_owned(), + row: original_path.row, + column: original_path.column, + }); + } + if let Some(stripped) = path_with_position.path.strip_prefix(prefix_str).ok() { + potential_paths.push(PathWithPosition { + path: stripped.to_owned(), + row: path_with_position.row, + column: path_with_position.column, + }); + } + } + + let insert_both_paths = original_path != path_with_position; + potential_paths.insert(0, original_path); + if insert_both_paths { + potential_paths.insert(1, path_with_position); + } + + let mut worktree_paths_to_check = Vec::new(); + let mut is_cwd_in_worktree = false; + let mut open_target = None; + 'worktree_loop: for worktree in &worktree_candidates { + let worktree_root = worktree.read(cx).abs_path(); + let mut paths_to_check = Vec::with_capacity(potential_paths.len()); + let relative_cwd = cwd + .and_then(|cwd| cwd.strip_prefix(&worktree_root).ok()) + .and_then(|cwd| RelPath::new(cwd, PathStyle::local()).ok()) + .and_then(|cwd_stripped| { + (cwd_stripped.as_ref() != RelPath::empty()).then(|| { + is_cwd_in_worktree = true; + cwd_stripped + }) + }); + + for path_with_position in &potential_paths { + let path_to_check = if worktree_root.ends_with(&path_with_position.path) { + let root_path_with_position = PathWithPosition { + path: worktree_root.to_path_buf(), + row: path_with_position.row, + column: path_with_position.column, + }; + match worktree.read(cx).root_entry() { + Some(root_entry) => { + open_target = Some(OpenTarget::Worktree( + root_path_with_position, + root_entry.clone(), + #[cfg(any(test, feature = "test-support"))] + OpenTargetFoundBy::WorktreeExact, + )); + break 'worktree_loop; + } + None => root_path_with_position, + } + } else { + PathWithPosition { + path: path_with_position + .path + .strip_prefix(&worktree_root) + .unwrap_or(&path_with_position.path) + .to_owned(), + row: path_with_position.row, + column: path_with_position.column, + } + }; + + let normalized_path = if path_to_check.path.is_relative() { + relative_cwd.as_ref().and_then(|relative_cwd| { + let joined = relative_cwd + .as_ref() + .as_std_path() + .join(&path_to_check.path); + normalize_lexically(&joined).ok().and_then(|path| { + RelPath::new(&path, PathStyle::local()) + .ok() + .map(std::borrow::Cow::into_owned) + }) + }) + } else { + None + }; + let original_path = RelPath::new(&path_to_check.path, PathStyle::local()).ok(); + + if !worktree.read(cx).is_single_file() + && let Some(entry) = normalized_path + .as_ref() + .and_then(|path| worktree.read(cx).entry_for_path(path)) + .or_else(|| { + original_path + .as_ref() + .and_then(|path| worktree.read(cx).entry_for_path(path.as_ref())) + }) + { + open_target = Some(OpenTarget::Worktree( + PathWithPosition { + path: worktree.read(cx).absolutize(&entry.path), + row: path_to_check.row, + column: path_to_check.column, + }, + entry.clone(), + #[cfg(any(test, feature = "test-support"))] + OpenTargetFoundBy::WorktreeExact, + )); + break 'worktree_loop; + } + + paths_to_check.push(path_to_check); + } + + if !paths_to_check.is_empty() { + worktree_paths_to_check.push((worktree.clone(), paths_to_check)); + } + } + + let enable_background_fs_checks = background_fs_checks + .map(|background_fs_checks| background_fs_checks == BackgroundFsChecks::Enabled) + .unwrap_or_else(|| workspace.read(cx).project().read(cx).is_local()); + + if open_target.is_some() { + if !enable_background_fs_checks || is_cwd_in_worktree { + return Task::ready(open_target); + } + } + + let fs_paths_to_check = if enable_background_fs_checks { + let fs_cwd_paths_to_check = cwd + .iter() + .flat_map(|cwd| { + let mut paths_to_check = Vec::new(); + for path_to_check in &potential_paths { + let maybe_path = &path_to_check.path; + if path_to_check.path.is_relative() { + paths_to_check.push(PathWithPosition { + path: cwd.join(maybe_path), + row: path_to_check.row, + column: path_to_check.column, + }); + } + } + paths_to_check + }) + .collect::>(); + fs_cwd_paths_to_check + .into_iter() + .chain( + potential_paths + .into_iter() + .flat_map(|path_to_check| { + let mut paths_to_check = Vec::new(); + let maybe_path = &path_to_check.path; + if maybe_path.starts_with("~") { + if let Some(home_path) = maybe_path + .strip_prefix("~") + .ok() + .and_then(|stripped| Some(dirs::home_dir()?.join(stripped))) + { + paths_to_check.push(PathWithPosition { + path: home_path, + row: path_to_check.row, + column: path_to_check.column, + }); + } + } else { + paths_to_check.push(PathWithPosition { + path: maybe_path.clone(), + row: path_to_check.row, + column: path_to_check.column, + }); + if maybe_path.is_relative() { + for worktree in &worktree_candidates { + if !worktree.read(cx).is_single_file() { + paths_to_check.push(PathWithPosition { + path: worktree.read(cx).abs_path().join(maybe_path), + row: path_to_check.row, + column: path_to_check.column, + }); + } + } + } + } + paths_to_check + }) + .collect::>(), + ) + .collect() + } else { + Vec::new() + }; + + let fs = workspace.read(cx).project().read(cx).fs().clone(); + let background_fs_checks_task = cx.background_spawn(async move { + for mut path_to_check in fs_paths_to_check { + if let Some(fs_path_to_check) = fs.canonicalize(&path_to_check.path).await.ok() + && let Some(metadata) = fs.metadata(&fs_path_to_check).await.ok().flatten() + { + if open_target + .as_ref() + .map(|open_target| open_target.path().path != fs_path_to_check) + .unwrap_or(true) + { + path_to_check.path = fs_path_to_check; + return Some(OpenTarget::File(path_to_check, metadata)); + } + + break; + } + } + + open_target + }); + + cx.spawn(async move |cx| { + background_fs_checks_task.await.or_else(|| { + for (worktree, worktree_paths_to_check) in worktree_paths_to_check { + if let Some(found_entry) = + worktree.update(cx, |worktree, _| -> Option { + let traversal = + worktree.traverse_from_path(true, true, false, RelPath::empty()); + for entry in traversal { + if let Some(path_in_worktree) = + worktree_paths_to_check.iter().find(|path_to_check| { + RelPath::new(&path_to_check.path, PathStyle::local()) + .is_ok_and(|path| entry.path.ends_with(&path)) + }) + { + return Some(OpenTarget::Worktree( + PathWithPosition { + path: worktree.absolutize(&entry.path), + row: path_in_worktree.row, + column: path_in_worktree.column, + }, + entry.clone(), + #[cfg(any(test, feature = "test-support"))] + OpenTargetFoundBy::WorktreeScan, + )); + } + } + None + }) + { + return Some(found_entry); + } + } + None + }) + }) +} diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index b3bafd0bb6b..dbe132add88 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -13,6 +13,7 @@ pub mod pane_group; pub mod path_list { pub use util::path_list::{PathList, SerializedPathList}; } +pub mod path_link; mod persistence; pub mod searchable; pub mod security_modal;