diff --git a/codex-rs/tui/src/bottom_pane/command_popup.rs b/codex-rs/tui/src/bottom_pane/command_popup.rs index 39bbfbd18..8aca5c4a6 100644 --- a/codex-rs/tui/src/bottom_pane/command_popup.rs +++ b/codex-rs/tui/src/bottom_pane/command_popup.rs @@ -182,9 +182,9 @@ impl CommandPopup { GenericDisplayRow { name, match_indices: indices.map(|v| v.into_iter().map(|i| i + 1).collect()), - is_current: false, display_shortcut: None, description: Some(description), + wrap_indent: None, } }) .collect() diff --git a/codex-rs/tui/src/bottom_pane/file_search_popup.rs b/codex-rs/tui/src/bottom_pane/file_search_popup.rs index 708b00474..064e4f013 100644 --- a/codex-rs/tui/src/bottom_pane/file_search_popup.rs +++ b/codex-rs/tui/src/bottom_pane/file_search_popup.rs @@ -129,9 +129,9 @@ impl WidgetRef for &FileSearchPopup { .indices .as_ref() .map(|v| v.iter().map(|&i| i as usize).collect()), - is_current: false, display_shortcut: None, description: None, + wrap_indent: None, }) .collect() }; diff --git a/codex-rs/tui/src/bottom_pane/list_selection_view.rs b/codex-rs/tui/src/bottom_pane/list_selection_view.rs index d294a4726..b58524185 100644 --- a/codex-rs/tui/src/bottom_pane/list_selection_view.rs +++ b/codex-rs/tui/src/bottom_pane/list_selection_view.rs @@ -28,6 +28,7 @@ use super::scroll_state::ScrollState; use super::selection_popup_common::GenericDisplayRow; use super::selection_popup_common::measure_rows_height; use super::selection_popup_common::render_rows; +use unicode_width::UnicodeWidthStr; /// One selectable item in the generic selection list. pub(crate) type SelectionAction = Box; @@ -192,23 +193,26 @@ impl ListSelectionView { item.name.clone() }; let n = visible_idx + 1; - let display_name = if self.is_searchable { + let wrap_prefix = if self.is_searchable { // The number keys don't work when search is enabled (since we let the // numbers be used for the search query). - format!("{prefix} {name_with_marker}") + format!("{prefix} ") } else { - format!("{prefix} {n}. {name_with_marker}") + format!("{prefix} {n}. ") }; + let wrap_prefix_width = UnicodeWidthStr::width(wrap_prefix.as_str()); + let display_name = format!("{wrap_prefix}{name_with_marker}"); let description = is_selected .then(|| item.selected_description.clone()) .flatten() .or_else(|| item.description.clone()); + let wrap_indent = description.is_none().then_some(wrap_prefix_width); GenericDisplayRow { name: display_name, display_shortcut: item.display_shortcut, match_indices: None, - is_current: item.is_current, description, + wrap_indent, } }) }) @@ -558,6 +562,47 @@ mod tests { ); } + #[test] + fn wraps_long_option_without_overflowing_columns() { + let (tx_raw, _rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx_raw); + let items = vec![ + SelectionItem { + name: "Yes, proceed".to_string(), + dismiss_on_select: true, + ..Default::default() + }, + SelectionItem { + name: "Yes, and don't ask again for commands that start with `python -mpre_commit run --files eslint-plugin/no-mixed-const-enum-exports.js`".to_string(), + dismiss_on_select: true, + ..Default::default() + }, + ]; + let view = ListSelectionView::new( + SelectionViewParams { + title: Some("Approval".to_string()), + items, + ..Default::default() + }, + tx, + ); + + let rendered = render_lines_with_width(&view, 60); + let command_line = rendered + .lines() + .find(|line| line.contains("python -mpre_commit run")) + .expect("rendered lines should include wrapped command"); + assert!( + command_line.starts_with(" `python -mpre_commit run"), + "wrapped command line should align under the numbered prefix:\n{rendered}" + ); + assert!( + rendered.contains("eslint-plugin/no-") + && rendered.contains("mixed-const-enum-exports.js"), + "long command should not be truncated even when wrapped:\n{rendered}" + ); + } + #[test] fn width_changes_do_not_hide_rows() { let (tx_raw, _rx) = unbounded_channel::(); 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 8042a75b2..5107ab0ca 100644 --- a/codex-rs/tui/src/bottom_pane/selection_popup_common.rs +++ b/codex-rs/tui/src/bottom_pane/selection_popup_common.rs @@ -19,8 +19,8 @@ pub(crate) struct GenericDisplayRow { pub name: String, pub display_shortcut: Option, pub match_indices: Option>, // indices to bold (char positions) - pub is_current: bool, - pub description: Option, // optional grey text after the name + pub description: Option, // optional grey text after the name + pub wrap_indent: Option, // optional indent for wrapped lines } /// Compute a shared description-column start based on the widest visible name @@ -47,13 +47,30 @@ fn compute_desc_col( desc_col } +/// Determine how many spaces to indent wrapped lines for a row. +fn wrap_indent(row: &GenericDisplayRow, desc_col: usize, max_width: u16) -> usize { + let max_indent = max_width.saturating_sub(1) as usize; + let indent = row.wrap_indent.unwrap_or_else(|| { + if row.description.is_some() { + desc_col + } else { + 0 + } + }); + indent.min(max_indent) +} + /// Build the full display line for a row with the description padded to start /// at `desc_col`. Applies fuzzy-match bolding when indices are present and /// dims the description. fn build_full_line(row: &GenericDisplayRow, desc_col: usize) -> Line<'static> { // Enforce single-line name: allow at most desc_col - 2 cells for name, // reserving two spaces before the description column. - let name_limit = desc_col.saturating_sub(2); + let name_limit = row + .description + .as_ref() + .map(|_| desc_col.saturating_sub(2)) + .unwrap_or(usize::MAX); let mut name_spans: Vec = Vec::with_capacity(row.name.len()); let mut used_width = 0usize; @@ -63,11 +80,12 @@ fn build_full_line(row: &GenericDisplayRow, desc_col: usize) -> Line<'static> { let mut idx_iter = idxs.iter().peekable(); for (char_idx, ch) in row.name.chars().enumerate() { let ch_w = UnicodeWidthChar::width(ch).unwrap_or(0); - if used_width + ch_w > name_limit { + let next_width = used_width.saturating_add(ch_w); + if next_width > name_limit { truncated = true; break; } - used_width += ch_w; + used_width = next_width; if idx_iter.peek().is_some_and(|next| **next == char_idx) { idx_iter.next(); @@ -79,11 +97,12 @@ fn build_full_line(row: &GenericDisplayRow, desc_col: usize) -> Line<'static> { } else { for ch in row.name.chars() { let ch_w = UnicodeWidthChar::width(ch).unwrap_or(0); - if used_width + ch_w > name_limit { + let next_width = used_width.saturating_add(ch_w); + if next_width > name_limit { truncated = true; break; } - used_width += ch_w; + used_width = next_width; name_spans.push(ch.to_string().into()); } } @@ -161,24 +180,7 @@ pub(crate) fn render_rows( break; } - let GenericDisplayRow { - name, - match_indices, - display_shortcut, - is_current: _is_current, - description, - } = row; - - let mut full_line = build_full_line( - &GenericDisplayRow { - name: name.clone(), - match_indices: match_indices.clone(), - display_shortcut: *display_shortcut, - is_current: *_is_current, - description: description.clone(), - }, - desc_col, - ); + let mut full_line = build_full_line(row, desc_col); if Some(i) == state.selected_idx { // Match previous behavior: cyan + bold for the selected row. // Reset the style first to avoid inheriting dim from keyboard shortcuts. @@ -190,9 +192,10 @@ pub(crate) fn render_rows( // Wrap with subsequent indent aligned to the description column. use crate::wrapping::RtOptions; use crate::wrapping::word_wrap_line; + let continuation_indent = wrap_indent(row, desc_col, area.width); let options = RtOptions::new(area.width as usize) .initial_indent(Line::from("")) - .subsequent_indent(Line::from(" ".repeat(desc_col))); + .subsequent_indent(Line::from(" ".repeat(continuation_indent))); let wrapped = word_wrap_line(&full_line, options); // Render the wrapped lines. @@ -256,9 +259,10 @@ pub(crate) fn measure_rows_height( .map(|(_, r)| r) { let full_line = build_full_line(row, desc_col); + let continuation_indent = wrap_indent(row, desc_col, content_width); let opts = RtOptions::new(content_width as usize) .initial_indent(Line::from("")) - .subsequent_indent(Line::from(" ".repeat(desc_col))); + .subsequent_indent(Line::from(" ".repeat(continuation_indent))); total = total.saturating_add(word_wrap_line(&full_line, opts).len() as u16); } total.max(1) diff --git a/codex-rs/tui/src/bottom_pane/skill_popup.rs b/codex-rs/tui/src/bottom_pane/skill_popup.rs index 74c1b137c..3e0f79f84 100644 --- a/codex-rs/tui/src/bottom_pane/skill_popup.rs +++ b/codex-rs/tui/src/bottom_pane/skill_popup.rs @@ -90,9 +90,9 @@ impl SkillPopup { GenericDisplayRow { name, match_indices: indices, - is_current: false, display_shortcut: None, description: Some(description), + wrap_indent: None, } }) .collect()