Skip to content

Commit d1ad779

Browse files
committed
fix wrap behavior for long commands
1 parent 2e4a402 commit d1ad779

File tree

5 files changed

+82
-28
lines changed

5 files changed

+82
-28
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,7 @@ impl CommandPopup {
185185
is_current: false,
186186
display_shortcut: None,
187187
description: Some(description),
188+
wrap_indent: None,
188189
}
189190
})
190191
.collect()

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ impl WidgetRef for &FileSearchPopup {
132132
is_current: false,
133133
display_shortcut: None,
134134
description: None,
135+
wrap_indent: None,
135136
})
136137
.collect()
137138
};

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

Lines changed: 49 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ use super::scroll_state::ScrollState;
2828
use super::selection_popup_common::GenericDisplayRow;
2929
use super::selection_popup_common::measure_rows_height;
3030
use super::selection_popup_common::render_rows;
31+
use unicode_width::UnicodeWidthStr;
3132

3233
/// One selectable item in the generic selection list.
3334
pub(crate) type SelectionAction = Box<dyn Fn(&AppEventSender) + Send + Sync>;
@@ -192,23 +193,27 @@ impl ListSelectionView {
192193
item.name.clone()
193194
};
194195
let n = visible_idx + 1;
195-
let display_name = if self.is_searchable {
196+
let wrap_prefix = if self.is_searchable {
196197
// The number keys don't work when search is enabled (since we let the
197198
// numbers be used for the search query).
198-
format!("{prefix} {name_with_marker}")
199+
format!("{prefix} ")
199200
} else {
200-
format!("{prefix} {n}. {name_with_marker}")
201+
format!("{prefix} {n}. ")
201202
};
203+
let wrap_prefix_width = UnicodeWidthStr::width(wrap_prefix.as_str());
204+
let display_name = format!("{wrap_prefix}{name_with_marker}");
202205
let description = is_selected
203206
.then(|| item.selected_description.clone())
204207
.flatten()
205208
.or_else(|| item.description.clone());
209+
let wrap_indent = description.is_none().then_some(wrap_prefix_width);
206210
GenericDisplayRow {
207211
name: display_name,
208212
display_shortcut: item.display_shortcut,
209213
match_indices: None,
210214
is_current: item.is_current,
211215
description,
216+
wrap_indent,
212217
}
213218
})
214219
})
@@ -558,6 +563,47 @@ mod tests {
558563
);
559564
}
560565

