Improve link pattern parsing to trim punctuation wrappers (#45457)

Closes # (none)

## Summary

Improved the jump-to-definition reliability for file paths in prose
strings (comments, markdown, etc.) by smarter stripping of surrounding
punctuation.

This allows `cmd/ctrl-click` to work on file paths in contexts like:
- **Markdown backticks**: `` `path/to/file` ``
- **Markdown links**: `[link](path/to/file)`
- **Parentheses**: `(see path/to/file)`
- **Sentence endings**: `Check path/to/file.`
- **Code spans**: `` `cat path/to/file` ``

## Technical Details

- Updated `link_pattern_file_candidates` in `hover_links.rs` to
iteratively trim common leading and trailing punctuation characters.
- Candidate generation now produces multiple variations (trimmed, regex
match, raw) ordered by specificity (most trimmed first).
- Refactored `test_hover_filenames` to be DRY: it now uses a single base
document string and targeted replacements, making it easier to add new
prose test cases without duplication.

Release Notes:

- Improved jump-to-definition reliability for file paths wrapped in
punctuation (backticks, parens, sentence endings).

---------

Co-authored-by: Martin Ye <martin@zed.dev>
Co-authored-by: MartinYe1234 <52641447+MartinYe1234@users.noreply.github.com>
This commit is contained in:
Tom Aylott 2026-05-20 22:51:14 +00:00 committed by GitHub
parent b3ce9a49f7
commit ee5c7b6d45
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -767,23 +767,66 @@ pub(crate) async fn find_file(
None None
} }
// Tries to capture potentially inlined links, like those found in markdown, // Generates candidate file paths by stripping common punctuation wrappers.
// e.g. [LinkTitle](link_file.txt) // Handles markdown patterns like [title](path), `path`, (path), as well as
// Since files can have parens, we should always return the full string // partial wrappers where punctuation only appears on one side (e.g. path) or path`).
// (literally, [LinkTitle](link_file.txt)) as a candidate. // Returns candidates ordered from most-specific (most trimmed) to least-specific (raw).
fn link_pattern_file_candidates(candidate: &str) -> Vec<(String, Range<usize>)> { fn link_pattern_file_candidates(candidate: &str) -> Vec<(String, Range<usize>)> {
static MD_LINK_REGEX: LazyLock<Regex> = static MD_LINK_REGEX: LazyLock<Regex> =
LazyLock::new(|| Regex::new(r"]\(([^)]*)\)").expect("Failed to create REGEX")); LazyLock::new(|| Regex::new(r"]\(([^)]*)\)").expect("Failed to create REGEX"));
// Punctuation that commonly wraps file paths in prose/markdown
const LEADING_PUNCTUATION: &[char] = &['`', '(', '[', '{', '<', '"', '\''];
const TRAILING_PUNCTUATION: &[char] = &[
'`', ')', ']', '}', '>', '"', '\'', '.', ',', ':', ';', '!', '?',
];
let candidate_len = candidate.len(); let candidate_len = candidate.len();
let mut candidates = Vec::new();
let mut candidates = vec![(candidate.to_string(), 0..candidate_len)]; // Trim leading and trailing punctuation iteratively
let mut start = 0;
let mut end = candidate_len;
if let Some(captures) = MD_LINK_REGEX.captures(candidate) { // Trim leading punctuation
if let Some(link) = captures.get(1) { for ch in candidate.chars() {
candidates.push((link.as_str().to_string(), link.range())); if LEADING_PUNCTUATION.contains(&ch) {
start += ch.len_utf8();
} else {
break;
} }
} }
// Trim trailing punctuation
for ch in candidate.chars().rev() {
if TRAILING_PUNCTUATION.contains(&ch) {
end -= ch.len_utf8();
} else {
break;
}
}
// Add trimmed candidate first (highest priority) if it differs from original
if start < end && (start > 0 || end < candidate_len) {
candidates.push((candidate[start..end].to_string(), start..end));
}
// Extract markdown link destination: [title](path) or ](path) -> path
// This also handles bare (path) wrapping.
if let Some(captures) = MD_LINK_REGEX.captures(candidate) {
if let Some(link) = captures.get(1) {
let link_str = link.as_str().to_string();
let link_range = link.range();
// Avoid duplicate if punctuation trimming already found this
if !candidates.iter().any(|(s, _)| s == &link_str) {
candidates.push((link_str, link_range));
}
}
}
// Always include the raw candidate as fallback (lowest priority)
candidates.push((candidate.to_string(), 0..candidate_len));
candidates candidates
} }
@ -816,7 +859,12 @@ fn surrounding_filename(
found_start = true; found_start = true;
break; break;
} }
if (ch == '"' || ch == '\'' || ch == '`') && !inside_quotes { // Quote characters open a quoted region that is stripped from the
// returned filename. Backticks and parens are NOT treated this way —
// they are kept as part of the token so that downstream candidate
// generation (link_pattern_file_candidates) can trim them and produce
// a tight highlight range via make_range.
if (ch == '"' || ch == '\'') && !inside_quotes {
found_start = true; found_start = true;
inside_quotes = true; inside_quotes = true;
break; break;
@ -849,7 +897,7 @@ fn surrounding_filename(
found_end = true; found_end = true;
break; break;
} }
if ch == '"' || ch == '\'' || ch == '`' { if ch == '"' || ch == '\'' {
// If we're inside quotes, we stop when we come across the next quote // If we're inside quotes, we stop when we come across the next quote
if inside_quotes { if inside_quotes {
found_end = true; found_end = true;
@ -1569,66 +1617,98 @@ mod tests {
#[test] #[test]
fn test_link_pattern_file_candidates() { fn test_link_pattern_file_candidates() {
// Full markdown link: [LinkTitle](link_file.txt)
// Trimmed strips [ and ), regex extracts link destination, raw is fallback
let candidates: Vec<String> = link_pattern_file_candidates("[LinkTitle](link_file.txt)") let candidates: Vec<String> = link_pattern_file_candidates("[LinkTitle](link_file.txt)")
.into_iter() .into_iter()
.map(|(c, _)| c) .map(|(c, _)| c)
.collect(); .collect();
assert_eq!( assert_eq!(
candidates, candidates,
vec!["[LinkTitle](link_file.txt)", "link_file.txt",] vec![
"LinkTitle](link_file.txt",
"link_file.txt",
"[LinkTitle](link_file.txt)"
]
); );
// Link title with spaces in it
// Link title with spaces (token starts mid-link)
let candidates: Vec<String> = link_pattern_file_candidates("LinkTitle](link_file.txt)") let candidates: Vec<String> = link_pattern_file_candidates("LinkTitle](link_file.txt)")
.into_iter() .into_iter()
.map(|(c, _)| c) .map(|(c, _)| c)
.collect(); .collect();
assert_eq!( assert_eq!(
candidates, candidates,
vec!["LinkTitle](link_file.txt)", "link_file.txt",] vec![
"LinkTitle](link_file.txt",
"link_file.txt",
"LinkTitle](link_file.txt)"
]
); );
// Link with spaces // Link with escaped spaces
let candidates: Vec<String> = link_pattern_file_candidates("LinkTitle](link\\ _file.txt)") let candidates: Vec<String> = link_pattern_file_candidates("LinkTitle](link\\ _file.txt)")
.into_iter() .into_iter()
.map(|(c, _)| c) .map(|(c, _)| c)
.collect(); .collect();
assert_eq!( assert_eq!(
candidates, candidates,
vec!["LinkTitle](link\\ _file.txt)", "link\\ _file.txt",] vec![
"LinkTitle](link\\ _file.txt",
"link\\ _file.txt",
"LinkTitle](link\\ _file.txt)"
]
); );
// Parentheses without preceding `]` should not extract inner content,
// to avoid matching function calls like `do_work(file2)` as file paths. // Bare parentheses: (link_file.txt)
let candidates: Vec<String> = link_pattern_file_candidates("(link_file.txt)") let candidates: Vec<String> = link_pattern_file_candidates("(link_file.txt)")
.into_iter() .into_iter()
.map(|(c, _)| c) .map(|(c, _)| c)
.collect(); .collect();
assert_eq!(candidates, vec!["(link_file.txt)"]); assert_eq!(candidates, vec!["link_file.txt", "(link_file.txt)"]);
let candidates: Vec<String> = link_pattern_file_candidates("do_work(file2);") // Trailing paren only: link_file.txt)
let candidates: Vec<String> = link_pattern_file_candidates("link_file.txt)")
.into_iter() .into_iter()
.map(|(c, _)| c) .map(|(c, _)| c)
.collect(); .collect();
assert_eq!(candidates, vec!["do_work(file2);"]); assert_eq!(candidates, vec!["link_file.txt", "link_file.txt)"]);
// Markdown links should still extract the path // Trailing backtick only: link_file.txt`
let candidates: Vec<String> = link_pattern_file_candidates("](readme.md)") let candidates: Vec<String> = link_pattern_file_candidates("link_file.txt`")
.into_iter() .into_iter()
.map(|(c, _)| c) .map(|(c, _)| c)
.collect(); .collect();
assert_eq!(candidates, vec!["](readme.md)", "readme.md"]); assert_eq!(candidates, vec!["link_file.txt", "link_file.txt`"]);
// No nesting // Wrapped in backticks: `link_file.txt`
let candidates: Vec<String> = link_pattern_file_candidates("`link_file.txt`")
.into_iter()
.map(|(c, _)| c)
.collect();
assert_eq!(candidates, vec!["link_file.txt", "`link_file.txt`"]);
// Trailing period (sentence ending): link_file.txt.
let candidates: Vec<String> = link_pattern_file_candidates("link_file.txt.")
.into_iter()
.map(|(c, _)| c)
.collect();
assert_eq!(candidates, vec!["link_file.txt", "link_file.txt."]);
// Nested parens - regex finds first (...) capturing inner content
let candidates: Vec<String> = let candidates: Vec<String> =
link_pattern_file_candidates("LinkTitle](link_(link_file)file.txt)") link_pattern_file_candidates("LinkTitle](link_(link_file)file.txt)")
.into_iter() .into_iter()
.map(|(c, _)| c) .map(|(c, _)| c)
.collect(); .collect();
assert_eq!( assert_eq!(
candidates, candidates,
vec!["LinkTitle](link_(link_file)file.txt)", "link_(link_file",] vec![
) "LinkTitle](link_(link_file)file.txt",
"link_(link_file",
"LinkTitle](link_(link_file)file.txt)"
]
);
} }
#[gpui::test] #[gpui::test]
@ -1671,16 +1751,12 @@ mod tests {
(" ˇ\"\"", Some("")), (" ˇ\"\"", Some("")),
(" \"ˇ常\"", Some("")), (" \"ˇ常\"", Some("")),
("ˇ\"\"", Some("")), ("ˇ\"\"", Some("")),
// Path with row:column suffix // Backticks (surrounding_filename returns the full token including backticks)
("fiˇle.rs:83:1", Some("file.rs:83:1")), ("`fiˇle.txt`", Some("`file.txt`")),
("file.rs:83ˇ:1 foo", Some("file.rs:83:1")), ("open `fiˇle.txt` please", Some("`file.txt`")),
("file.rs:20ˇ:in bar", Some("file.rs:20:in")), // Parentheses (surrounding_filename returns the full token including parens)
// Backtick delimiters ("(fiˇle.txt)", Some("(file.txt)")),
("`fˇile.txt`", Some("file.txt")), ("open (fiˇle.txt) please", Some("(file.txt)")),
("ˇ`file.txt`", Some("file.txt")),
("`fˇile.txt` and more", Some("file.txt")),
// Backtick with row:col
("`fiˇle.rs:83:1`", Some("file.rs:83:1")),
]; ];
for (input, expected) in test_cases { for (input, expected) in test_cases {
@ -1739,222 +1815,130 @@ mod tests {
) )
.await; .await;
// Base document with {ABS} placeholder for absolute path prefix.
// Each test case replaces a specific line to add cursor (ˇ) or highlight («»ˇ) markers.
#[cfg(not(target_os = "windows"))] #[cfg(not(target_os = "windows"))]
cx.set_state(indoc! {" const ABS: &str = "/root/dir";
You can't go to a file that does_not_exist.txt.
Go to file2.rs if you want.
Or go to ../dir/file2.rs if you want.
Or go to /root/dir/file2.rs if project is local.
Or go to /root/dir/file2 if this is a Rust file.ˇ
"});
#[cfg(target_os = "windows")] #[cfg(target_os = "windows")]
cx.set_state(indoc! {" const ABS: &str = "C:/root/dir";
You can't go to a file that does_not_exist.txt.
Go to file2.rs if you want.
Or go to ../dir/file2.rs if you want.
Or go to C:/root/dir/file2.rs if project is local.
Or go to C:/root/dir/file2 if this is a Rust file.ˇ
"});
// File does not exist let base = format!(
#[cfg(not(target_os = "windows"))] "\
let screen_coord = cx.pixel_position(indoc! {" You can't go to a file that does_not_exist.txt.
You can't go to a file that dˇoes_not_exist.txt. Go to file2.rs if you want.
Go to file2.rs if you want. Or go to ../dir/file2.rs if you want.
Or go to ../dir/file2.rs if you want. Or go to {ABS}/file2.rs if project is local.
Or go to /root/dir/file2.rs if project is local. Or go to {ABS}/file2 if this is a Rust file.
Or go to /root/dir/file2 if this is a Rust file. Or `file2.rs` in backticks.
"}); Or (file2.rs) in parens.
#[cfg(target_os = "windows")] Or [link](file2.rs) markdown style.
let screen_coord = cx.pixel_position(indoc! {" A file (named file2.rs) in prose.
You can't go to a file that dˇoes_not_exist.txt. Read with `cat file2.rs` command.
Go to file2.rs if you want. Sentence ending file2.rs.
Or go to ../dir/file2.rs if you want. "
Or go to C:/root/dir/file2.rs if project is local.
Or go to C:/root/dir/file2 if this is a Rust file.
"});
cx.simulate_mouse_move(screen_coord, None, Modifiers::secondary_key());
// No highlight
cx.update_editor(|editor, window, cx| {
assert!(
editor
.snapshot(window, cx)
.text_highlight_ranges(HighlightKey::HoveredLinkState)
.unwrap_or_default()
.1
.is_empty()
);
});
// Moving the mouse over a file that does exist should highlight it.
#[cfg(not(target_os = "windows"))]
let screen_coord = cx.pixel_position(indoc! {"
You can't go to a file that does_not_exist.txt.
Go to fˇile2.rs if you want.
Or go to ../dir/file2.rs if you want.
Or go to /root/dir/file2.rs if project is local.
Or go to /root/dir/file2 if this is a Rust file.
"});
#[cfg(target_os = "windows")]
let screen_coord = cx.pixel_position(indoc! {"
You can't go to a file that does_not_exist.txt.
Go to fˇile2.rs if you want.
Or go to ../dir/file2.rs if you want.
Or go to C:/root/dir/file2.rs if project is local.
Or go to C:/root/dir/file2 if this is a Rust file.
"});
cx.simulate_mouse_move(screen_coord, None, Modifiers::secondary_key());
#[cfg(not(target_os = "windows"))]
cx.assert_editor_text_highlights(
HighlightKey::HoveredLinkState,
indoc! {"
You can't go to a file that does_not_exist.txt.
Go to «file2.rsˇ» if you want.
Or go to ../dir/file2.rs if you want.
Or go to /root/dir/file2.rs if project is local.
Or go to /root/dir/file2 if this is a Rust file.
"},
);
#[cfg(target_os = "windows")]
cx.assert_editor_text_highlights(
HighlightKey::HoveredLinkState,
indoc! {"
You can't go to a file that does_not_exist.txt.
Go to «file2.rsˇ» if you want.
Or go to ../dir/file2.rs if you want.
Or go to C:/root/dir/file2.rs if project is local.
Or go to C:/root/dir/file2 if this is a Rust file.
"},
); );
// Moving the mouse over a relative path that does exist should highlight it cx.set_state(&format!("{base}ˇ"));
#[cfg(not(target_os = "windows"))]
let screen_coord = cx.pixel_position(indoc! {"
You can't go to a file that does_not_exist.txt.
Go to file2.rs if you want.
Or go to ../dir/fˇile2.rs if you want.
Or go to /root/dir/file2.rs if project is local.
Or go to /root/dir/file2 if this is a Rust file.
"});
#[cfg(target_os = "windows")]
let screen_coord = cx.pixel_position(indoc! {"
You can't go to a file that does_not_exist.txt.
Go to file2.rs if you want.
Or go to ../dir/fˇile2.rs if you want.
Or go to C:/root/dir/file2.rs if project is local.
Or go to C:/root/dir/file2 if this is a Rust file.
"});
cx.simulate_mouse_move(screen_coord, None, Modifiers::secondary_key()); // Test cases: (original_line, cursor_line, highlight_line)
#[cfg(not(target_os = "windows"))] // - cursor_line: the line with ˇ to position the mouse
cx.assert_editor_text_highlights( // - highlight_line: None = expect no highlight, Some(...) = expect this highlight
HighlightKey::HoveredLinkState, let test_cases: &[(&str, &str, Option<&str>)] = &[
indoc! {" // File does not exist - no highlight
You can't go to a file that does_not_exist.txt. ("does_not_exist.txt", "dˇoes_not_exist.txt", None),
Go to file2.rs if you want. // Simple filename
Or go to «../dir/file2.rsˇ» if you want. (
Or go to /root/dir/file2.rs if project is local. "Go to file2.rs if",
Or go to /root/dir/file2 if this is a Rust file. "Go to fˇile2.rs if",
"}, Some("Go to «file2.rsˇ» if"),
),
// Relative path
(
"Or go to ../dir/file2.rs if",
"Or go to ../dir/fˇile2.rs if",
Some("Or go to «../dir/file2.rsˇ» if"),
),
// Absolute path
(
&format!("Or go to {ABS}/file2.rs if"),
&format!("Or go to {ABS}/fiˇle2.rs if"),
Some(&format!("Or go to «{ABS}/file2.rsˇ» if")),
),
// Path without extension (language suffix added)
(
&format!("Or go to {ABS}/file2 if"),
&format!("Or go to {ABS}/fiˇle2 if"),
Some(&format!("Or go to «{ABS}/file2ˇ» if")),
),
// Backticks
(
"Or `file2.rs` in backticks",
"Or `fiˇle2.rs` in backticks",
Some("Or `«file2.rsˇ»` in backticks"),
),
// Parentheses
(
"Or (file2.rs) in parens",
"Or (fiˇle2.rs) in parens",
Some("Or («file2.rsˇ») in parens"),
),
// Markdown link
(
"Or [link](file2.rs) markdown",
"Or [link](fiˇle2.rs) markdown",
Some("Or [link](«file2.rsˇ») markdown"),
),
// Partial wrapper: trailing paren in prose like "(named file2.rs)"
(
"A file (named file2.rs) in",
"A file (named fiˇle2.rs) in",
Some("A file (named «file2.rsˇ») in"),
),
// Partial wrapper: inside code span like "`cat file2.rs`"
(
"Read with `cat file2.rs` command",
"Read with `cat fiˇle2.rs` command",
Some("Read with `cat «file2.rsˇ»` command"),
),
// Trailing period at end of sentence
(
"Sentence ending file2.rs.",
"Sentence ending fiˇle2.rs.",
Some("Sentence ending «file2.rsˇ»."),
),
];
for (original, cursor_version, highlight_version) in test_cases {
let position_text = base.replace(original, cursor_version);
let screen_coord = cx.pixel_position(&position_text);
cx.simulate_mouse_move(screen_coord, None, Modifiers::secondary_key());
if let Some(highlight) = highlight_version {
let expected = base.replace(original, highlight);
cx.assert_editor_text_highlights(HighlightKey::HoveredLinkState, &expected);
} else {
// Expect no highlight
cx.update_editor(|editor, window, cx| {
assert!(
editor
.snapshot(window, cx)
.text_highlight_ranges(HighlightKey::HoveredLinkState)
.unwrap_or_default()
.1
.is_empty(),
"Expected no highlight for cursor at: {}",
cursor_version
);
});
}
}
// Test click navigation on markdown link
let position_text = base.replace(
"Or [link](file2.rs) markdown",
"Or [link](fiˇle2.rs) markdown",
); );
#[cfg(target_os = "windows")] let screen_coord = cx.pixel_position(&position_text);
cx.assert_editor_text_highlights(
HighlightKey::HoveredLinkState,
indoc! {"
You can't go to a file that does_not_exist.txt.
Go to file2.rs if you want.
Or go to «../dir/file2.rsˇ» if you want.
Or go to C:/root/dir/file2.rs if project is local.
Or go to C:/root/dir/file2 if this is a Rust file.
"},
);
// Moving the mouse over an absolute path that does exist should highlight it
#[cfg(not(target_os = "windows"))]
let screen_coord = cx.pixel_position(indoc! {"
You can't go to a file that does_not_exist.txt.
Go to file2.rs if you want.
Or go to ../dir/file2.rs if you want.
Or go to /root/diˇr/file2.rs if project is local.
Or go to /root/dir/file2 if this is a Rust file.
"});
#[cfg(target_os = "windows")]
let screen_coord = cx.pixel_position(indoc! {"
You can't go to a file that does_not_exist.txt.
Go to file2.rs if you want.
Or go to ../dir/file2.rs if you want.
Or go to C:/root/diˇr/file2.rs if project is local.
Or go to C:/root/dir/file2 if this is a Rust file.
"});
cx.simulate_mouse_move(screen_coord, None, Modifiers::secondary_key());
#[cfg(not(target_os = "windows"))]
cx.assert_editor_text_highlights(
HighlightKey::HoveredLinkState,
indoc! {"
You can't go to a file that does_not_exist.txt.
Go to file2.rs if you want.
Or go to ../dir/file2.rs if you want.
Or go to «/root/dir/file2.rsˇ» if project is local.
Or go to /root/dir/file2 if this is a Rust file.
"},
);
#[cfg(target_os = "windows")]
cx.assert_editor_text_highlights(
HighlightKey::HoveredLinkState,
indoc! {"
You can't go to a file that does_not_exist.txt.
Go to file2.rs if you want.
Or go to ../dir/file2.rs if you want.
Or go to «C:/root/dir/file2.rsˇ» if project is local.
Or go to C:/root/dir/file2 if this is a Rust file.
"},
);
// Moving the mouse over a path that exists, if we add the language-specific suffix, it should highlight it
#[cfg(not(target_os = "windows"))]
let screen_coord = cx.pixel_position(indoc! {"
You can't go to a file that does_not_exist.txt.
Go to file2.rs if you want.
Or go to ../dir/file2.rs if you want.
Or go to /root/dir/file2.rs if project is local.
Or go to /root/diˇr/file2 if this is a Rust file.
"});
#[cfg(target_os = "windows")]
let screen_coord = cx.pixel_position(indoc! {"
You can't go to a file that does_not_exist.txt.
Go to file2.rs if you want.
Or go to ../dir/file2.rs if you want.
Or go to C:/root/dir/file2.rs if project is local.
Or go to C:/root/diˇr/file2 if this is a Rust file.
"});
cx.simulate_mouse_move(screen_coord, None, Modifiers::secondary_key());
#[cfg(not(target_os = "windows"))]
cx.assert_editor_text_highlights(
HighlightKey::HoveredLinkState,
indoc! {"
You can't go to a file that does_not_exist.txt.
Go to file2.rs if you want.
Or go to ../dir/file2.rs if you want.
Or go to /root/dir/file2.rs if project is local.
Or go to «/root/dir/file2ˇ» if this is a Rust file.
"},
);
#[cfg(target_os = "windows")]
cx.assert_editor_text_highlights(
HighlightKey::HoveredLinkState,
indoc! {"
You can't go to a file that does_not_exist.txt.
Go to file2.rs if you want.
Or go to ../dir/file2.rs if you want.
Or go to C:/root/dir/file2.rs if project is local.
Or go to «C:/root/dir/file2ˇ» if this is a Rust file.
"},
);
cx.simulate_click(screen_coord, Modifiers::secondary_key()); cx.simulate_click(screen_coord, Modifiers::secondary_key());
cx.update_workspace(|workspace, _, cx| assert_eq!(workspace.items(cx).count(), 2)); cx.update_workspace(|workspace, _, cx| assert_eq!(workspace.items(cx).count(), 2));