mirror of
https://github.com/openai/codex.git
synced 2026-10-19 17:28:46 +00:00
Respect rendered width when adding TUI history (#38075)
## What changed - Initialize new chat widgets with the current terminal width. - Determine whether an added history cell is visible using the active history render mode and the width available after ambient-pet reservations. - Saturate diff-summary width calculations so extremely narrow terminals remain renderable. ## Testing - Cover width-sensitive history insertion with an ambient pet, raw-output mode, and a diff summary narrower than its indentation. GitOrigin-RevId: 82d20900e0637431be7646b4f4aba61810ff74c1
This commit is contained in:
committed by
copyberry
parent
a817d9424d
commit
33aaf91366
@@ -1019,6 +1019,7 @@ impl App {
|
||||
(ChatWidget::new_with_app_event(init), Some(forked))
|
||||
}
|
||||
};
|
||||
chat_widget.note_rendered_width(tui.terminal.last_known_screen_size.width);
|
||||
chat_widget.remote_connection = remote_connection;
|
||||
let thread_and_widget_ms = thread_and_widget_started_at.elapsed().as_millis();
|
||||
chat_widget
|
||||
|
||||
@@ -533,6 +533,8 @@ impl App {
|
||||
/*initial_user_message*/ None,
|
||||
);
|
||||
self.replace_chat_widget(ChatWidget::new_with_app_event(init));
|
||||
self.chat_widget
|
||||
.note_rendered_width(tui.terminal.last_known_screen_size.width);
|
||||
if blocks_direct_input {
|
||||
self.chat_widget.set_parent_owned_thread();
|
||||
}
|
||||
@@ -754,6 +756,8 @@ impl App {
|
||||
initial_user_message,
|
||||
);
|
||||
self.replace_chat_widget(ChatWidget::new_with_app_event(init));
|
||||
self.chat_widget
|
||||
.note_rendered_width(tui.terminal.last_known_screen_size.width);
|
||||
if started.blocks_direct_input {
|
||||
self.mark_primary_thread_parent_owned(started.session.thread_id);
|
||||
}
|
||||
|
||||
@@ -705,7 +705,7 @@ pub(crate) struct ChatWidget {
|
||||
quit_shortcut_key: Option<KeyBinding>,
|
||||
// Runtime metrics accumulated across delta snapshots for the active turn.
|
||||
turn_runtime_metrics: RuntimeMetricsSummary,
|
||||
last_rendered_width: std::cell::Cell<Option<usize>>,
|
||||
last_rendered_width: std::cell::Cell<Option<u16>>,
|
||||
// Feedback sink for /feedback
|
||||
feedback: codex_feedback::CodexFeedback,
|
||||
// Current session rollout path (if known)
|
||||
@@ -1239,8 +1239,17 @@ impl ChatWidget {
|
||||
.active_cell
|
||||
.as_ref()
|
||||
.is_some_and(|c| c.as_any().is::<history_cell::SessionHeaderHistoryCell>());
|
||||
let history_width = self
|
||||
.last_rendered_width
|
||||
.get()
|
||||
.map(|width| self.history_wrap_width(width))
|
||||
.unwrap_or(u16::MAX);
|
||||
|
||||
if !keep_placeholder_header_active && !cell.display_lines(u16::MAX).is_empty() {
|
||||
if !keep_placeholder_header_active
|
||||
&& !cell
|
||||
.display_lines_for_mode(history_width, self.history_render_mode())
|
||||
.is_empty()
|
||||
{
|
||||
// Only break exec grouping if the cell renders visible lines.
|
||||
if !self.has_active_stream_tail() {
|
||||
self.flush_active_cell();
|
||||
@@ -1583,7 +1592,6 @@ impl ChatWidget {
|
||||
if width == 0 {
|
||||
None
|
||||
} else {
|
||||
let width = u16::try_from(width).unwrap_or(u16::MAX);
|
||||
let width = usize::from(self.history_wrap_width(width));
|
||||
Some(crate::width::usable_content_width(width, reserved_cols).unwrap_or(1))
|
||||
}
|
||||
@@ -1643,7 +1651,7 @@ impl ChatWidget {
|
||||
/// rebuilding runs through app-level resize reflow.
|
||||
pub(crate) fn on_terminal_resize(&mut self, width: u16) {
|
||||
let had_rendered_width = self.last_rendered_width.get().is_some();
|
||||
self.last_rendered_width.set(Some(width as usize));
|
||||
self.last_rendered_width.set(Some(width));
|
||||
let stream_width = self.current_stream_width(/*reserved_cols*/ 2);
|
||||
let plan_stream_width = self.current_stream_width(/*reserved_cols*/ 4);
|
||||
if let Some(controller) = self.stream_controller.as_mut() {
|
||||
|
||||
@@ -58,7 +58,7 @@ impl ChatWidget {
|
||||
}
|
||||
|
||||
pub(crate) fn note_rendered_width(&self, width: u16) {
|
||||
self.last_rendered_width.set(Some(width as usize));
|
||||
self.last_rendered_width.set(Some(width));
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -8,14 +8,10 @@ use super::*;
|
||||
impl ChatWidget {
|
||||
pub(super) fn open_theme_picker(&mut self) {
|
||||
let codex_home = codex_utils_home_dir::find_codex_home().ok();
|
||||
let terminal_width = self
|
||||
.last_rendered_width
|
||||
.get()
|
||||
.and_then(|width| u16::try_from(width).ok());
|
||||
let params = crate::theme_picker::build_theme_picker_params(
|
||||
self.config.tui_theme.as_deref(),
|
||||
codex_home.as_deref(),
|
||||
terminal_width,
|
||||
self.last_rendered_width.get(),
|
||||
);
|
||||
self.bottom_pane.show_selection_view(params);
|
||||
}
|
||||
|
||||
@@ -2409,6 +2409,61 @@ async fn ambient_pet_can_be_disabled() {
|
||||
assert!(chat.ambient_pet.is_none());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn added_history_uses_pet_adjusted_terminal_width() {
|
||||
#[derive(Debug)]
|
||||
struct WidthCell(std::sync::Arc<std::sync::atomic::AtomicU16>);
|
||||
|
||||
impl HistoryCell for WidthCell {
|
||||
fn display_lines(&self, width: u16) -> Vec<Line<'static>> {
|
||||
self.0.store(width, std::sync::atomic::Ordering::Relaxed);
|
||||
if width == u16::MAX {
|
||||
Vec::new()
|
||||
} else {
|
||||
vec!["width-sensitive history".into()]
|
||||
}
|
||||
}
|
||||
|
||||
fn raw_lines(&self) -> Vec<Line<'static>> {
|
||||
Vec::new()
|
||||
}
|
||||
}
|
||||
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
enable_test_ambient_pet(&mut chat);
|
||||
chat.last_rendered_width.set(Some(80));
|
||||
set_active_cell(
|
||||
&mut chat,
|
||||
Box::new(PlainHistoryCell::new(vec!["active response".into()])),
|
||||
);
|
||||
let width = std::sync::Arc::new(std::sync::atomic::AtomicU16::new(0));
|
||||
|
||||
chat.add_to_history(WidthCell(std::sync::Arc::clone(&width)));
|
||||
|
||||
assert_eq!(width.load(std::sync::atomic::Ordering::Relaxed), 69);
|
||||
assert!(chat.transcript.needs_final_message_separator);
|
||||
let backend = VT100Backend::new(/*width*/ 80, /*height*/ 4);
|
||||
let mut terminal = crate::custom_terminal::Terminal::with_options(backend).expect("terminal");
|
||||
terminal.set_viewport_area(Rect::new(
|
||||
/*x*/ 0, /*y*/ 3, /*width*/ 80, /*height*/ 1,
|
||||
));
|
||||
for lines in drain_insert_history(&mut rx) {
|
||||
crate::insert_history::insert_history_lines(&mut terminal, lines)
|
||||
.expect("insert history lines");
|
||||
}
|
||||
insta::assert_snapshot!(terminal.backend().vt100().screen().contents().trim(), @r"
|
||||
active response
|
||||
|
||||
width-sensitive history
|
||||
");
|
||||
|
||||
width.store(0, std::sync::atomic::Ordering::Relaxed);
|
||||
chat.set_raw_output_mode(/*enabled*/ true);
|
||||
chat.last_rendered_width.set(Some(12));
|
||||
chat.add_to_history(WidthCell(std::sync::Arc::clone(&width)));
|
||||
assert_eq!(width.load(std::sync::atomic::Ordering::Relaxed), 0);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[serial]
|
||||
async fn ambient_pet_reserves_history_wrap_width() {
|
||||
|
||||
@@ -464,8 +464,10 @@ fn render_changes_block(rows: Vec<Row<'_>>, wrap_cols: usize, cwd: &Path) -> Vec
|
||||
let lang_path = r.move_path.unwrap_or(r.path);
|
||||
let lang = detect_lang_for_path(lang_path);
|
||||
let mut lines = vec![];
|
||||
render_change(r.change, &mut lines, wrap_cols - 4, lang.as_deref());
|
||||
out.extend(prefix_lines(lines, " ".into(), " ".into()));
|
||||
let prefix = " ";
|
||||
let content_width = wrap_cols.saturating_sub(prefix.len());
|
||||
render_change(r.change, &mut lines, content_width, lang.as_deref());
|
||||
out.extend(prefix_lines(lines, prefix.into(), prefix.into()));
|
||||
}
|
||||
|
||||
out
|
||||
@@ -1631,6 +1633,12 @@ mod tests {
|
||||
/*width*/ 80,
|
||||
/*height*/ 10,
|
||||
);
|
||||
|
||||
let narrow = create_diff_summary(&changes, &PathBuf::from("/"), /*wrap_cols*/ 3);
|
||||
assert_snapshot!(format!("{}\n{}", narrow[1], narrow[2]), @r"
|
||||
1 +a
|
||||
l
|
||||
");
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user