mirror of
https://github.com/openai/codex.git
synced 2026-10-20 17:22:37 +00:00
Enable plain clicks on transcript links and style bare URLs (#46884)
## Why Transcript links required a modified click to open, and bare URLs lacked the visual styling of explicit Markdown links. ## What changed - Open links on release after an unmodified, stationary left click. Cancel activation after dragging, scrolling, or releasing elsewhere so text selection remains usable. - Apply Markdown link styling to bare URLs while preserving surrounding prose, existing emphasis, Unicode, and table wrapping. ## Testing Add gesture tests for bare and Markdown links at narrow and wide widths, drag and scroll cancellation, and existing modified-click behavior. Add styling tests and snapshots covering punctuation, emphasis, tabs, Unicode, and wrapped table URLs. GitOrigin-RevId: 23d12c901932d7e3586d841e5a0ece00e422fccd
This commit is contained in:
@@ -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);
|
||||
}
|
||||
|
||||
+6
@@ -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()]))
|
||||
+20
@@ -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(),
|
||||
]),
|
||||
])
|
||||
@@ -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<Span<'static>> = 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 {
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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<Arc<dyn HistoryCell>>) {
|
||||
let cells: Vec<Arc<dyn HistoryCell>> = 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());
|
||||
}
|
||||
}
|
||||
@@ -16,6 +16,7 @@ pub(super) struct Selection {
|
||||
pub(super) moved: bool,
|
||||
pub(super) resume_on_empty: bool,
|
||||
pub(super) pointer: Option<ScreenPosition>,
|
||||
pub(super) pressed_link: Option<String>,
|
||||
origin: (Anchor, Anchor),
|
||||
unit: SelectionUnit,
|
||||
preferred_column: Option<u16>,
|
||||
@@ -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();
|
||||
|
||||
Reference in New Issue
Block a user