clone/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-04-24 14:45:27 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix wrap behavior for long commands (#7655)

Browse Source 
before:
<img width="720" height="225" alt="image"
src="https://github.com/user-attachments/assets/19b7ad7c-db14-4792-97cc-80677a3a52ec"
/>
after:
<img width="500" height="219" alt="Screenshot 2025-12-05 at 4 37 14 PM"
src="https://github.com/user-attachments/assets/f877f846-5943-4ca7-8949-89e8524ffdb9"
/>


also removes `is_current`, which is deadcode
This commit is contained in:
zhao-oai
2025-12-08 09:35:03 -08:00
committed by GitHub
parent 5e888ab48e
commit cfda44b98b
5 changed files with 83 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
codex-rs/tui/src/bottom_pane/command_popup.rs
View File

@@ -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()

2
codex-rs/tui/src/bottom_pane/file_search_popup.rs
View File

@@ -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()
};

53
codex-rs/tui/src/bottom_pane/list_selection_view.rs
View File

@@ -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>;
@@ -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::<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>();

58
codex-rs/tui/src/bottom_pane/selection_popup_common.rs
View File

@@ -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
@@ -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;
@@ -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)

2
codex-rs/tui/src/bottom_pane/skill_popup.rs
View File

@@ -90,9 +90,9 @@ impl SkillPopup {
GenericDisplayRow {
name,
match_indices: indices,
is_current: false,
display_shortcut: None,
description: Some(description),
wrap_indent: None,
}
})
.collect()