From 298b3e8d7b37da657df4310d238e997f377cbe8d Mon Sep 17 00:00:00 2001 From: MartinYe1234 <52641447+MartinYe1234@users.noreply.github.com> Date: Tue, 21 Jul 2026 14:55:19 +0000 Subject: [PATCH] Open relative file links at line numbers (#61006) ## Summary - Open relative file links with GitHub-style line fragments at the referenced line - Support line navigation from agent responses and Markdown previews - Preserve decoded-first path resolution with literal percent-escape fallback ## Testing - cargo test -q -p util test_source_line_from_fragment -- --nocapture - cargo test -q -p acp_thread test_hyperlink_percent_escapes_are_decoded -- --nocapture - cargo test -q -p agent_ui test_open_link -- --nocapture - cargo test -q -p markdown_preview -- --nocapture - cargo fmt --all -- --check Release Notes: - Improved relative file links with line numbers to open at the referenced line. --- crates/acp_thread/src/mention.rs | 2 +- .../src/conversation_view/thread_view.rs | 79 +++++++++- crates/editor/src/hover_links.rs | 74 +-------- .../src/markdown_preview_view.rs | 149 ++++++++++++++++-- crates/util/src/markdown.rs | 29 ++++ 5 files changed, 244 insertions(+), 89 deletions(-) diff --git a/crates/acp_thread/src/mention.rs b/crates/acp_thread/src/mention.rs index 3686f2955c..f52e281212 100644 --- a/crates/acp_thread/src/mention.rs +++ b/crates/acp_thread/src/mention.rs @@ -709,7 +709,7 @@ fn normalize_path_mention( /// traverses. Invalid sequences and non-UTF-8 results leave the input /// unchanged. Returns `Cow::Owned` iff decoding changed the input /// (`parse_hyperlink_literal` relies on this). -fn decode_path_escapes(input: &str) -> Cow<'_, str> { +pub fn decode_path_escapes(input: &str) -> Cow<'_, str> { fn hex_digit(byte: u8) -> Option { match byte { b'0'..=b'9' => Some(byte - b'0'), diff --git a/crates/agent_ui/src/conversation_view/thread_view.rs b/crates/agent_ui/src/conversation_view/thread_view.rs index ffec927c09..d1e90180fc 100644 --- a/crates/agent_ui/src/conversation_view/thread_view.rs +++ b/crates/agent_ui/src/conversation_view/thread_view.rs @@ -10,7 +10,7 @@ use std::cell::RefCell; use acp_thread::{ Elicitation, ElicitationEntryId, ElicitationStatus, PlanEntry, SandboxAuthorizationDetails, - SandboxFallbackAuthorizationDetails, SandboxNotAppliedReason, + SandboxFallbackAuthorizationDetails, SandboxNotAppliedReason, decode_path_escapes, }; use agent::{ SandboxStatusKey, SandboxStatusRefresh, SkillLoadingIssue, SkillLoadingIssueKind, @@ -45,6 +45,7 @@ use ui::{ ButtonLike, CalloutBorderPosition, Checkbox, SpinnerLabel, SpinnerVariant, SplitButton, SplitButtonStyle, Tab, ToggleState, }; +use util::markdown::{source_position_from_fragment, split_local_url_fragment}; use workspace::{OpenOptions, SERIALIZATION_THROTTLE_TIME}; use super::elicitation::{ @@ -12287,6 +12288,33 @@ pub(crate) fn open_link( }; let path_style = workspace.read(cx).path_style(cx); + let (relative_path, fragment) = split_local_url_fragment(&url); + if let Some(fragment) = fragment + && !relative_path.is_empty() + && !path_style.is_absolute(relative_path) + { + let project = workspace.read(cx).project().clone(); + let decoded_path = decode_path_escapes(relative_path); + let abs_path = project.update(cx, |project, cx| { + let resolve_path = |path: &str| { + let project_path = project.find_project_path(path, cx)?; + project.entry_for_path(&project_path, cx)?; + project.absolute_path(&project_path, cx) + }; + resolve_path(&decoded_path).or_else(|| resolve_path(relative_path)) + }); + if let Some(abs_path) = abs_path { + let point = fragment + .strip_prefix('L') + .and_then(source_position_from_fragment) + .map(|(row, _)| Point::new(row, 0)); + workspace.update(cx, |workspace, cx| { + open_abs_path_at_point(workspace, abs_path, point, window, cx); + }); + return; + } + } + if let Some(mention) = MentionUri::parse_hyperlink(&url, path_style).log_err() { // Percent escapes in bare paths are ambiguous: prefer the decoded // interpretation, falling back to the literal one (e.g. a file @@ -12503,8 +12531,11 @@ mod tests { crate::test_support::init_test(cx); let fs = FakeFs::new(cx.executor()); - fs.insert_tree(path!("/project"), json!({"src": {"main.rs": ""}})) - .await; + fs.insert_tree( + path!("/project"), + json!({"src": {"main.rs": "first\nsecond\nthird\n"}}), + ) + .await; let project = Project::test(fs, [path!("/project").as_ref()], cx).await; let (multi_workspace, cx) = @@ -12525,6 +12556,21 @@ mod tests { assert!(*active.path == *"src/main.rs"); }); + multi_workspace.update_in(cx, |_, window, cx| { + open_link("src/main.rs#L2".into(), &workspace_weak, window, cx); + }); + cx.run_until_parked(); + let editor = workspace.read_with(cx, |workspace, cx| { + workspace + .active_item(cx) + .and_then(|item| item.downcast::()) + .expect("file should be open in an editor") + }); + editor.update_in(cx, |editor, window, cx| { + let snapshot = editor.snapshot(window, cx); + assert_eq!(editor.selections.newest::(&snapshot).head().row, 1); + }); + // Absolute path let abs_path: SharedString = path!("/project/src/main.rs").to_string().into(); multi_workspace.update_in(cx, |_, window, cx| { @@ -12548,10 +12594,10 @@ mod tests { fs.insert_tree( path!("/project"), json!({ - "a%20b.rs": "literal", - "a b.rs": "decoded", - "c d.rs": "", - "e%20f.rs": "", + "a%20b.rs": "literal\nsecond\n", + "a b.rs": "decoded\nsecond\n", + "c d.rs": "first\nsecond\n", + "e%20f.rs": "first\nsecond\n", }), ) .await; @@ -12587,6 +12633,25 @@ mod tests { // Only the literally-named file exists: fall back to it. let path = open_link_and_active_path(path!("/project/e%20f.rs").to_string(), cx); assert_eq!(*path, *"e%20f.rs"); + + let path = open_link_and_active_path("a%20b.rs#L2".to_string(), cx); + assert_eq!(*path, *"a b.rs"); + + let path = open_link_and_active_path("c%20d.rs#L2".to_string(), cx); + assert_eq!(*path, *"c d.rs"); + + let path = open_link_and_active_path("e%20f.rs#L2".to_string(), cx); + assert_eq!(*path, *"e%20f.rs"); + let editor = workspace.read_with(cx, |workspace, cx| { + workspace + .active_item(cx) + .and_then(|item| item.downcast::()) + .expect("file should be open in an editor") + }); + editor.update_in(cx, |editor, window, cx| { + let snapshot = editor.snapshot(window, cx); + assert_eq!(editor.selections.newest::(&snapshot).head().row, 1); + }); } #[gpui::test] diff --git a/crates/editor/src/hover_links.rs b/crates/editor/src/hover_links.rs index 2f7fe2787d..b013797952 100644 --- a/crates/editor/src/hover_links.rs +++ b/crates/editor/src/hover_links.rs @@ -17,7 +17,9 @@ use settings::Settings; use std::{ops::Range, str::FromStr as _, sync::LazyLock}; use text::OffsetRangeExt; use theme::ActiveTheme as _; -use util::{ResultExt, TryFutureExt as _, paths::PathWithPosition}; +use util::{ + ResultExt, TryFutureExt as _, markdown::source_position_from_fragment, paths::PathWithPosition, +}; #[derive(Debug)] pub struct HoveredLinkState { @@ -85,7 +87,8 @@ pub fn document_link_target_to_hover_link(target: &str, server_id: LanguageServe { let position = url .fragment() - .and_then(parse_uri_fragment_position) + .and_then(source_position_from_fragment) + .map(|(line, character)| lsp::Position { line, character }) .unwrap_or_default(); return HoverLink::LspLocation( lsp::Location { @@ -98,24 +101,6 @@ pub fn document_link_target_to_hover_link(target: &str, server_id: LanguageServe HoverLink::Url(target.to_string()) } -/// Parse a URI fragment such as `9,16`, `9:16`, `L9`, or `L9:16` into an -/// LSP position (1-based input, 0-based output). Servers like the JSON -/// language server attach this fragment to `file://` document link -/// targets to point at a specific row/column inside the file. -fn parse_uri_fragment_position(fragment: &str) -> Option { - let stripped = fragment.strip_prefix('L').unwrap_or(fragment); - let (line_str, column_str) = match stripped.split_once([',', ':']) { - Some((line, column)) => (line, Some(column)), - None => (stripped, None), - }; - let line = line_str.parse::().ok()?.checked_sub(1)?; - let character = column_str - .and_then(|column| column.parse::().ok()) - .and_then(|column| column.checked_sub(1)) - .unwrap_or(0); - Some(lsp::Position { line, character }) -} - #[derive(Debug, Clone, PartialEq, Eq)] pub struct InlayHighlight { pub inlay: InlayId, @@ -1090,55 +1075,6 @@ mod tests { use util::{assert_set_eq, path}; use workspace::item::Item; - #[test] - fn test_parse_uri_fragment_position() { - // json-language-server style: 1-based `line,column`. - assert_eq!( - parse_uri_fragment_position("9,16"), - Some(lsp::Position { - line: 8, - character: 15, - }) - ); - assert_eq!( - parse_uri_fragment_position("33,33"), - Some(lsp::Position { - line: 32, - character: 32, - }) - ); - - // GitHub-style `L` and `L:`. - assert_eq!( - parse_uri_fragment_position("L42"), - Some(lsp::Position { - line: 41, - character: 0, - }) - ); - assert_eq!( - parse_uri_fragment_position("L42:7"), - Some(lsp::Position { - line: 41, - character: 6, - }) - ); - - // Bare line number, no column. - assert_eq!( - parse_uri_fragment_position("5"), - Some(lsp::Position { - line: 4, - character: 0, - }) - ); - - // Garbage / unparseable / 0-based fragments are rejected. - assert_eq!(parse_uri_fragment_position(""), None); - assert_eq!(parse_uri_fragment_position("section-name"), None); - assert_eq!(parse_uri_fragment_position("0,0"), None); - } - #[test] fn test_document_link_target_to_hover_link_file_uri_with_fragment() { let server_id = LanguageServerId(0); diff --git a/crates/markdown_preview/src/markdown_preview_view.rs b/crates/markdown_preview/src/markdown_preview_view.rs index b80fe288f7..bf9bfcf7a6 100644 --- a/crates/markdown_preview/src/markdown_preview_view.rs +++ b/crates/markdown_preview/src/markdown_preview_view.rs @@ -14,7 +14,7 @@ use gpui::{ InteractiveElement, IntoElement, IsZero, Pixels, Render, Resource, RetainAllImageCache, ScrollHandle, SharedString, SharedUri, Subscription, Task, WeakEntity, Window, point, px, }; -use language::LanguageRegistry; +use language::{LanguageRegistry, Point}; use markdown::{ CodeBlockRenderer, CopyButtonVisibility, Markdown, MarkdownElement, MarkdownFont, MarkdownOptions, MarkdownStyle, @@ -26,8 +26,10 @@ use theme::{SystemAppearance, Theme, ThemeRegistry}; use theme_settings::ThemeSettings; use ui::utils::WithRemSize; use ui::{ContextMenu, LinkPreview, WithScrollbar, prelude::*, right_click_menu}; -use util::ResultExt; -use util::markdown::split_local_url_fragment; +use util::{ + ResultExt, + markdown::{source_position_from_fragment, split_local_url_fragment}, +}; use workspace::item::{Item, ItemBufferKind, ItemHandle, SaveOptions, SerializableItem}; use workspace::notifications::NotifyResultExt; use workspace::searchable::{ @@ -1067,6 +1069,7 @@ fn handle_url_click( } else { open_preview_url( SharedString::from(path_part.to_string()), + fragment.map(|fragment| SharedString::from(fragment.to_string())), base_directory, workspace, window, @@ -1077,15 +1080,30 @@ fn handle_url_click( fn open_preview_url( url: SharedString, + fragment: Option, base_directory: Option, workspace: &WeakEntity, window: &mut Window, cx: &mut App, ) { - let (path_text, _) = split_preview_url(url.as_ref()); + let decoded_path = urlencoding::decode(&url).unwrap_or_else(|_| Cow::Borrowed(&url)); - // URL-decode the path for proper handling of encoded characters - let decoded_path = urlencoding::decode(path_text).unwrap_or_else(|_| Cow::Borrowed(path_text)); + if let Some(row) = fragment + .as_deref() + .and_then(|fragment| fragment.strip_prefix('L')) + .and_then(source_position_from_fragment) + .map(|(row, _)| row) + && open_preview_path_at_line( + decoded_path.to_string(), + base_directory.clone(), + workspace, + row, + window, + cx, + ) + { + return; + } if let Some(workspace) = workspace.upgrade() { workspace.update(cx, |workspace, cx| { @@ -1096,11 +1114,75 @@ fn open_preview_url( } } -fn split_preview_url(url: &str) -> (&str, Option<&str>) { - match url.split_once('#') { - Some((path, fragment)) => (path, Some(fragment)), - None => (url, None), - } +enum PreviewPathTarget { + Project(ProjectPath), + Absolute(PathBuf), +} + +fn open_preview_path_at_line( + path: String, + base_directory: Option, + workspace: &WeakEntity, + row: u32, + window: &mut Window, + cx: &mut App, +) -> bool { + let Some(workspace) = workspace.upgrade() else { + return false; + }; + let project = workspace.read(cx).project().clone(); + let path_style = project.read(cx).path_style(cx); + let target = if path_style.is_absolute(&path) { + Some(PreviewPathTarget::Absolute(PathBuf::from(path))) + } else if project.read(cx).is_local() + && let Some(resolved) = base_directory + .as_deref() + .map(|base_directory| base_directory.join(&path)) + && resolved.exists() + { + Some(PreviewPathTarget::Absolute(resolved)) + } else { + project + .update(cx, |project, cx| project.find_project_path(&path, cx)) + .map(PreviewPathTarget::Project) + }; + let Some(target) = target else { + return false; + }; + + let task = workspace.update(cx, |workspace, cx| match target { + PreviewPathTarget::Project(project_path) => { + workspace.open_path(project_path, None, true, window, cx) + } + PreviewPathTarget::Absolute(abs_path) => workspace.open_abs_path( + abs_path, + workspace::OpenOptions { + focus: Some(true), + ..Default::default() + }, + window, + cx, + ), + }); + window + .spawn(cx, async move |cx| { + let item = task.await?; + let Some(editor) = item.downcast::() else { + return anyhow::Ok(()); + }; + editor.update_in(cx, |editor, window, cx| { + let point = Point::new(row, 0); + editor.change_selections( + SelectionEffects::scroll(Autoscroll::center()), + window, + cx, + |selections| selections.select_ranges([point..point]), + ); + })?; + anyhow::Ok(()) + }) + .detach_and_log_err(cx); + true } fn resolve_preview_image( @@ -1721,6 +1803,8 @@ mod tests { use buffer_diff::BufferDiff; use editor::Editor; use gpui::{AppContext as _, Entity, Focusable as _, TestAppContext, WindowHandle}; + use language::Point; + use project::{FakeFs, Project}; use serde_json::json; use std::path::PathBuf; use std::sync::Arc; @@ -1733,7 +1817,7 @@ mod tests { AppState, ItemId, MultiWorkspace, SaveIntent, Workspace, WorkspaceId, open_paths, }; - use super::MarkdownPreviewView; + use super::{MarkdownPreviewView, open_preview_url}; #[test] fn resolves_workspace_absolute_preview_image_path_and_rejects_missing() { @@ -1762,6 +1846,47 @@ mod tests { assert!(missing.is_none()); } + #[gpui::test] + async fn opens_preview_file_link_at_line(cx: &mut TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + path!("/project"), + json!({"src": {"main.rs": "first\nsecond\nthird\n"}}), + ) + .await; + let project = Project::test(fs, [path!("/project").as_ref()], cx).await; + let (multi_workspace, cx) = + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project, window, cx)); + let workspace = + multi_workspace.read_with(cx, |multi_workspace, _| multi_workspace.workspace().clone()); + let workspace_weak = workspace.downgrade(); + + multi_workspace.update_in(cx, |_, window, cx| { + open_preview_url( + "src/main.rs".into(), + Some("L2".into()), + None, + &workspace_weak, + window, + cx, + ); + }); + cx.run_until_parked(); + + let editor = workspace.read_with(cx, |workspace, cx| { + workspace + .active_item(cx) + .and_then(|item| item.downcast::()) + .expect("file should be open in an editor") + }); + editor.update_in(cx, |editor, window, cx| { + let snapshot = editor.snapshot(window, cx); + assert_eq!(editor.selections.newest::(&snapshot).head().row, 1); + }); + } + #[gpui::test] async fn toggles_task_checkbox_and_saves_when_preview_is_active(cx: &mut TestAppContext) { let app_state = init_test(cx); diff --git a/crates/util/src/markdown.rs b/crates/util/src/markdown.rs index e42ce13b59..8c45f98bf8 100644 --- a/crates/util/src/markdown.rs +++ b/crates/util/src/markdown.rs @@ -57,6 +57,23 @@ pub fn split_local_url_fragment(url: &str) -> (&str, Option<&str>) { } } +pub fn source_position_from_fragment(fragment: &str) -> Option<(u32, u32)> { + let fragment = fragment.strip_prefix('L').unwrap_or(fragment); + let (line, column) = match fragment.split_once([',', ':']) { + Some((line, column)) => (line, Some(column)), + None => ( + fragment.split_once('-').map_or(fragment, |(line, _)| line), + None, + ), + }; + let line = line.parse::().ok()?.checked_sub(1)?; + let column = column + .and_then(|column| column.parse::().ok()) + .and_then(|column| column.checked_sub(1)) + .unwrap_or(0); + Some((line, column)) +} + /// Indicates that the wrapped `String` is markdown text. #[derive(Debug, Clone)] pub struct MarkdownString(pub String); @@ -350,6 +367,18 @@ mod tests { ); } + #[test] + fn test_source_position_from_fragment() { + assert_eq!(source_position_from_fragment("9,16"), Some((8, 15))); + assert_eq!(source_position_from_fragment("33,33"), Some((32, 32))); + assert_eq!(source_position_from_fragment("L42"), Some((41, 0))); + assert_eq!(source_position_from_fragment("L42:7"), Some((41, 6))); + assert_eq!(source_position_from_fragment("5"), Some((4, 0))); + assert_eq!(source_position_from_fragment("L10-L20"), Some((9, 0))); + assert_eq!(source_position_from_fragment("heading"), None); + assert_eq!(source_position_from_fragment("0,0"), None); + } + #[test] fn test_generate_heading_slug() { assert_eq!(generate_heading_slug("Hello World"), "hello-world");