diff --git a/codex-rs/tui/src/bottom_pane/selection_popup_common.rs b/codex-rs/tui/src/bottom_pane/selection_popup_common.rs index c012f9dd49..fa2c9f2196 100644 --- a/codex-rs/tui/src/bottom_pane/selection_popup_common.rs +++ b/codex-rs/tui/src/bottom_pane/selection_popup_common.rs @@ -17,12 +17,16 @@ use crate::render::Insets; use crate::render::RectExt as _; use crate::style::accent_style; use crate::style::user_message_style; -use crate::width::display_width; use super::scroll_state::ScrollState; use super::selection_row_layout::SelectionDescriptionLayout; use super::selection_row_layout::build_full_line; -use super::selection_row_layout::line_to_owned; +#[path = "selection_row_links.rs"] +mod hyperlinks; +use hyperlinks::full_hyperlink_row; +use hyperlinks::wrap_row_lines; +#[cfg(test)] +use hyperlinks::wrap_two_column_row; /// Render-ready representation of one row in a selection popup. /// @@ -207,104 +211,6 @@ fn should_wrap_name_in_column(row: &GenericDisplayRow) -> bool { && row.name_prefix_spans.is_empty() } -fn wrap_two_column_row(row: &GenericDisplayRow, desc_col: usize, width: u16) -> Vec> { - use crate::wrapping::RtOptions; - use crate::wrapping::word_wrap_lines; - - let Some(description) = row.description.as_deref() else { - return Vec::new(); - }; - - let width = width.max(1); - let max_desc_col = width.saturating_sub(1) as usize; - if max_desc_col == 0 { - // No valid description column exists at this width; let callers fall - // back to single-line wrapping path. - return Vec::new(); - } - - let desc_col = desc_col.clamp(1, max_desc_col); - let left_width = desc_col.saturating_sub(2).max(1); - let right_width = width.saturating_sub(desc_col as u16).max(1) as usize; - let name_wrap_indent = row - .wrap_indent - .unwrap_or(0) - .min(left_width.saturating_sub(1)); - - let name_options = RtOptions::new(left_width) - .initial_indent(Line::from("")) - .subsequent_indent(Line::from(" ".repeat(name_wrap_indent))); - let name_lines = word_wrap_lines(row.name.lines(), name_options); - - let desc_options = RtOptions::new(right_width).initial_indent(Line::from("")); - let desc_lines = word_wrap_lines(description.lines(), desc_options); - - let rows = name_lines.len().max(desc_lines.len()).max(1); - let mut out = Vec::with_capacity(rows); - for idx in 0..rows { - let mut spans: Vec> = Vec::new(); - if let Some(name) = name_lines.get(idx) { - spans.push(name.to_string().into()); - } - - if let Some(desc) = desc_lines.get(idx) { - let left_used = spans - .iter() - .map(|span| display_width(span.content.as_ref())) - .sum::(); - let gap = if left_used == 0 { - desc_col - } else { - desc_col.saturating_sub(left_used).max(2) - }; - if gap > 0 { - spans.push(" ".repeat(gap).into()); - } - spans.push(desc.to_string().dim()); - } - - out.push(Line::from(spans)); - } - - out -} - -fn wrap_standard_row( - row: &GenericDisplayRow, - desc_col: usize, - width: u16, - description_layout: SelectionDescriptionLayout, -) -> Vec> { - use crate::wrapping::RtOptions; - use crate::wrapping::word_wrap_line; - - let full_line = build_full_line(row, desc_col, width, description_layout); - let continuation_indent = wrap_indent(row, desc_col, width); - let options = RtOptions::new(width.max(1) as usize) - .initial_indent(Line::from("")) - .subsequent_indent(Line::from(" ".repeat(continuation_indent))); - word_wrap_line(&full_line, options) - .into_iter() - .map(line_to_owned) - .collect() -} - -fn wrap_row_lines( - row: &GenericDisplayRow, - desc_col: usize, - width: u16, - description_layout: SelectionDescriptionLayout, -) -> Vec> { - if desc_col > 0 && should_wrap_name_in_column(row) { - let wrapped = wrap_two_column_row(row, desc_col, width); - if !wrapped.is_empty() { - return wrapped; - } - } - - wrap_standard_row(row, desc_col, width, description_layout) -} - fn apply_row_state_style( lines: &mut [Line<'static>], selected: bool, @@ -524,12 +430,14 @@ fn render_rows_inner( let mut wrapped = wrap_row_lines(row, desc_col, area.width, column_width.description_layout); clipped |= wrapped.len() > usize::from(area.bottom().saturating_sub(cur_y)); - apply_row_state_style( - &mut wrapped, - Some(i) == state.selected_idx && !row.is_disabled, - row.is_disabled, - row.selection_style, - ); + for line in &mut wrapped { + apply_row_state_style( + std::slice::from_mut(&mut line.line), + Some(i) == state.selected_idx && !row.is_disabled, + row.is_disabled, + row.selection_style, + ); + } // Render the wrapped lines. let mut rendered_item = false; @@ -537,7 +445,7 @@ fn render_rows_inner( if cur_y >= area.y + area.height { break; } - line.render( + (&line).render( Rect { x: area.x, y: cur_y, @@ -680,8 +588,16 @@ pub(crate) fn render_rows_single_line_with_col_width_mode( row.selection_style, ); - let full_line = truncate_line_with_ellipsis_if_overflow(full_line, area.width as usize); - full_line.render( + let mut full_line = full_hyperlink_row(row, full_line); + let clipped = line_width(&full_line.line) > usize::from(area.width); + full_line.line = + truncate_line_with_ellipsis_if_overflow(full_line.line, area.width as usize); + let visible_width = line_width(&full_line.line).saturating_sub(usize::from(clipped)); + full_line.hyperlinks.retain_mut(|link| { + link.columns.end = link.columns.end.min(visible_width); + !link.columns.is_empty() + }); + (&full_line).render( Rect { x: area.x, y: cur_y, @@ -1036,7 +952,7 @@ mod tests { let rendered = wrap_two_column_row(&row, /*desc_col*/ 8, /*width*/ 24) .into_iter() - .map(|line| line.to_string()) + .map(|line| line.line.to_string()) .collect::>() .join("\n"); diff --git a/codex-rs/tui/src/bottom_pane/selection_row_layout.rs b/codex-rs/tui/src/bottom_pane/selection_row_layout.rs index e69083cfe5..50678b8e39 100644 --- a/codex-rs/tui/src/bottom_pane/selection_row_layout.rs +++ b/codex-rs/tui/src/bottom_pane/selection_row_layout.rs @@ -1,5 +1,3 @@ -use std::borrow::Cow; - use ratatui::style::Stylize; use ratatui::text::Line; use ratatui::text::Span; @@ -32,21 +30,6 @@ impl SelectionDescriptionLayout { } } -pub(super) fn line_to_owned(line: Line<'_>) -> Line<'static> { - Line { - style: line.style, - alignment: line.alignment, - spans: line - .spans - .into_iter() - .map(|span| Span { - style: span.style, - content: Cow::Owned(span.content.into_owned()), - }) - .collect(), - } -} - fn combined_description( row: &GenericDisplayRow, description_layout: SelectionDescriptionLayout, diff --git a/codex-rs/tui/src/bottom_pane/selection_row_links.rs b/codex-rs/tui/src/bottom_pane/selection_row_links.rs new file mode 100644 index 0000000000..0c2e7ebc11 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/selection_row_links.rs @@ -0,0 +1,180 @@ +//! Preserve complete hyperlink destinations through selection-row wrapping and clipping. + +use super::GenericDisplayRow; +use super::SelectionDescriptionLayout; +use super::build_full_line; +use super::should_wrap_name_in_column; +use super::wrap_indent; +use crate::terminal_hyperlinks::HyperlinkLine; +use crate::width::display_width; +use ratatui::style::Stylize; +use ratatui::text::Line; +use ratatui::text::Span; + +pub(super) fn full_hyperlink_row(row: &GenericDisplayRow, line: Line<'static>) -> HyperlinkLine { + let prefix = row + .name_prefix_spans + .iter() + .map(|span| span.content.as_ref()) + .collect::(); + let text = line.to_string(); + let visible_name = text.strip_prefix(&prefix).unwrap_or(""); + let common = row + .name + .chars() + .zip(visible_name.chars()) + .take_while(|(a, b)| a == b) + .map(|(ch, _)| ch) + .collect::(); + let start = display_width(&prefix); + let end = start + display_width(&common); + let mut source = crate::terminal_hyperlinks::annotate_web_urls_in_line(line); + source + .hyperlinks + .retain(|link| link.columns.end <= start || link.columns.start >= end); + let name = crate::terminal_hyperlinks::annotate_web_urls_in_line(row.name.clone().into()); + source + .hyperlinks + .extend(name.hyperlinks.into_iter().filter_map(|link| { + let last = (start + link.columns.end).min(end); + let first = start + link.columns.start; + (first < last).then(|| link.with_columns(first..last)) + })); + source.hyperlinks.sort_by_key(|link| link.columns.start); + source +} + +fn wrap_column(text: &str, options: crate::wrapping::RtOptions<'static>) -> Vec { + text.lines() + .enumerate() + .flat_map(|(index, text)| { + let options = if index == 0 { + options.clone() + } else { + options + .clone() + .initial_indent(options.subsequent_indent.clone()) + }; + let source = + crate::terminal_hyperlinks::annotate_web_urls_in_line(text.to_owned().into()); + let wrapped = + crate::wrapping::word_wrap_line_with_source(&source.line, options.clone()); + crate::terminal_hyperlinks::remap_source_wrapped_line(&source, wrapped) + }) + .collect() +} + +pub(super) fn wrap_two_column_row( + row: &GenericDisplayRow, + desc_col: usize, + width: u16, +) -> Vec { + use crate::wrapping::RtOptions; + + let Some(description) = row.description.as_deref() else { + return Vec::new(); + }; + + let width = width.max(1); + let max_desc_col = width.saturating_sub(1) as usize; + if max_desc_col == 0 { + // No valid description column exists at this width; let callers fall + // back to single-line wrapping path. + return Vec::new(); + } + + let desc_col = desc_col.clamp(1, max_desc_col); + let left_width = desc_col.saturating_sub(2).max(1); + let right_width = width.saturating_sub(desc_col as u16).max(1) as usize; + let name_wrap_indent = row + .wrap_indent + .unwrap_or(0) + .min(left_width.saturating_sub(1)); + + let name_options = RtOptions::new(left_width) + .initial_indent(Line::from("")) + .subsequent_indent(Line::from(" ".repeat(name_wrap_indent))); + let name_lines = wrap_column(&row.name, name_options); + + let desc_options = RtOptions::new(right_width).initial_indent(Line::from("")); + let desc_lines = wrap_column(description, desc_options); + + let rows = name_lines.len().max(desc_lines.len()).max(1); + let mut out = Vec::with_capacity(rows); + for idx in 0..rows { + let mut hyperlinks = name_lines + .get(idx) + .map(|name| name.hyperlinks.clone()) + .unwrap_or_default(); + let mut spans: Vec> = Vec::new(); + if let Some(name) = name_lines.get(idx) { + spans.push(name.line.to_string().into()); + } + + if let Some(desc) = desc_lines.get(idx) { + let left_used = spans + .iter() + .map(|span| display_width(span.content.as_ref())) + .sum::(); + let gap = if left_used == 0 { + desc_col + } else { + desc_col.saturating_sub(left_used).max(2) + }; + if gap > 0 { + spans.push(" ".repeat(gap).into()); + } + let column = left_used + gap; + hyperlinks.extend(desc.hyperlinks.iter().map(|link| { + link.with_columns(column + link.columns.start..column + link.columns.end) + })); + spans.push(desc.line.to_string().dim()); + } + + let mut line = HyperlinkLine::new(Line::from(spans)); + line.hyperlinks = hyperlinks; + out.push(line); + } + + out +} + +fn wrap_standard_row( + row: &GenericDisplayRow, + desc_col: usize, + width: u16, + description_layout: SelectionDescriptionLayout, +) -> Vec { + use crate::wrapping::RtOptions; + + let full_line = full_hyperlink_row( + row, + build_full_line(row, desc_col, width, description_layout), + ); + let continuation_indent = wrap_indent(row, desc_col, width); + let options = RtOptions::new(width.max(1) as usize) + .initial_indent(Line::from("")) + .subsequent_indent(Line::from(" ".repeat(continuation_indent))); + let wrapped = crate::wrapping::word_wrap_line_with_source(&full_line.line, options); + crate::terminal_hyperlinks::remap_source_wrapped_line(&full_line, wrapped) +} + +pub(super) fn wrap_row_lines( + row: &GenericDisplayRow, + desc_col: usize, + width: u16, + description_layout: SelectionDescriptionLayout, +) -> Vec { + if desc_col > 0 && should_wrap_name_in_column(row) { + let wrapped = wrap_two_column_row(row, desc_col, width); + if !wrapped.is_empty() { + return wrapped; + } + } + + wrap_standard_row(row, desc_col, width, description_layout) +} + +#[cfg(test)] +#[path = "selection_row_links_tests.rs"] +mod tests; diff --git a/codex-rs/tui/src/bottom_pane/selection_row_links_tests.rs b/codex-rs/tui/src/bottom_pane/selection_row_links_tests.rs new file mode 100644 index 0000000000..3f19d03c11 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/selection_row_links_tests.rs @@ -0,0 +1,91 @@ +use super::super::*; + +#[test] +fn choice_labels_and_descriptions_keep_complete_url_destinations() { + let label = "https://github.com/openai/codex/pull/12345?diff=split"; + let description = "https://example.com/releases/review/complete-destination"; + let mut snapshots = Vec::new(); + for prefix_label in [false, true] { + for wrap_indent in [Some(0), None] { + let mut row = GenericDisplayRow { + name: format!("日本語 {label}"), + description: Some(description.into()), + wrap_indent, + ..Default::default() + }; + if prefix_label { + row.name_prefix_spans = vec!["1. ".into(), std::mem::take(&mut row.name).bold()]; + row.description = None; + } + let rows = [row]; + for single_line in [false, true] { + let area = Rect::new(0, 0, 48, if single_line { 1 } else { 12 }); + let mut buf = Buffer::empty(area); + if single_line { + render_rows_single_line_with_col_width_mode( + area, + &mut buf, + &rows, + &ScrollState::new(), + /*max_results*/ 1, + "", + ColumnWidthConfig::default(), + ); + } else { + render_rows( + area, + &mut buf, + &rows, + &ScrollState::new(), + /*max_results*/ 1, + "", + ); + } + let mut linked = 0; + for cell in &buf.content { + if cell.symbol().contains("\x1b]8;;") { + linked += 1; + assert!( + [label, description].iter().any(|url| cell + .symbol() + .starts_with(&format!("\x1b]8;;{url}\x07"))) + ); + assert_ne!(crate::terminal_hyperlinks::strip_osc8(cell.symbol()), "…"); + } + } + assert!(linked > 0); + let urls = if prefix_label { + vec![label] + } else { + vec![label, description] + }; + for url in urls { + let visible = buf + .content + .iter() + .filter(|cell| cell.symbol().starts_with(&format!("\x1b]8;;{url}\x07"))) + .map(|cell| crate::terminal_hyperlinks::strip_osc8(cell.symbol())) + .collect::(); + assert!(!visible.is_empty() && url.starts_with(&visible)); + } + let visible = buf + .content + .chunks(48) + .map(|row| { + row.iter() + .map(|cell| crate::terminal_hyperlinks::strip_osc8(cell.symbol())) + .collect::() + .trim_end() + .to_owned() + }) + .collect::>() + .join("\n"); + snapshots.push(format!( + "prefix label: {prefix_label}, wrapped label: {}, single line: {single_line}\n{visible}", + wrap_indent.is_some() + )); + } + } + } + insta::assert_snapshot!("choice_wrapped_urls", snapshots.join("\n\n")); +} diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__selection_popup_common__hyperlinks__tests__choice_wrapped_urls.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__selection_popup_common__hyperlinks__tests__choice_wrapped_urls.snap new file mode 100644 index 0000000000..68562f08ae --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__selection_popup_common__hyperlinks__tests__choice_wrapped_urls.snap @@ -0,0 +1,71 @@ +--- +source: tui/src/bottom_pane/selection_row_links_tests.rs +expression: "snapshots.join(\"\\n\\n\")" +--- +prefix label: false, wrapped label: true, single line: false +日 本 語 https://github.com/ https:// +openai/codex/pull/12345? example.com/ +diff=split releases/ + review/ + complete- + destination + + + + + + + +prefix label: false, wrapped label: true, single line: true +日 本 語 https://github.com/opena… https://exampl… + +prefix label: false, wrapped label: false, single line: false +日 本 語 https://github.com/opena… https:// + example.com/ + releases/ + review/ + complete- + destination + + + + + + + +prefix label: false, wrapped label: false, single line: true +日 本 語 https://github.com/opena… https://exampl… + +prefix label: true, wrapped label: true, single line: false +1. 日 本 語 https://github.com/openai/codex/ +pull/12345?diff=split + + + + + + + + + + + +prefix label: true, wrapped label: true, single line: true +1. 日 本 語 https://github.com/openai/codex/pull/… + +prefix label: true, wrapped label: false, single line: false +1. 日 本 語 https://github.com/openai/codex/ +pull/12345?diff=split + + + + + + + + + + + +prefix label: true, wrapped label: false, single line: true +1. 日 本 語 https://github.com/openai/codex/pull/…