Skip to content

Commit 71ed69b

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

File tree

5 files changed

+83
-34
lines changed

5 files changed

+83
-34
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,9 +182,9 @@ impl CommandPopup {
182182
GenericDisplayRow {
183183
name,
184184
match_indices: indices.map(|v| v.into_iter().map(|i| i + 1).collect()),
185-
is_current: false,
186185
display_shortcut: None,
187186
description: Some(description),
187+
wrap_indent: None,
188188
}
189189
})
190190
.collect()

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,9 +129,9 @@ impl WidgetRef for &FileSearchPopup {
129129
.indices
130130
.as_ref()
131131
.map(|v| v.iter().map(|&i| i as usize).collect()),
132-
is_current: false,
133132
display_shortcut: None,
134133
description: None,
134+
wrap_indent: None,
135135
})
136136
.collect()
137137
};

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

Lines changed: 49 additions & 4 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,26 @@ 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,
210-
is_current: item.is_current,
211214
description,
215+
wrap_indent,
212216
}
213217
})
214218
})
@@ -558,6 +562,47 @@ mod tests {
558562
);
559563
}
560564

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

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

Lines changed: 31 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ pub(crate) struct GenericDisplayRow {
1919
pub name: String,
2020
pub display_shortcut: Option<KeyBinding>,
2121
pub match_indices: Option<Vec<usize>>, // indices to bold (char positions)
22-
pub is_current: bool,
23-
pub description: Option<String>, // optional grey text after the name
22+
pub description: Option<String>, // optional grey text after the name
23+
pub wrap_indent: Option<usize>, // optional indent for wrapped lines
2424
}
2525

2626
/// Compute a shared description-column start based on the widest visible name
@@ -47,13 +47,30 @@ fn compute_desc_col(
4747
desc_col
4848
}
4949

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

5875
let mut name_spans: Vec<Span> = Vec::with_capacity(row.name.len());
5976
let mut used_width = 0usize;
@@ -63,11 +80,12 @@ fn build_full_line(row: &GenericDisplayRow, desc_col: usize) -> Line<'static> {
6380
let mut idx_iter = idxs.iter().peekable();
6481
for (char_idx, ch) in row.name.chars().enumerate() {
6582
let ch_w = UnicodeWidthChar::width(ch).unwrap_or(0);
66-
if used_width + ch_w > name_limit {
83+
let next_width = used_width.saturating_add(ch_w);
84+
if next_width > name_limit {
6785
truncated = true;
6886
break;
6987
}
70-
used_width += ch_w;
88+
used_width = next_width;
7189

7290
if idx_iter.peek().is_some_and(|next| **next == char_idx) {
7391
idx_iter.next();
@@ -79,11 +97,12 @@ fn build_full_line(row: &GenericDisplayRow, desc_col: usize) -> Line<'static> {
7997
} else {
8098
for ch in row.name.chars() {
8199
let ch_w = UnicodeWidthChar::width(ch).unwrap_or(0);
82-
if used_width + ch_w > name_limit {
100+
let next_width = used_width.saturating_add(ch_w);
101+
if next_width > name_limit {
83102
truncated = true;
84103
break;
85104
}
86-
used_width += ch_w;
105+
used_width = next_width;
87106
name_spans.push(ch.to_string().into());
88107
}
89108
}
@@ -161,24 +180,7 @@ pub(crate) fn render_rows(
161180
break;
162181
}
163182

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-
);
183+
let mut full_line = build_full_line(row, desc_col);
182184
if Some(i) == state.selected_idx {
183185
// Match previous behavior: cyan + bold for the selected row.
184186
// Reset the style first to avoid inheriting dim from keyboard shortcuts.
@@ -190,9 +192,10 @@ pub(crate) fn render_rows(
190192
// Wrap with subsequent indent aligned to the description column.
191193
use crate::wrapping::RtOptions;
192194
use crate::wrapping::word_wrap_line;
195+
let continuation_indent = wrap_indent(row, desc_col, area.width);
193196
let options = RtOptions::new(area.width as usize)
194197
.initial_indent(Line::from(""))
195-
.subsequent_indent(Line::from(" ".repeat(desc_col)));
198+
.subsequent_indent(Line::from(" ".repeat(continuation_indent)));
196199
let wrapped = word_wrap_line(&full_line, options);
197200

198201
// Render the wrapped lines.
@@ -256,9 +259,10 @@ pub(crate) fn measure_rows_height(
256259
.map(|(_, r)| r)
257260
{
258261
let full_line = build_full_line(row, desc_col);
262+
let continuation_indent = wrap_indent(row, desc_col, content_width);
259263
let opts = RtOptions::new(content_width as usize)
260264
.initial_indent(Line::from(""))
261-
.subsequent_indent(Line::from(" ".repeat(desc_col)));
265+
.subsequent_indent(Line::from(" ".repeat(continuation_indent)));
262266
total = total.saturating_add(word_wrap_line(&full_line, opts).len() as u16);
263267
}
264268
total.max(1)

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,9 +90,9 @@ impl SkillPopup {
9090
GenericDisplayRow {
9191
name,
9292
match_indices: indices,
93-
is_current: false,
9493
display_shortcut: None,
9594
description: Some(description),
95+
wrap_indent: None,
9696
}
9797
})
9898
.collect()

0 commit comments

Comments
 (0)