From 6acede5a28c05cd313bdc19a9d0330c8552e87b0 Mon Sep 17 00:00:00 2001 From: pash-openai Date: Thu, 26 Feb 2026 10:29:54 +0000 Subject: [PATCH] tui: restore visible line numbers for hidden file links (#12870) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit we recently changed file linking so the model uses markdown links when it wants something to be clickable. This works well across the GUI surfaces because they can render markdown cleanly and use the full absolute path in the anchor target. A previous pass hid the absolute path in the TUI (and only showed the label), but that also meant we could lose useful location info when the model put the line number or range in the anchor target instead of the label. This follow-up keeps the TUI behavior simple while making local file links feel closer to the old TUI file reference style. key changes: - Local markdown file links in the TUI keep the old file-ref feel: code styling, no underline, no visible absolute path. - If the hidden local anchor target includes a location suffix and the label does not already include one, we append that suffix to the visible label. - This works for single lines, line/column references, and ranges. - If the label already includes the location, we leave it alone. - normal web links keep the old TUI markdown-link behavior some examples: - `[foo.rs](/abs/path/foo.rs)` renders as `foo.rs` - `[foo.rs](/abs/path/foo.rs:45)` renders as `foo.rs:45` - `[foo.rs](/abs/path/foo.rs:45:3-48:9)` renders as `foo.rs:45:3-48:9` - `[foo.rs:45](/abs/path/foo.rs:45)` stays `foo.rs:45` - `[docs](https://example.com/docs)` still renders like a normal web link how it looks: Screenshot 2026-02-26 at 9 27 55 AM --- codex-rs/Cargo.lock | 1 + codex-rs/tui/Cargo.toml | 1 + codex-rs/tui/src/markdown_render.rs | 84 ++++++++++++- codex-rs/tui/src/markdown_render_tests.rs | 115 +++++++++++++++++- ...s__markdown_render_file_link_snapshot.snap | 1 + codex-rs/utils/string/src/lib.rs | 52 ++++++++ 6 files changed, 246 insertions(+), 8 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index c888f9368..4fdcc9ecd 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2399,6 +2399,7 @@ dependencies = [ "codex-utils-pty", "codex-utils-sandbox-summary", "codex-utils-sleep-inhibitor", + "codex-utils-string", "codex-windows-sandbox", "color-eyre", "cpal", diff --git a/codex-rs/tui/Cargo.toml b/codex-rs/tui/Cargo.toml index 1a87314e9..594acb33d 100644 --- a/codex-rs/tui/Cargo.toml +++ b/codex-rs/tui/Cargo.toml @@ -50,6 +50,7 @@ codex-utils-fuzzy-match = { workspace = true } codex-utils-oss = { workspace = true } codex-utils-sandbox-summary = { workspace = true } codex-utils-sleep-inhibitor = { workspace = true } +codex-utils-string = { workspace = true } color-eyre = { workspace = true } crossterm = { workspace = true, features = ["bracketed-paste", "event-stream"] } derive_more = { workspace = true, features = ["is_variant"] } diff --git a/codex-rs/tui/src/markdown_render.rs b/codex-rs/tui/src/markdown_render.rs index a7e80acae..2bbe19b6f 100644 --- a/codex-rs/tui/src/markdown_render.rs +++ b/codex-rs/tui/src/markdown_render.rs @@ -2,6 +2,7 @@ use crate::render::highlight::highlight_code_to_lines; use crate::render::line_utils::line_to_static; use crate::wrapping::RtOptions; use crate::wrapping::adaptive_wrap_line; +use codex_utils_string::normalize_markdown_hash_location_suffix; use pulldown_cmark::CodeBlockKind; use pulldown_cmark::CowStr; use pulldown_cmark::Event; @@ -14,6 +15,8 @@ use ratatui::style::Style; use ratatui::text::Line; use ratatui::text::Span; use ratatui::text::Text; +use regex_lite::Regex; +use std::sync::LazyLock; struct MarkdownStyles { h1: Style, @@ -89,12 +92,30 @@ pub(crate) fn render_markdown_text_with_width(input: &str, width: Option) struct LinkState { destination: String, show_destination: bool, + hidden_location_suffix: Option, + label_start_span_idx: usize, + label_styled: bool, } fn should_render_link_destination(dest_url: &str) -> bool { !is_local_path_like_link(dest_url) } +static COLON_LOCATION_SUFFIX_RE: LazyLock = + LazyLock::new( + || match Regex::new(r":\d+(?::\d+)?(?:[-–]\d+(?::\d+)?)?$") { + Ok(regex) => regex, + Err(error) => panic!("invalid location suffix regex: {error}"), + }, + ); + +// Covered by load_location_suffix_regexes. +static HASH_LOCATION_SUFFIX_RE: LazyLock = + LazyLock::new(|| match Regex::new(r"^L\d+(?:C\d+)?(?:-L\d+(?:C\d+)?)?$") { + Ok(regex) => regex, + Err(error) => panic!("invalid hash location regex: {error}"), + }); + fn is_local_path_like_link(dest_url: &str) -> bool { dest_url.starts_with("file://") || dest_url.starts_with('/') @@ -491,20 +512,77 @@ where } fn push_link(&mut self, dest_url: String) { - self.push_inline_style(self.styles.link); + let show_destination = should_render_link_destination(&dest_url); + let label_styled = !show_destination; + let label_start_span_idx = self + .current_line_content + .as_ref() + .map(|line| line.spans.len()) + .unwrap_or(0); + if label_styled { + self.push_inline_style(self.styles.code); + } self.link = Some(LinkState { - show_destination: should_render_link_destination(&dest_url), + show_destination, + hidden_location_suffix: if is_local_path_like_link(&dest_url) { + dest_url + .rsplit_once('#') + .and_then(|(_, fragment)| { + HASH_LOCATION_SUFFIX_RE + .is_match(fragment) + .then(|| format!("#{fragment}")) + }) + .and_then(|suffix| normalize_markdown_hash_location_suffix(&suffix)) + .or_else(|| { + COLON_LOCATION_SUFFIX_RE + .find(&dest_url) + .map(|m| m.as_str().to_string()) + }) + } else { + None + }, + label_start_span_idx, + label_styled, destination: dest_url, }); } fn pop_link(&mut self) { if let Some(link) = self.link.take() { - self.pop_inline_style(); if link.show_destination { + if link.label_styled { + self.pop_inline_style(); + } self.push_span(" (".into()); self.push_span(Span::styled(link.destination, self.styles.link)); self.push_span(")".into()); + } else if let Some(location_suffix) = link.hidden_location_suffix.as_deref() { + let label_text = self + .current_line_content + .as_ref() + .and_then(|line| { + line.spans.get(link.label_start_span_idx..).map(|spans| { + spans + .iter() + .map(|span| span.content.as_ref()) + .collect::() + }) + }) + .unwrap_or_default(); + if label_text + .rsplit_once('#') + .is_some_and(|(_, fragment)| HASH_LOCATION_SUFFIX_RE.is_match(fragment)) + || COLON_LOCATION_SUFFIX_RE.find(&label_text).is_some() + { + // The label already carries a location suffix; don't duplicate it. + } else { + self.push_span(Span::styled(location_suffix.to_string(), self.styles.code)); + } + if link.label_styled { + self.pop_inline_style(); + } + } else if link.label_styled { + self.pop_inline_style(); } } } diff --git a/codex-rs/tui/src/markdown_render_tests.rs b/codex-rs/tui/src/markdown_render_tests.rs index 6457bf02c..998124609 100644 --- a/codex-rs/tui/src/markdown_render_tests.rs +++ b/codex-rs/tui/src/markdown_render_tests.rs @@ -4,6 +4,8 @@ use ratatui::text::Line; use ratatui::text::Span; use ratatui::text::Text; +use crate::markdown_render::COLON_LOCATION_SUFFIX_RE; +use crate::markdown_render::HASH_LOCATION_SUFFIX_RE; use crate::markdown_render::render_markdown_text; use insta::assert_snapshot; @@ -643,7 +645,7 @@ fn strong_emphasis() { fn link() { let text = render_markdown_text("[Link](https://example.com)"); let expected = Text::from(Line::from_iter([ - "Link".cyan().underlined(), + "Link".into(), " (".into(), "https://example.com".cyan().underlined(), ")".into(), @@ -651,11 +653,114 @@ fn link() { assert_eq!(text, expected); } +#[test] +fn load_location_suffix_regexes() { + let _colon = &*COLON_LOCATION_SUFFIX_RE; + let _hash = &*HASH_LOCATION_SUFFIX_RE; +} + #[test] fn file_link_hides_destination() { - let text = - render_markdown_text("[markdown_render.rs:74](/Users/example/code/codex/codex-rs/tui/src/markdown_render.rs:74)"); - let expected = Text::from(Line::from("markdown_render.rs:74".cyan().underlined())); + let text = render_markdown_text( + "[codex-rs/tui/src/markdown_render.rs](/Users/example/code/codex/codex-rs/tui/src/markdown_render.rs)", + ); + let expected = Text::from(Line::from_iter(["codex-rs/tui/src/markdown_render.rs".cyan()])); + assert_eq!(text, expected); +} + +#[test] +fn file_link_appends_line_number_when_label_lacks_it() { + let text = render_markdown_text( + "[markdown_render.rs](/Users/example/code/codex/codex-rs/tui/src/markdown_render.rs:74)", + ); + let expected = Text::from(Line::from_iter([ + "markdown_render.rs".cyan(), + ":74".cyan(), + ])); + assert_eq!(text, expected); +} + +#[test] +fn file_link_uses_label_for_line_number() { + let text = render_markdown_text( + "[markdown_render.rs:74](/Users/example/code/codex/codex-rs/tui/src/markdown_render.rs:74)", + ); + let expected = Text::from(Line::from_iter(["markdown_render.rs:74".cyan()])); + assert_eq!(text, expected); +} + +#[test] +fn file_link_appends_hash_anchor_when_label_lacks_it() { + let text = render_markdown_text( + "[markdown_render.rs](file:///Users/example/code/codex/codex-rs/tui/src/markdown_render.rs#L74C3)", + ); + let expected = Text::from(Line::from_iter([ + "markdown_render.rs".cyan(), + ":74:3".cyan(), + ])); + assert_eq!(text, expected); +} + +#[test] +fn file_link_uses_label_for_hash_anchor() { + let text = render_markdown_text( + "[markdown_render.rs#L74C3](file:///Users/example/code/codex/codex-rs/tui/src/markdown_render.rs#L74C3)", + ); + let expected = Text::from(Line::from_iter(["markdown_render.rs#L74C3".cyan()])); + assert_eq!(text, expected); +} + +#[test] +fn file_link_appends_range_when_label_lacks_it() { + let text = render_markdown_text( + "[markdown_render.rs](/Users/example/code/codex/codex-rs/tui/src/markdown_render.rs:74:3-76:9)", + ); + let expected = Text::from(Line::from_iter([ + "markdown_render.rs".cyan(), + ":74:3-76:9".cyan(), + ])); + assert_eq!(text, expected); +} + +#[test] +fn file_link_uses_label_for_range() { + let text = render_markdown_text( + "[markdown_render.rs:74:3-76:9](/Users/example/code/codex/codex-rs/tui/src/markdown_render.rs:74:3-76:9)", + ); + let expected = Text::from(Line::from_iter(["markdown_render.rs:74:3-76:9".cyan()])); + assert_eq!(text, expected); +} + +#[test] +fn file_link_appends_hash_range_when_label_lacks_it() { + let text = render_markdown_text( + "[markdown_render.rs](file:///Users/example/code/codex/codex-rs/tui/src/markdown_render.rs#L74C3-L76C9)", + ); + let expected = Text::from(Line::from_iter([ + "markdown_render.rs".cyan(), + ":74:3-76:9".cyan(), + ])); + assert_eq!(text, expected); +} + +#[test] +fn multiline_file_link_label_after_styled_prefix_does_not_panic() { + let text = render_markdown_text( + "**bold** plain [foo\nbar](file:///Users/example/code/codex/codex-rs/tui/src/markdown_render.rs#L74C3)", + ); + let expected = Text::from_iter([ + Line::from_iter(["bold".bold(), " plain ".into(), "foo".cyan()]), + Line::from_iter(["bar".cyan(), ":74:3".cyan()]), + ]); + assert_eq!(text, expected); +} + +#[test] +fn file_link_uses_label_for_hash_range() { + let text = render_markdown_text( + "[markdown_render.rs#L74C3-L76C9](file:///Users/example/code/codex/codex-rs/tui/src/markdown_render.rs#L74C3-L76C9)", + ); + let expected = Text::from(Line::from_iter(["markdown_render.rs#L74C3-L76C9".cyan()])); assert_eq!(text, expected); } @@ -663,7 +768,7 @@ fn file_link_hides_destination() { fn url_link_shows_destination() { let text = render_markdown_text("[docs](https://example.com/docs)"); let expected = Text::from(Line::from_iter([ - "docs".cyan().underlined(), + "docs".into(), " (".into(), "https://example.com/docs".cyan().underlined(), ")".into(), diff --git a/codex-rs/tui/src/snapshots/codex_tui__markdown_render__markdown_render_tests__markdown_render_file_link_snapshot.snap b/codex-rs/tui/src/snapshots/codex_tui__markdown_render__markdown_render_tests__markdown_render_file_link_snapshot.snap index 6e571852e..1b1f1210f 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__markdown_render__markdown_render_tests__markdown_render_file_link_snapshot.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__markdown_render__markdown_render_tests__markdown_render_file_link_snapshot.snap @@ -1,5 +1,6 @@ --- source: tui/src/markdown_render_tests.rs +assertion_line: 714 expression: rendered --- See markdown_render.rs:74. diff --git a/codex-rs/utils/string/src/lib.rs b/codex-rs/utils/string/src/lib.rs index df9ae59e1..0cb218f34 100644 --- a/codex-rs/utils/string/src/lib.rs +++ b/codex-rs/utils/string/src/lib.rs @@ -76,9 +76,45 @@ pub fn find_uuids(s: &str) -> Vec { re.find_iter(s).map(|m| m.as_str().to_string()).collect() } +/// Convert a markdown-style `#L..` location suffix into a terminal-friendly +/// `:line[:column][-line[:column]]` suffix. +pub fn normalize_markdown_hash_location_suffix(suffix: &str) -> Option { + let fragment = suffix.strip_prefix('#')?; + let (start, end) = match fragment.split_once('-') { + Some((start, end)) => (start, Some(end)), + None => (fragment, None), + }; + let (start_line, start_column) = parse_markdown_hash_location_point(start)?; + let mut normalized = String::from(":"); + normalized.push_str(start_line); + if let Some(column) = start_column { + normalized.push(':'); + normalized.push_str(column); + } + if let Some(end) = end { + let (end_line, end_column) = parse_markdown_hash_location_point(end)?; + normalized.push('-'); + normalized.push_str(end_line); + if let Some(column) = end_column { + normalized.push(':'); + normalized.push_str(column); + } + } + Some(normalized) +} + +fn parse_markdown_hash_location_point(point: &str) -> Option<(&str, Option<&str>)> { + let point = point.strip_prefix('L')?; + match point.split_once('C') { + Some((line, column)) => Some((line, Some(column))), + None => Some((point, None)), + } +} + #[cfg(test)] mod tests { use super::find_uuids; + use super::normalize_markdown_hash_location_suffix; use super::sanitize_metric_tag_value; use pretty_assertions::assert_eq; @@ -121,4 +157,20 @@ mod tests { let msg = "bad value!"; assert_eq!(sanitize_metric_tag_value(msg), "bad_value"); } + + #[test] + fn normalize_markdown_hash_location_suffix_converts_single_location() { + assert_eq!( + normalize_markdown_hash_location_suffix("#L74C3"), + Some(":74:3".to_string()) + ); + } + + #[test] + fn normalize_markdown_hash_location_suffix_converts_ranges() { + assert_eq!( + normalize_markdown_hash_location_suffix("#L74C3-L76C9"), + Some(":74:3-76:9".to_string()) + ); + } }