diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index c888f9368d7..4fdcc9ecd20 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 1a87314e988..594acb33d46 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 a7e80acae82..2bbe19b6f7b 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 6457bf02c75..9981246093f 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 6e571852e6d..1b1f1210f43 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 df9ae59e1b7..0cb218f349f 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()) + ); + } }