markdown_preview: Open relative links at line (#60864)

Closes #60733

# Objective

Markdown Preview treats a relative `path:line[:column]` destination as a
literal filename. When that file does not exist, the link falls through
to the system URL handler instead of opening the workspace file at the
requested position.

## Solution

- Recognize position suffixes on relative Markdown links.
- Convert GitHub-style `#L42` fragments into the same positioned target
used by `path:42`, so both forms share one resolution and opening path.
- Resolve links with `workspace::path_link::possible_open_target` using
the Markdown source directory. This keeps the remote-project path
resolution added in #61438.
- Open resolved targets with `editor::items::open_resolved_target`,
including its one-based row and column handling.
- Preserve valid URI schemes and fall back to the existing Markdown link
behavior when no workspace target resolves.

## Testing

- Extended the Markdown Preview GPUI regression test to cover relative
`path:line:column`, GitHub-style `#Lline`, subfolder-relative
resolution, and custom URI schemes.
- `git diff --check` passes.
- The focused Rust test could not start in the current macOS environment
because the Rust toolchain binaries hang in the dynamic loader before
Cargo runs. PR Actions are awaiting maintainer approval.

## Self-Review Checklist:

- [x] I have reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content adheres to Zed UI and icon guidelines
- [x] Tests cover the new and changed behavior
- [x] Performance impact has been considered and is acceptable

Release Notes:

- Fixed Markdown Preview links using workspace-relative paths with line
and column suffixes.

---------

Co-authored-by: Smit Barmase <heysmitbarmase@gmail.com>
This commit is contained in:
Kevin BravoandSmit Barmase authored and GitHub committed 2026-07-28 05:45:13 +00:00
1 parent 1d61d7f42a
commit 0c69ff7a3d
3 files changed
+216 -100

No files matched your search

@@ -7,6 +7,7 @@ use std::sync::Arc;
use std::time::Duration;
use anyhow::{Context as _, Result};
use editor::items::open_resolved_target;
use editor::scroll::Autoscroll;
use editor::{Editor, EditorEvent, MultiBufferOffset, SelectionEffects};
use gpui::{
@@ -14,7 +15,7 @@ use gpui::{
InteractiveElement, IntoElement, IsZero, Pixels, Render, Resource, RetainAllImageCache,
ScrollHandle, SharedString, SharedUri, Subscription, Task, WeakEntity, Window, point, px,
};
use language::{LanguageRegistry, Point};
use language::LanguageRegistry;
use markdown::{
CodeBlockRenderer, CopyButtonVisibility, Markdown, MarkdownElement, MarkdownFont,
MarkdownOptions, MarkdownStyle,
@@ -29,9 +30,11 @@ use ui::{ContextMenu, LinkPreview, WithScrollbar, prelude::*, right_click_menu};
use util::{
ResultExt,
markdown::{source_position_from_fragment, split_local_url_fragment},
paths::PathWithPosition,
};
use workspace::item::{Item, ItemBufferKind, ItemHandle, SaveOptions, SerializableItem};
use workspace::notifications::NotifyResultExt;
use workspace::notifications::{NotifyResultExt, NotifyTaskExt};
use workspace::path_link::{PathMatching, resolve_open_target};
use workspace::searchable::{
Direction, SearchEvent, SearchOptions, SearchToken, SearchableItem, SearchableItemHandle,
};
@@ -1093,20 +1096,36 @@ fn open_preview_url(
) {
let decoded_path = urlencoding::decode(&url).unwrap_or_else(|_| Cow::Borrowed(&url));
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(),
// Only `://` URLs skip position handling (a scheme check would misclassify `main.rs:2`); ambiguous links like `tel:123` are tried as files first, then fall back to the URL handler.
let path_with_position = if !decoded_path.contains("://") {
if PathWithPosition::parse_str(&decoded_path).row.is_some() {
Some(decoded_path.to_string())
} else {
fragment
.as_deref()
.and_then(|fragment| fragment.strip_prefix('L'))
.and_then(source_position_from_fragment)
.map(|(row, column)| {
if column == 0 {
format!("{decoded_path}:{}", row + 1)
} else {
format!("{decoded_path}:{}:{}", row + 1, column + 1)
}
})
}
} else {
None
};
if let Some(path_with_position) = path_with_position {
open_preview_path_with_position(
path_with_position,
decoded_path.into_owned(),
base_directory,
workspace,
row,
window,
cx,
)
{
);
return;
}
@@ -1119,75 +1138,41 @@ fn open_preview_url(
}
}
enum PreviewPathTarget {
Project(ProjectPath),
Absolute(PathBuf),
}
fn open_preview_path_at_line(
path: String,
fn open_preview_path_with_position(
path_with_position: String,
fallback_path: String,
base_directory: Option<PathBuf>,
workspace: &WeakEntity<Workspace>,
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 open_target = resolve_open_target(
workspace,
PathMatching::Exact,
&path_with_position,
base_directory.as_deref(),
cx,
);
let workspace = workspace.clone();
let workspace_for_error = workspace.clone();
let task = window.spawn(cx, async move |cx| {
let opened = if let Some(open_target) = open_target.await {
open_resolved_target(&workspace, &open_target, cx)
.await
.context("opening Markdown preview link")?
} else {
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::<Editor>() 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]),
);
if !opened {
workspace.update_in(cx, |workspace, window, cx| {
workspace.open_url_or_file(&fallback_path, base_directory.as_deref(), window, cx);
})?;
anyhow::Ok(())
})
.detach_and_log_err(cx);
true
}
anyhow::Ok(())
});
task.detach_and_notify_err(workspace_for_error, window, cx);
}
fn resolve_preview_image(
@@ -1853,13 +1838,16 @@ mod tests {
}
#[gpui::test]
async fn opens_preview_file_link_at_line(cx: &mut TestAppContext) {
async fn opens_preview_file_links_at_positions(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"}}),
json!({
"docs": {"guide.md": "alpha\nbeta\ngamma\n"},
"src": {"main.rs": "first\nsecond\nthird\n"}
}),
)
.await;
let project = Project::test(fs, [path!("/project").as_ref()], cx).await;
@@ -1871,9 +1859,9 @@ mod tests {
multi_workspace.update_in(cx, |_, window, cx| {
open_preview_url(
"src/main.rs".into(),
Some("L2".into()),
"../src/main.rs:2:4".into(),
None,
Some(PathBuf::from(path!("/project/docs"))),
&workspace_weak,
window,
cx,
@@ -1889,8 +1877,101 @@ mod tests {
});
editor.update_in(cx, |editor, window, cx| {
let snapshot = editor.snapshot(window, cx);
assert_eq!(editor.selections.newest::<Point>(&snapshot).head().row, 1);
assert_eq!(
editor.selections.newest::<Point>(&snapshot).head(),
Point::new(1, 3)
);
});
multi_workspace.update_in(cx, |_, window, cx| {
open_preview_url(
"../src/main.rs".into(),
Some("L3".into()),
Some(PathBuf::from(path!("/project/docs"))),
&workspace_weak,
window,
cx,
);
});
cx.run_until_parked();
editor.update_in(cx, |editor, window, cx| {
let snapshot = editor.snapshot(window, cx);
assert_eq!(
editor.selections.newest::<Point>(&snapshot).head(),
Point::new(2, 0)
);
});
// `guide.md:2` must open the sibling file at line 2 despite `guide.md:` being a valid URI scheme prefix.
multi_workspace.update_in(cx, |_, window, cx| {
open_preview_url(
"guide.md:2".into(),
None,
Some(PathBuf::from(path!("/project/docs"))),
&workspace_weak,
window,
cx,
);
});
cx.run_until_parked();
let guide_editor = workspace.read_with(cx, |workspace, cx| {
workspace
.active_item(cx)
.and_then(|item| item.downcast::<Editor>())
.expect("guide.md should be open in an editor")
});
guide_editor.update_in(cx, |editor, window, cx| {
let file_path = editor
.buffer()
.read(cx)
.as_singleton()
.and_then(|buffer| buffer.read(cx).file().map(|file| file.path().clone()))
.expect("opened editor should have a file");
assert_eq!(file_path.as_ref(), rel_path("docs/guide.md"));
let snapshot = editor.snapshot(window, cx);
assert_eq!(
editor.selections.newest::<Point>(&snapshot).head(),
Point::new(1, 0)
);
});
// An unresolved link must not open an unrelated file that merely shares its trailing path components.
multi_workspace.update_in(cx, |_, window, cx| {
open_preview_url(
"main.rs:2".into(),
None,
Some(PathBuf::from(path!("/project/docs"))),
&workspace_weak,
window,
cx,
);
});
cx.run_until_parked();
assert_eq!(cx.opened_url().as_deref(), Some("main.rs:2"));
editor.update_in(cx, |editor, window, cx| {
let snapshot = editor.snapshot(window, cx);
assert_eq!(
editor.selections.newest::<Point>(&snapshot).head(),
Point::new(2, 0),
"broken link must not move the cursor in a suffix-matching file"
);
});
multi_workspace.update_in(cx, |_, window, cx| {
open_preview_url(
"tel:123".into(),
None,
Some(PathBuf::from(path!("/project/docs"))),
&workspace_weak,
window,
cx,
);
});
cx.run_until_parked();
assert_eq!(cx.opened_url().as_deref(), Some("tel:123"));
}
#[gpui::test]
@@ -4,11 +4,12 @@ use editor::items::open_resolved_target;
use gpui::{Context, Task, TaskExt, WeakEntity, Window};
use std::path::PathBuf;
use terminal::PathLikeTarget;
use workspace::path_link::PathMatching;
#[cfg(not(test))]
use workspace::path_link::possible_open_target;
use workspace::path_link::resolve_open_target;
#[cfg(test)]
use workspace::path_link::{
BackgroundPathChecks, OpenTargetFoundBy, possible_open_target_with_fs_checks,
BackgroundPathChecks, OpenTargetFoundBy, resolve_open_target_with_fs_checks,
};
use workspace::{Workspace, path_link::OpenTarget};
@@ -42,15 +43,17 @@ fn possible_hover_target(
#[cfg(test)] background_path_checks: BackgroundPathChecks,
) -> Task<()> {
#[cfg(not(test))]
let file_to_open_task = possible_open_target(
let file_to_open_task = resolve_open_target(
workspace,
PathMatching::Heuristic,
&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(
let file_to_open_task = resolve_open_target_with_fs_checks(
workspace,
PathMatching::Heuristic,
&path_like_target.maybe_path,
path_like_target.terminal_dir.as_deref(),
cx,
@@ -119,8 +122,9 @@ fn possibly_open_target(
.update(cx, |_, cx| {
#[cfg(not(test))]
{
possible_open_target(
resolve_open_target(
&workspace,
PathMatching::Heuristic,
&path_like_target.maybe_path,
path_like_target.terminal_dir.as_deref(),
cx,
@@ -128,8 +132,9 @@ fn possibly_open_target(
}
#[cfg(test)]
{
possible_open_target_with_fs_checks(
resolve_open_target_with_fs_checks(
&workspace,
PathMatching::Heuristic,
&path_like_target.maybe_path,
path_like_target.terminal_dir.as_deref(),
cx,
+43 -13
View File
@@ -22,6 +22,15 @@ pub enum BackgroundPathChecks {
ProjectPathResolution,
}
/// How a path-like string gets matched against files by [`resolve_open_target`].
#[derive(Debug, Clone, Copy, Eq, PartialEq)]
pub enum PathMatching {
/// Only exact matches (absolute, `cwd`-relative, or worktree-root-relative), so broken document links resolve to `None`.
Exact,
/// Additionally guess terminal-output paths: strip git diff `a/`/`b/` prefixes and scan worktrees for trailing path matches.
Heuristic,
}
#[derive(Debug, Clone)]
pub enum OpenTarget {
Worktree(
@@ -126,28 +135,38 @@ pub fn first_unbalanced_open_paren(text: &str) -> Option<usize> {
first_unmatched.filter(|_| balance > 0)
}
pub fn possible_open_target(
pub fn resolve_open_target(
workspace: &WeakEntity<Workspace>,
matching: PathMatching,
maybe_path: &str,
cwd: Option<&Path>,
cx: &App,
) -> Task<Option<OpenTarget>> {
possible_open_target_internal(workspace, maybe_path, cwd, cx, None)
resolve_open_target_internal(workspace, matching, maybe_path, cwd, cx, None)
}
#[cfg(any(test, feature = "test-support"))]
pub fn possible_open_target_with_fs_checks(
pub fn resolve_open_target_with_fs_checks(
workspace: &WeakEntity<Workspace>,
matching: PathMatching,
maybe_path: &str,
cwd: Option<&Path>,
cx: &App,
background_path_checks: BackgroundPathChecks,
) -> Task<Option<OpenTarget>> {
possible_open_target_internal(workspace, maybe_path, cwd, cx, Some(background_path_checks))
resolve_open_target_internal(
workspace,
matching,
maybe_path,
cwd,
cx,
Some(background_path_checks),
)
}
fn possible_open_target_internal(
fn resolve_open_target_internal(
workspace: &WeakEntity<Workspace>,
matching: PathMatching,
maybe_path: &str,
cwd: Option<&Path>,
cx: &App,
@@ -172,8 +191,11 @@ fn possible_open_target_internal(
})
.collect::<Vec<_>>();
const GIT_DIFF_PATH_PREFIXES: &[&str] = &["a", "b"];
for prefix_str in GIT_DIFF_PATH_PREFIXES.iter().chain(std::iter::once(&".")) {
let prefixes_to_strip: &[&str] = match matching {
PathMatching::Heuristic => &["a", "b", "."],
PathMatching::Exact => &["."],
};
for prefix_str in prefixes_to_strip {
if let Some(stripped) = original_path.path.strip_prefix(prefix_str).ok() {
potential_paths.push(PathWithPosition {
path: stripped.to_owned(),
@@ -309,7 +331,7 @@ fn possible_open_target_internal(
let background_resolution_task = match background_path_checks {
BackgroundPathChecks::LocalFileSystem => {
let fs_paths_to_check =
local_paths_to_check(&potential_paths, cwd, &worktree_candidates, cx);
local_paths_to_check(&potential_paths, matching, cwd, &worktree_candidates, cx);
let fs = project.read(cx).fs().clone();
cx.background_spawn(async move {
for mut path_to_check in fs_paths_to_check {
@@ -384,6 +406,11 @@ fn possible_open_target_internal(
return Some(open_target);
}
// An inexact path must not resolve to an unrelated file sharing its trailing components.
if matching == PathMatching::Exact {
return None;
}
let worktree_paths_to_check = worktree_paths_to_check
.into_iter()
.map(|(worktree, paths_to_check)| {
@@ -434,6 +461,7 @@ fn possible_open_target_internal(
fn local_paths_to_check(
potential_paths: &[PathWithPosition],
matching: PathMatching,
cwd: Option<&Path>,
worktree_candidates: &[Entity<Worktree>],
cx: &App,
@@ -467,11 +495,13 @@ fn local_paths_to_check(
});
}
} else {
paths_to_check.push(PathWithPosition {
path: maybe_path.clone(),
row: path_to_check.row,
column: path_to_check.column,
});
if maybe_path.is_absolute() || matching == PathMatching::Heuristic {
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() {