Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion codex-rs/tui/src/bottom_pane/command_popup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion codex-rs/tui/src/bottom_pane/file_search_popup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
};
Expand Down
53 changes: 49 additions & 4 deletions codex-rs/tui/src/bottom_pane/list_selection_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<dyn Fn(&AppEventSender) + Send + Sync>;
Expand Down Expand Up @@ -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,
}
})
})
Expand Down Expand Up @@ -558,6 +562,47 @@ mod tests {
);
}

#[test]
fn wraps_long_option_without_overflowing_columns() {
let (tx_raw, _rx) = unbounded_channel::<AppEvent>();
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::<AppEvent>();
Expand Down
58 changes: 31 additions & 27 deletions codex-rs/tui/src/bottom_pane/selection_popup_common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ pub(crate) struct GenericDisplayRow {
pub name: String,
pub display_shortcut: Option<KeyBinding>,
pub match_indices: Option<Vec<usize>>, // indices to bold (char positions)
pub is_current: bool,
pub description: Option<String>, // optional grey text after the name
pub description: Option<String>, // optional grey text after the name
pub wrap_indent: Option<usize>, // optional indent for wrapped lines
}

/// Compute a shared description-column start based on the widest visible name
Expand All @@ -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<Span> = Vec::with_capacity(row.name.len());
let mut used_width = 0usize;
Expand All @@ -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();
Expand All @@ -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());
}
}
Expand Down Expand Up @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion codex-rs/tui/src/bottom_pane/skill_popup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,9 @@ impl SkillPopup {
GenericDisplayRow {
name,
match_indices: indices,
is_current: false,
display_shortcut: None,
description: Some(description),
wrap_indent: None,
}
})
.collect()
Expand Down
Loading