diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 341bfc94e3..541604a5fd 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -171,7 +171,6 @@ use ratatui::style::Modifier; use ratatui::style::Style; use ratatui::style::Stylize; use ratatui::text::Line; -use ratatui::text::Text; use ratatui::widgets::Clear; use ratatui::widgets::Paragraph; use ratatui::widgets::Widget; diff --git a/codex-rs/tui/src/chatwidget/rendering.rs b/codex-rs/tui/src/chatwidget/rendering.rs index 67bb5005f0..fd6fc7aa57 100644 --- a/codex-rs/tui/src/chatwidget/rendering.rs +++ b/codex-rs/tui/src/chatwidget/rendering.rs @@ -3,6 +3,7 @@ use super::transcript::ActiveCellLayoutCache; use super::transcript::ActiveCellLayoutCacheKey; use super::*; +use crate::terminal_hyperlinks::HyperlinkParagraph; use std::cell::Cell; impl ChatWidget { @@ -112,8 +113,8 @@ struct PersistentActiveCellLayout<'a> { impl Renderable for TranscriptAreaRenderable<'_> { fn render(&self, area: Rect, buf: &mut Buffer) { let area = self.child_area(area); - let lines = self.child.display_lines(area.width); - let paragraph = Paragraph::new(Text::from(lines)).wrap(Wrap { trim: false }); + let lines = self.child.display_hyperlink_lines(area.width); + let paragraph = HyperlinkParagraph::new(&lines, Style::default()); let y = if area.height == 0 { 0 } else { @@ -133,7 +134,7 @@ impl Renderable for TranscriptAreaRenderable<'_> { u16::try_from(overflow).unwrap_or(u16::MAX) }; Clear.render(area, buf); - paragraph.scroll((y, 0)).render(area, buf); + paragraph.scroll(y).render(area, buf); } fn desired_height(&self, width: u16) -> u16 { diff --git a/codex-rs/tui/src/chatwidget/rendering_tests.rs b/codex-rs/tui/src/chatwidget/rendering_tests.rs index e3cf4abc28..beac194da6 100644 --- a/codex-rs/tui/src/chatwidget/rendering_tests.rs +++ b/codex-rs/tui/src/chatwidget/rendering_tests.rs @@ -75,6 +75,73 @@ fn contains_text(buffer: &Buffer, text: &str) -> bool { }) } +#[test] +fn active_transcript_preserves_clipped_markdown_hyperlinks() { + let cell = history_cell::AgentMarkdownCell::new( + "Earlier content\n\n[OSC8 label](https://example.com/)".to_string(), + std::path::Path::new("/tmp"), + ); + let renderable = TranscriptAreaRenderable { + child: &cell, + top: 1, + right: 2, + persistent_layout: None, + }; + let area = Rect::new( + /*x*/ 2, /*y*/ 1, /*width*/ 40, /*height*/ 3, + ); + let mut buffer = Buffer::empty(area); + renderable.render(area, &mut buffer); + + let linked_text = buffer + .content + .iter() + .map(ratatui::buffer::Cell::symbol) + .filter(|symbol| symbol.starts_with("\x1b]8;;https://example.com/\x07")) + .map(crate::terminal_hyperlinks::strip_osc8) + .collect::(); + assert_eq!(linked_text, "OSC8 labelhttps://example.com/"); + + let visible_rows = buffer + .content + .chunks(usize::from(area.width)) + .map(|row| { + row.iter() + .map(|cell| crate::terminal_hyperlinks::strip_osc8(cell.symbol())) + .collect::() + .trim_end() + .to_string() + }) + .collect::>(); + insta::assert_debug_snapshot!(visible_rows, @r#" + [ + "", + "", + " OSC8 label (https://example.com/)", + ] + "#); + + let size = ratatui::layout::Size::new(/*width*/ 44, /*height*/ 5); + let mut terminal = + crate::custom_terminal::Terminal::with_screen_size_and_cursor_position_for_test( + ratatui::backend::CrosstermBackend::new(Vec::new()), + size, + area.as_position(), + ); + terminal.set_viewport_area(Rect::new( + /*x*/ 0, + /*y*/ 0, + size.width, + size.height, + )); + terminal + .draw_with_size(size, |frame| renderable.render(area, frame.buffer_mut())) + .expect("render terminal frame"); + let output = String::from_utf8(terminal.backend().writer().clone()).expect("UTF-8 output"); + assert!(output.contains("\x1b]8;;https://example.com/\x07OSC8 label\x1b]8;;\x07")); + assert!(output.contains("\x1b]8;;https://example.com/\x07https://example.com/\x1b]8;;\x07")); +} + #[tokio::test] async fn initial_session_header_starts_at_the_top_of_the_viewport() { let (mut widget, _sender, _events, _operations) = make_chatwidget_manual_with_sender().await; diff --git a/codex-rs/tui/src/history_cell/mod.rs b/codex-rs/tui/src/history_cell/mod.rs index 6eae683282..4e51d50f7c 100644 --- a/codex-rs/tui/src/history_cell/mod.rs +++ b/codex-rs/tui/src/history_cell/mod.rs @@ -33,11 +33,10 @@ use crate::session_state::ThreadSessionState; use crate::style::proposed_plan_style; use crate::style::user_message_style; use crate::terminal_hyperlinks::HyperlinkLine; -use crate::terminal_hyperlinks::mark_buffer_hyperlinks; +use crate::terminal_hyperlinks::HyperlinkParagraph; use crate::terminal_hyperlinks::plain_hyperlink_lines; use crate::terminal_hyperlinks::prefix_hyperlink_lines; use crate::terminal_hyperlinks::visible_lines; -use crate::terminal_hyperlinks::visible_lines_ref; #[cfg(test)] use crate::test_support::PathBufExt; #[cfg(test)] @@ -291,8 +290,7 @@ pub(crate) trait HistoryCell: std::fmt::Debug + Send + Sync + Any { impl Renderable for Box { fn render(&self, area: Rect, buf: &mut Buffer) { let hyperlink_lines = self.display_hyperlink_lines(area.width); - let lines = visible_lines_ref(&hyperlink_lines); - let paragraph = Paragraph::new(Text::from(lines)).wrap(Wrap { trim: false }); + let paragraph = HyperlinkParagraph::new(&hyperlink_lines, Style::default()); let y = if area.height == 0 { 0 } else { @@ -304,8 +302,7 @@ impl Renderable for Box { // Active-cell content can reflow dramatically during resize/stream updates. Clear the // entire draw area first so stale glyphs from previous frames never linger. Clear.render(area, buf); - paragraph.scroll((y, 0)).render(area, buf); - mark_buffer_hyperlinks(buf, area, &hyperlink_lines, usize::from(y)); + paragraph.scroll(y).render(area, buf); } fn desired_height(&self, width: u16) -> u16 { HistoryCell::desired_height(self.as_ref(), width) diff --git a/codex-rs/tui/src/insert_history.rs b/codex-rs/tui/src/insert_history.rs index 1c594f5308..165bc56c77 100644 --- a/codex-rs/tui/src/insert_history.rs +++ b/codex-rs/tui/src/insert_history.rs @@ -539,6 +539,55 @@ mod tests { assert_eq!(line.line.spans[0].content, destination); } + #[test] + fn markdown_link_label_survives_history_wrapping() { + use pretty_assertions::assert_eq; + + let destination = "https://developers.openai.com/"; + let markdown = "Instead of a Markdown-labeled link. That is the most reliably clickable form. Official OpenAI documentation does not appear to document this exact terminal-link behavior; this is an inference from how terminal hyperlink rendering works. [OpenAI Developers](https://developers.openai.com/)"; + for label in ["OpenAI Developers", "`OpenAI Developers`"] { + for markdown in [ + markdown.replace("[OpenAI Developers]", &format!("[{label}]")), + format!("| Link |\n| --- |\n| [{label}]({destination}) |"), + ] { + for width in [32, 80, 200] { + let lines = crate::markdown::render_markdown_agent_with_links_and_cwd( + &markdown, + Some(width), + /*cwd*/ None, + ); + let lines = crate::terminal_hyperlinks::prefix_hyperlink_lines( + lines, + " ".into(), + " ".into(), + ); + let (wrapped, _) = wrap_history_hyperlink_lines( + &lines, + width + 2, + HistoryLineWrapPolicy::PreWrap, + ); + let mut actual = Vec::new(); + for line in &wrapped { + write_history_line(&mut actual, line, width + 2) + .expect("write history line"); + } + let output = String::from_utf8(actual).expect("UTF-8 terminal output"); + let open = format!("\x1b]8;;{destination}\x07"); + let linked_text = output + .split(&open) + .skip(1) + .map(|part| part.split("\x1b]8;;\x07").next().unwrap()) + .collect::(); + assert_eq!( + linked_text.replace(' ', ""), + format!("OpenAIDevelopers{destination}"), + "label {label}, width {width}: {wrapped:?}" + ); + } + } + } + } + #[test] fn vt100_blockquote_line_emits_green_fg() { // Set up a small off-screen terminal diff --git a/codex-rs/tui/src/markdown.rs b/codex-rs/tui/src/markdown.rs index 6375a789b0..a6c562bb25 100644 --- a/codex-rs/tui/src/markdown.rs +++ b/codex-rs/tui/src/markdown.rs @@ -94,8 +94,10 @@ pub(crate) fn render_markdown_agent_with_links_cwd_and_visualizations( ) -> Vec { let rewritten = rewrite_inline_visualizations(markdown_source, inline_visualization_context); let normalized = unwrap_markdown_fences(&rewritten.markdown); - let is_hidden_link_destination = - |destination: &str| rewritten.trusted_file_links.contains_key(destination); + let is_hidden_link_destination = |destination: &str| { + rewritten.trusted_file_links.contains_key(destination) + || crate::markdown_render::hide_web_link_destination(destination) + }; let mut lines = crate::markdown_render::render_markdown_lines_with_width_cwd_and_hidden_link_destinations( &normalized, @@ -126,6 +128,7 @@ pub(crate) fn render_streaming_markdown_agent_with_links_and_cwd( &normalized, width, cwd, + &crate::markdown_render::hide_web_link_destination, ); if normalized != markdown_source { // Fence unwrapping removes opening/closing lines. A normalized tail that is still a raw diff --git a/codex-rs/tui/src/markdown_render.rs b/codex-rs/tui/src/markdown_render.rs index 7a7cc4cad4..e08b722a5b 100644 --- a/codex-rs/tui/src/markdown_render.rs +++ b/codex-rs/tui/src/markdown_render.rs @@ -78,9 +78,11 @@ use url::Url; mod streaming; mod table_key_value; +mod web_links; pub(crate) use streaming::StreamingMarkdownRender; pub(crate) use streaming::render_streaming_markdown_lines_with_width_and_cwd; +pub(crate) use web_links::hide_web_link_destination; const TABLE_COLUMN_GAP: usize = 2; const TABLE_CELL_PADDING: usize = 1; @@ -161,14 +163,6 @@ impl TableCell { } } - #[inline] - fn push_span(&mut self, span: Span<'static>) { - self.ensure_line(); - if let Some(line) = self.lines.last_mut() { - line.line.push_span(span); - } - } - fn push_annotated(&mut self, mut appended: HyperlinkLine) { self.ensure_line(); if let Some(line) = self.lines.last_mut() { @@ -318,6 +312,8 @@ pub(crate) fn render_markdown_text_with_width_and_cwd( ))) } +/// Keep destinations visible by default, including for callers that discard hyperlink metadata. +/// Semantic output paths supply their hidden-destination policy explicitly. pub(crate) fn render_markdown_lines_with_width_and_cwd( input: &str, width: Option, @@ -355,6 +351,7 @@ struct LinkState { destination: String, show_destination: bool, style_label: bool, + has_visible_label: bool, /// Pre-rendered display text for local file links. /// /// When this is present, the markdown label is intentionally suppressed so the rendered @@ -1024,10 +1021,16 @@ where } fn push_span_to_table_cell(&mut self, span: Span<'static>) { + let span = self.style_link_label(span); + let mut annotated = HyperlinkLine::new(Line::default()); + annotated.push_span( + span, + self.link.as_ref().map(|link| link.destination.as_str()), + ); if let Some(table_state) = self.table_state.as_mut() && let Some(cell) = table_state.current_cell.as_mut() { - cell.push_span(span); + cell.push_annotated(annotated); } } @@ -1050,7 +1053,7 @@ where } fn push_text_spans_to_table_cell(&mut self, text: &str, style: Style) { - let span = Span::styled(text.to_string(), style); + let span = self.style_link_label(Span::styled(text.to_string(), style)); let destination = self .link .as_ref() @@ -1794,6 +1797,17 @@ where self.inline_styles.pop(); } + fn style_link_label(&mut self, mut span: Span<'static>) -> Span<'static> { + if let Some(link) = self.link.as_mut() + && web_destination(&link.destination).is_some() + { + link.has_visible_label |= + !span.content.trim().is_empty() && display_width(&span.content) > 0; + span.style = span.style.patch(self.styles.link); + } + span + } + fn push_link(&mut self, dest_url: String) { let style_label = (self.is_hidden_link_destination)(&dest_url); if style_label { @@ -1803,6 +1817,7 @@ where self.link = Some(LinkState { show_destination, 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()) } else { @@ -1817,7 +1832,9 @@ where if link.style_label { self.pop_inline_style(); } - if link.show_destination { + if link.show_destination + || (!link.has_visible_label && web_destination(&link.destination).is_some()) + { // Link destinations are rendered as " (url)" suffixes. When parsing table cells, // append the suffix into the active cell buffer rather than the outer paragraph // line to avoid detached url lines. @@ -1970,10 +1987,15 @@ where } fn push_span(&mut self, span: Span<'static>) { + let span = self.style_link_label(span); + if self.current_line_content.is_none() { + self.push_line(Line::default()); + } if let Some(line) = self.current_line_content.as_mut() { - line.line.push_span(span); - } else { - self.push_line(Line::from(vec![span])); + line.push_span( + span, + self.link.as_ref().map(|link| link.destination.as_str()), + ); } } @@ -1993,7 +2015,7 @@ where } fn push_text_spans(&mut self, text: &str, style: Style) { - let span = Span::styled(text.to_string(), style); + let span = self.style_link_label(Span::styled(text.to_string(), style)); let destination = self .link .as_ref() @@ -2469,9 +2491,9 @@ mod tests { #[test] fn wrap_cell_preserves_hard_break_lines() { let mut cell = TableCell::default(); - cell.push_span("first line".into()); + cell.push_annotated(Line::from("first line").into()); cell.hard_break(); - cell.push_span("second line".into()); + cell.push_annotated(Line::from("second line").into()); let writer = W::new( "", @@ -2506,7 +2528,7 @@ mod tests { /// Build a single-line `TableCell` from plain text. fn make_cell(text: &str) -> TableCell { let mut cell = TableCell::default(); - cell.push_span(Span::raw(text.to_string())); + cell.push_annotated(Line::from(text.to_string()).into()); cell } diff --git a/codex-rs/tui/src/markdown_render/snapshots/codex_tui__markdown_render__web_links__tests__label_only_and_fallback_presentations_snapshot.snap b/codex-rs/tui/src/markdown_render/snapshots/codex_tui__markdown_render__web_links__tests__label_only_and_fallback_presentations_snapshot.snap new file mode 100644 index 0000000000..7d84cf65fe --- /dev/null +++ b/codex-rs/tui/src/markdown_render/snapshots/codex_tui__markdown_render__web_links__tests__label_only_and_fallback_presentations_snapshot.snap @@ -0,0 +1,56 @@ +--- +source: tui/src/markdown_render/web_links_tests.rs +expression: presentations +--- +[ + Text::from_iter([ + Line::from_iter([ + Span::from("plain "), + Span::from("plain").cyan().underlined(), + Span::from(" "), + Span::from("code").cyan(), + Span::from(" "), + Span::from("code").cyan().underlined(), + ]), + Line::default(), + Line::from_iter([ + Span::from(" "), + Span::from("Resource"), + ]).fg(Color::Rgb(249, 226, 175)).bold(), + Line::from(Span::from("━━━━━━━━━━").dim()), + Line::from_iter([ + Span::from(" "), + Span::from("docs").cyan().underlined(), + ]), + ]), + Text::from_iter([ + Line::from_iter([ + Span::from("plain "), + Span::from("plain").cyan().underlined(), + Span::from(" ("), + Span::from("https://example.com").cyan().underlined(), + Span::from(")"), + Span::from(" "), + Span::from("code").cyan(), + ]), + Line::from_iter([ + Span::from("code").cyan().underlined(), + Span::from(" ("), + Span::from("https://example.com").cyan().underlined(), + Span::from(")"), + ]), + Line::default(), + Line::from_iter([ + Span::from(" "), + Span::from("Resource"), + ]).fg(Color::Rgb(249, 226, 175)).bold(), + Line::from(Span::from("━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━").dim()), + Line::from_iter([ + Span::from(" "), + Span::from("docs").cyan().underlined(), + Span::from(" ("), + Span::from("https://example.com/docs").cyan().underlined(), + Span::from(")"), + ]), + ]), +] diff --git a/codex-rs/tui/src/markdown_render/streaming.rs b/codex-rs/tui/src/markdown_render/streaming.rs index a4657c41ae..535774ff72 100644 --- a/codex-rs/tui/src/markdown_render/streaming.rs +++ b/codex-rs/tui/src/markdown_render/streaming.rs @@ -10,7 +10,6 @@ use super::Options; use super::Parser; use super::Tag; use super::Writer; -use super::never_hide_link_destination; use std::ops::Range; use std::path::Path; @@ -34,6 +33,7 @@ pub(crate) fn render_streaming_markdown_lines_with_width_and_cwd( input: &str, width: Option, cwd: Option<&Path>, + is_hidden_link_destination: &dyn Fn(&str) -> bool, ) -> StreamingMarkdownRender { let mut options = Options::empty(); options.insert(Options::ENABLE_STRIKETHROUGH); @@ -47,7 +47,7 @@ pub(crate) fn render_streaming_markdown_lines_with_width_and_cwd( last_start: 0, first_is_html: false, }; - let mut writer = Writer::new(input, parser, width, cwd, &never_hide_link_destination); + let mut writer = Writer::new(input, parser, width, cwd, is_hidden_link_destination); writer.run(); StreamingMarkdownRender { lines: writer.text, diff --git a/codex-rs/tui/src/markdown_render/web_links.rs b/codex-rs/tui/src/markdown_render/web_links.rs new file mode 100644 index 0000000000..4604e8c126 --- /dev/null +++ b/codex-rs/tui/src/markdown_render/web_links.rs @@ -0,0 +1,64 @@ +//! Presentation policy for web-link destinations in semantic Markdown output. +//! +//! Terminal identity is a conservative heuristic, not end-to-end capability negotiation. Unknown +//! terminals and multiplexers retain the destination so a label never hides the only usable URL. + +use codex_terminal_detection::TerminalInfo; +use codex_terminal_detection::TerminalName; +use codex_terminal_detection::terminal_info; +use std::io::IsTerminal; +use std::sync::LazyLock; + +use crate::terminal_hyperlinks::web_destination; + +#[derive(Clone, Copy, PartialEq, Eq)] +enum WebLinkDisplay { + LabelOnly, + WithDestination, +} + +impl WebLinkDisplay { + fn for_terminal(terminal: &TerminalInfo, term: Option<&str>) -> Self { + if terminal.multiplexer.is_some() + || term.is_some_and(|term| { + term == "dumb" || term.starts_with("screen") || term.starts_with("tmux") + }) + { + return Self::WithDestination; + } + match terminal.name { + TerminalName::Ghostty + | TerminalName::Iterm2 + | TerminalName::WezTerm + | TerminalName::Kitty + | TerminalName::VsCode + | TerminalName::Alacritty + | TerminalName::WindowsTerminal + | TerminalName::Konsole + | TerminalName::GnomeTerminal + | TerminalName::Vte => Self::LabelOnly, + TerminalName::AppleTerminal + | TerminalName::WarpTerminal + | TerminalName::Dumb + | TerminalName::Unknown => Self::WithDestination, + } + } + + fn hide_destination(self, destination: &str) -> bool { + self == Self::LabelOnly && web_destination(destination).is_some() + } +} + +pub(crate) fn hide_web_link_destination(destination: &str) -> bool { + static DISPLAY: LazyLock = LazyLock::new(|| { + if !std::io::stdout().is_terminal() || std::env::var_os("STY").is_some() { + return WebLinkDisplay::WithDestination; + } + WebLinkDisplay::for_terminal(&terminal_info(), std::env::var("TERM").ok().as_deref()) + }); + DISPLAY.hide_destination(destination) +} + +#[cfg(test)] +#[path = "web_links_tests.rs"] +mod tests; diff --git a/codex-rs/tui/src/markdown_render/web_links_tests.rs b/codex-rs/tui/src/markdown_render/web_links_tests.rs new file mode 100644 index 0000000000..614b5a6efa --- /dev/null +++ b/codex-rs/tui/src/markdown_render/web_links_tests.rs @@ -0,0 +1,194 @@ +use super::WebLinkDisplay; +use crate::markdown_render::render_markdown_lines_with_width_cwd_and_hidden_link_destinations; +use crate::markdown_render::render_streaming_markdown_lines_with_width_and_cwd; +use crate::terminal_hyperlinks::HyperlinkLine; +use crate::terminal_hyperlinks::TerminalHyperlink; +use crate::terminal_hyperlinks::visible_lines; +use codex_terminal_detection::Multiplexer; +use codex_terminal_detection::TerminalInfo; +use codex_terminal_detection::TerminalName; +use insta::assert_debug_snapshot; +use pretty_assertions::assert_eq; +use ratatui::style::Stylize; +use ratatui::text::Line; +use ratatui::text::Text; +use std::collections::BTreeSet; + +fn terminal(name: TerminalName) -> TerminalInfo { + TerminalInfo { + name, + term_program: None, + version: None, + term: None, + multiplexer: None, + } +} + +fn render(markdown: &str, width: usize, display: WebLinkDisplay) -> Vec { + render_markdown_lines_with_width_cwd_and_hidden_link_destinations( + markdown, + Some(width), + /*cwd*/ None, + &|destination| display.hide_destination(destination), + ) +} + +#[test] +fn supporting_terminals_render_only_the_styled_label_and_keep_its_target() { + for name in [ + TerminalName::Ghostty, + TerminalName::Iterm2, + TerminalName::WezTerm, + TerminalName::Kitty, + TerminalName::VsCode, + TerminalName::Alacritty, + TerminalName::WindowsTerminal, + TerminalName::Konsole, + TerminalName::GnomeTerminal, + TerminalName::Vte, + ] { + let display = WebLinkDisplay::for_terminal(&terminal(name), /*term*/ None); + for (markdown, label) in [ + ("[label](https://example.com)", "label".cyan().underlined()), + ( + "[`label`](https://example.com)", + "label".cyan().underlined(), + ), + ( + "[**label**](https://example.com)", + "label".cyan().bold().underlined(), + ), + ( + "[*label*](https://example.com)", + "label".cyan().italic().underlined(), + ), + ("[](https://example.com)", "".cyan().underlined()), + ( + "[https://example.com](https://example.com)", + "https://example.com".cyan().underlined(), + ), + ] { + let label_width = label.width(); + assert_eq!( + render(markdown, /*width*/ 80, display), + vec![HyperlinkLine { + line: Line::from(label), + hyperlinks: vec![TerminalHyperlink::web( + 0..label_width, + "https://example.com".into(), + )], + }], + "terminal: {name:?}" + ); + } + } +} + +#[test] +fn unknown_terminals_and_multiplexers_keep_visible_destinations() { + let mut tmux = terminal(TerminalName::Ghostty); + tmux.multiplexer = Some(Multiplexer::Tmux { version: None }); + let mut zellij = terminal(TerminalName::Ghostty); + zellij.multiplexer = Some(Multiplexer::Zellij { version: None }); + let markdown = "[label](https://example.com)"; + let expected = render(markdown, /*width*/ 80, WebLinkDisplay::WithDestination); + for (terminal, term) in [ + (terminal(TerminalName::Unknown), None), + (terminal(TerminalName::AppleTerminal), None), + (terminal(TerminalName::Dumb), None), + (tmux, None), + (zellij, None), + (terminal(TerminalName::Ghostty), Some("screen-256color")), + (terminal(TerminalName::Ghostty), Some("tmux-256color")), + (terminal(TerminalName::Ghostty), Some("dumb")), + ] { + assert_eq!( + render( + markdown, + /*width*/ 80, + WebLinkDisplay::for_terminal(&terminal, term), + ), + expected, + ); + } +} + +#[test] +fn label_only_policy_keeps_non_web_links_and_literal_urls_unchanged() { + let markdown = "[file](./src/main.rs) [mail](mailto:a@example.com)\n\nhttps://example.com\n\n`https://example.com/code`"; + assert_eq!( + render(markdown, /*width*/ 80, WebLinkDisplay::LabelOnly), + render(markdown, /*width*/ 80, WebLinkDisplay::WithDestination), + ); +} + +#[test] +fn empty_web_link_labels_keep_a_visible_destination() { + for markdown in [ + "[](https://example.com)", + "[ ](https://example.com)", + "[\u{200d}](https://example.com)", + ] { + assert_eq!( + render(markdown, /*width*/ 80, WebLinkDisplay::LabelOnly), + render(markdown, /*width*/ 80, WebLinkDisplay::WithDestination), + ); + } +} + +#[test] +fn label_only_web_links_keep_targets_after_prose_and_table_wrapping() { + let links = "[plain label](https://example.com/plain) [`code label`](https://example.com/code) [](https://example.com/html)"; + for markdown in [ + links.to_string(), + format!("| Links |\n| --- |\n| {links} |"), + ] { + for width in [12, 40] { + let lines = render(&markdown, width, WebLinkDisplay::LabelOnly); + let targets = lines + .iter() + .flat_map(|line| &line.hyperlinks) + .map(|link| link.destination.as_str()) + .collect::>(); + assert_eq!( + targets, + BTreeSet::from([ + "https://example.com/plain", + "https://example.com/code", + "https://example.com/html", + ]), + ); + assert!( + lines + .iter() + .all(|line| !line.line.to_string().contains("https://")) + ); + } + } +} + +#[test] +fn streaming_and_full_render_agree_with_label_only_links() { + let markdown = "See [**label**](https://example.com).\n\n| Resource |\n| --- |\n| [`code`](https://example.com/code) |\n\nTrailing text."; + let display = WebLinkDisplay::LabelOnly; + for width in [12, 40] { + for end in 1..=markdown.len() { + let prefix = &markdown[..end]; + let streamed = render_streaming_markdown_lines_with_width_and_cwd( + prefix, + Some(width), + /*cwd*/ None, + &|destination| display.hide_destination(destination), + ); + assert_eq!(streamed.lines, render(prefix, width, display)); + } + } +} + +#[test] +fn label_only_and_fallback_presentations_snapshot() { + let markdown = "plain [plain](https://example.com) `code` [`code`](https://example.com)\n\n| Resource |\n| --- |\n| [docs](https://example.com/docs) |"; + let presentations = [WebLinkDisplay::LabelOnly, WebLinkDisplay::WithDestination] + .map(|display| Text::from(visible_lines(render(markdown, /*width*/ 40, display)))); + assert_debug_snapshot!(presentations); +} diff --git a/codex-rs/tui/src/markdown_render_tests.rs b/codex-rs/tui/src/markdown_render_tests.rs index 62fa55ca99..5506085cd6 100644 --- a/codex-rs/tui/src/markdown_render_tests.rs +++ b/codex-rs/tui/src/markdown_render_tests.rs @@ -757,7 +757,7 @@ fn strong_emphasis() { fn link() { let text = render_markdown_text("[Link](https://example.com)"); let expected = Text::from(Line::from_iter([ - "Link".into(), + "Link".cyan().underlined(), " (".into(), "https://example.com".cyan().underlined(), ")".into(), @@ -765,6 +765,65 @@ fn link() { assert_eq!(text, expected); } +#[test] +fn web_link_labels_use_link_style_and_preserve_inline_formatting() { + for (label, expected_label) in [ + ("plain", "plain".cyan().underlined()), + ("`code`", "code".cyan().underlined()), + ("**bold**", "bold".cyan().bold().underlined()), + ("*italic*", "italic".cyan().italic().underlined()), + ] { + let text = render_markdown_text(&format!( + "before [{label}](https://example.com) after `code`" + )); + let expected = Text::from(Line::from_iter([ + "before ".into(), + expected_label, + " (".into(), + "https://example.com".cyan().underlined(), + ")".into(), + " after ".into(), + "code".cyan(), + ])); + assert_eq!(text, expected, "label: {label}"); + } +} + +#[test] +fn web_link_labels_keep_link_style_in_wrapped_prose_and_tables() { + let links = + "[plain](https://example.com) [`code`](https://example.com) [](https://example.com)"; + for markdown in [links.to_string(), format!("| Links |\n| --- |\n| {links} |")] + { + for width in [32, 80] { + let text = render_markdown_text_with_width(&markdown, Some(width)); + let labels = text + .lines + .iter() + .flat_map(|line| &line.spans) + .filter(|span| matches!(span.content.as_ref(), "plain" | "code" | "")) + .cloned() + .collect::>(); + assert_eq!( + labels, + vec![ + "plain".cyan().underlined(), + "code".cyan().underlined(), + "".cyan().underlined() + ] + ); + } + } +} + +#[test] +fn web_link_labels_have_a_visible_underline_snapshot() { + let text = render_markdown_text( + "plain [plain](https://example.com) `code` [`code`](https://example.com)", + ); + assert_debug_snapshot!(text); +} + #[test] fn load_location_suffix_regexes() { let _colon = &*COLON_LOCATION_SUFFIX_RE; @@ -912,7 +971,7 @@ fn file_link_uses_target_path_for_hash_range() { fn url_link_shows_destination() { let text = render_markdown_text("[docs](https://example.com/docs)"); let expected = Text::from(Line::from_iter([ - "docs".into(), + "docs".cyan().underlined(), " (".into(), "https://example.com/docs".cyan().underlined(), ")".into(), diff --git a/codex-rs/tui/src/pager_overlay/scrolling.rs b/codex-rs/tui/src/pager_overlay/scrolling.rs index 6f21b16afc..9cfdd11e7c 100644 --- a/codex-rs/tui/src/pager_overlay/scrolling.rs +++ b/codex-rs/tui/src/pager_overlay/scrolling.rs @@ -7,15 +7,11 @@ use crate::history_cell::UserHistoryCell; use crate::render::renderable::Renderable; use crate::style::user_message_style; use crate::terminal_hyperlinks::HyperlinkLine; -use crate::terminal_hyperlinks::mark_buffer_hyperlinks; -use crate::terminal_hyperlinks::visible_lines_ref; +use crate::terminal_hyperlinks::HyperlinkParagraph; use ratatui::buffer::Buffer; use ratatui::layout::Rect; use ratatui::style::Style; -use ratatui::text::Text; -use ratatui::widgets::Paragraph; use ratatui::widgets::Widget; -use ratatui::widgets::Wrap; /// Renders a committed history cell directly into the visible transcript viewport. pub(super) struct CellRenderable { @@ -40,12 +36,9 @@ impl Renderable for CellRenderable { } else { Style::default() }; - let p = Paragraph::new(Text::from(visible_lines_ref(&hyperlink_lines))) - .style(style) - .wrap(Wrap { trim: false }) - .scroll((scroll_offset, 0)); - p.render(area, buf); - mark_buffer_hyperlinks(buf, area, &hyperlink_lines, usize::from(scroll_offset)); + HyperlinkParagraph::new(&hyperlink_lines, style) + .scroll(scroll_offset) + .render(area, buf); true } @@ -66,17 +59,14 @@ impl Renderable for HyperlinkLinesRenderable { /// Keep live-tail hyperlinks aligned with the same visible rows as their text. fn render_scrolled(&self, area: Rect, buf: &mut Buffer, scroll_offset: u16) -> bool { - Paragraph::new(Text::from(visible_lines_ref(&self.lines))) - .wrap(Wrap { trim: false }) - .scroll((scroll_offset, 0)) + HyperlinkParagraph::new(&self.lines, Style::default()) + .scroll(scroll_offset) .render(area, buf); - mark_buffer_hyperlinks(buf, area, &self.lines, usize::from(scroll_offset)); true } fn desired_height(&self, width: u16) -> u16 { - Paragraph::new(Text::from(visible_lines_ref(&self.lines))) - .wrap(Wrap { trim: false }) + HyperlinkParagraph::new(&self.lines, Style::default()) .line_count(width) .try_into() .unwrap_or(/*default*/ 0) diff --git a/codex-rs/tui/src/snapshots/codex_tui__markdown_render__markdown_render_tests__web_link_labels_have_a_visible_underline_snapshot.snap b/codex-rs/tui/src/snapshots/codex_tui__markdown_render__markdown_render_tests__web_link_labels_have_a_visible_underline_snapshot.snap new file mode 100644 index 0000000000..655ab46f78 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__markdown_render__markdown_render_tests__web_link_labels_have_a_visible_underline_snapshot.snap @@ -0,0 +1,5 @@ +--- +source: tui/src/markdown_render_tests.rs +expression: text +--- +Text::from(Line::from_iter([Span::from("plain "), Span::from("plain").cyan().underlined(), Span::from(" ("), Span::from("https://example.com").cyan().underlined(), Span::from(")"), Span::from(" "), Span::from("code").cyan(), Span::from(" "), Span::from("code").cyan().underlined(), Span::from(" ("), Span::from("https://example.com").cyan().underlined(), Span::from(")")])) diff --git a/codex-rs/tui/src/streaming/snapshots/codex_tui__streaming__render__tests__incremental_render_representative_stream.snap b/codex-rs/tui/src/streaming/snapshots/codex_tui__streaming__render__tests__incremental_render_representative_stream.snap index c93d483c22..59fe18f1c8 100644 --- a/codex-rs/tui/src/streaming/snapshots/codex_tui__streaming__render__tests__incremental_render_representative_stream.snap +++ b/codex-rs/tui/src/streaming/snapshots/codex_tui__streaming__render__tests__incremental_render_representative_stream.snap @@ -17,7 +17,7 @@ expression: render.lines HyperlinkLine { line: Line::from_iter([ Span::from("First paragraph with a "), - Span::from("link"), + Span::from("link").cyan().underlined(), ]), hyperlinks: [ TerminalHyperlink { diff --git a/codex-rs/tui/src/terminal_hyperlinks.rs b/codex-rs/tui/src/terminal_hyperlinks.rs index 62bc57f5c3..78bee452db 100644 --- a/codex-rs/tui/src/terminal_hyperlinks.rs +++ b/codex-rs/tui/src/terminal_hyperlinks.rs @@ -3,6 +3,10 @@ //! Layout code measures and wraps ordinary ratatui lines. Hyperlink annotations are applied only //! when text reaches a terminal buffer or scrollback writer so OSC 8 bytes never affect geometry. +mod paragraph; + +pub(crate) use paragraph::HyperlinkParagraph; + use std::num::NonZeroU16; use std::ops::Range; @@ -778,10 +782,7 @@ mod tests { ); let mut buf = Buffer::empty(area); - Paragraph::new(Text::from(line.line.clone())) - .wrap(Wrap { trim: false }) - .render(area, &mut buf); - mark_buffer_hyperlinks(&mut buf, area, &[line], /*scroll_rows*/ 0); + HyperlinkParagraph::new(&[line], Style::default()).render(area, &mut buf); let linked_text = area .positions() @@ -819,11 +820,9 @@ mod tests { terminal .draw(|frame| { let buf = frame.buffer_mut(); - Paragraph::new(Text::from(visible_lines_ref(&lines))) - .wrap(Wrap { trim: false }) - .scroll((/*vertical*/ 2, /*horizontal*/ 0)) + HyperlinkParagraph::new(&lines, Style::default()) + .scroll(/*rows*/ 2) .render(area, buf); - mark_buffer_hyperlinks(buf, area, &lines, /*scroll_rows*/ 2); let linked_text = area .positions() diff --git a/codex-rs/tui/src/terminal_hyperlinks/paragraph.rs b/codex-rs/tui/src/terminal_hyperlinks/paragraph.rs new file mode 100644 index 0000000000..3a5975ee32 --- /dev/null +++ b/codex-rs/tui/src/terminal_hyperlinks/paragraph.rs @@ -0,0 +1,49 @@ +//! Paragraph rendering that keeps visible text and hyperlink annotations aligned. + +use super::HyperlinkLine; +use super::mark_buffer_hyperlinks; +use super::visible_lines_ref; +use ratatui::buffer::Buffer; +use ratatui::layout::Rect; +use ratatui::style::Style; +use ratatui::text::Text; +use ratatui::widgets::Paragraph; +use ratatui::widgets::Widget; +use ratatui::widgets::Wrap; + +/// Word-wraps without trimming and applies the same vertical scroll to text and links. +pub(crate) struct HyperlinkParagraph<'a> { + lines: &'a [HyperlinkLine], + paragraph: Paragraph<'a>, + scroll_rows: u16, +} + +impl<'a> HyperlinkParagraph<'a> { + pub(crate) fn new(lines: &'a [HyperlinkLine], style: Style) -> Self { + Self { + lines, + paragraph: Paragraph::new(Text::from(visible_lines_ref(lines))) + .style(style) + .wrap(Wrap { trim: false }), + scroll_rows: 0, + } + } + + pub(crate) fn line_count(&self, width: u16) -> usize { + self.paragraph.line_count(width) + } + + pub(crate) fn scroll(mut self, rows: u16) -> Self { + self.scroll_rows = rows; + self + } +} + +impl Widget for HyperlinkParagraph<'_> { + fn render(self, area: Rect, buf: &mut Buffer) { + self.paragraph + .scroll((self.scroll_rows, 0)) + .render(area, buf); + mark_buffer_hyperlinks(buf, area, self.lines, usize::from(self.scroll_rows)); + } +}