Keep wrapped option labels within left column

codex-rs/tui/src/bottom_pane/selection_popup_common.rs

@@ -9,6 +9,7 @@ use ratatui::text::Line;
 use ratatui::text::Span;
 use ratatui::widgets::Block;
 use ratatui::widgets::Widget;
+use std::borrow::Cow;
 use unicode_width::UnicodeWidthChar;
 use unicode_width::UnicodeWidthStr;
 
@@ -113,6 +114,21 @@ fn line_width(line: &Line<'_>) -> usize {
         .sum()
 }
 
+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(),
+    }
+}
+
 pub(crate) fn truncate_line_to_width(line: Line<'static>, max_width: usize) -> Line<'static> {
     if max_width == 0 {
         return Line::from(Vec::<Span<'static>>::new());
@@ -275,26 +291,16 @@ fn wrap_indent(row: &GenericDisplayRow, desc_col: usize, max_width: u16) -> usiz
     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> {
-    let combined_description = match (&row.description, &row.disabled_reason) {
+fn combined_description_text(row: &GenericDisplayRow) -> Option<String> {
+    match (&row.description, &row.disabled_reason) {
         (Some(desc), Some(reason)) => Some(format!("{desc} (disabled: {reason})")),
         (Some(desc), None) => Some(desc.clone()),
         (None, Some(reason)) => Some(format!("disabled: {reason}")),
         (None, None) => None,
-    };
-
-    // Enforce single-line names only when no explicit wrap indent is set.
-    // Callers that set `wrap_indent` can opt into wrapped names even when a
-    // description is present.
-    let name_limit = if combined_description.is_some() && row.wrap_indent.is_none() {
-        desc_col.saturating_sub(2)
-    } else {
-        usize::MAX
-    };
+    }
+}
 
+fn build_name_spans(row: &GenericDisplayRow, name_limit: usize) -> Vec<Span<'static>> {
     let mut name_spans: Vec<Span> = Vec::with_capacity(row.name.len());
     let mut used_width = 0usize;
     let mut truncated = false;
@@ -340,6 +346,35 @@ fn build_full_line(row: &GenericDisplayRow, desc_col: usize) -> Line<'static> {
         name_spans.push(" (disabled)".dim());
     }
 
+    name_spans
+}
+
+fn build_name_line(row: &GenericDisplayRow, name_limit: usize) -> Line<'static> {
+    let mut name_spans = build_name_spans(row, name_limit);
+    if let Some(display_shortcut) = row.display_shortcut {
+        name_spans.push(" (".into());
+        name_spans.push(display_shortcut.into());
+        name_spans.push(")".into());
+    }
+    Line::from(name_spans)
+}
+
+/// 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> {
+    let combined_description = combined_description_text(row);
+
+    // Enforce single-line names only when no explicit wrap indent is set.
+    // Callers that set `wrap_indent` can opt into wrapped names even when a
+    // description is present.
+    let name_limit = if combined_description.is_some() && row.wrap_indent.is_none() {
+        desc_col.saturating_sub(2)
+    } else {
+        usize::MAX
+    };
+
+    let name_spans = build_name_spans(row, name_limit);
     let this_name_width = Line::from(name_spans.clone()).width();
     let mut full_spans: Vec<Span> = name_spans;
     if let Some(display_shortcut) = row.display_shortcut {
@@ -365,6 +400,118 @@ fn build_full_line(row: &GenericDisplayRow, desc_col: usize) -> Line<'static> {
     Line::from(full_spans)
 }
 
+fn should_wrap_name_in_column(row: &GenericDisplayRow) -> bool {
+    row.wrap_indent.is_some()
+        && (row.description.is_some() || row.disabled_reason.is_some())
+        && row.category_tag.is_none()
+}
+
+fn wrap_two_column_row(row: &GenericDisplayRow, desc_col: usize, width: u16) -> Vec<Line<'static>> {
+    use crate::wrapping::RtOptions;
+    use crate::wrapping::word_wrap_line;
+
+    let Some(combined_desc) = combined_description_text(row) else {
+        return Vec::new();
+    };
+
+    let width = width.max(1);
+    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_line = build_name_line(row, usize::MAX);
+    let name_lines = word_wrap_line(
+        &name_line,
+        RtOptions::new(left_width)
+            .initial_indent(Line::from(""))
+            .subsequent_indent(Line::from(" ".repeat(name_wrap_indent))),
+    )
+    .into_iter()
+    .map(line_to_owned)
+    .collect::<Vec<_>>();
+
+    let desc_line = Line::from(combined_desc.dim());
+    let desc_lines = word_wrap_line(
+        &desc_line,
+        RtOptions::new(right_width)
+            .initial_indent(Line::from(""))
+            .subsequent_indent(Line::from("")),
+    )
+    .into_iter()
+    .map(line_to_owned)
+    .collect::<Vec<_>>();
+
+    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::new();
+        if let Some(name) = name_lines.get(idx) {
+            spans.extend(name.spans.clone());
+        }
+
+        if let Some(desc) = desc_lines.get(idx) {
+            let left_used = spans
+                .iter()
+                .map(|span| UnicodeWidthStr::width(span.content.as_ref()))
+                .sum::<usize>();
+            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.extend(desc.spans.clone());
+        }
+
+        out.push(Line::from(spans));
+    }
+    out
+}
+
+fn wrap_row_lines(row: &GenericDisplayRow, desc_col: usize, width: u16) -> Vec<Line<'static>> {
+    use crate::wrapping::RtOptions;
+    use crate::wrapping::word_wrap_line;
+
+    if should_wrap_name_in_column(row) {
+        let wrapped = wrap_two_column_row(row, desc_col, width);
+        if !wrapped.is_empty() {
+            return wrapped;
+        }
+    }
+
+    let full_line = build_full_line(row, desc_col);
+    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 apply_row_state_style(lines: &mut [Line<'static>], selected: bool, is_disabled: bool) {
+    if selected {
+        for line in lines.iter_mut() {
+            line.spans.iter_mut().for_each(|span| {
+                span.style = Style::default().fg(Color::Cyan).bold();
+            });
+        }
+    }
+    if is_disabled {
+        for line in lines.iter_mut() {
+            line.spans.iter_mut().for_each(|span| {
+                span.style = span.style.dim();
+            });
+        }
+    }
+}
+
 /// Render a list of rows using the provided ScrollState, with shared styling
 /// and behavior for selection popups.
 fn render_rows_inner(
@@ -422,28 +569,12 @@ fn render_rows_inner(
             break;
         }
 
-        let mut full_line = build_full_line(row, desc_col);
-        if Some(i) == state.selected_idx && !row.is_disabled {
-            // Match previous behavior: cyan + bold for the selected row.
-            // Reset the style first to avoid inheriting dim from keyboard shortcuts.
-            full_line.spans.iter_mut().for_each(|span| {
-                span.style = Style::default().fg(Color::Cyan).bold();
-            });
-        }
-        if row.is_disabled {
-            full_line.spans.iter_mut().for_each(|span| {
-                span.style = span.style.dim();
-            });
-        }
-
-        // 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(continuation_indent)));
-        let wrapped = word_wrap_line(&full_line, options);
+        let mut wrapped = wrap_row_lines(row, desc_col, area.width);
+        apply_row_state_style(
+            &mut wrapped,
+            Some(i) == state.selected_idx && !row.is_disabled,
+            row.is_disabled,
+        );
 
         // Render the wrapped lines.
         for line in wrapped {
@@ -710,8 +841,6 @@ fn measure_rows_height_inner(
         col_width_mode,
     );
 
-    use crate::wrapping::RtOptions;
-    use crate::wrapping::word_wrap_line;
     let mut total: u16 = 0;
     for row in rows_all
         .iter()
@@ -720,12 +849,7 @@ fn measure_rows_height_inner(
         .take(visible_items)
         .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(continuation_indent)));
-        let wrapped_lines = word_wrap_line(&full_line, opts).len();
+        let wrapped_lines = wrap_row_lines(row, desc_col, content_width).len();
         total = total.saturating_add(wrapped_lines as u16);
     }
     total.max(1)