566+
#[test]
567+
fn wraps_long_option_without_overflowing_columns() {
568+
let (tx_raw, _rx) = unbounded_channel::<AppEvent>();
569+
let tx = AppEventSender::new(tx_raw);
570+
let items = vec![
571+
SelectionItem {
572+
name: "Yes, proceed".to_string(),
573+
dismiss_on_select: true,
574+
..Default::default()
575+
},
576+
SelectionItem {
577+
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(),
578+
dismiss_on_select: true,
579+
..Default::default()
580+
},
581+
];
582+
let view = ListSelectionView::new(
583+
SelectionViewParams {
584+
title: Some("Approval".to_string()),
585+
items,
586+
..Default::default()
587+
},
588+
tx,
589+
);
590+
591+
let rendered = render_lines_with_width(&view, 60);
592+
let command_line = rendered
593+
.lines()
594+
.find(|line| line.contains("python -mpre_commit run"))
595+
.expect("rendered lines should include wrapped command");
596+
assert!(
597+
command_line.starts_with(" `python -mpre_commit run"),
598+
"wrapped command line should align under the numbered prefix:\n{rendered}"
599+
);
600+
assert!(
601+
rendered.contains("eslint-plugin/no-")
602+
&& rendered.contains("mixed-const-enum-exports.js"),
603+
"long command should not be truncated even when wrapped:\n{rendered}"
604+
);
605+
}
606+
561607
#[test]
562608
fn width_changes_do_not_hide_rows() {
563609
let (tx_raw, _rx) = unbounded_channel::<AppEvent>();

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

Lines changed: 30 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ pub(crate) struct GenericDisplayRow {
2121
pub match_indices: Option<Vec<usize>>, // indices to bold (char positions)
2222
pub is_current: bool,
2323
pub description: Option<String>, // optional grey text after the name
24+
pub wrap_indent: Option<usize>, // optional indent for wrapped lines
2425
}
2526

2627
/// Compute a shared description-column start based on the widest visible name
@@ -47,13 +48,30 @@ fn compute_desc_col(
4748
desc_col
4849
}
4950

51+
/// Determine how many spaces to indent wrapped lines for a row.
52+
fn wrap_indent(row: &GenericDisplayRow, desc_col: usize, max_width: u16) -> usize {
53+
let max_indent = max_width.saturating_sub(1) as usize;
54+
let indent = row.wrap_indent.unwrap_or_else(|| {
55+
if row.description.is_some() {
56+
desc_col
57+
} else {
58+
0
59+
}
60+
});
61+
indent.min(max_indent)
62+
}
63+
5064
/// Build the full display line for a row with the description padded to start
5165
/// at `desc_col`. Applies fuzzy-match bolding when indices are present and
5266
/// dims the description.
5367
fn build_full_line(row: &GenericDisplayRow, desc_col: usize) -> Line<'static> {
5468
// Enforce single-line name: allow at most desc_col - 2 cells for name,
5569
// reserving two spaces before the description column.
56-
let name_limit = desc_col.saturating_sub(2);
70+
let name_limit = row
71+
.description
72+
.as_ref()
73+
.map(|_| desc_col.saturating_sub(2))
74+
.unwrap_or(usize::MAX);
5775

5876
let mut name_spans: Vec<Span> = Vec::with_capacity(row.name.len());
5977
let mut used_width = 0usize;
@@ -63,11 +81,12 @@ fn build_full_line(row: &GenericDisplayRow, desc_col: usize) -> Line<'static> {
6381
let mut idx_iter = idxs.iter().peekable();
6482
for (char_idx, ch) in row.name.chars().enumerate() {
6583
let ch_w = UnicodeWidthChar::width(ch).unwrap_or(0);
66-
if used_width + ch_w > name_limit {
84+
let next_width = used_width.saturating_add(ch_w);
85+
if next_width > name_limit {
6786
truncated = true;
6887
break;
6988
}
70-
used_width += ch_w;
89+
used_width = next_width;
7190

7291
if idx_iter.peek().is_some_and(|next| **next == char_idx) {
7392
idx_iter.next();
@@ -79,11 +98,12 @@ fn build_full_line(row: &GenericDisplayRow, desc_col: usize) -> Line<'static> {
7998
} else {
8099
for ch in row.name.chars() {
81100
let ch_w = UnicodeWidthChar::width(ch).unwrap_or(0);
82-
if used_width + ch_w > name_limit {
101+
let next_width = used_width.saturating_add(ch_w);
102+
if next_width > name_limit {
83103
truncated = true;
84104
break;
85105
}
86-
used_width += ch_w;
106+
used_width = next_width;
87107
name_spans.push(ch.to_string().into());
88108
}
89109
}
@@ -161,24 +181,7 @@ pub(crate) fn render_rows(
161181
break;
162182
}
163183

164-
let GenericDisplayRow {
165-
name,
166-
match_indices,
167-
display_shortcut,
168-
is_current: _is_current,
169-
description,
170-
} = row;
171-
172-
let mut full_line = build_full_line(
173-
&GenericDisplayRow {
174-
name: name.clone(),
175-
match_indices: match_indices.clone(),
176-
display_shortcut: *display_shortcut,
177-
is_current: *_is_current,
178-
description: description.clone(),
179-
},
180-
desc_col,
181-
);
184+
let mut full_line = build_full_line(row, desc_col);
182185
if Some(i) == state.selected_idx {
183186
// Match previous behavior: cyan + bold for the selected row.
184187
// Reset the style first to avoid inheriting dim from keyboard shortcuts.
@@ -190,9 +193,10 @@ pub(crate) fn render_rows(
190193
// Wrap with subsequent indent aligned to the description column.
191194
use crate::wrapping::RtOptions;
192195
use crate::wrapping::word_wrap_line;
196+
let continuation_indent = wrap_indent(row, desc_col, area.width);
193197
let options = RtOptions::new(area.width as usize)
194198
.initial_indent(Line::from(""))
195-
.subsequent_indent(Line::from(" ".repeat(desc_col)));
199+
.subsequent_indent(Line::from(" ".repeat(continuation_indent)));
196200
let wrapped = word_wrap_line(&full_line, options);
197201

198202
// Render the wrapped lines.
@@ -256,9 +260,10 @@ pub(crate) fn measure_rows_height(
256260
.map(|(_, r)| r)
257261
{
258262
let full_line = build_full_line(row, desc_col);
263+
let continuation_indent = wrap_indent(row, desc_col, content_width);
259264
let opts = RtOptions::new(content_width as usize)
260265
.initial_indent(Line::from(""))
261-
.subsequent_indent(Line::from(" ".repeat(desc_col)));
266+
.subsequent_indent(Line::from(" ".repeat(continuation_indent)));
262267
total = total.saturating_add(word_wrap_line(&full_line, opts).len() as u16);
263268
}
264269
total.max(1)

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ impl SkillPopup {
9393
is_current: false,
9494
display_shortcut: None,
9595
description: Some(description),
96+
wrap_indent: None,
9697
}
9798
})
9899
.collect()

0 commit comments

Comments
 (0)