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"
/>
This commit is contained in:
Jeremy Rose
2025-10-09 10:37:13 -07:00
committed by GitHub
parent 0308febc23
commit bf82353f45
4 changed files with 184 additions and 96 deletions

View File

@@ -16,7 +16,6 @@ use crate::key_hint::KeyBinding;
use crate::render::highlight::highlight_bash_to_lines;
use crate::render::renderable::ColumnRenderable;
use crate::render::renderable::Renderable;
use crate::text_formatting::truncate_text;
use codex_core::protocol::FileChange;
use codex_core::protocol::Op;
use codex_core::protocol::ReviewDecision;
@@ -160,11 +159,8 @@ impl ApprovalOverlay {
}
fn handle_exec_decision(&self, id: &str, command: &[String], decision: ReviewDecision) {
if let Some(lines) = build_exec_history_lines(command.to_vec(), decision) {
self.app_event_tx.send(AppEvent::InsertHistoryCell(Box::new(
history_cell::new_user_approval_decision(lines),
)));
}
let cell = history_cell::new_approval_decision_cell(command.to_vec(), decision);
self.app_event_tx.send(AppEvent::InsertHistoryCell(cell));
self.app_event_tx.send(AppEvent::CodexOp(Op::ExecApproval {
id: id.to_string(),
decision,
@@ -396,91 +392,11 @@ fn patch_options() -> Vec<ApprovalOption> {
]
}
fn build_exec_history_lines(
command: Vec<String>,
decision: ReviewDecision,
) -> Option<Vec<Line<'static>>> {
use ReviewDecision::*;
let (symbol, summary): (Span<'static>, Vec<Span<'static>>) = match decision {
Approved => {
let snippet = Span::from(exec_snippet(&command)).dim();
(
"".green(),
vec![
"You ".into(),
"approved".bold(),
" codex to run ".into(),
snippet,
" this time".bold(),
],
)
}
ApprovedForSession => {
let snippet = Span::from(exec_snippet(&command)).dim();
(
"".green(),
vec![
"You ".into(),
"approved".bold(),
" codex to run ".into(),
snippet,
" every time this session".bold(),
],
)
}
Denied => {
let snippet = Span::from(exec_snippet(&command)).dim();
(
"".red(),
vec![
"You ".into(),
"did not approve".bold(),
" codex to run ".into(),
snippet,
],
)
}
Abort => {
let snippet = Span::from(exec_snippet(&command)).dim();
(
"".red(),
vec![
"You ".into(),
"canceled".bold(),
" the request to run ".into(),
snippet,
],
)
}
};
let mut lines = Vec::new();
let mut spans = Vec::new();
spans.push(symbol);
spans.extend(summary);
lines.push(Line::from(spans));
Some(lines)
}
fn truncate_exec_snippet(full_cmd: &str) -> String {
let mut snippet = match full_cmd.split_once('\n') {
Some((first, _)) => format!("{first} ..."),
None => full_cmd.to_string(),
};
snippet = truncate_text(&snippet, 80);
snippet
}
fn exec_snippet(command: &[String]) -> String {
let full_cmd = strip_bash_lc_and_escape(command);
truncate_exec_snippet(&full_cmd)
}
#[cfg(test)]
mod tests {
use super::*;
use crate::app_event::AppEvent;
use pretty_assertions::assert_eq;
use tokio::sync::mpsc::unbounded_channel;
fn make_exec_request() -> ApprovalRequest {
@@ -550,6 +466,34 @@ mod tests {
);
}
#[test]
fn exec_history_cell_wraps_with_two_space_indent() {
let command = vec![
"/bin/zsh".into(),
"-lc".into(),
"git add tui/src/render/mod.rs tui/src/render/renderable.rs".into(),
];
let cell = history_cell::new_approval_decision_cell(command, ReviewDecision::Approved);
let lines = cell.display_lines(28);
let rendered: Vec<String> = lines
.iter()
.map(|line| {
line.spans
.iter()
.map(|span| span.content.as_ref())
.collect::<String>()
})
.collect();
let expected = vec![
"✔ You approved codex to".to_string(),
" run /bin/zsh -lc 'git add".to_string(),
" tui/src/render/mod.rs tui/".to_string(),
" src/render/renderable.rs'".to_string(),
" this time".to_string(),
];
assert_eq!(rendered, expected);
}
#[test]
fn enter_sets_last_selected_index_without_dismissing() {
let (tx_raw, mut rx) = unbounded_channel::<AppEvent>();

View File

@@ -1,5 +1,7 @@
---
source: tui/src/chatwidget/tests.rs
assertion_line: 495
expression: lines_to_single_string(&aborted_long)
---
✗ You canceled the request to run echo aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...
✗ You canceled the request to run echo
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...

View File

@@ -6,12 +6,15 @@ use crate::exec_cell::TOOL_CALL_MAX_LINES;
use crate::exec_cell::output_lines;
use crate::exec_cell::spinner;
use crate::exec_command::relativize_to_home;
use crate::exec_command::strip_bash_lc_and_escape;
use crate::markdown::MarkdownCitationContext;
use crate::markdown::append_markdown;
use crate::render::line_utils::line_to_static;
use crate::render::line_utils::prefix_lines;
use crate::render::line_utils::push_owned_lines;
use crate::style::user_message_style;
use crate::text_formatting::format_and_truncate_tool_result;
use crate::text_formatting::truncate_text;
use crate::ui_consts::LIVE_PREFIX_COLS;
use crate::wrapping::RtOptions;
use crate::wrapping::word_wrap_line;
@@ -262,6 +265,126 @@ impl HistoryCell for PlainHistoryCell {
}
}
#[derive(Debug)]
pub(crate) struct PrefixedWrappedHistoryCell {
text: Text<'static>,
initial_prefix: Line<'static>,
subsequent_prefix: Line<'static>,
}
impl PrefixedWrappedHistoryCell {
pub(crate) fn new(
text: impl Into<Text<'static>>,
initial_prefix: impl Into<Line<'static>>,
subsequent_prefix: impl Into<Line<'static>>,
) -> Self {
Self {
text: text.into(),
initial_prefix: initial_prefix.into(),
subsequent_prefix: subsequent_prefix.into(),
}
}
}
impl HistoryCell for PrefixedWrappedHistoryCell {
fn display_lines(&self, width: u16) -> Vec<Line<'static>> {
if width == 0 {
return Vec::new();
}
let opts = RtOptions::new(width.max(1) as usize)
.initial_indent(self.initial_prefix.clone())
.subsequent_indent(self.subsequent_prefix.clone());
let wrapped = word_wrap_lines(&self.text, opts);
let mut out = Vec::new();
push_owned_lines(&wrapped, &mut out);
out
}
fn desired_height(&self, width: u16) -> u16 {
self.display_lines(width).len() as u16
}
}
fn truncate_exec_snippet(full_cmd: &str) -> String {
let mut snippet = match full_cmd.split_once('\n') {
Some((first, _)) => format!("{first} ..."),
None => full_cmd.to_string(),
};
snippet = truncate_text(&snippet, 80);
snippet
}
fn exec_snippet(command: &[String]) -> String {
let full_cmd = strip_bash_lc_and_escape(command);
truncate_exec_snippet(&full_cmd)
}
pub fn new_approval_decision_cell(
command: Vec<String>,
decision: codex_core::protocol::ReviewDecision,
) -> Box<dyn HistoryCell> {
use codex_core::protocol::ReviewDecision::*;
let (symbol, summary): (Span<'static>, Vec<Span<'static>>) = match decision {
Approved => {
let snippet = Span::from(exec_snippet(&command)).dim();
(
"".green(),
vec![
"You ".into(),
"approved".bold(),
" codex to run ".into(),
snippet,
" this time".bold(),
],
)
}
ApprovedForSession => {
let snippet = Span::from(exec_snippet(&command)).dim();
(
"".green(),
vec![
"You ".into(),
"approved".bold(),
" codex to run ".into(),
snippet,
" every time this session".bold(),
],
)
}
Denied => {
let snippet = Span::from(exec_snippet(&command)).dim();
(
"".red(),
vec![
"You ".into(),
"did not approve".bold(),
" codex to run ".into(),
snippet,
],
)
}
Abort => {
let snippet = Span::from(exec_snippet(&command)).dim();
(
"".red(),
vec![
"You ".into(),
"canceled".bold(),
" the request to run ".into(),
snippet,
],
)
}
};
Box::new(PrefixedWrappedHistoryCell::new(
Line::from(summary),
symbol,
" ",
))
}
/// Cyan history cell line showing the current review status.
pub(crate) fn new_review_status_line(message: String) -> PlainHistoryCell {
PlainHistoryCell {
@@ -447,10 +570,6 @@ pub(crate) fn new_user_prompt(message: String) -> UserHistoryCell {
UserHistoryCell { message }
}
pub(crate) fn new_user_approval_decision(lines: Vec<Line<'static>>) -> PlainHistoryCell {
PlainHistoryCell { lines }
}
#[derive(Debug)]
struct SessionHeaderHistoryCell {
version: &'static str,
@@ -1203,6 +1322,29 @@ mod tests {
assert_eq!(cell.desired_transcript_height(80), 1);
}
#[test]
fn prefixed_wrapped_history_cell_indents_wrapped_lines() {
let summary = Line::from(vec![
"You ".into(),
"approved".bold(),
" codex to run ".into(),
"echo something really long to ensure wrapping happens".dim(),
" this time".bold(),
]);
let cell = PrefixedWrappedHistoryCell::new(summary, "".green(), " ");
let rendered = render_lines(&cell.display_lines(24));
assert_eq!(
rendered,
vec![
"✔ You approved codex".to_string(),
" to run echo something".to_string(),
" really long to ensure".to_string(),
" wrapping happens this".to_string(),
" time".to_string(),
]
);
}
#[test]
fn active_mcp_tool_call_snapshot() {
let invocation = McpInvocation {

View File

@@ -582,6 +582,7 @@ fn render_offset_content(
#[cfg(test)]
mod tests {
use super::*;
use codex_core::protocol::ReviewDecision;
use insta::assert_snapshot;
use std::collections::HashMap;
use std::path::PathBuf;
@@ -589,6 +590,7 @@ mod tests {
use std::time::Duration;
use crate::exec_cell::CommandOutput;
use crate::history_cell;
use crate::history_cell::HistoryCell;
use crate::history_cell::new_patch_event;
use codex_core::protocol::FileChange;
@@ -712,10 +714,8 @@ mod tests {
cells.push(apply_begin_cell);
let apply_end_cell: Arc<dyn HistoryCell> =
Arc::new(crate::history_cell::new_user_approval_decision(vec![
"✓ Patch applied".green().bold().into(),
"src/foo.txt".dim().into(),
]));
history_cell::new_approval_decision_cell(vec!["ls".into()], ReviewDecision::Approved)
.into();
cells.push(apply_end_cell);
let mut exec_cell = crate::exec_cell::new_active_exec_command(