Preserve local Markdown link labels in the TUI (#50695)

## Why

Path-like link labels were collapsed into their destinations, discarding the author's label text and formatting.

## What changed

Preserve nonblank local link labels alongside the formatted destination as `label (target)`, including in tables. Empty or whitespace-only labels show only the destination. Keep destination formatting and fall back to the original destination when a local file URL cannot be parsed.

Render file citations without a label so they continue to display only the formatted destination.

## Testing

Update rendering and transcript-copy tests and snapshots to cover preserved path labels, styled labels, empty labels, malformed file URLs, tables, and Markdown escaping.

GitOrigin-RevId: 2810b70f1ecdc59d7f90fa1b254b1e9069c8fc29
This commit is contained in:
Benjamin Carlsson
2026-10-03 18:44:14 +00:00
committed by copyberry
parent 58ae3ba611
commit 3a69ec3ef8
13 changed files with 188 additions and 180 deletions
+6 -7
View File
@@ -92,7 +92,6 @@ pub(crate) use list_spacing::ListSpacing;
use list_spacing::UniformList;
use local_links::is_local_path_like_link;
use local_links::render_local_link_target;
use local_links::should_render_local_link_label;
pub(crate) use streaming::StreamingMarkdownRender;
pub(crate) use streaming::render_streaming_markdown_lines_with_width_and_cwd;
#[cfg(test)]
@@ -2172,7 +2171,10 @@ impl<'a, 'policy> Writer<'a, 'policy> {
style_label,
has_visible_label: false,
local_target_display: if is_local_path_like_link(&dest_url) {
render_local_link_target(&dest_url, self.cwd.as_deref())
Some(
render_local_link_target(&dest_url, self.cwd.as_deref())
.unwrap_or_else(|| dest_url.clone()),
)
} else {
None
},
@@ -2217,13 +2219,10 @@ impl<'a, 'policy> Writer<'a, 'policy> {
self.push_span(")".into());
}
} else if let Some(local_target_display) = link.local_target_display {
let local_label_text = link
let show_label = link
.local_label_spans
.iter()
.map(|span| span.content.as_ref())
.collect::<String>();
let show_label =
should_render_local_link_label(&local_label_text, &link.destination);
.any(|span| !span.content.trim().is_empty());
let style = self
.inline_styles
.last()
@@ -24,6 +24,7 @@ fn file_citation_paths_preserve_markdown_significant_characters() {
"/tmp/report:10",
"/tmp/report%20final.xlsx",
"/tmp/report?final.xlsx",
"file://[",
] {
let markdown = format!(":codex-file-citation{{path=\"{path}\"}}");
assert_eq!(rendered_text(&markdown, /*cwd*/ None), path);
@@ -233,18 +233,17 @@ impl<'a> InlineDirectives<'a> {
let suffix_start = destination.len() - suffix.len();
destination.replace_range(suffix_start.., &suffix.replace(':', "%3A"));
}
// Citations have no descriptive label; compare the same encoded path on both sides.
// Citations have no author-provided label; render the formatted destination alone.
events.extend([
(
Event::Start(Tag::Link {
link_type: LinkType::Inline,
dest_url: destination.clone().into(),
dest_url: destination.into(),
title: "".into(),
id: "".into(),
}),
span.clone(),
),
(Event::Text(destination.into()), span.clone()),
(Event::End(TagEnd::Link), span.clone()),
]);
}
@@ -1,11 +1,9 @@
//! Local file-link parsing, label comparison, and display for Markdown transcripts.
//!
//! Markdown rendering intentionally treats local file links differently from normal web links. For
//! local paths, transcripts always show the real file target (including normalized location
//! suffixes) and can shorten absolute paths relative to a known working directory. Descriptive
//! Markdown labels remain visible alongside that target, while path-like labels collapse to the
//! canonical target to avoid duplicate file references.
//! Local file-link destination parsing and display for Markdown transcripts.
//!
//! Local Markdown links preserve their labels. Since ordinary file destinations have no trusted
//! navigation capability, the renderer also displays their target, shortening absolute paths
//! relative to the session working directory when possible. This module only formats destinations;
//! it does not resolve files or infer execution-host paths from the frontend filesystem.
use codex_utils_path_uri::PathUri;
use codex_utils_string::normalize_markdown_hash_location_suffix;
@@ -43,53 +41,6 @@ pub(super) fn is_local_path_like_link(dest_url: &str) -> bool {
)
}
/// Decide whether a local-file link label adds meaning beyond its canonical target.
///
/// Matching path-like labels collapse to the target; prose labels remain visible.
pub(super) fn should_render_local_link_label(label: &str, destination: &str) -> bool {
let label = label.trim();
if label.is_empty() {
return false;
}
let Some(parsed_label) = comparable_local_link_path(label) else {
return true;
};
let Some(target) = comparable_local_link_path(destination) else {
return true;
};
let target_path = trim_trailing_local_path_separator(target.trim_start_matches("./"));
let has_boundary_suffix = |path: &str, suffix: &str| {
!suffix.is_empty()
&& path
.strip_suffix(suffix)
.is_some_and(|prefix| prefix.is_empty() || prefix.ends_with('/'))
};
// Labels can spell a filename literally or URL-encode it. Compare both without decoding
// the destination twice (for example, percent%2520.rs denotes percent%20.rs).
let literal_label = normalize_local_link_path_text(label).to_lowercase();
![literal_label, parsed_label].iter().any(|label| {
let label_path = trim_trailing_local_path_separator(label.trim_start_matches("./"));
has_boundary_suffix(target_path, label_path)
|| (is_absolute_local_link_path(label_path)
&& has_boundary_suffix(label_path, target_path))
})
}
/// Normalize original Markdown strings for comparison only, never already-rendered path text.
/// Case and URL spelling are intentionally forgiving; the visible target retains its spelling.
fn comparable_local_link_path(text: &str) -> Option<String> {
let text = if text
.get(..7)
.is_some_and(|scheme| scheme.eq_ignore_ascii_case("file://"))
{
std::borrow::Cow::Owned(format!("file://{}", &text[7..]))
} else {
std::borrow::Cow::Borrowed(text)
};
let (path, _) = parse_local_link_target(&text)?;
Some(normalize_local_link_path_text(&path).to_lowercase())
}
/// Parse a local link target into normalized path text plus an optional location suffix.
///
/// This accepts the path shapes Codex emits today: `file://` URLs, absolute and relative paths,
+95 -58
View File
@@ -738,7 +738,7 @@ fn inline_code() {
#[test]
fn inline_code_and_file_paths_follow_syntax_theme() {
let markdown = "Use `src/main.rs` and [src/lib.rs](./src/lib.rs).";
let markdown = "Use `src/main.rs` and [](./src/lib.rs).";
let mut rendered = Vec::new();
for name in [
"catppuccin-mocha",
@@ -911,15 +911,20 @@ fn web_link_labels_have_a_visible_underline_snapshot() {
}
#[test]
fn file_link_hides_destination() {
fn file_link_preserves_label_and_decodes_fallback_destination() {
let text = render_markdown_text_for_cwd(
"[/Users/example/code/codex/codex-rs/tui/src/My%20File.rs](/Users/example/code/codex/codex-rs/tui/src/My%20File.rs)",
Path::new("/Users/example/code/codex"),
);
let expected = Text::from(Line::from_iter([Span::styled(
"codex-rs/tui/src/My File.rs",
MarkdownStyles::default().code,
)]));
let expected = Text::from(Line::from_iter([
"/Users/example/code/codex/codex-rs/tui/src/My%20File.rs".into(),
" (".into(),
Span::styled(
"codex-rs/tui/src/My File.rs",
MarkdownStyles::default().code,
),
")".into(),
]));
assert_eq!(text, expected);
}
@@ -973,7 +978,7 @@ fn file_link_preserves_tilde_and_absolute_destinations() {
}
#[test]
fn file_link_compares_path_spellings_without_changing_display() {
fn file_link_preserves_path_labels_and_formats_destinations() {
let markdown = r"[file:///repo/src/lib.rs](file:///repo/src/lib.rs)
[SRC\LIB.RS](/repo/src/lib.rs#L12)
@@ -999,7 +1004,7 @@ fn file_link_compares_path_spellings_without_changing_display() {
[other/src/lib.rs](/repo/src/lib.rs)";
let text = render_markdown_text_for_cwd(markdown, Path::new("/repo"));
// UNC file-URL display currently preserves escapes on Unix but decodes them on Windows.
// Keep that existing display behavior separate from comparison normalization.
// Keep that platform-dependent destination display out of this shared snapshot.
let rendered = plain_lines(&text)
.join("\n")
.replace("My%20File.rs", "My File.rs");
@@ -1034,7 +1039,7 @@ fn file_link_preserves_labels_with_invalid_percent_encoding() {
}
#[test]
fn file_link_ignores_trailing_separators_when_comparing_paths() {
fn file_link_preserves_directory_labels_and_destinations() {
let text = render_markdown_text_for_cwd(
"[dir](./dir/)\n\n[dir/](./dir)\n\n[dir](/outside/dir/)\n\n[dir/](/outside/dir)",
Path::new("/repo"),
@@ -1062,10 +1067,15 @@ fn file_link_appends_line_number_when_label_lacks_it() {
"[markdown_render.rs](/Users/example/code/codex/codex-rs/tui/src/markdown_render.rs:74)",
Path::new("/Users/example/code/codex"),
);
let expected = Text::from(Line::from_iter([Span::styled(
"codex-rs/tui/src/markdown_render.rs:74",
MarkdownStyles::default().code,
)]));
let expected = Text::from(Line::from_iter([
"markdown_render.rs".into(),
" (".into(),
Span::styled(
"codex-rs/tui/src/markdown_render.rs:74",
MarkdownStyles::default().code,
),
")".into(),
]));
assert_eq!(text, expected);
}
@@ -1075,10 +1085,15 @@ fn file_link_keeps_absolute_paths_outside_cwd() {
"[README.md:74](/Users/example/code/codex/README.md:74)",
Path::new("/Users/example/code/codex/codex-rs/tui"),
);
let expected = Text::from(Line::from_iter([Span::styled(
"/Users/example/code/codex/README.md:74",
MarkdownStyles::default().code,
)]));
let expected = Text::from(Line::from_iter([
"README.md:74".into(),
" (".into(),
Span::styled(
"/Users/example/code/codex/README.md:74",
MarkdownStyles::default().code,
),
")".into(),
]));
assert_eq!(text, expected);
}
@@ -1109,10 +1124,15 @@ fn file_link_appends_hash_anchor_when_label_lacks_it() {
"[markdown_render.rs](file:///Users/example/code/codex/codex-rs/tui/src/markdown_render.rs#L74C3)",
Path::new("/Users/example/code/codex"),
);
let expected = Text::from(Line::from_iter([Span::styled(
"codex-rs/tui/src/markdown_render.rs:74:3",
MarkdownStyles::default().code,
)]));
let expected = Text::from(Line::from_iter([
"markdown_render.rs".into(),
" (".into(),
Span::styled(
"codex-rs/tui/src/markdown_render.rs:74:3",
MarkdownStyles::default().code,
),
")".into(),
]));
assert_eq!(text, expected);
}
@@ -1122,10 +1142,15 @@ fn file_link_uses_target_path_for_hash_anchor() {
"[markdown_render.rs#L74C3](file:///Users/example/code/codex/codex-rs/tui/src/markdown_render.rs#L74C3)",
Path::new("/Users/example/code/codex"),
);
let expected = Text::from(Line::from_iter([Span::styled(
"codex-rs/tui/src/markdown_render.rs:74:3",
MarkdownStyles::default().code,
)]));
let expected = Text::from(Line::from_iter([
"markdown_render.rs#L74C3".into(),
" (".into(),
Span::styled(
"codex-rs/tui/src/markdown_render.rs:74:3",
MarkdownStyles::default().code,
),
")".into(),
]));
assert_eq!(text, expected);
}
@@ -1135,10 +1160,15 @@ fn file_link_appends_range_when_label_lacks_it() {
"[markdown_render.rs](/Users/example/code/codex/codex-rs/tui/src/markdown_render.rs:74:3-76:9)",
Path::new("/Users/example/code/codex"),
);
let expected = Text::from(Line::from_iter([Span::styled(
"codex-rs/tui/src/markdown_render.rs:74:3-76:9",
MarkdownStyles::default().code,
)]));
let expected = Text::from(Line::from_iter([
"markdown_render.rs".into(),
" (".into(),
Span::styled(
"codex-rs/tui/src/markdown_render.rs:74:3-76:9",
MarkdownStyles::default().code,
),
")".into(),
]));
assert_eq!(text, expected);
}
@@ -1148,10 +1178,15 @@ fn file_link_uses_target_path_for_range() {
"[markdown_render.rs:74:3-76:9](/Users/example/code/codex/codex-rs/tui/src/markdown_render.rs:74:3-76:9)",
Path::new("/Users/example/code/codex"),
);
let expected = Text::from(Line::from_iter([Span::styled(
"codex-rs/tui/src/markdown_render.rs:74:3-76:9",
MarkdownStyles::default().code,
)]));
let expected = Text::from(Line::from_iter([
"markdown_render.rs:74:3-76:9".into(),
" (".into(),
Span::styled(
"codex-rs/tui/src/markdown_render.rs:74:3-76:9",
MarkdownStyles::default().code,
),
")".into(),
]));
assert_eq!(text, expected);
}
@@ -1161,10 +1196,15 @@ fn file_link_appends_hash_range_when_label_lacks_it() {
"[markdown_render.rs](file:///Users/example/code/codex/codex-rs/tui/src/markdown_render.rs#L74C3-L76C9)",
Path::new("/Users/example/code/codex"),
);
let expected = Text::from(Line::from_iter([Span::styled(
"codex-rs/tui/src/markdown_render.rs:74:3-76:9",
MarkdownStyles::default().code,
)]));
let expected = Text::from(Line::from_iter([
"markdown_render.rs".into(),
" (".into(),
Span::styled(
"codex-rs/tui/src/markdown_render.rs:74:3-76:9",
MarkdownStyles::default().code,
),
")".into(),
]));
assert_eq!(text, expected);
}
@@ -1196,10 +1236,15 @@ fn file_link_uses_target_path_for_hash_range() {
"[markdown_render.rs#L74C3-L76C9](file:///Users/example/code/codex/codex-rs/tui/src/markdown_render.rs#L74C3-L76C9)",
Path::new("/Users/example/code/codex"),
);
let expected = Text::from(Line::from_iter([Span::styled(
"codex-rs/tui/src/markdown_render.rs:74:3-76:9",
MarkdownStyles::default().code,
)]));
let expected = Text::from(Line::from_iter([
"markdown_render.rs#L74C3-L76C9".into(),
" (".into(),
Span::styled(
"codex-rs/tui/src/markdown_render.rs:74:3-76:9",
MarkdownStyles::default().code,
),
")".into(),
]));
assert_eq!(text, expected);
}
@@ -1217,23 +1262,15 @@ fn url_link_shows_destination() {
#[test]
fn markdown_render_file_link_snapshot() {
let text = render_markdown_text_for_cwd(
"See [markdown_render.rs:74](/Users/example/code/codex/codex-rs/tui/src/markdown_render.rs:74).",
Path::new("/Users/example/code/codex"),
);
let rendered = text
.lines
.iter()
.map(|l| {
l.spans
.iter()
.map(|s| s.content.clone())
.collect::<String>()
})
.collect::<Vec<_>>()
.join("\n");
assert_snapshot!(rendered);
let markdown = "[analysis.md](/tmp/codex-link-repro/analysis/021_all_experiments_best_per_group/analysis.md)\n\n\
[**analysis** `.md`](/tmp/codex-link-repro/analysis.md:12)\n\n\
[](/tmp/codex-link-repro/analysis.md#L12C3)\n\n\
[ ](../analysis.md)\n\n\
[](<file://[>)\n\n\
[broken](<file://[>)\n\n\
| File |\n| --- |\n| [analysis.md](/tmp/codex-link-repro/analysis.md) |\n| [](./empty.md) |";
let text = render_markdown_text_for_cwd(markdown, Path::new("/tmp/codex-link-repro"));
assert_snapshot!(plain_lines(&text).join("\n"));
}
#[test]
@@ -1,27 +0,0 @@
---
source: tui/src/markdown_render_tests.rs
expression: rendered
---
src/lib.rs
src/lib.rs:12
src/lib.rs
C:/Repo/Src/Lib.rs
C:/Repo/Src/Lib.rs
//server/share/My File.rs
//server/share/My File.rs
My File.rs
percent%20.rs
percent%20.rs
open My File.rs (My File.rs)
other/src/lib.rs (src/lib.rs)
@@ -2,10 +2,10 @@
source: tui/src/markdown_render_tests.rs
expression: "plain_lines(&text).join(\"\\n\")"
---
./dir/
dir (./dir/)
./dir
dir/ (./dir)
/outside/dir/
dir (/outside/dir/)
/outside/dir
dir/ (/outside/dir)
@@ -0,0 +1,27 @@
---
source: tui/src/markdown_render_tests.rs
expression: rendered
---
file:///repo/src/lib.rs (src/lib.rs)
SRC\LIB.RS (src/lib.rs:12)
./src/lib.rs (src/lib.rs)
file:///C:/Repo/Src/Lib.rs (C:/Repo/Src/Lib.rs)
FILE:///C:/Repo/Src/Lib.rs (C:/Repo/Src/Lib.rs)
file://server/share/My File.rs (//server/share/My File.rs)
//SERVER/SHARE/My File.rs (//server/share/My File.rs)
file:///repo/My File.rs (My File.rs)
file:///repo/percent%2520.rs (percent%20.rs)
percent%20.rs (percent%20.rs)
open My File.rs (My File.rs)
other/src/lib.rs (src/lib.rs)
@@ -2,9 +2,9 @@
source: tui/src/markdown_render_tests.rs
expression: "plain_lines(&text).join(\"\\n\")"
---
~/notes
~/notes (~/notes)
/home/alice/notes
/home/alice/notes (/home/alice/notes)
/home/alice/notes (~/notes)
@@ -12,8 +12,8 @@ expression: "plain_lines(&text).join(\"\\n\")"
my notes (~/notes:12:3)
~/project/src/lib.rs
~/project/src/lib.rs (~/project/src/lib.rs)
src/lib.rs
/home/alice/project/src/lib.rs (src/lib.rs)
~ (/home/alice:12)
@@ -1,5 +1,21 @@
---
source: tui/src/markdown_render_tests.rs
expression: rendered
expression: "plain_lines(&text).join(\"\\n\")"
---
See codex-rs/tui/src/markdown_render.rs:74.
analysis.md (analysis/021_all_experiments_best_per_group/analysis.md)
analysis .md (analysis.md:12)
analysis.md:12:3
../analysis.md
file://[
broken (file://[)
File
━━━━━━━━━━━━━━━━━━━━━━━━━━━
analysis.md (analysis.md)
───────────────────────────
./empty.md
@@ -4,22 +4,30 @@ expression: "plain_lines(&text).join(\"\\n\")"
---
Unit Files Adds Removes What It Adds
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ━━━━━━━━━━━━━━━━━━ ━━━━━━ ━━━━━━━━━ ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Suggestion engine and unit coverage codex-rs/core/ 704 0 Sampling workflow, stable-history
src/ checks, tool-flow suppression, fast
next_prompt_sugg reasoning profile, filtering rules,
estion.rs:1, cancellation and timeout.
codex-rs/core/
Suggestion engine and unit coverage next_prompt_sugg 704 0 Sampling workflow, stable-history
estion.rs checks, tool-flow suppression, fast
(codex-rs/core/ reasoning profile, filtering rules,
src/ cancellation and timeout.
next_prompt_sugg
estion.rs:1),
next_prompt_sugg
estion_tests.rs
(codex-rs/core/
src/
next_prompt_sugg
estion_tests.rs:
1
1)
─────────────────────────────────────── ────────────────── ────── ───────── ────────────────────────────────────────
Model instruction fragment and codex-rs/core/ 54 0 Synthetic suggestion prompt and an
contextual isolation src/context/ isolation test for ordinary user text.
Model instruction fragment and next_prompt_sugg 54 0 Synthetic suggestion prompt and an
contextual isolation estion.rs isolation test for ordinary user text.
(codex-rs/core/
src/context/
next_prompt_sugg
estion.rs:1,
codex-rs/core/
estion.rs:1),
contextual_user_
message_tests.rs
(codex-rs/core/
src/context/
contextual_user_
message_tests.rs
:1
:1)
@@ -133,11 +133,8 @@ fn inline_code_selection_copies_only_selected_content() {
(r"before ``real\_literal\!`` after", r"real\_literal\!"),
("before [`foo_bar!`](/repo/foo_bar!) after", "repo/foo_bar!"),
("before [`foo_bar!`](/repo/foo_bar!) after", "bar!"),
(
"[foo_bar!](/foo_bar!)[baz_qux!](/baz_qux!)",
"foo_bar!baz_qux!",
),
("`foo_bar!`[baz_qux!](/baz_qux!)", "foo_bar!baz_qux!"),
("[](/foo_bar!)[](/baz_qux!)", "foo_bar!baz_qux!"),
("`foo_bar!`[](/baz_qux!)", "foo_bar!baz_qux!"),
("before **[file](/repo/foo_bar!)** after", "foo_bar!"),
(
"| File |\n|---|\n| [`foo_bar!`](/repo/foo_bar!) |",
@@ -168,11 +165,11 @@ fn inline_code_selection_copies_only_selected_content() {
#[test]
fn mixed_file_targets_preserve_markdown_escaping() {
for (source, expected) in [
("before [+](/+) after", "before + after"),
("before [1.)](</1.)>) after", "before 1.) after"),
("before [+](/+) after", "before + (+) after"),
("before [1.)](</1.)>) after", "before 1.) (1.)) after"),
(
"| File |\n|---|\n| [`foo_bar!`](/repo/foo_bar!) |",
"| File |\n|---|\n| repo/foo\\_bar\\! |",
"| File |\n|---|\n| `foo_bar!` (repo/foo\\_bar\\!) |",
),
] {
let layout = markdown_layout(source, /*width*/ 80);
@@ -22,9 +22,9 @@ before [`foo_bar!`](/repo/foo_bar!) after
Selected "repo/foo_bar!" → ("repo/foo_bar!", PlainText)
before [`foo_bar!`](/repo/foo_bar!) after
Selected "bar!" → ("bar!", PlainText)
[foo_bar!](/foo_bar!)[baz_qux!](/baz_qux!)
[](/foo_bar!)[](/baz_qux!)
Selected "foo_bar!baz_qux!" → ("foo_bar!baz_qux!", PlainText)
`foo_bar!`[baz_qux!](/baz_qux!)
`foo_bar!`[](/baz_qux!)
Selected "foo_bar!baz_qux!" → ("foo_bar!baz_qux!", PlainText)
before **[file](/repo/foo_bar!)** after
Selected "foo_bar!" → ("foo_bar!", PlainText)