mirror of
https://github.com/openai/codex.git
synced 2026-03-05 21:45:28 +03:00
tui: restore visible line numbers for hidden file links (#12870)
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: <img width="732" height="813" alt="Screenshot 2026-02-26 at 9 27 55 AM" src="https://github.com/user-attachments/assets/d51bf236-653a-4e83-96e4-9427f0804471" />
This commit is contained in:
1
codex-rs/Cargo.lock
generated
1
codex-rs/Cargo.lock
generated
@@ -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",
|
||||
|
||||
@@ -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"] }
|
||||
|
||||
@@ -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<usize>)
|
||||
struct LinkState {
|
||||
destination: String,
|
||||
show_destination: bool,
|
||||
hidden_location_suffix: Option<String>,
|
||||
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<Regex> =
|
||||
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<Regex> =
|
||||
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::<String>()
|
||||
})
|
||||
})
|
||||
.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();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
---
|
||||
source: tui/src/markdown_render_tests.rs
|
||||
assertion_line: 714
|
||||
expression: rendered
|
||||
---
|
||||
See markdown_render.rs:74.
|
||||
|
||||
@@ -76,9 +76,45 @@ pub fn find_uuids(s: &str) -> Vec<String> {
|
||||
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<String> {
|
||||
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())
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user