Skip to content

Commit bf82353

Browse files
tui: fix wrapping in user approval decisions (#5008)
before: <img width="706" height="71" alt="Screenshot 2025-10-09 at 10 20 57 AM" src="https://github.com/user-attachments/assets/ff758b77-4e64-4736-b867-7ebf596e4e65" /> after: <img width="706" height="71" alt="Screenshot 2025-10-09 at 10 20 35 AM" src="https://github.com/user-attachments/assets/6a44efc0-d9ee-40ce-a709-cce969d6e3b3" />
1 parent 0308feb commit bf82353

File tree

4 files changed

+184
-96
lines changed

4 files changed

+184
-96
lines changed

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

Lines changed: 31 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ use crate::key_hint::KeyBinding;
1616
use crate::render::highlight::highlight_bash_to_lines;
1717
use crate::render::renderable::ColumnRenderable;
1818
use crate::render::renderable::Renderable;
19-
use crate::text_formatting::truncate_text;
2019
use codex_core::protocol::FileChange;
2120
use codex_core::protocol::Op;
2221
use codex_core::protocol::ReviewDecision;
@@ -160,11 +159,8 @@ impl ApprovalOverlay {
160159
}
161160

162161
fn handle_exec_decision(&self, id: &str, command: &[String], decision: ReviewDecision) {
163-
if let Some(lines) = build_exec_history_lines(command.to_vec(), decision) {
164-
self.app_event_tx.send(AppEvent::InsertHistoryCell(Box::new(
165-
history_cell::new_user_approval_decision(lines),
166-
)));
167-
}
162+
let cell = history_cell::new_approval_decision_cell(command.to_vec(), decision);
163+
self.app_event_tx.send(AppEvent::InsertHistoryCell(cell));
168164
self.app_event_tx.send(AppEvent::CodexOp(Op::ExecApproval {
169165
id: id.to_string(),
170166
decision,
@@ -396,91 +392,11 @@ fn patch_options() -> Vec<ApprovalOption> {
396392
]
397393
}
398394

399-
fn build_exec_history_lines(
400-
command: Vec<String>,
401-
decision: ReviewDecision,
402-
) -> Option<Vec<Line<'static>>> {
403-
use ReviewDecision::*;
404-
405-
let (symbol, summary): (Span<'static>, Vec<Span<'static>>) = match decision {
406-
Approved => {
407-
let snippet = Span::from(exec_snippet(&command)).dim();
408-
(
409-
"✔ ".green(),
410-
vec![
411-
"You ".into(),
412-
"approved".bold(),
413-
" codex to run ".into(),
414-
snippet,
415-
" this time".bold(),
416-
],
417-
)
418-
}
419-
ApprovedForSession => {
420-
let snippet = Span::from(exec_snippet(&command)).dim();
421-
(
422-
"✔ ".green(),
423-
vec![
424-
"You ".into(),
425-
"approved".bold(),
426-
" codex to run ".into(),
427-
snippet,
428-
" every time this session".bold(),
429-
],
430-
)
431-
}
432-
Denied => {
433-
let snippet = Span::from(exec_snippet(&command)).dim();
434-
(
435-
"✗ ".red(),
436-
vec![
437-
"You ".into(),
438-
"did not approve".bold(),
439-
" codex to run ".into(),
440-
snippet,
441-
],
442-
)
443-
}
444-
Abort => {
445-
let snippet = Span::from(exec_snippet(&command)).dim();
446-
(
447-
"✗ ".red(),
448-
vec![
449-
"You ".into(),
450-
"canceled".bold(),
451-
" the request to run ".into(),
452-
snippet,
453-
],
454-
)
455-
}
456-
};
457-
458-
let mut lines = Vec::new();
459-
let mut spans = Vec::new();
460-
spans.push(symbol);
461-
spans.extend(summary);
462-
lines.push(Line::from(spans));
463-
Some(lines)
464-
}
465-
466-
fn truncate_exec_snippet(full_cmd: &str) -> String {
467-
let mut snippet = match full_cmd.split_once('\n') {
468-
Some((first, _)) => format!("{first} ..."),
469-
None => full_cmd.to_string(),
470-
};
471-
snippet = truncate_text(&snippet, 80);
472-
snippet
473-
}
474-
475-
fn exec_snippet(command: &[String]) -> String {
476-
let full_cmd = strip_bash_lc_and_escape(command);
477-
truncate_exec_snippet(&full_cmd)
478-
}
479-
480395
#[cfg(test)]
481396
mod tests {
482397
use super::*;
483398
use crate::app_event::AppEvent;
399+
use pretty_assertions::assert_eq;
484400
use tokio::sync::mpsc::unbounded_channel;
485401

486402
fn make_exec_request() -> ApprovalRequest {
@@ -550,6 +466,34 @@ mod tests {
550466
);
551467
}
552468

469+
#[test]
470+
fn exec_history_cell_wraps_with_two_space_indent() {
471+
let command = vec![
472+
"/bin/zsh".into(),
473+
"-lc".into(),
474+
"git add tui/src/render/mod.rs tui/src/render/renderable.rs".into(),
475+
];
476+
let cell = history_cell::new_approval_decision_cell(command, ReviewDecision::Approved);
477+
let lines = cell.display_lines(28);
478+
let rendered: Vec<String> = lines
479+
.iter()
480+
.map(|line| {
481+
line.spans
482+
.iter()
483+
.map(|span| span.content.as_ref())
484+
.collect::<String>()
485+
})
486+
.collect();
487+
let expected = vec![
488+
"✔ You approved codex to".to_string(),
489+
" run /bin/zsh -lc 'git add".to_string(),
490+
" tui/src/render/mod.rs tui/".to_string(),
491+
" src/render/renderable.rs'".to_string(),
492+
" this time".to_string(),
493+
];
494+
assert_eq!(rendered, expected);
495+
}
496+
553497
#[test]
554498
fn enter_sets_last_selected_index_without_dismissing() {
555499
let (tx_raw, mut rx) = unbounded_channel::<AppEvent>();
Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
---
22
source: tui/src/chatwidget/tests.rs
3+
assertion_line: 495
34
expression: lines_to_single_string(&aborted_long)
45
---
5-
You canceled the request to run echo aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...
6+
You canceled the request to run echo
7+
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...

codex-rs/tui/src/history_cell.rs

Lines changed: 146 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,15 @@ use crate::exec_cell::TOOL_CALL_MAX_LINES;
66
use crate::exec_cell::output_lines;
77
use crate::exec_cell::spinner;
88
use crate::exec_command::relativize_to_home;
9+
use crate::exec_command::strip_bash_lc_and_escape;
910
use crate::markdown::MarkdownCitationContext;
1011
use crate::markdown::append_markdown;
1112
use crate::render::line_utils::line_to_static;
1213
use crate::render::line_utils::prefix_lines;
14+
use crate::render::line_utils::push_owned_lines;
1315
use crate::style::user_message_style;
1416
use crate::text_formatting::format_and_truncate_tool_result;
17+
use crate::text_formatting::truncate_text;
1518
use crate::ui_consts::LIVE_PREFIX_COLS;
1619
use crate::wrapping::RtOptions;
1720
use crate::wrapping::word_wrap_line;
@@ -262,6 +265,126 @@ impl HistoryCell for PlainHistoryCell {
262265
}
263266
}
264267

268+
#[derive(Debug)]
269+
pub(crate) struct PrefixedWrappedHistoryCell {
270+
text: Text<'static>,
271+
initial_prefix: Line<'static>,
272+
subsequent_prefix: Line<'static>,
273+
}
274+
275+
impl PrefixedWrappedHistoryCell {
276+
pub(crate) fn new(
277+
text: impl Into<Text<'static>>,
278+
initial_prefix: impl Into<Line<'static>>,
279+
subsequent_prefix: impl Into<Line<'static>>,
280+
) -> Self {
281+
Self {
282+
text: text.into(),
283+
initial_prefix: initial_prefix.into(),
284+
subsequent_prefix: subsequent_prefix.into(),
285+
}
286+
}
287+
}
288+
289+
impl HistoryCell for PrefixedWrappedHistoryCell {
290+
fn display_lines(&self, width: u16) -> Vec<Line<'static>> {
291+
if width == 0 {
292+
return Vec::new();
293+
}
294+
let opts = RtOptions::new(width.max(1) as usize)
295+
.initial_indent(self.initial_prefix.clone())
296+
.subsequent_indent(self.subsequent_prefix.clone());
297+
let wrapped = word_wrap_lines(&self.text, opts);
298+
let mut out = Vec::new();
299+
push_owned_lines(&wrapped, &mut out);
300+
out
301+
}
302+
303+
fn desired_height(&self, width: u16) -> u16 {
304+
self.display_lines(width).len() as u16
305+
}
306+
}
307+
308+
fn truncate_exec_snippet(full_cmd: &str) -> String {
309+
let mut snippet = match full_cmd.split_once('\n') {
310+
Some((first, _)) => format!("{first} ..."),
311+
None => full_cmd.to_string(),
312+
};
313+
snippet = truncate_text(&snippet, 80);
314+
snippet
315+
}
316+
317+
fn exec_snippet(command: &[String]) -> String {
318+
let full_cmd = strip_bash_lc_and_escape(command);
319+
truncate_exec_snippet(&full_cmd)
320+
}
321+
322+
pub fn new_approval_decision_cell(
323+
command: Vec<String>,
324+
decision: codex_core::protocol::ReviewDecision,
325+
) -> Box<dyn HistoryCell> {
326+
use codex_core::protocol::ReviewDecision::*;
327+
328+
let (symbol, summary): (Span<'static>, Vec<Span<'static>>) = match decision {
329+
Approved => {
330+
let snippet = Span::from(exec_snippet(&command)).dim();
331+
(
332+
"✔ ".green(),
333+
vec![
334+
"You ".into(),
335+
"approved".bold(),
336+
" codex to run ".into(),
337+
snippet,
338+
" this time".bold(),
339+
],
340+
)
341+
}
342+
ApprovedForSession => {
343+
let snippet = Span::from(exec_snippet(&command)).dim();
344+
(
345+
"✔ ".green(),
346+
vec![
347+
"You ".into(),
348+
"approved".bold(),
349+
" codex to run ".into(),
350+
snippet,
351+
" every time this session".bold(),
352+
],
353+
)
354+
}
355+
Denied => {
356+
let snippet = Span::from(exec_snippet(&command)).dim();
357+
(
358+
"✗ ".red(),
359+
vec![
360+
"You ".into(),
361+
"did not approve".bold(),
362+
" codex to run ".into(),
363+
snippet,
364+
],
365+
)
366+
}
367+
Abort => {
368+
let snippet = Span::from(exec_snippet(&command)).dim();
369+
(
370+
"✗ ".red(),
371+
vec![
372+
"You ".into(),
373+
"canceled".bold(),
374+
" the request to run ".into(),
375+
snippet,
376+
],
377+
)
378+
}
379+
};
380+
381+
Box::new(PrefixedWrappedHistoryCell::new(
382+
Line::from(summary),
383+
symbol,
384+
" ",
385+
))
386+
}
387+
265388
/// Cyan history cell line showing the current review status.
266389
pub(crate) fn new_review_status_line(message: String) -> PlainHistoryCell {
267390
PlainHistoryCell {
@@ -447,10 +570,6 @@ pub(crate) fn new_user_prompt(message: String) -> UserHistoryCell {
447570
UserHistoryCell { message }
448571
}
449572

450-
pub(crate) fn new_user_approval_decision(lines: Vec<Line<'static>>) -> PlainHistoryCell {
451-
PlainHistoryCell { lines }
452-
}
453-
454573
#[derive(Debug)]
455574
struct SessionHeaderHistoryCell {
456575
version: &'static str,
@@ -1203,6 +1322,29 @@ mod tests {
12031322
assert_eq!(cell.desired_transcript_height(80), 1);
12041323
}
12051324

1325+
#[test]
1326+
fn prefixed_wrapped_history_cell_indents_wrapped_lines() {
1327+
let summary = Line::from(vec![
1328+
"You ".into(),
1329+
"approved".bold(),
1330+
" codex to run ".into(),
1331+
"echo something really long to ensure wrapping happens".dim(),
1332+
" this time".bold(),
1333+
]);
1334+
let cell = PrefixedWrappedHistoryCell::new(summary, "✔ ".green(), " ");
1335+
let rendered = render_lines(&cell.display_lines(24));
1336+
assert_eq!(
1337+
rendered,
1338+
vec![
1339+
"✔ You approved codex".to_string(),
1340+
" to run echo something".to_string(),
1341+
" really long to ensure".to_string(),
1342+
" wrapping happens this".to_string(),
1343+
" time".to_string(),
1344+
]
1345+
);
1346+
}
1347+
12061348
#[test]
12071349
fn active_mcp_tool_call_snapshot() {
12081350
let invocation = McpInvocation {

codex-rs/tui/src/pager_overlay.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -582,13 +582,15 @@ fn render_offset_content(
582582
#[cfg(test)]
583583
mod tests {
584584
use super::*;
585+
use codex_core::protocol::ReviewDecision;
585586
use insta::assert_snapshot;
586587
use std::collections::HashMap;
587588
use std::path::PathBuf;
588589
use std::sync::Arc;
589590
use std::time::Duration;
590591

591592
use crate::exec_cell::CommandOutput;
593+
use crate::history_cell;
592594
use crate::history_cell::HistoryCell;
593595
use crate::history_cell::new_patch_event;
594596
use codex_core::protocol::FileChange;
@@ -712,10 +714,8 @@ mod tests {
712714
cells.push(apply_begin_cell);
713715

714716
let apply_end_cell: Arc<dyn HistoryCell> =
715-
Arc::new(crate::history_cell::new_user_approval_decision(vec![
716-
"✓ Patch applied".green().bold().into(),
717-
"src/foo.txt".dim().into(),
718-
]));
717+
history_cell::new_approval_decision_cell(vec!["ls".into()], ReviewDecision::Approved)
718+
.into();
719719
cells.push(apply_end_cell);
720720

721721
let mut exec_cell = crate::exec_cell::new_active_exec_command(

0 commit comments

Comments
 (0)