tui: preserve kill buffer across submit and slash-command clears (#12006)
## Problem Before this change, composer paths that cleared the textarea after submit or slash-command dispatch also cleared the textarea kill buffer. That meant a user could `Ctrl+K` part of a draft, trigger a composer action that cleared the visible draft, and then lose the ability to `Ctrl+Y` the killed text back. This was especially awkward for workflows where the user wants to temporarily remove text, run a composer action such as changing reasoning level or dispatching a slash command, and then restore the killed text into the now-empty draft. ## Mental model This change separates visible draft state from editing-history state. The visible draft includes the current textarea contents and text elements that should be cleared when the composer submits or dispatches a command. The kill buffer is different: it represents the most recent killed text and should survive those composer-driven clears so the user can still yank it back afterward. After this change, submit and slash-command dispatch still clear the visible textarea contents, but they no longer erase the most recent kill. ## Non-goals This does not implement a multi-entry kill ring or change the semantics of `Ctrl+K` and `Ctrl+Y` beyond preserving the existing yank target across these clears. It also does not change how submit, slash-command parsing, prompt expansion, or attachment handling work, except that those flows no longer discard the textarea kill buffer as a side effect of clearing the draft. ## Tradeoffs The main tradeoff is that clearing the visible textarea is no longer equivalent to fully resetting all editing state. That is intentional here, because submit and slash-command dispatch are composer actions, not requests to forget the user's most recent kill. The benefit is better editing continuity. The cost is that callers must understand that full-buffer replacement resets visible draft state but not the kill buffer. ## Architecture The behavioral change is in `TextArea`: full-buffer replacement now rebuilds text and elements without clearing `kill_buffer`. `ChatComposer` already clears the textarea after successful submit and slash-command dispatch by calling into those textarea replacement paths. With this change, those existing composer flows inherit the new behavior automatically: the visible draft is cleared, but the last killed text remains available for `Ctrl+Y`. The tests cover both layers: - `TextArea` verifies that the kill buffer survives full-buffer replacement. - `ChatComposer` verifies that it survives submit. - `ChatComposer` also verifies that it survives slash-command dispatch. ## Observability There is no dedicated logging for kill-buffer preservation. The most direct way to reason about the behavior is to inspect textarea-wide replacement paths and confirm whether they treat the kill buffer as visible-buffer state or as editing-history state. If this regresses in the future, the likely failure mode is simple and user-visible: `Ctrl+Y` stops restoring text after submit or slash-command clears even though ordinary kill/yank still works within a single uninterrupted draft. ## Tests Added focused regression coverage for the new contract: - `kill_buffer_persists_across_set_text` - `kill_buffer_persists_after_submit` - `kill_buffer_persists_after_slash_command_dispatch` Local verification: - `just fmt` - `cargo test -p codex-tui` --------- Co-authored-by: Josh McKinney <joshka@openai.com>
This commit is contained in:
rakan-oai
GitHub
parent
0bb152b01d
commit
56cc2c71f4
3 changed files with 142 additions and 3 deletions
|
|
@ -43,6 +43,11 @@
|
|||
//! - Prunes local attached images so only placeholders that survive expansion are sent.
|
||||
//! - Preserves remote image URLs as separate attachments even when text is empty.
|
||||
//!
|
||||
//! When these paths clear the visible textarea after a successful submit or slash-command
|
||||
//! dispatch, they intentionally preserve the textarea kill buffer. That lets users `Ctrl+K` part
|
||||
//! of a draft, perform a composer action such as changing reasoning level, and then `Ctrl+Y` the
|
||||
//! killed text back into the now-empty draft.
|
||||
//!
|
||||
//! The numeric auto-submit path used by the slash popup performs the same pending-paste expansion
|
||||
//! and attachment pruning, and clears pending paste state on success.
|
||||
//! Slash commands with arguments (like `/plan` and `/review`) reuse the same preparation path so
|
||||
|
|
@ -6388,6 +6393,78 @@ mod tests {
|
|||
assert!(composer.textarea.is_empty(), "composer should be cleared");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn kill_buffer_persists_after_submit() {
|
||||
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(
|
||||
true,
|
||||
sender,
|
||||
false,
|
||||
"Ask Codex to do anything".to_string(),
|
||||
false,
|
||||
);
|
||||
composer.set_steer_enabled(true);
|
||||
composer.textarea.insert_str("restore me");
|
||||
composer.textarea.set_cursor(0);
|
||||
|
||||
let (_result, _needs_redraw) =
|
||||
composer.handle_key_event(KeyEvent::new(KeyCode::Char('k'), KeyModifiers::CONTROL));
|
||||
assert!(composer.textarea.is_empty());
|
||||
|
||||
composer.textarea.insert_str("hello");
|
||||
let (result, _needs_redraw) =
|
||||
composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
|
||||
assert!(matches!(result, InputResult::Submitted { .. }));
|
||||
assert!(composer.textarea.is_empty());
|
||||
|
||||
let (_result, _needs_redraw) =
|
||||
composer.handle_key_event(KeyEvent::new(KeyCode::Char('y'), KeyModifiers::CONTROL));
|
||||
assert_eq!(composer.textarea.text(), "restore me");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn kill_buffer_persists_after_slash_command_dispatch() {
|
||||
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(
|
||||
true,
|
||||
sender,
|
||||
false,
|
||||
"Ask Codex to do anything".to_string(),
|
||||
false,
|
||||
);
|
||||
composer.textarea.insert_str("restore me");
|
||||
composer.textarea.set_cursor(0);
|
||||
|
||||
let (_result, _needs_redraw) =
|
||||
composer.handle_key_event(KeyEvent::new(KeyCode::Char('k'), KeyModifiers::CONTROL));
|
||||
assert!(composer.textarea.is_empty());
|
||||
|
||||
composer.textarea.insert_str("/diff");
|
||||
let (result, _needs_redraw) =
|
||||
composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
|
||||
match result {
|
||||
InputResult::Command(cmd) => {
|
||||
assert_eq!(cmd.command(), "diff");
|
||||
}
|
||||
_ => panic!("expected Command result for '/diff'"),
|
||||
}
|
||||
assert!(composer.textarea.is_empty());
|
||||
|
||||
let (_result, _needs_redraw) =
|
||||
composer.handle_key_event(KeyEvent::new(KeyCode::Char('y'), KeyModifiers::CONTROL));
|
||||
assert_eq!(composer.textarea.text(), "restore me");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn slash_command_disabled_while_task_running_keeps_text() {
|
||||
use crossterm::event::KeyCode;
|
||||
|
|
|
|||
|
|
@ -1,3 +1,15 @@
|
|||
//! The textarea owns editable composer text, placeholder elements, cursor/wrap state, and a
|
||||
//! single-entry kill buffer.
|
||||
//!
|
||||
//! Whole-buffer replacement APIs intentionally rebuild only the visible draft state. They clear
|
||||
//! element ranges and derived cursor/wrapping caches, but they keep the kill buffer intact so a
|
||||
//! caller can clear or rewrite the draft and still allow `Ctrl+Y` to restore the user's most
|
||||
//! recent `Ctrl+K`. This is the contract higher-level composer flows rely on after submit,
|
||||
//! slash-command dispatch, and other synthetic clears.
|
||||
//!
|
||||
//! This module does not implement an Emacs-style multi-entry kill ring. It keeps only the most
|
||||
//! recent killed span.
|
||||
|
||||
use crate::key_hint::is_altgr;
|
||||
use codex_protocol::user_input::ByteRange;
|
||||
use codex_protocol::user_input::TextElement as UserTextElement;
|
||||
|
|
@ -38,6 +50,14 @@ pub(crate) struct TextElementSnapshot {
|
|||
pub(crate) text: String,
|
||||
}
|
||||
|
||||
/// `TextArea` is the editable buffer behind the TUI composer.
|
||||
///
|
||||
/// It owns the raw UTF-8 text, placeholder-like text elements that must move atomically with
|
||||
/// edits, cursor/wrapping state for rendering, and a single-entry kill buffer for `Ctrl+K` /
|
||||
/// `Ctrl+Y` style editing. Callers may replace the entire visible buffer through
|
||||
/// [`Self::set_text_clearing_elements`] or [`Self::set_text_with_elements`] without disturbing the
|
||||
/// kill buffer; if they incorrectly assume those methods fully reset editing state, a later yank
|
||||
/// will appear to restore stale text from the user's perspective.
|
||||
#[derive(Debug)]
|
||||
pub(crate) struct TextArea {
|
||||
text: String,
|
||||
|
|
@ -74,12 +94,21 @@ impl TextArea {
|
|||
}
|
||||
}
|
||||
|
||||
/// Replace the textarea text and clear any existing text elements.
|
||||
/// Replace the visible textarea text and clear any existing text elements.
|
||||
///
|
||||
/// This is the "fresh buffer" path for callers that want plain text with no placeholder
|
||||
/// ranges. It intentionally preserves the current kill buffer, because higher-level flows such
|
||||
/// as submit or slash-command dispatch clear the draft through this method and still want
|
||||
/// `Ctrl+Y` to recover the user's most recent kill.
|
||||
pub fn set_text_clearing_elements(&mut self, text: &str) {
|
||||
self.set_text_inner(text, None);
|
||||
}
|
||||
|
||||
/// Replace the textarea text and set the provided text elements.
|
||||
/// Replace the visible textarea text and rebuild the provided text elements.
|
||||
///
|
||||
/// As with [`Self::set_text_clearing_elements`], this resets only state derived from the
|
||||
/// visible buffer. The kill buffer survives so callers restoring drafts or external edits do
|
||||
/// not silently discard a pending yank target.
|
||||
pub fn set_text_with_elements(&mut self, text: &str, elements: &[UserTextElement]) {
|
||||
self.set_text_inner(text, Some(elements));
|
||||
}
|
||||
|
|
@ -109,10 +138,11 @@ impl TextArea {
|
|||
self.elements.sort_by_key(|e| e.range.start);
|
||||
}
|
||||
// Stage 3: clamp the cursor and reset derived state tied to the prior content.
|
||||
// The kill buffer is editing history rather than visible-buffer state, so full-buffer
|
||||
// replacements intentionally leave it alone.
|
||||
self.cursor_pos = self.clamp_pos_to_nearest_boundary(self.cursor_pos);
|
||||
self.wrap_cache.replace(None);
|
||||
self.preferred_col = None;
|
||||
self.kill_buffer.clear();
|
||||
}
|
||||
|
||||
pub fn text(&self) -> &str {
|
||||
|
|
@ -552,6 +582,12 @@ impl TextArea {
|
|||
}
|
||||
}
|
||||
|
||||
/// Kill from the cursor to the end of the current logical line.
|
||||
///
|
||||
/// If the cursor is already at end-of-line and a trailing newline exists, this kills that
|
||||
/// newline so repeated invocations continue making progress. The removed text becomes the next
|
||||
/// yank target and remains available even if a caller later clears or rewrites the visible
|
||||
/// buffer via `set_text_*`.
|
||||
pub fn kill_to_end_of_line(&mut self) {
|
||||
let eol = self.end_of_current_line();
|
||||
let range = if self.cursor_pos == eol {
|
||||
|
|
@ -582,6 +618,11 @@ impl TextArea {
|
|||
}
|
||||
}
|
||||
|
||||
/// Insert the most recently killed text at the cursor.
|
||||
///
|
||||
/// This uses the textarea's single-entry kill buffer. Because whole-buffer replacement APIs do
|
||||
/// not clear that buffer, `yank` can restore text after composer-level clears such as submit
|
||||
/// and slash-command dispatch.
|
||||
pub fn yank(&mut self) {
|
||||
if self.kill_buffer.is_empty() {
|
||||
return;
|
||||
|
|
@ -1735,6 +1776,21 @@ mod tests {
|
|||
assert_eq!(t.cursor(), 5);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn kill_buffer_persists_across_set_text() {
|
||||
let mut t = ta_with("restore me");
|
||||
t.set_cursor(0);
|
||||
t.kill_to_end_of_line();
|
||||
assert!(t.text().is_empty());
|
||||
|
||||
t.set_text_clearing_elements("/diff");
|
||||
t.set_text_clearing_elements("");
|
||||
t.yank();
|
||||
|
||||
assert_eq!(t.text(), "restore me");
|
||||
assert_eq!(t.cursor(), "restore me".len());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cursor_left_and_right_handle_graphemes() {
|
||||
let mut t = ta_with("a👍b");
|
||||
|
|
|
|||
|
|
@ -118,6 +118,12 @@ the input starts with `!` (shell command).
|
|||
The same preparation path is reused for slash commands with arguments (for example `/plan` and
|
||||
`/review`) so pasted content and text elements are preserved when extracting args.
|
||||
|
||||
The composer also treats the textarea kill buffer as separate editing state from the visible draft.
|
||||
After submit or slash-command dispatch clears the textarea, the most recent `Ctrl+K` payload is
|
||||
still available for `Ctrl+Y`. This supports flows where a user kills part of a draft, runs a
|
||||
composer action such as changing reasoning level, and then yanks that text back into the cleared
|
||||
draft.
|
||||
|
||||
### Numeric auto-submit path
|
||||
|
||||
When the slash popup is open and the first line matches a numeric-only custom prompt with
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue