mirror of
https://github.com/openai/codex.git
synced 2026-06-01 19:02:59 +00:00
fix(tui): external editor expansion for same-size large pastes (#21190)
## Why We found this while reviewing #21091, but confirmed it is not introduced by that PR: the order-sensitive `current_text_with_pending()` replacement loop already existed, and `main` already allowed active same-size large pastes to use prefix-overlapping labels such as `[Pasted Content N chars]` and `[Pasted Content N chars] #2`. #21091 fixes placeholder numbering after a draft is cleared, so a fresh same-size paste can reuse the base label. This PR fixes a different path: when a draft already contains multiple active same-size large pastes, the placeholders can overlap by prefix, for example `[Pasted Content N chars]` and `[Pasted Content N chars] #2`. That overlap breaks `current_text_with_pending()` when the composer materializes the draft text for the external editor. Replacing the base placeholder first can partially rewrite the `#2` placeholder, leaving the external editor seeded with corrupted text instead of both paste payloads. | Before | After | |---|---| | <img width="1230" height="1008" alt="CleanShot 2026-05-05 at 10 18 09" src="https://github.com/user-attachments/assets/88a2936c-cf00-4adc-8567-8fd8f398b4a8" /> | <img width="1230" height="1008" alt="CleanShot 2026-05-05 at 10 20 31" src="https://github.com/user-attachments/assets/119cff52-43c8-432a-9367-418d82f4ed82" /> | | <img width="1230" height="1008" alt="CleanShot 2026-05-05 at 10 18 57" src="https://github.com/user-attachments/assets/026031bb-839b-4252-a0fd-9ba9616435fe" /> | <img width="1230" height="1008" alt="CleanShot 2026-05-05 at 10 21 31" src="https://github.com/user-attachments/assets/8cb6f2c8-3a5d-411b-8623-dca666ee3c08" /> | ## What Changed - Changed `current_text_with_pending()` to expand pending pastes through the existing element-range based `expand_pending_pastes()` helper instead of global string replacement. - Added a regression test with two different same-length large pastes to ensure both overlapping placeholders expand to their original payloads. ## How to Test 1. Start Codex TUI. 2. Paste a large string, for example 1004 `A` characters. ```shell perl -e 'print "A" x 1004' | pbcopy ``` 3. Paste a second large string with the same length, for example 1004 `B` characters. ```shell perl -e 'print "B" x 1004' | pbcopy ``` 4. Open the external editor from the composer. 5. Confirm the editor is seeded with the full `A...` payload followed by the full `B...` payload, with no literal `#2` left behind. Targeted tests: - `cargo test -p codex-tui current_text_with_pending_expands_overlapping_placeholders` - `just argument-comment-lint-from-source -p codex-tui` I also ran `cargo test -p codex-tui`; it reached the full crate suite but failed two unrelated local status tests because this machine's `/etc/codex/requirements.toml` rejects `DangerFullAccess`.
This commit is contained in:
@@ -1129,12 +1129,13 @@ impl ChatComposer {
|
||||
}
|
||||
|
||||
pub(crate) fn current_text_with_pending(&self) -> String {
|
||||
let mut text = self.current_text();
|
||||
for (placeholder, actual) in &self.pending_pastes {
|
||||
if text.contains(placeholder) {
|
||||
text = text.replace(placeholder, actual);
|
||||
}
|
||||
let text = self.current_text();
|
||||
if self.pending_pastes.is_empty() {
|
||||
return text;
|
||||
}
|
||||
|
||||
let (text, _) =
|
||||
Self::expand_pending_pastes(&text, self.current_text_elements(), &self.pending_pastes);
|
||||
text
|
||||
}
|
||||
|
||||
@@ -10225,6 +10226,33 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn current_text_with_pending_expands_overlapping_placeholders() {
|
||||
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 first_paste = "a".repeat(LARGE_PASTE_CHAR_THRESHOLD + 4);
|
||||
let second_paste = "b".repeat(LARGE_PASTE_CHAR_THRESHOLD + 4);
|
||||
let base = format!("[Pasted Content {} chars]", first_paste.chars().count());
|
||||
let second = format!("{base} #2");
|
||||
|
||||
composer.handle_paste(first_paste.clone());
|
||||
composer.handle_paste(second_paste.clone());
|
||||
|
||||
assert_eq!(composer.current_text(), format!("{base}{second}"));
|
||||
assert_eq!(
|
||||
composer.current_text_with_pending(),
|
||||
format!("{first_paste}{second_paste}")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn apply_external_edit_limits_duplicates_to_occurrences() {
|
||||
let (tx, _rx) = unbounded_channel::<AppEvent>();
|
||||
|
||||
Reference in New Issue
Block a user