mirror of
https://github.com/openai/codex.git
synced 2026-10-19 17:28:46 +00:00
Show hidden output line counts in compact terminal activity (#48761)
## What changed Replace generic output disclosure labels with counts such as `+ 5 lines (ctrl+t to expand)` for compact command activity, both live and in history. Count retained output lines hidden or clipped by the preview, excluding output discarded by storage limits. Use the configured `open_transcript` shortcut and omit the hint when it does not fit or the action is unbound. Invalidate cached layouts when keybindings change. Keep generic `Show details` labels for other hidden details and keep disclosure controls outside copyable and searchable text. ## Testing Add regression tests for live/history consistency, remapped and disabled shortcuts, and exclusion of hints from source text. Add snapshot coverage for fully visible output, hidden and clipped lines, command-only details, and storage-truncated output. GitOrigin-RevId: c468be9b88b69dc3ca1abdb6b7be023244d32a57
This commit is contained in:
@@ -38,8 +38,9 @@ impl ChatWidget {
|
||||
render_compact(cell, width)
|
||||
}
|
||||
});
|
||||
let has_hidden_details =
|
||||
has_activity && active.is_some_and(|cell| cell.has_hidden_activity_details(width));
|
||||
let disclosure = active
|
||||
.filter(|_| has_activity)
|
||||
.and_then(|cell| cell.activity_disclosure(width));
|
||||
let mut auxiliary = self
|
||||
.active_cell_hyperlink_lines_with(width, |cell, width| {
|
||||
if active.is_some_and(|active| std::ptr::eq(active, cell)) {
|
||||
@@ -58,7 +59,7 @@ impl ChatWidget {
|
||||
Some(ActivityTranscriptLines {
|
||||
activity,
|
||||
auxiliary,
|
||||
has_hidden_details,
|
||||
disclosure,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
use super::model::ExecCell;
|
||||
use crate::exec_command::strip_bash_lc_and_escape;
|
||||
use crate::history_cell::ActivityDisclosure;
|
||||
use crate::history_cell::HistoryRenderMode;
|
||||
use crate::history_cell::activity_preview::DETAIL_PREVIEW_LINES;
|
||||
use crate::history_cell::activity_preview::clipped_line;
|
||||
@@ -16,17 +17,33 @@ use ratatui::style::Stylize;
|
||||
use ratatui::text::Line;
|
||||
|
||||
impl ExecCell {
|
||||
pub(super) fn command_has_hidden_details(&self, width: u16) -> bool {
|
||||
pub(super) fn command_disclosure(&self, width: u16) -> Option<ActivityDisclosure> {
|
||||
let [call] = self.group.calls.as_slice() else {
|
||||
return false;
|
||||
return None;
|
||||
};
|
||||
// Interaction output is intentionally absent from the expanded transcript.
|
||||
if !call.is_unified_exec_interaction()
|
||||
&& let Some(output) = &call.output
|
||||
{
|
||||
let retained = output.line_counts().1;
|
||||
let clipped = output
|
||||
.lines()
|
||||
.rev()
|
||||
.take(DETAIL_PREVIEW_LINES)
|
||||
.filter(|raw| ansi_escape_line(raw.as_ref()).width() + 4 > usize::from(width))
|
||||
.count();
|
||||
let hidden = retained.saturating_sub(DETAIL_PREVIEW_LINES) + clipped;
|
||||
if hidden > 0 {
|
||||
return Some(ActivityDisclosure::OutputLines(hidden));
|
||||
}
|
||||
}
|
||||
if !self
|
||||
.group
|
||||
.details
|
||||
.lines_after(/*after_calls*/ 1, width, HistoryRenderMode::Rich)
|
||||
.is_empty()
|
||||
{
|
||||
return true;
|
||||
return Some(ActivityDisclosure::Generic);
|
||||
}
|
||||
let script = strip_bash_lc_and_escape(&call.command);
|
||||
if script.lines().nth(/*n*/ 1).is_some()
|
||||
@@ -35,14 +52,17 @@ impl ExecCell {
|
||||
.first()
|
||||
.is_some_and(|header| header.width() > usize::from(width))
|
||||
{
|
||||
return true;
|
||||
return Some(ActivityDisclosure::Generic);
|
||||
}
|
||||
call.output.as_ref().is_some_and(|output| {
|
||||
output.line_counts().1 > DETAIL_PREVIEW_LINES
|
||||
|| output
|
||||
.transcript_lines()
|
||||
.any(|line| ansi_escape_line(line.as_ref()).width() + 4 > usize::from(width))
|
||||
})
|
||||
call.output
|
||||
.as_ref()
|
||||
.is_some_and(|output| {
|
||||
output.line_counts().1 > DETAIL_PREVIEW_LINES
|
||||
|| output.transcript_lines().any(|line| {
|
||||
ansi_escape_line(line.as_ref()).width() + 4 > usize::from(width)
|
||||
})
|
||||
})
|
||||
.then_some(ActivityDisclosure::Generic)
|
||||
}
|
||||
|
||||
pub(super) fn compact_command_lines(&self, width: u16) -> Vec<HyperlinkLine> {
|
||||
|
||||
@@ -6,6 +6,7 @@ use super::model::CommandOutput;
|
||||
use super::model::ExecCall;
|
||||
use super::model::ExecCell;
|
||||
use crate::exec_command::strip_bash_lc_and_escape;
|
||||
use crate::history_cell::ActivityDisclosure;
|
||||
use crate::history_cell::HistoryCell;
|
||||
use crate::history_cell::HistoryRenderMode;
|
||||
use crate::history_cell::plain_lines;
|
||||
@@ -218,18 +219,19 @@ impl HistoryCell for ExecCell {
|
||||
}
|
||||
}
|
||||
|
||||
fn has_hidden_activity_details(&self, width: u16) -> bool {
|
||||
fn activity_disclosure(&self, width: u16) -> Option<ActivityDisclosure> {
|
||||
if self.is_exploring_cell() {
|
||||
return true;
|
||||
return Some(ActivityDisclosure::Generic);
|
||||
}
|
||||
if self.group.calls.iter().any(ExecCall::is_user_shell_command) {
|
||||
// User-shell previews already retain wrapped commands and up to fifty output rows.
|
||||
// Reuse its actual truncation decision instead of comparing differently styled text.
|
||||
return self
|
||||
.command_display_lines_with_hidden_details(width)
|
||||
.hidden_details;
|
||||
.hidden_details
|
||||
.then_some(ActivityDisclosure::Generic);
|
||||
}
|
||||
self.command_has_hidden_details(width)
|
||||
self.command_disclosure(width)
|
||||
}
|
||||
|
||||
fn compact_hyperlink_lines(&self, width: u16) -> Vec<HyperlinkLine> {
|
||||
|
||||
@@ -6,6 +6,14 @@ use ratatui::text::Line;
|
||||
|
||||
pub(crate) const DETAIL_PREVIEW_LINES: usize = 3;
|
||||
|
||||
/// Details an activity can reveal without materializing its expanded transcript.
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
|
||||
pub(crate) enum ActivityDisclosure {
|
||||
Generic,
|
||||
/// Retained output lines not fully visible in the preview, excluding storage omissions.
|
||||
OutputLines(usize),
|
||||
}
|
||||
|
||||
/// Clip a preview row without teaching selection to copy text that is currently hidden.
|
||||
pub(crate) fn clipped_line(line: Line<'static>, width: u16) -> HyperlinkLine {
|
||||
truncate_line_with_ellipsis_if_overflow(line, usize::from(width)).into()
|
||||
|
||||
@@ -123,6 +123,7 @@ mod startup_warnings;
|
||||
mod warnings;
|
||||
|
||||
pub(crate) use activity_details::ActivityDetails;
|
||||
pub(crate) use activity_preview::ActivityDisclosure;
|
||||
pub(crate) use approvals::*;
|
||||
pub(crate) use base::*;
|
||||
pub(crate) use dynamic::DynamicToolCallCell;
|
||||
@@ -254,10 +255,10 @@ pub(crate) trait HistoryCell: std::fmt::Debug + Send + Sync + Any {
|
||||
self.transcript_hyperlink_lines(width)
|
||||
}
|
||||
|
||||
/// Whether an activity offers details beyond its compact presentation.
|
||||
/// Details an activity offers beyond its compact presentation, if any.
|
||||
/// Prefer source metadata so collapsed rendering does not materialize hidden content.
|
||||
fn has_hidden_activity_details(&self, _width: u16) -> bool {
|
||||
true
|
||||
fn activity_disclosure(&self, _width: u16) -> Option<ActivityDisclosure> {
|
||||
Some(ActivityDisclosure::Generic)
|
||||
}
|
||||
|
||||
fn display_lines_for_mode(&self, width: u16, mode: HistoryRenderMode) -> Vec<Line<'static>> {
|
||||
|
||||
@@ -207,8 +207,9 @@ impl HistoryCell for PlanUpdateCell {
|
||||
vec![self.activity_id.clone()]
|
||||
}
|
||||
|
||||
fn has_hidden_activity_details(&self, width: u16) -> bool {
|
||||
self.explanation
|
||||
fn activity_disclosure(&self, width: u16) -> Option<super::ActivityDisclosure> {
|
||||
(self
|
||||
.explanation
|
||||
.as_ref()
|
||||
.is_some_and(|explanation| !explanation.trim().is_empty())
|
||||
|| self.plan.len() > super::activity_preview::DETAIL_PREVIEW_LINES
|
||||
@@ -216,7 +217,8 @@ impl HistoryCell for PlanUpdateCell {
|
||||
let step = item.step.split_whitespace().collect::<Vec<_>>().join(" ");
|
||||
// The compact row reserves four columns for indentation and two for its marker.
|
||||
Line::from(step).width() + 6 > usize::from(width)
|
||||
})
|
||||
}))
|
||||
.then_some(super::ActivityDisclosure::Generic)
|
||||
}
|
||||
|
||||
fn compact_hyperlink_lines(&self, width: u16) -> Vec<HyperlinkLine> {
|
||||
|
||||
+33
@@ -0,0 +1,33 @@
|
||||
---
|
||||
source: tui/src/transcript_view_tests.rs
|
||||
expression: "frames.join(\"\\n\\n\")"
|
||||
---
|
||||
three visible
|
||||
• Ran echo
|
||||
└ a
|
||||
b
|
||||
c
|
||||
|
||||
one hidden
|
||||
• Ran echo
|
||||
└ b
|
||||
c
|
||||
d
|
||||
+ 1 line (ctrl+t to expand)
|
||||
|
||||
clipped line
|
||||
• Ran echo
|
||||
└ abcdefghijklmno…
|
||||
+ 1 line
|
||||
|
||||
command only
|
||||
• Ran echo a …
|
||||
└ (no output)
|
||||
+ Show details
|
||||
|
||||
storage truncated
|
||||
• Running echo
|
||||
└ x
|
||||
x
|
||||
x
|
||||
+ 97 lines (ctrl+t to expand)
|
||||
@@ -287,8 +287,12 @@ impl TranscriptView {
|
||||
expanded: bool,
|
||||
lines: impl FnOnce(u16) -> Option<ActivityTranscriptLines>,
|
||||
) -> bool {
|
||||
let shortcut = self
|
||||
.disclosure
|
||||
.keymap
|
||||
.primary_hint(crate::keymap::KeymapContext::Global, "open_transcript");
|
||||
self.sync_live_layout(width, key, |width| {
|
||||
lines(width).map(|lines| layout::activity_layout(lines, width, expanded))
|
||||
lines(width).map(|lines| layout::activity_layout(lines, width, expanded, shortcut))
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -414,7 +414,7 @@ fn activity_focus_keeps_controls_without_passive_hints() {
|
||||
cell.compact_hyperlink_lines(width)
|
||||
},
|
||||
auxiliary: Vec::new(),
|
||||
has_hidden_details: true,
|
||||
disclosure: cell.activity_disclosure(width),
|
||||
})
|
||||
});
|
||||
render(view, &[])
|
||||
|
||||
@@ -3,6 +3,9 @@
|
||||
//! Mutable cells refresh each frame, then share that frame's layout across measurement and paint.
|
||||
//! Stable cells also invalidate when animation ticks, syntax themes, or terminal colors change.
|
||||
|
||||
use crate::history_cell::ActivityDisclosure;
|
||||
use crate::key_hint::ShortcutHint;
|
||||
use crate::keymap::KeymapContext;
|
||||
use std::sync::Weak;
|
||||
|
||||
use super::*;
|
||||
@@ -14,7 +17,7 @@ const MAX_CACHED_TEXT_BYTES: usize = 8 * 1024 * 1024;
|
||||
pub(crate) struct ActivityTranscriptLines {
|
||||
pub(crate) activity: Vec<HyperlinkLine>,
|
||||
pub(crate) auxiliary: Vec<HyperlinkLine>,
|
||||
pub(crate) has_hidden_details: bool,
|
||||
pub(crate) disclosure: Option<ActivityDisclosure>,
|
||||
}
|
||||
|
||||
/// Keep disclosure controls outside selectable source without rendering hidden details.
|
||||
@@ -22,8 +25,9 @@ pub(super) fn activity_layout(
|
||||
mut lines: ActivityTranscriptLines,
|
||||
width: u16,
|
||||
expanded: bool,
|
||||
shortcut: Option<ShortcutHint>,
|
||||
) -> TextLayout {
|
||||
let has_details = !lines.activity.is_empty() && (expanded || lines.has_hidden_details);
|
||||
let has_details = !lines.activity.is_empty() && (expanded || lines.disclosure.is_some());
|
||||
let source_offset = (!lines.auxiliary.is_empty())
|
||||
.then(|| TextLayout::new(lines.activity.clone(), width).text().len());
|
||||
lines.activity.extend(lines.auxiliary);
|
||||
@@ -31,12 +35,22 @@ pub(super) fn activity_layout(
|
||||
if !has_details {
|
||||
return layout;
|
||||
}
|
||||
let label = if expanded {
|
||||
"− Show less"
|
||||
} else {
|
||||
"+ Show details"
|
||||
}
|
||||
.to_owned();
|
||||
let label = match (expanded, lines.disclosure) {
|
||||
(true, _) => "− Show less".to_owned(),
|
||||
(false, Some(ActivityDisclosure::OutputLines(count))) => {
|
||||
let noun = if count == 1 { "line" } else { "lines" };
|
||||
let mut label = format!("+ {count} {noun}");
|
||||
if let Some(shortcut) = shortcut {
|
||||
let hint = format!(" ({} to expand)", shortcut.display_label());
|
||||
let indent = usize::from(width / 4).min(/*other*/ 4);
|
||||
if Line::from(format!("{label}{hint}")).width() + indent <= usize::from(width) {
|
||||
label.push_str(&hint);
|
||||
}
|
||||
}
|
||||
label
|
||||
}
|
||||
(false, Some(ActivityDisclosure::Generic) | None) => "+ Show details".to_owned(),
|
||||
};
|
||||
match source_offset {
|
||||
Some(offset) => layout.with_disclosure_control_at(label, offset),
|
||||
None => layout.with_disclosure_control(label),
|
||||
@@ -141,6 +155,10 @@ impl TranscriptView {
|
||||
expanded,
|
||||
disclosure,
|
||||
};
|
||||
let shortcut = self
|
||||
.disclosure
|
||||
.keymap
|
||||
.primary_hint(KeymapContext::Global, "open_transcript");
|
||||
Some(self.cache.get(cell, width, presentation, || {
|
||||
if disclosure {
|
||||
activity_layout(
|
||||
@@ -151,10 +169,11 @@ impl TranscriptView {
|
||||
cell.compact_hyperlink_lines(width)
|
||||
},
|
||||
auxiliary: Vec::new(),
|
||||
has_hidden_details: cell.has_hidden_activity_details(width),
|
||||
disclosure: cell.activity_disclosure(width),
|
||||
},
|
||||
width,
|
||||
expanded,
|
||||
shortcut,
|
||||
)
|
||||
} else {
|
||||
TextLayout::new(
|
||||
|
||||
@@ -111,6 +111,8 @@ impl TranscriptView {
|
||||
}
|
||||
self.search.editor.set_keymap_bindings(keymap);
|
||||
self.disclosure.keymap = keymap.clone();
|
||||
self.cache.clear();
|
||||
self.live_key = None;
|
||||
}
|
||||
|
||||
pub(crate) fn is_search_active(&self) -> bool {
|
||||
|
||||
@@ -63,6 +63,143 @@ pub(super) fn text(buffer: &Buffer) -> String {
|
||||
.join("\n")
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn terminal_output_disclosure_follows_live_history_and_keymap() {
|
||||
use crate::exec_cell::CommandOutput;
|
||||
use crate::exec_cell::new_active_exec_command;
|
||||
use crate::keymap::RuntimeKeymap;
|
||||
use codex_app_server_protocol::CommandExecutionSource;
|
||||
|
||||
let mut cell = new_active_exec_command(
|
||||
"output-test".into(),
|
||||
vec!["echo".into()],
|
||||
Vec::new(),
|
||||
CommandExecutionSource::Agent,
|
||||
/*interaction_input*/ None,
|
||||
/*animations_enabled*/ false,
|
||||
);
|
||||
cell.complete_call(
|
||||
"output-test",
|
||||
CommandOutput::new(/*exit_code*/ 0, "1\n2\n3\n4\n5\n6\n7\n8\n".into()),
|
||||
std::time::Duration::ZERO,
|
||||
);
|
||||
let cell: Arc<dyn HistoryCell> = Arc::new(cell);
|
||||
let mut view = TranscriptView::default();
|
||||
view.sync_live_activity(&[], cell.activity_ids());
|
||||
view.sync_live_activity_tail(
|
||||
/*width*/ 72,
|
||||
/*key*/ None,
|
||||
/*expanded*/ false,
|
||||
|width| {
|
||||
Some(ActivityTranscriptLines {
|
||||
activity: cell.compact_hyperlink_lines(width),
|
||||
auxiliary: Vec::new(),
|
||||
disclosure: cell.activity_disclosure(width),
|
||||
})
|
||||
},
|
||||
);
|
||||
let live = text(&render(
|
||||
&mut view,
|
||||
&[],
|
||||
/*width*/ 72,
|
||||
/*height*/ 6,
|
||||
));
|
||||
let cells = [cell];
|
||||
view.sync_live_activity(&cells, Vec::new());
|
||||
view.sync_live_tail(/*width*/ 72, /*key*/ None, |_| None);
|
||||
let committed = text(&render(
|
||||
&mut view, &cells, /*width*/ 72, /*height*/ 6,
|
||||
));
|
||||
assert_eq!(live, committed);
|
||||
let hint = " (ctrl+t to expand)";
|
||||
assert!(live.contains(&format!("+ 5 lines{hint}")));
|
||||
// Check where the hint is visible, so the copy/search exclusion cannot pass vacuously.
|
||||
assert!(
|
||||
!view
|
||||
.layout(&cells, /*index*/ 0)
|
||||
.unwrap()
|
||||
.text()
|
||||
.contains(hint)
|
||||
);
|
||||
// Warm layouts must follow config changes, including chords and disabling the action.
|
||||
for (configured, expected) in [
|
||||
(serde_json::json!("f12"), " (f12 to expand)"),
|
||||
(serde_json::json!("ctrl-x t"), " (ctrl+x t to expand)"),
|
||||
(serde_json::json!([]), ""),
|
||||
] {
|
||||
let config = serde_json::from_value(serde_json::json!({
|
||||
"global": {"open_transcript": configured}
|
||||
}))
|
||||
.unwrap();
|
||||
let keymap = RuntimeKeymap::from_config(&config).unwrap();
|
||||
view.set_keymap_bindings(&keymap);
|
||||
assert_eq!(
|
||||
text(&render(
|
||||
&mut view, &cells, /*width*/ 72, /*height*/ 6,
|
||||
)),
|
||||
live.replace(hint, expected),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn terminal_output_disclosure_counts_only_revealable_lines() {
|
||||
use crate::exec_cell::CommandOutput;
|
||||
use crate::exec_cell::new_active_exec_command;
|
||||
use codex_app_server_protocol::CommandExecutionSource;
|
||||
|
||||
let mut frames = Vec::new();
|
||||
for (name, command, output, width, streamed) in [
|
||||
("three visible", "echo", "a\nb\nc\n".to_owned(), 64, false),
|
||||
("one hidden", "echo", "a\nb\nc\nd\n".to_owned(), 64, false),
|
||||
(
|
||||
"clipped line",
|
||||
"echo",
|
||||
"abcdefghijklmnopqrstuvwxyz\n".to_owned(),
|
||||
20,
|
||||
false,
|
||||
),
|
||||
("command only", "echo a\necho b", String::new(), 64, false),
|
||||
(
|
||||
"storage truncated",
|
||||
"echo",
|
||||
"x\n".repeat(/*n*/ 530_000),
|
||||
64,
|
||||
true,
|
||||
),
|
||||
] {
|
||||
let mut cell = new_active_exec_command(
|
||||
"output-test".into(),
|
||||
vec!["sh".into(), "-c".into(), command.into()],
|
||||
Vec::new(),
|
||||
CommandExecutionSource::Agent,
|
||||
/*interaction_input*/ None,
|
||||
/*animations_enabled*/ false,
|
||||
);
|
||||
if streamed {
|
||||
cell.append_output("output-test", &output);
|
||||
} else {
|
||||
cell.complete_call(
|
||||
"output-test",
|
||||
CommandOutput::new(/*exit_code*/ 0, output),
|
||||
std::time::Duration::from_secs(/*secs*/ 1),
|
||||
);
|
||||
}
|
||||
let cells: [Arc<dyn HistoryCell>; 1] = [Arc::new(cell)];
|
||||
frames.push(format!(
|
||||
"{name}\n{}",
|
||||
text(&render(
|
||||
&mut TranscriptView::default(),
|
||||
&cells,
|
||||
width,
|
||||
/*height*/ 6,
|
||||
))
|
||||
.trim_end()
|
||||
));
|
||||
}
|
||||
insta::assert_snapshot!(frames.join("\n\n"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn startup_warning_keeps_first_visible_header_in_place() {
|
||||
let header: Arc<dyn HistoryCell> =
|
||||
|
||||
Reference in New Issue
Block a user