[codex] Fix TUI large paste placeholder numbering after Ctrl+C (#21091)

Fixes #19940.

Large-paste placeholder numbering was backed by a per-size counter, so
clearing a draft with `Ctrl+C` left numbering state behind even though
the active pending paste state was gone. This updates the composer to
derive the next placeholder suffix from active pending pastes instead,
which keeps simultaneous same-size pastes distinct while letting fresh
drafts reuse the base label. This is also a small code cleanup: pending
paste state is now the source of truth instead of maintaining a separate
counter.

Credit to @Sungyoun-Kim for the issue report, root-cause notes, and fork
with the proposed fix, and to @charley-oai for the earlier related
#10032 proposal.

Changes:
- Remove the monotonic large-paste counter from the composer.
- Compute suffixes from currently active pending paste placeholders.
- Document large-paste placeholder behavior in the composer module docs.
- Add regression coverage for `Ctrl+C` clearing and deletion/reset
behavior.

Testing:
- `just fmt`
- `git diff --check`
This commit is contained in:
canvrno-oai
2026-05-05 10:33:37 -07:00
committed by GitHub
Unverified
parent af86be529c
commit 1feaa7d85b
+97 -10
View File
@@ -59,6 +59,17 @@
//! Slash commands with arguments (like `/plan` and `/review`) reuse the same preparation path so
//! pasted content and text elements are preserved when extracting args.
//!
//! # Large Paste Placeholders
//!
//! Large pastes insert an element placeholder in the buffer and store the full text in
//! `pending_pastes`. The placeholder label is derived from the pasted character count:
//!
//! - First paste of a given size uses `[Pasted Content N chars]`.
//! - Additional pending pastes of the same size add a numeric suffix (`#2`, `#3`, ...), where the
//! next suffix is computed from the placeholders that still exist in `pending_pastes`.
//! - When all placeholders for a size are cleared or deleted, the next paste of that size reuses
//! the base label without a suffix.
//!
//! # Remote Image Rows (Up/Down/Delete)
//!
//! Remote image URLs are rendered as non-editable `[Image #N]` rows above the textarea (inside the
@@ -338,7 +349,6 @@ pub(crate) struct ChatComposer {
dismissed_file_popup_token: Option<String>,
current_file_query: Option<String>,
pending_pastes: Vec<(String, String)>,
large_paste_counters: HashMap<usize, usize>,
has_focus: bool,
frame_requester: Option<FrameRequester>,
/// Invariant: attached images are labeled in vec order as
@@ -536,7 +546,6 @@ impl ChatComposer {
dismissed_file_popup_token: None,
current_file_query: None,
pending_pastes: Vec::new(),
large_paste_counters: HashMap::new(),
has_focus: has_input_focus,
frame_requester: None,
attached_images: Vec::new(),
@@ -1625,14 +1634,27 @@ impl ChatComposer {
.is_some_and(|expires_at| Instant::now() < expires_at)
}
fn next_large_paste_placeholder(&mut self, char_count: usize) -> String {
fn next_large_paste_placeholder(&self, char_count: usize) -> String {
let base = format!("[Pasted Content {char_count} chars]");
let next_suffix = self.large_paste_counters.entry(char_count).or_insert(0);
*next_suffix += 1;
if *next_suffix == 1 {
let prefix = format!("{base} #");
let mut max_suffix = 0usize;
for (placeholder, _) in &self.pending_pastes {
if placeholder == &base {
max_suffix = max_suffix.max(1);
continue;
}
if let Some(suffix) = placeholder.strip_prefix(&prefix)
&& let Ok(value) = suffix.parse::<usize>()
{
max_suffix = max_suffix.max(value);
}
}
if max_suffix == 0 {
base
} else {
format!("{base} #{next_suffix}")
format!("{base} #{}", max_suffix + 1)
}
}
@@ -5721,6 +5743,35 @@ mod tests {
}
}
#[test]
fn large_paste_numbering_reuses_after_ctrl_c_clear() {
let (tx, _rx) = unbounded_channel::<AppEvent>();
let sender = AppEventSender::new(tx);
let mut composer = ChatComposer::new(
/*has_input_focus*/ true,
sender,
/*enhanced_keys_supported*/ false,
"Ask Codex to do anything".to_string(),
/*disable_paste_burst*/ false,
);
let paste = "x".repeat(LARGE_PASTE_CHAR_THRESHOLD + 4);
let base = format!("[Pasted Content {} chars]", paste.chars().count());
composer.handle_paste(paste.clone());
assert_eq!(composer.textarea.text(), base);
assert_eq!(composer.pending_pastes.len(), 1);
assert_eq!(composer.clear_for_ctrl_c(), Some(base.clone()));
assert!(composer.textarea.text().is_empty());
assert!(composer.pending_pastes.is_empty());
composer.handle_paste(paste);
assert_eq!(composer.textarea.text(), base);
assert_eq!(composer.pending_pastes.len(), 1);
assert_eq!(composer.pending_pastes[0].0, base);
}
#[test]
fn vim_mode_resets_to_normal_after_submission() {
use crossterm::event::KeyCode;
@@ -8539,10 +8590,10 @@ mod tests {
assert_eq!(composer.pending_pastes[0].1, paste);
}
/// Behavior: large-paste placeholder numbering does not get reused after deletion, so a new
/// paste of the same length gets a new unique placeholder label.
/// Behavior: large-paste placeholder numbering continues when another placeholder of the
/// same length still exists, so a new paste gets a new unique placeholder label.
#[test]
fn large_paste_numbering_does_not_reuse_after_deletion() {
fn large_paste_numbering_continues_with_same_length_placeholder() {
use crossterm::event::KeyCode;
use crossterm::event::KeyEvent;
use crossterm::event::KeyModifiers;
@@ -8581,6 +8632,42 @@ mod tests {
assert_eq!(composer.pending_pastes[1].0, third);
}
/// Behavior: if all placeholders of a given length are removed, numbering resets to the
/// base placeholder on the next paste.
#[test]
fn large_paste_numbering_reuses_after_all_deleted() {
use crossterm::event::KeyCode;
use crossterm::event::KeyEvent;
use crossterm::event::KeyModifiers;
let (tx, _rx) = unbounded_channel::<AppEvent>();
let sender = AppEventSender::new(tx);
let mut composer = ChatComposer::new(
/*has_input_focus*/ true,
sender,
/*enhanced_keys_supported*/ false,
"Ask Codex to do anything".to_string(),
/*disable_paste_burst*/ false,
);
let paste = "x".repeat(LARGE_PASTE_CHAR_THRESHOLD + 4);
let base = format!("[Pasted Content {} chars]", paste.chars().count());
composer.handle_paste(paste.clone());
assert_eq!(composer.textarea.text(), base);
assert_eq!(composer.pending_pastes.len(), 1);
composer.textarea.set_cursor(composer.textarea.text().len());
composer.handle_key_event(KeyEvent::new(KeyCode::Backspace, KeyModifiers::NONE));
assert!(composer.textarea.text().is_empty());
assert!(composer.pending_pastes.is_empty());
composer.handle_paste(paste);
assert_eq!(composer.textarea.text(), base);
assert_eq!(composer.pending_pastes.len(), 1);
assert_eq!(composer.pending_pastes[0].0, base);
}
#[test]
fn test_partial_placeholder_deletion() {
use crossterm::event::KeyCode;