diff --git a/codex-rs/tui/src/markdown_render.rs b/codex-rs/tui/src/markdown_render.rs index 4decb1eb01..b028fa7f2b 100644 --- a/codex-rs/tui/src/markdown_render.rs +++ b/codex-rs/tui/src/markdown_render.rs @@ -47,7 +47,6 @@ use crate::render::line_utils::line_to_static; use crate::style::accent_color; use crate::style::table_separator_style; use crate::terminal_hyperlinks::HyperlinkLine; -use crate::terminal_hyperlinks::annotate_web_urls_in_line; use crate::terminal_hyperlinks::remap_wrapped_line; use crate::terminal_hyperlinks::visible_lines; use crate::terminal_hyperlinks::web_destination; @@ -87,6 +86,7 @@ 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; pub(crate) use web_links::hide_web_link_destination; +use web_links::style_bare_web_urls; const TABLE_COLUMN_GAP: usize = 2; const TABLE_CELL_PADDING: usize = 1; @@ -1115,7 +1115,7 @@ where } else if self.link.is_some() || self.in_code_block { HyperlinkLine::new(Line::from(span)) } else { - annotate_web_urls_in_line(Line::from(span)) + style_bare_web_urls(span, self.styles.link) }; if let Some(table_state) = self.table_state.as_mut() && let Some(cell) = table_state.current_cell.as_mut() @@ -2123,7 +2123,7 @@ where } else if self.link.is_some() || self.in_code_block { HyperlinkLine::new(Line::from(span)) } else { - annotate_web_urls_in_line(Line::from(span)) + style_bare_web_urls(span, self.styles.link) }; self.push_annotated(annotated); } diff --git a/codex-rs/tui/src/markdown_render/snapshots/codex_tui__markdown_render__web_links__tests__bare_urls_are_styled_without_coloring_surrounding_prose.snap b/codex-rs/tui/src/markdown_render/snapshots/codex_tui__markdown_render__web_links__tests__bare_urls_are_styled_without_coloring_surrounding_prose.snap new file mode 100644 index 0000000000..da67cbc905 --- /dev/null +++ b/codex-rs/tui/src/markdown_render/snapshots/codex_tui__markdown_render__web_links__tests__bare_urls_are_styled_without_coloring_surrounding_prose.snap @@ -0,0 +1,6 @@ +--- +source: tui/src/markdown_render/web_links_tests.rs +assertion_line: 37 +expression: text +--- +Text::from(Line::from_iter([Span::from("界 ("), Span::from("https://example.com/a").fg(Color::Reset).underlined(), Span::from("); "), Span::from("https://example.com/b").fg(Color::Reset).underlined(), Span::from("! "), Span::from("https://example.com/c").fg(Color::Reset).bold().underlined()])) diff --git a/codex-rs/tui/src/markdown_render/snapshots/codex_tui__markdown_render__web_links__tests__bare_urls_keep_link_style_when_wrapped_in_tables.snap b/codex-rs/tui/src/markdown_render/snapshots/codex_tui__markdown_render__web_links__tests__bare_urls_keep_link_style_when_wrapped_in_tables.snap new file mode 100644 index 0000000000..7e88fd7d36 --- /dev/null +++ b/codex-rs/tui/src/markdown_render/snapshots/codex_tui__markdown_render__web_links__tests__bare_urls_keep_link_style_when_wrapped_in_tables.snap @@ -0,0 +1,20 @@ +--- +source: tui/src/markdown_render/web_links_tests.rs +assertion_line: 63 +expression: text +--- +Text::from_iter([ + 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("https://").fg(Color::Reset).underlined(), + ]), + Line::from_iter([ + Span::from(" "), + Span::from("example.com/docs").fg(Color::Reset).underlined(), + ]), +]) diff --git a/codex-rs/tui/src/markdown_render/web_links.rs b/codex-rs/tui/src/markdown_render/web_links.rs index 4604e8c126..0ed41d9f3f 100644 --- a/codex-rs/tui/src/markdown_render/web_links.rs +++ b/codex-rs/tui/src/markdown_render/web_links.rs @@ -1,4 +1,4 @@ -//! Presentation policy for web-link destinations in semantic Markdown output. +//! Styling and destination visibility for web links 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. @@ -9,7 +9,50 @@ use codex_terminal_detection::terminal_info; use std::io::IsTerminal; use std::sync::LazyLock; +use ratatui::style::Style; +use ratatui::text::Line; +use ratatui::text::Span; +use unicode_segmentation::UnicodeSegmentation; + +use crate::terminal_hyperlinks::HyperlinkLine; +use crate::terminal_hyperlinks::annotate_web_urls_in_line; use crate::terminal_hyperlinks::web_destination; +use crate::width::display_width; + +/// Give detected URLs the same affordance as explicit Markdown links, preserving surrounding text. +pub(super) fn style_bare_web_urls(span: Span<'static>, link_style: Style) -> HyperlinkLine { + let mut line = annotate_web_urls_in_line(Line::from(span)); + if line.hyperlinks.is_empty() { + return line; + } + let mut spans: Vec> = Vec::new(); + let mut links = line.hyperlinks.iter().peekable(); + let mut column = 0; + let span = &line.line.spans[0]; + for grapheme in span.content.graphemes(/*is_extended*/ true) { + while links.peek().is_some_and(|link| link.columns.end <= column) { + links.next(); + } + let style = if links + .peek() + .is_some_and(|link| link.columns.contains(&column)) + { + span.style.patch(link_style) + } else { + span.style + }; + if let Some(last) = spans.last_mut() + && last.style == style + { + last.content.to_mut().push_str(grapheme); + } else { + spans.push(Span::styled(grapheme.to_owned(), style)); + } + column += display_width(grapheme); + } + line.line.spans = spans; + line +} #[derive(Clone, Copy, PartialEq, Eq)] enum WebLinkDisplay { diff --git a/codex-rs/tui/src/markdown_render/web_links_tests.rs b/codex-rs/tui/src/markdown_render/web_links_tests.rs index 608f77f406..2657b863a3 100644 --- a/codex-rs/tui/src/markdown_render/web_links_tests.rs +++ b/codex-rs/tui/src/markdown_render/web_links_tests.rs @@ -10,11 +10,62 @@ use codex_terminal_detection::TerminalInfo; use codex_terminal_detection::TerminalName; use insta::assert_debug_snapshot; use pretty_assertions::assert_eq; +use ratatui::style::Style; use ratatui::style::Stylize; use ratatui::text::Line; use ratatui::text::Text; use std::collections::BTreeSet; +#[test] +fn bare_urls_are_styled_without_coloring_surrounding_prose() { + let text = Text::from(visible_lines(render( + "界 (https://example.com/a); https://example.com/b! **https://example.com/c**", + /*width*/ 100, + WebLinkDisplay::LabelOnly, + ))); + assert_eq!( + text, + Text::from(Line::from(vec![ + "界 (".into(), + "https://example.com/a".fg(accent_color()).underlined(), + "); ".into(), + "https://example.com/b".fg(accent_color()).underlined(), + "! ".into(), + "https://example.com/c" + .bold() + .fg(accent_color()) + .underlined(), + ])), + ); + assert_debug_snapshot!(text); +} + +#[test] +fn bare_url_styling_preserves_tabs_and_unicode() { + let line = super::style_bare_web_urls( + "界\t(https://example.com).".bold(), + Style::new().fg(accent_color()).underlined(), + ); + assert_eq!( + line.line, + Line::from(vec![ + "界\t(".bold(), + "https://example.com".bold().fg(accent_color()).underlined(), + ").".bold(), + ]), + ); +} + +#[test] +fn bare_urls_keep_link_style_when_wrapped_in_tables() { + let text = Text::from(visible_lines(render( + "| Resource |\n| --- |\n| https://example.com/docs |", + /*width*/ 20, + WebLinkDisplay::LabelOnly, + ))); + assert_debug_snapshot!(text); +} + fn terminal(name: TerminalName) -> TerminalInfo { TerminalInfo { name, diff --git a/codex-rs/tui/src/transcript_view/input.rs b/codex-rs/tui/src/transcript_view/input.rs index 4a9351c288..3c3a7d745f 100644 --- a/codex-rs/tui/src/transcript_view/input.rs +++ b/codex-rs/tui/src/transcript_view/input.rs @@ -1,4 +1,5 @@ //! Transcript gestures leave ordinary typing and composer editing with the existing input path. +//! Stationary link clicks open on release; dragging or scrolling keeps the gesture in selection. use crate::key_hint::KeyBindingListExt; use crossterm::event::KeyCode; @@ -206,6 +207,27 @@ impl TranscriptView { self.extend_selection(event.column, event.row); } MouseEventKind::Up(MouseButton::Left) if dragging => { + // Open only a stationary click. Dragging back to the origin is still selection, + // and wheel scrolling clears the pointer even when it cannot move the viewport. + let link = self.selection.as_mut().and_then(|selection| { + (inside + && !selection.moved + && selection.pointer == Some(ScreenPosition::new(event.column, event.row)) + && event.modifiers.is_empty()) + .then(|| selection.pressed_link.take()) + .flatten() + }); + let link = link.filter(|destination| { + self.visible + .get(usize::from(event.row - self.area.y)) + .and_then(|visible| { + visible + .layout + .link_at(visible.row, event.column - self.area.x) + }) + .as_ref() + == Some(destination) + }); if self .selection .as_ref() @@ -217,6 +239,9 @@ impl TranscriptView { if self.selected_text(cells).is_none() { self.end_selection(cells); } + if let Some(link) = link { + return Some(ViewAction::OpenLink(link)); + } } _ => return None, } @@ -317,7 +342,21 @@ impl TranscriptView { { return None; } + let link = (clicks == 1 && event.modifiers.is_empty()) + .then(|| { + visible + .layout + .link_at(visible.row, event.column.saturating_sub(self.area.x)) + }) + .flatten(); self.begin_selection(cells, event.column, event.row, clicks); + if let Some(selection) = &mut self.selection { + selection.pressed_link = link; + } Some(ViewAction::Changed) } } + +#[cfg(test)] +#[path = "input_tests.rs"] +mod tests; diff --git a/codex-rs/tui/src/transcript_view/input_tests.rs b/codex-rs/tui/src/transcript_view/input_tests.rs new file mode 100644 index 0000000000..0480b9b080 --- /dev/null +++ b/codex-rs/tui/src/transcript_view/input_tests.rs @@ -0,0 +1,173 @@ +//! Link activation shares pointer gestures with selection without opening after a drag or scroll. + +use super::*; +use crate::history_cell::AgentMarkdownCell; +use pretty_assertions::assert_eq; + +fn transcript(markdown: &str, width: u16) -> (TranscriptView, Vec>) { + let cells: Vec> = vec![Arc::new(AgentMarkdownCell::new( + markdown.into(), + std::path::Path::new("/"), + ))]; + let mut view = TranscriptView::default(); + let area = Rect::new(/*x*/ 2, /*y*/ 1, width, /*height*/ 8); + view.render(area, &mut Buffer::empty(area), &cells); + (view, cells) +} + +fn mouse(kind: MouseEventKind, column: u16, row: u16) -> MouseEvent { + MouseEvent { + kind, + column, + row, + modifiers: KeyModifiers::NONE, + } +} + +#[test] +fn ordinary_click_opens_bare_and_markdown_links_on_release() { + for markdown in [ + "https://example.com/docs", + "[docs](https://example.com/docs)", + ] { + for width in [12, 60] { + let (mut view, cells) = transcript(markdown, width); + let down = mouse( + MouseEventKind::Down(MouseButton::Left), + /*column*/ 4, + /*row*/ 1, + ); + assert!(matches!( + view.handle_mouse(down, &cells), + Some(ViewAction::Changed) + )); + // Match the app's redraw before dispatching the next mouse event. + let area = view.area; + view.render(area, &mut Buffer::empty(area), &cells); + let up = MouseEvent { + kind: MouseEventKind::Up(MouseButton::Left), + ..down + }; + let Some(ViewAction::OpenLink(url)) = view.handle_mouse(up, &cells) else { + panic!("click should open {markdown}"); + }; + assert_eq!( + ( + url.as_str(), + view.selected_text(&cells), + view.is_following() + ), + ("https://example.com/docs", None, true) + ); + assert!(view.handle_mouse(up, &cells).is_none()); + } + } +} + +#[test] +fn dragging_a_link_selects_text_and_never_opens_it() { + for return_to_origin in [false, true] { + let (mut view, cells) = transcript( + "[documentation](https://example.com/docs)", + /*width*/ 60, + ); + view.handle_mouse( + mouse( + MouseEventKind::Down(MouseButton::Left), + /*column*/ 4, + /*row*/ 1, + ), + &cells, + ); + view.handle_mouse( + mouse( + MouseEventKind::Drag(MouseButton::Left), + /*column*/ 7, + /*row*/ 1, + ), + &cells, + ); + assert_eq!(view.selected_text(&cells).as_deref(), Some("doc")); + let column = if return_to_origin { 4 } else { 7 }; + let action = view.handle_mouse( + mouse( + MouseEventKind::Up(MouseButton::Left), + column, + /*row*/ 1, + ), + &cells, + ); + assert!(matches!(action, Some(ViewAction::Changed))); + assert_eq!( + view.selected_text(&cells).as_deref(), + (!return_to_origin).then_some("doc") + ); + } +} + +#[test] +fn scrolling_or_releasing_elsewhere_cancels_link_activation() { + for interruption in [ + mouse(MouseEventKind::ScrollUp, /*column*/ 4, /*row*/ 1), + mouse( + MouseEventKind::ScrollDown, + /*column*/ 4, + /*row*/ 1, + ), + mouse( + MouseEventKind::Up(MouseButton::Left), + /*column*/ 5, + /*row*/ 1, + ), + mouse( + MouseEventKind::Up(MouseButton::Left), + /*column*/ 0, + /*row*/ 0, + ), + ] { + let (mut view, cells) = transcript("[docs](https://example.com/docs)", /*width*/ 60); + view.handle_mouse( + mouse( + MouseEventKind::Down(MouseButton::Left), + /*column*/ 4, + /*row*/ 1, + ), + &cells, + ); + assert!(!matches!( + view.handle_mouse(interruption, &cells), + Some(ViewAction::OpenLink(_)) + )); + assert!(!matches!( + view.handle_mouse( + mouse( + MouseEventKind::Up(MouseButton::Left), + /*column*/ 4, + /*row*/ 1 + ), + &cells + ), + Some(ViewAction::OpenLink(_)) + )); + } +} + +#[test] +fn modified_clicks_still_open_immediately() { + for modifiers in [KeyModifiers::CONTROL, KeyModifiers::SUPER] { + let (mut view, cells) = transcript("[docs](https://example.com/docs)", /*width*/ 60); + let event = MouseEvent { + modifiers, + ..mouse( + MouseEventKind::Down(MouseButton::Left), + /*column*/ 4, + /*row*/ 1, + ) + }; + let Some(ViewAction::OpenLink(url)) = view.handle_mouse(event, &cells) else { + panic!("modified click should open link"); + }; + assert_eq!(url, "https://example.com/docs"); + assert!(view.selection.is_none()); + } +} diff --git a/codex-rs/tui/src/transcript_view/selection.rs b/codex-rs/tui/src/transcript_view/selection.rs index 7dd259716c..1cc62623bd 100644 --- a/codex-rs/tui/src/transcript_view/selection.rs +++ b/codex-rs/tui/src/transcript_view/selection.rs @@ -16,6 +16,7 @@ pub(super) struct Selection { pub(super) moved: bool, pub(super) resume_on_empty: bool, pub(super) pointer: Option, + pub(super) pressed_link: Option, origin: (Anchor, Anchor), unit: SelectionUnit, preferred_column: Option, @@ -57,6 +58,7 @@ impl TranscriptView { moved: false, resume_on_empty: was_following, pointer: Some(ScreenPosition::new(column, row)), + pressed_link: None, }); if was_following { self.hold_position();