From 84ebc5be714180553c02e5f90e72fd394715a00a Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Fri, 5 Jun 2026 13:17:44 +0200 Subject: [PATCH] Correctly handle file links in markdown and agent threads (#56024) Closes https://github.com/zed-industries/zed/issues/54840 There are likely more places we can use this new workspace function in, but these are the known ones to me right now Release Notes: - Fixed file hyperlinks in the agent panel not opening files when clicked Co-authored-by: saberoueslati --- Cargo.lock | 1 + .../src/conversation_view/thread_view.rs | 55 ++++++- crates/diagnostics/src/diagnostic_renderer.rs | 3 +- crates/editor/src/code_context_menus.rs | 13 +- crates/editor/src/hover_popover.rs | 31 +++- crates/editor/src/signature_help.rs | 28 +++- .../src/markdown_preview_view.rs | 110 ++----------- crates/workspace/Cargo.toml | 1 + crates/workspace/src/notifications.rs | 10 +- crates/workspace/src/workspace.rs | 148 ++++++++++++++++++ 10 files changed, 292 insertions(+), 108 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index cc85216406..635d7c6c10 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -22364,6 +22364,7 @@ dependencies = [ "theme", "theme_settings", "ui", + "url", "util", "uuid", "windows 0.61.3", diff --git a/crates/agent_ui/src/conversation_view/thread_view.rs b/crates/agent_ui/src/conversation_view/thread_view.rs index ed0f75da9b..e0a72ee77e 100644 --- a/crates/agent_ui/src/conversation_view/thread_view.rs +++ b/crates/agent_ui/src/conversation_view/thread_view.rs @@ -10373,7 +10373,60 @@ pub(crate) fn open_link( } }) } else { - cx.open_url(&url); + workspace.update(cx, |workspace, cx| { + workspace.open_url_or_file(&url, None, window, cx); + }); + } +} + +#[cfg(test)] +mod tests { + use super::*; + use project::{FakeFs, Project}; + use serde_json::json; + use util::path; + use workspace::MultiWorkspace; + + #[gpui::test] + async fn test_open_link_bare_path(cx: &mut gpui::TestAppContext) { + crate::test_support::init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree(path!("/project"), json!({"src": {"main.rs": ""}})) + .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.clone(), window, cx)); + let workspace = multi_workspace.read_with(cx, |mw, _| mw.workspace().clone()); + let workspace_weak = workspace.downgrade(); + + // Relative path — call from multi_workspace so the inner workspace entity is not locked + multi_workspace.update_in(cx, |_, window, cx| { + open_link("src/main.rs".into(), &workspace_weak, window, cx); + }); + cx.run_until_parked(); + workspace.read_with(cx, |workspace, cx| { + let active = workspace + .active_item(cx) + .and_then(|item| item.project_path(cx)) + .expect("file should be open"); + assert!(*active.path == *"src/main.rs"); + }); + + // Absolute path + let abs_path: SharedString = path!("/project/src/main.rs").to_string().into(); + multi_workspace.update_in(cx, |_, window, cx| { + open_link(abs_path, &workspace_weak, window, cx); + }); + cx.run_until_parked(); + workspace.read_with(cx, |workspace, cx| { + let active = workspace + .active_item(cx) + .and_then(|item| item.project_path(cx)) + .expect("file should be open"); + assert!(*active.path == *"src/main.rs"); + }); } } diff --git a/crates/diagnostics/src/diagnostic_renderer.rs b/crates/diagnostics/src/diagnostic_renderer.rs index b86b691546..f44e6a4ee5 100644 --- a/crates/diagnostics/src/diagnostic_renderer.rs +++ b/crates/diagnostics/src/diagnostic_renderer.rs @@ -274,7 +274,8 @@ impl DiagnosticBlock { cx: &mut Context, ) { let Some(diagnostic_link) = link.strip_prefix("file://#diagnostic-") else { - editor::hover_popover::open_markdown_url(link, window, cx); + let workspace = editor.workspace(); + editor::hover_popover::open_markdown_url(workspace, link, window, cx); return; }; let Some((buffer_id, group_id, ix)) = maybe!({ diff --git a/crates/editor/src/code_context_menus.rs b/crates/editor/src/code_context_menus.rs index d54cc667c3..02634fcbd5 100644 --- a/crates/editor/src/code_context_menus.rs +++ b/crates/editor/src/code_context_menus.rs @@ -1256,6 +1256,7 @@ impl CompletionsMenu { window: &mut Window, cx: &mut Context, ) -> Div { + let editor = cx.weak_entity(); div().child( MarkdownElement::new(markdown, hover_markdown_style(window, cx)) .code_block_renderer(markdown::CodeBlockRenderer::Default { @@ -1263,7 +1264,17 @@ impl CompletionsMenu { wrap_button_visibility: markdown::WrapButtonVisibility::Hidden, border: false, }) - .on_url_click(open_markdown_url), + .on_url_click(move |link, window, cx| { + open_markdown_url( + editor + .read_with(cx, |editor, _| editor.workspace()) + .ok() + .flatten(), + link, + window, + cx, + ) + }), ) } diff --git a/crates/editor/src/hover_popover.rs b/crates/editor/src/hover_popover.rs index 53f24cc1fd..c915232ba5 100644 --- a/crates/editor/src/hover_popover.rs +++ b/crates/editor/src/hover_popover.rs @@ -797,10 +797,15 @@ pub fn diagnostics_markdown_style(window: &Window, cx: &App) -> MarkdownStyle { } } -pub fn open_markdown_url(link: SharedString, window: &mut Window, cx: &mut App) { +pub fn open_markdown_url( + workspace: Option>, + link: SharedString, + window: &mut Window, + cx: &mut App, +) { if let Ok(uri) = Url::parse(&link) && uri.scheme() == "file" - && let Some(workspace) = Workspace::for_window(window, cx) + && let Some(workspace) = workspace { workspace.update(cx, |workspace, cx| { let task = workspace.open_abs_path( @@ -847,7 +852,14 @@ pub fn open_markdown_url(link: SharedString, window: &mut Window, cx: &mut App) }); return; } - cx.open_url(&link); + + if let Some(workspace) = workspace { + workspace.update(cx, |workspace, cx| { + workspace.open_url_or_file(&link, None, window, cx); + }); + } else { + cx.open_url(&link); + } } #[derive(Default)] @@ -1033,6 +1045,7 @@ impl InfoPopover { ) -> AnyElement { let keyboard_grace = Rc::clone(&self.keyboard_grace); let this = cx.entity().downgrade(); + let this2 = this.clone(); let bounds_cell = self.last_bounds.clone(); div() .id("info_popover") @@ -1083,7 +1096,17 @@ impl InfoPopover { wrap_button_visibility: markdown::WrapButtonVisibility::Hidden, border: false, }) - .on_url_click(open_markdown_url) + .on_url_click(move |link, window, cx| { + open_markdown_url( + this2 + .read_with(cx, |editor, _| editor.workspace()) + .ok() + .flatten(), + link, + window, + cx, + ) + }) .p_2(), ), ) diff --git a/crates/editor/src/signature_help.rs b/crates/editor/src/signature_help.rs index 6ec2fe6152..99d2ed710b 100644 --- a/crates/editor/src/signature_help.rs +++ b/crates/editor/src/signature_help.rs @@ -382,6 +382,7 @@ impl SignatureHelpPopover { return div().into_any_element(); }; + let editor = cx.weak_entity(); let main_content = div() .occlude() .p_2() @@ -413,7 +414,20 @@ impl SignatureHelpPopover { markdown::WrapButtonVisibility::Hidden, border: false, }) - .on_url_click(open_markdown_url), + .on_url_click({ + let editor = editor.clone(); + move |link, window, cx| { + open_markdown_url( + editor + .read_with(cx, |editor, _| editor.workspace()) + .ok() + .flatten(), + link, + window, + cx, + ) + } + }), ) }, ) @@ -427,7 +441,17 @@ impl SignatureHelpPopover { markdown::WrapButtonVisibility::Hidden, border: false, }) - .on_url_click(open_markdown_url), + .on_url_click(move |link, window, cx| { + open_markdown_url( + editor + .read_with(cx, |editor, _| editor.workspace()) + .ok() + .flatten(), + link, + window, + cx, + ) + }), ) }), ) diff --git a/crates/markdown_preview/src/markdown_preview_view.rs b/crates/markdown_preview/src/markdown_preview_view.rs index 2db1e9b0a2..ec33f3d867 100644 --- a/crates/markdown_preview/src/markdown_preview_view.rs +++ b/crates/markdown_preview/src/markdown_preview_view.rs @@ -1,4 +1,5 @@ use std::any::TypeId; +use std::borrow::Cow; use std::cmp::min; use std::ops::Range; use std::path::{Path, PathBuf}; @@ -25,12 +26,11 @@ use theme::{SystemAppearance, Theme, ThemeRegistry}; use theme_settings::ThemeSettings; use ui::{ContextMenu, WithScrollbar, prelude::*, right_click_menu}; use util::markdown::split_local_url_fragment; -use util::normalize_path; use workspace::item::{Item, ItemBufferKind, ItemHandle, SaveOptions}; use workspace::searchable::{ Direction, SearchEvent, SearchOptions, SearchToken, SearchableItem, SearchableItemHandle, }; -use workspace::{OpenOptions, OpenVisible, Pane, Workspace}; +use workspace::{Pane, Workspace}; use crate::{ OpenFollowingPreview, OpenPreview, OpenPreviewToTheSide, ScrollDown, ScrollDownByItem, @@ -785,26 +785,16 @@ fn open_preview_url( ) { let (path_text, _) = split_preview_url(url.as_ref()); - if let Some(path) = resolve_preview_path(path_text, base_directory.as_deref()) - && let Some(workspace) = workspace.upgrade() - { - let _ = workspace.update(cx, |workspace, cx| { - workspace - .open_abs_path( - normalize_path(path.as_path()), - OpenOptions { - visible: Some(OpenVisible::None), - ..Default::default() - }, - window, - cx, - ) - .detach(); - }); - return; - } + // URL-decode the path for proper handling of encoded characters + let decoded_path = urlencoding::decode(path_text).unwrap_or_else(|_| Cow::Borrowed(path_text)); - cx.open_url(url.as_ref()); + if let Some(workspace) = workspace.upgrade() { + workspace.update(cx, |workspace, cx| { + workspace.open_url_or_file(&decoded_path, base_directory.as_deref(), window, cx); + }); + } else { + cx.open_url(url.as_ref()); + } } fn split_preview_url(url: &str) -> (&str, Option<&str>) { @@ -814,30 +804,6 @@ fn split_preview_url(url: &str) -> (&str, Option<&str>) { } } -fn resolve_preview_path(url: &str, base_directory: Option<&Path>) -> Option { - if url.starts_with("http://") || url.starts_with("https://") { - return None; - } - - let (path_text, _) = split_preview_url(url); - let decoded_url = urlencoding::decode(path_text) - .map(|decoded| decoded.into_owned()) - .unwrap_or_else(|_| path_text.to_string()); - let candidate = PathBuf::from(&decoded_url); - - if candidate.is_absolute() && candidate.exists() { - return Some(candidate); - } - - let base_directory = base_directory?; - let resolved = base_directory.join(decoded_url); - if resolved.exists() { - Some(resolved) - } else { - None - } -} - fn resolve_preview_image( dest_url: &str, base_directory: Option<&Path>, @@ -1212,53 +1178,7 @@ mod tests { use util::test::TempTree; use workspace::{AppState, MultiWorkspace, SaveIntent, Workspace, open_paths}; - use super::{MarkdownPreviewView, resolve_preview_path}; - - #[test] - fn resolves_relative_preview_path_and_missing_cases() { - let tree = markdown_fixture_tree(json!({ - "notes.md": "# Notes" - })); - let base_directory = markdown_fixture_directory(&tree); - let file = base_directory.join("notes.md"); - - assert_eq!( - resolve_preview_path("notes.md", Some(base_directory.as_path())), - Some(file) - ); - assert_eq!( - resolve_preview_path("nonexistent.md", Some(base_directory.as_path())), - None - ); - assert_eq!(resolve_preview_path("notes.md", None), None); - } - - #[test] - fn resolves_urlencoded_preview_path_and_ignores_fragment_component() { - let tree = markdown_fixture_tree(json!({ - "release notes.md": "# Release Notes", - "notes.md": "# Notes" - })); - let base_directory = markdown_fixture_directory(&tree); - - assert_eq!( - resolve_preview_path( - "release%20notes.md#overview", - Some(base_directory.as_path()) - ), - Some(base_directory.join("release notes.md")) - ); - assert_eq!( - resolve_preview_path("notes.md#L10", Some(base_directory.as_path())), - Some(base_directory.join("notes.md")) - ); - } - - #[test] - fn does_not_treat_web_links_as_preview_files() { - assert_eq!(resolve_preview_path("https://zed.dev", None), None); - assert_eq!(resolve_preview_path("http://example.com", None), None); - } + use super::MarkdownPreviewView; #[test] fn resolves_workspace_absolute_preview_image_path_and_rejects_missing() { @@ -1463,12 +1383,6 @@ mod tests { }) } - fn markdown_fixture_tree(docs_tree: serde_json::Value) -> TempTree { - TempTree::new(json!({ - "docs": docs_tree - })) - } - fn markdown_fixture_directory(tree: &TempTree) -> PathBuf { tree.path().join("docs") } diff --git a/crates/workspace/Cargo.toml b/crates/workspace/Cargo.toml index 4bee4ba8c0..ed91016bca 100644 --- a/crates/workspace/Cargo.toml +++ b/crates/workspace/Cargo.toml @@ -66,6 +66,7 @@ telemetry.workspace = true theme.workspace = true theme_settings.workspace = true ui.workspace = true +url.workspace = true util.workspace = true uuid.workspace = true zed_actions.workspace = true diff --git a/crates/workspace/src/notifications.rs b/crates/workspace/src/notifications.rs index ffe70b8301..e7a1dbd5e0 100644 --- a/crates/workspace/src/notifications.rs +++ b/crates/workspace/src/notifications.rs @@ -405,7 +405,15 @@ impl Render for LanguageServerPrompt { wrap_button_visibility: markdown::WrapButtonVisibility::Hidden, border: false, }) - .on_url_click(|link, _, cx| cx.open_url(&link)), + .on_url_click(|link, window, cx| { + if let Some(workspace) = Workspace::for_window(window, cx) { + workspace.update(cx, |workspace, cx| { + workspace.open_url_or_file(&link, None, window, cx); + }); + } else { + cx.open_url(&link); + } + }), ) .children(request.actions.iter().enumerate().map(|(ix, action)| { let this_handle = cx.entity(); diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 5f522e5cc6..f9534d838a 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -145,6 +145,7 @@ pub use toolbar::{ }; pub use ui; use ui::{Window, prelude::*}; +use url::Url; use util::{ ResultExt, TryFutureExt, paths::{PathStyle, SanitizedPath}, @@ -4672,6 +4673,108 @@ impl Workspace { }) } + /// Opens a URL or file path, intelligently routing to the appropriate handler: + /// + /// - `http://` and `https://` URLs are opened in the system's default browser + /// - `file://` URIs are opened as files in the editor + /// - Paths without a URL scheme are treated as file paths: + /// - Absolute paths are opened directly + /// - Relative paths are first resolved against `base_path` (if provided), + /// then against visible project worktrees + /// - Other URI schemes (e.g., `mailto:`, `vscode:`) are passed to the system handler + /// + /// # Arguments + /// * `url_or_path` - The URL or file path to open + /// * `base_path` - Optional base directory for resolving relative paths (e.g., the + /// directory containing a markdown file). If not provided, relative paths are + /// resolved against project worktrees. + /// + /// This method provides a unified way to handle links that may be either URLs + /// or file paths, such as those found in markdown documents, terminal output, + /// or LSP responses. + pub fn open_url_or_file( + &mut self, + url_or_path: &str, + base_path: Option<&Path>, + window: &mut Window, + cx: &mut Context, + ) { + let mut open_abs_path = |this: &mut Self, path, cx: &mut _| { + let url_or_path = url_or_path.to_owned(); + let task = this.open_abs_path( + path, + OpenOptions { + visible: Some(OpenVisible::None), + ..Default::default() + }, + window, + cx, + ); + (**cx) + .spawn(async move |cx| { + if let Err(_) = task.await { + cx.update(|cx| cx.open_url(&url_or_path)); + } + }) + .detach(); + }; + + if let Ok(url) = Url::parse(url_or_path) { + match url.scheme() { + "http" | "https" => cx.open_url(url_or_path), + "file" => open_abs_path(self, PathBuf::from(url.path()), cx), + _ => cx.open_url(url_or_path), + } + return; + } + + // Not a valid URL - treat as a file path + let project = self.project(); + let path_style = project.read(cx).path_style(cx); + + // If it's an absolute path, open it directly + if path_style.is_absolute(url_or_path) { + open_abs_path(self, PathBuf::from(url_or_path), cx); + return; + } + + let path = Path::new(url_or_path); + // Try to resolve relative path against base_path first + if let Some(base) = base_path + // TODO: remotes, the exists check below hits the local FS, unsure + // if this runs on the remote or not + && project.read(cx).is_local() + { + let resolved = path_style.join(base, path).map(PathBuf::from); + if let Some(resolved) = resolved + && resolved.exists() + { + open_abs_path(self, resolved, cx); + return; + } + } + + // Try to resolve against project worktrees + if let Some(project_path) = + project.update(cx, |project, cx| project.find_project_path(url_or_path, cx)) + { + let url_or_path = url_or_path.to_owned(); + let task = self.open_path(project_path, None, true, window, cx); + (**cx) + .spawn(async move |cx| { + if let Err(_) = task.await { + cx.update(|cx| cx.open_url(&url_or_path)); + } + }) + .detach(); + return; + } + + // Couldn't resolve as a file path - try opening as URL anyway + // (the OS might be able to handle it) + cx.open_url(url_or_path); + } + pub fn split_path( &mut self, path: impl Into, @@ -15996,4 +16099,49 @@ mod tests { let path = workspace.read_with(cx, |workspace, cx| workspace.most_recent_active_path(cx)); assert_eq!(path, None); } + + #[gpui::test] + async fn test_open_url_or_file_routes_urls(cx: &mut TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + "/root", + json!({ + "file.txt": "content", + "subdir": { + "nested.txt": "nested content" + } + }), + ) + .await; + + let project = Project::test(fs, [path!("/root").as_ref()], cx).await; + let (workspace, cx) = + cx.add_window_view(|window, cx| Workspace::test_new(project.clone(), window, cx)); + + // Test opening an HTTPS URL - should go to open_url + workspace.update_in(cx, |workspace, window, cx| { + workspace.open_url_or_file("https://example.com", None, window, cx); + }); + assert_eq!(cx.opened_url(), Some("https://example.com".to_string())); + + // Test opening an HTTP URL - should go to open_url + workspace.update_in(cx, |workspace, window, cx| { + workspace.open_url_or_file("http://example.com", None, window, cx); + }); + assert_eq!(cx.opened_url(), Some("http://example.com".to_string())); + + // Test opening other URI schemes - should go to open_url + workspace.update_in(cx, |workspace, window, cx| { + workspace.open_url_or_file("mailto:test@example.com", None, window, cx); + }); + assert_eq!(cx.opened_url(), Some("mailto:test@example.com".to_string())); + + // Test opening a path that doesn't exist and doesn't match project - should fallback to open_url + workspace.update_in(cx, |workspace, window, cx| { + workspace.open_url_or_file("nonexistent.txt", None, window, cx); + }); + assert_eq!(cx.opened_url(), Some("nonexistent.txt".to_string())); + } }