diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 0d5fe44064d..1bbc616a97d 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -683,6 +683,42 @@ impl ChatComposer { if self.paste_burst.try_append_char_if_active(ch, now) { return (InputResult::None, true); } + // Non-ASCII input often comes from IMEs and can arrive in quick bursts. + // We do not want to hold the first char (flicker suppression) on this path, but we + // still want to detect paste-like bursts. Before applying any non-ASCII input, flush + // any existing burst buffer (including a pending first char from the ASCII path) so + // we don't carry that transient state forward. + if let Some(pasted) = self.paste_burst.flush_before_modified_input() { + self.handle_paste(pasted); + } + if let Some(decision) = self.paste_burst.on_plain_char_no_hold(now) { + match decision { + CharDecision::BufferAppend => { + self.paste_burst.append_char_to_buffer(ch, now); + return (InputResult::None, true); + } + CharDecision::BeginBuffer { retro_chars } => { + let cur = self.textarea.cursor(); + let txt = self.textarea.text(); + let safe_cur = Self::clamp_to_char_boundary(txt, cur); + let before = &txt[..safe_cur]; + // If decision is to buffer, seed the paste burst buffer with the grabbed chars + new. + // Otherwise, fall through to normal insertion below. + if let Some(grab) = + self.paste_burst + .decide_begin_buffer(now, before, retro_chars as usize) + { + if !grab.grabbed.is_empty() { + self.textarea.replace_range(grab.start_byte..safe_cur, ""); + } + // seed the paste burst buffer with everything (grabbed + new) + self.paste_burst.append_char_to_buffer(ch, now); + return (InputResult::None, true); + } + } + _ => unreachable!("on_plain_char_no_hold returned unexpected variant"), + } + } } if let Some(pasted) = self.paste_burst.flush_before_modified_input() { self.handle_paste(pasted); @@ -1346,9 +1382,8 @@ impl ChatComposer { { let has_ctrl_or_alt = has_ctrl_or_alt(modifiers); if !has_ctrl_or_alt { - // Non-ASCII characters (e.g., from IMEs) can arrive in quick bursts and be - // misclassified by paste heuristics. Flush any active burst buffer and insert - // non-ASCII characters directly. + // Non-ASCII characters (e.g., from IMEs) can arrive in quick bursts, so avoid + // holding the first char while still allowing burst detection for paste input. if !ch.is_ascii() { return self.handle_non_ascii_char(input); } @@ -1370,7 +1405,6 @@ impl ChatComposer { if !grab.grabbed.is_empty() { self.textarea.replace_range(grab.start_byte..safe_cur, ""); } - self.paste_burst.begin_with_retro_grabbed(grab.grabbed, now); self.paste_burst.append_char_to_buffer(ch, now); return (InputResult::None, true); } @@ -2271,8 +2305,7 @@ mod tests { composer.handle_key_event(KeyEvent::new(KeyCode::Char('?'), KeyModifiers::NONE)); assert_eq!(result, InputResult::None); assert!(needs_redraw, "typing should still mark the view dirty"); - std::thread::sleep(ChatComposer::recommended_paste_flush_delay()); - let _ = composer.flush_paste_burst_if_due(); + let _ = flush_after_paste_burst(&mut composer); assert_eq!(composer.textarea.text(), "h?"); assert_eq!(composer.footer_mode, FooterMode::ShortcutSummary); assert_eq!(composer.footer_mode(), FooterMode::ContextOnly); @@ -2294,14 +2327,18 @@ mod tests { false, ); + // Force an active paste burst so this test doesn't depend on tight timing. + composer + .paste_burst + .begin_with_retro_grabbed(String::new(), Instant::now()); + for ch in ['h', 'i', '?', 't', 'h', 'e', 'r', 'e'] { let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Char(ch), KeyModifiers::NONE)); } assert!(composer.is_in_paste_burst()); assert_eq!(composer.textarea.text(), ""); - std::thread::sleep(ChatComposer::recommended_paste_flush_delay()); - let _ = composer.flush_paste_burst_if_due(); + let _ = flush_after_paste_burst(&mut composer); assert_eq!(composer.textarea.text(), "hi?there"); assert_ne!(composer.footer_mode, FooterMode::ShortcutOverlay); @@ -2510,6 +2547,116 @@ mod tests { } } + #[test] + fn non_ascii_char_inserts_immediately_without_burst_state() { + use crossterm::event::KeyCode; + use crossterm::event::KeyEvent; + use crossterm::event::KeyModifiers; + + let (tx, _rx) = unbounded_channel::(); + let sender = AppEventSender::new(tx); + let mut composer = ChatComposer::new( + true, + sender, + false, + "Ask Codex to do anything".to_string(), + false, + ); + + let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Char('あ'), KeyModifiers::NONE)); + + assert_eq!(composer.textarea.text(), "あ"); + assert!(!composer.is_in_paste_burst()); + } + + // test a variety of non-ascii char sequences to ensure we are handling them correctly + #[test] + fn non_ascii_burst_handles_newline() { + let test_cases = [ + // triggers on windows + "天地玄黄 宇宙洪荒 +日月盈昃 辰宿列张 +寒来暑往 秋收冬藏 + +你好世界 编码测试 +汉字处理 UTF-8 +终端显示 正确无误 + +风吹竹林 月照大江 +白云千载 青山依旧 +程序员 与 Unicode 同行", + // Simulate pasting "你 好\nhi" with an ideographic space to trigger pastey heuristics. + "你 好\nhi", + ]; + + for test_case in test_cases { + use crossterm::event::KeyCode; + use crossterm::event::KeyEvent; + use crossterm::event::KeyModifiers; + + let (tx, _rx) = unbounded_channel::(); + let sender = AppEventSender::new(tx); + let mut composer = ChatComposer::new( + true, + sender, + false, + "Ask Codex to do anything".to_string(), + false, + ); + + for c in test_case.chars() { + let _ = + composer.handle_key_event(KeyEvent::new(KeyCode::Char(c), KeyModifiers::NONE)); + } + + assert!( + composer.textarea.text().is_empty(), + "non-empty textarea before flush: {test_case}", + ); + let _ = flush_after_paste_burst(&mut composer); + assert_eq!(composer.textarea.text(), test_case); + } + } + + #[test] + fn ascii_burst_treats_enter_as_newline() { + use crossterm::event::KeyCode; + use crossterm::event::KeyEvent; + use crossterm::event::KeyModifiers; + + let (tx, _rx) = unbounded_channel::(); + let sender = AppEventSender::new(tx); + let mut composer = ChatComposer::new( + true, + sender, + false, + "Ask Codex to do anything".to_string(), + false, + ); + + // Force an active burst so this test doesn't depend on tight timing. + composer + .paste_burst + .begin_with_retro_grabbed(String::new(), Instant::now()); + + let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Char('h'), KeyModifiers::NONE)); + let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Char('i'), KeyModifiers::NONE)); + + let (result, _) = + composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + assert!( + matches!(result, InputResult::None), + "Enter during a burst should insert newline, not submit" + ); + + for ch in ['t', 'h', 'e', 'r', 'e'] { + let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Char(ch), KeyModifiers::NONE)); + } + + let _ = flush_after_paste_burst(&mut composer); + assert_eq!(composer.textarea.text(), "hi\nthere"); + } + #[test] fn handle_paste_small_inserts_text() { use crossterm::event::KeyCode; @@ -2800,6 +2947,11 @@ mod tests { } } + fn flush_after_paste_burst(composer: &mut ChatComposer) -> bool { + std::thread::sleep(PasteBurst::recommended_active_flush_delay()); + composer.flush_paste_burst_if_due() + } + // Test helper: simulate human typing with a brief delay and flush the paste-burst buffer fn type_chars_humanlike(composer: &mut ChatComposer, chars: &[char]) { use crossterm::event::KeyCode; @@ -3987,6 +4139,33 @@ mod tests { assert_eq!(InputResult::Submitted(expected), result); } + #[test] + fn pending_first_ascii_char_flushes_as_typed() { + use crossterm::event::KeyCode; + use crossterm::event::KeyEvent; + use crossterm::event::KeyModifiers; + + let (tx, _rx) = unbounded_channel::(); + let sender = AppEventSender::new(tx); + let mut composer = ChatComposer::new( + true, + sender, + false, + "Ask Codex to do anything".to_string(), + false, + ); + + let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Char('h'), KeyModifiers::NONE)); + assert!(composer.is_in_paste_burst()); + assert!(composer.textarea.text().is_empty()); + + std::thread::sleep(ChatComposer::recommended_paste_flush_delay()); + let flushed = composer.flush_paste_burst_if_due(); + assert!(flushed, "expected pending first char to flush"); + assert_eq!(composer.textarea.text(), "h"); + assert!(!composer.is_in_paste_burst()); + } + #[test] fn burst_paste_fast_small_buffers_and_flushes_on_stop() { use crossterm::event::KeyCode; @@ -4021,8 +4200,7 @@ mod tests { composer.textarea.text().is_empty(), "text should remain empty until flush" ); - std::thread::sleep(ChatComposer::recommended_paste_flush_delay()); - let flushed = composer.flush_paste_burst_if_due(); + let flushed = flush_after_paste_burst(&mut composer); assert!(flushed, "expected buffered text to flush after stop"); assert_eq!(composer.textarea.text(), "a".repeat(count)); assert!( @@ -4055,8 +4233,7 @@ mod tests { // Nothing should appear until we stop and flush assert!(composer.textarea.text().is_empty()); - std::thread::sleep(ChatComposer::recommended_paste_flush_delay()); - let flushed = composer.flush_paste_burst_if_due(); + let flushed = flush_after_paste_burst(&mut composer); assert!(flushed, "expected flush after stopping fast input"); let expected_placeholder = format!("[Pasted Content {count} chars]"); diff --git a/codex-rs/tui/src/bottom_pane/paste_burst.rs b/codex-rs/tui/src/bottom_pane/paste_burst.rs index 49377cb21c5..96ed095b8f3 100644 --- a/codex-rs/tui/src/bottom_pane/paste_burst.rs +++ b/codex-rs/tui/src/bottom_pane/paste_burst.rs @@ -6,6 +6,12 @@ use std::time::Instant; const PASTE_BURST_MIN_CHARS: u16 = 3; const PASTE_BURST_CHAR_INTERVAL: Duration = Duration::from_millis(8); const PASTE_ENTER_SUPPRESS_WINDOW: Duration = Duration::from_millis(120); +// Slower paste burts have been observed in windows environments, but ideally +// we want to keep this low +#[cfg(not(windows))] +const PASTE_BURST_ACTIVE_IDLE_TIMEOUT: Duration = Duration::from_millis(8); +#[cfg(windows)] +const PASTE_BURST_ACTIVE_IDLE_TIMEOUT: Duration = Duration::from_millis(60); #[derive(Default)] pub(crate) struct PasteBurst { @@ -52,16 +58,14 @@ impl PasteBurst { PASTE_BURST_CHAR_INTERVAL + Duration::from_millis(1) } + #[cfg(test)] + pub(crate) fn recommended_active_flush_delay() -> Duration { + PASTE_BURST_ACTIVE_IDLE_TIMEOUT + Duration::from_millis(1) + } + /// Entry point: decide how to treat a plain char with current timing. pub fn on_plain_char(&mut self, ch: char, now: Instant) -> CharDecision { - match self.last_plain_char_time { - Some(prev) if now.duration_since(prev) <= PASTE_BURST_CHAR_INTERVAL => { - self.consecutive_plain_char_burst = - self.consecutive_plain_char_burst.saturating_add(1) - } - _ => self.consecutive_plain_char_burst = 1, - } - self.last_plain_char_time = Some(now); + self.note_plain_char(now); if self.active { self.burst_window_until = Some(now + PASTE_ENTER_SUPPRESS_WINDOW); @@ -92,6 +96,40 @@ impl PasteBurst { CharDecision::RetainFirstChar } + /// Like on_plain_char(), but never holds the first char. + /// + /// Used for non-ASCII input paths (e.g., IMEs) where holding a character can + /// feel like dropped input, while still allowing burst-based paste detection. + /// + /// Note: This method will only ever return BufferAppend or BeginBuffer. + pub fn on_plain_char_no_hold(&mut self, now: Instant) -> Option { + self.note_plain_char(now); + + if self.active { + self.burst_window_until = Some(now + PASTE_ENTER_SUPPRESS_WINDOW); + return Some(CharDecision::BufferAppend); + } + + if self.consecutive_plain_char_burst >= PASTE_BURST_MIN_CHARS { + return Some(CharDecision::BeginBuffer { + retro_chars: self.consecutive_plain_char_burst.saturating_sub(1), + }); + } + + None + } + + fn note_plain_char(&mut self, now: Instant) { + match self.last_plain_char_time { + Some(prev) if now.duration_since(prev) <= PASTE_BURST_CHAR_INTERVAL => { + self.consecutive_plain_char_burst = + self.consecutive_plain_char_burst.saturating_add(1) + } + _ => self.consecutive_plain_char_burst = 1, + } + self.last_plain_char_time = Some(now); + } + /// Flush the buffered burst if the inter-key timeout has elapsed. /// /// Returns Some(String) when either: @@ -102,9 +140,14 @@ impl PasteBurst { /// /// Returns None if the timeout has not elapsed or there is nothing to flush. pub fn flush_if_due(&mut self, now: Instant) -> FlushResult { + let timeout = if self.is_active_internal() { + PASTE_BURST_ACTIVE_IDLE_TIMEOUT + } else { + PASTE_BURST_CHAR_INTERVAL + }; let timed_out = self .last_plain_char_time - .is_some_and(|t| now.duration_since(t) > PASTE_BURST_CHAR_INTERVAL); + .is_some_and(|t| now.duration_since(t) > timeout); if timed_out && self.is_active_internal() { self.active = false; let out = std::mem::take(&mut self.buffer); diff --git a/codex-rs/tui/src/bottom_pane/textarea.rs b/codex-rs/tui/src/bottom_pane/textarea.rs index 2fd415c7f65..4fc673a11de 100644 --- a/codex-rs/tui/src/bottom_pane/textarea.rs +++ b/codex-rs/tui/src/bottom_pane/textarea.rs @@ -63,9 +63,10 @@ impl TextArea { pub fn set_text(&mut self, text: &str) { self.text = text.to_string(); self.cursor_pos = self.cursor_pos.clamp(0, self.text.len()); + self.elements.clear(); + self.cursor_pos = self.clamp_pos_to_nearest_boundary(self.cursor_pos); self.wrap_cache.replace(None); self.preferred_col = None; - self.elements.clear(); self.kill_buffer.clear(); } @@ -735,18 +736,36 @@ impl TextArea { .position(|e| pos > e.range.start && pos < e.range.end) } - fn clamp_pos_to_nearest_boundary(&self, mut pos: usize) -> usize { - if pos > self.text.len() { - pos = self.text.len(); + fn clamp_pos_to_char_boundary(&self, pos: usize) -> usize { + let pos = pos.min(self.text.len()); + if self.text.is_char_boundary(pos) { + return pos; + } + let mut prev = pos; + while prev > 0 && !self.text.is_char_boundary(prev) { + prev -= 1; + } + let mut next = pos; + while next < self.text.len() && !self.text.is_char_boundary(next) { + next += 1; + } + if pos.saturating_sub(prev) <= next.saturating_sub(pos) { + prev + } else { + next } + } + + fn clamp_pos_to_nearest_boundary(&self, pos: usize) -> usize { + let pos = self.clamp_pos_to_char_boundary(pos); if let Some(idx) = self.find_element_containing(pos) { let e = &self.elements[idx]; let dist_start = pos.saturating_sub(e.range.start); let dist_end = e.range.end.saturating_sub(pos); if dist_start <= dist_end { - e.range.start + self.clamp_pos_to_char_boundary(e.range.start) } else { - e.range.end + self.clamp_pos_to_char_boundary(e.range.end) } } else { pos @@ -754,6 +773,7 @@ impl TextArea { } fn clamp_pos_for_insertion(&self, pos: usize) -> usize { + let pos = self.clamp_pos_to_char_boundary(pos); // Do not allow inserting into the middle of an element if let Some(idx) = self.find_element_containing(pos) { let e = &self.elements[idx]; @@ -761,9 +781,9 @@ impl TextArea { let dist_start = pos.saturating_sub(e.range.start); let dist_end = e.range.end.saturating_sub(pos); if dist_start <= dist_end { - e.range.start + self.clamp_pos_to_char_boundary(e.range.start) } else { - e.range.end + self.clamp_pos_to_char_boundary(e.range.end) } } else { pos @@ -1041,6 +1061,7 @@ impl TextArea { mod tests { use super::*; // crossterm types are intentionally not imported here to avoid unused warnings + use pretty_assertions::assert_eq; use rand::prelude::*; fn rand_grapheme(rng: &mut rand::rngs::StdRng) -> String { @@ -1133,6 +1154,27 @@ mod tests { assert_eq!(t.cursor(), 5); } + #[test] + fn insert_str_at_clamps_to_char_boundary() { + let mut t = TextArea::new(); + t.insert_str("你"); + t.set_cursor(0); + t.insert_str_at(1, "A"); + assert_eq!(t.text(), "A你"); + assert_eq!(t.cursor(), 1); + } + + #[test] + fn set_text_clamps_cursor_to_char_boundary() { + let mut t = TextArea::new(); + t.insert_str("abcd"); + t.set_cursor(1); + t.set_text("你"); + assert_eq!(t.cursor(), 0); + t.insert_str("a"); + assert_eq!(t.text(), "a你"); + } + #[test] fn delete_backward_and_forward_edges() { let mut t = ta_with("abc"); diff --git a/codex-rs/tui2/src/bottom_pane/chat_composer.rs b/codex-rs/tui2/src/bottom_pane/chat_composer.rs index 0073173fdc7..6493c8d28ec 100644 --- a/codex-rs/tui2/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui2/src/bottom_pane/chat_composer.rs @@ -600,6 +600,42 @@ impl ChatComposer { if self.paste_burst.try_append_char_if_active(ch, now) { return (InputResult::None, true); } + // Non-ASCII input often comes from IMEs and can arrive in quick bursts. + // We do not want to hold the first char (flicker suppression) on this path, but we + // still want to detect paste-like bursts. Before applying any non-ASCII input, flush + // any existing burst buffer (including a pending first char from the ASCII path) so + // we don't carry that transient state forward. + if let Some(pasted) = self.paste_burst.flush_before_modified_input() { + self.handle_paste(pasted); + } + if let Some(decision) = self.paste_burst.on_plain_char_no_hold(now) { + match decision { + CharDecision::BufferAppend => { + self.paste_burst.append_char_to_buffer(ch, now); + return (InputResult::None, true); + } + CharDecision::BeginBuffer { retro_chars } => { + let cur = self.textarea.cursor(); + let txt = self.textarea.text(); + let safe_cur = Self::clamp_to_char_boundary(txt, cur); + let before = &txt[..safe_cur]; + // If decision is to buffer, seed the paste burst buffer with the grabbed chars + new. + // Otherwise, fall through to normal insertion below. + if let Some(grab) = + self.paste_burst + .decide_begin_buffer(now, before, retro_chars as usize) + { + if !grab.grabbed.is_empty() { + self.textarea.replace_range(grab.start_byte..safe_cur, ""); + } + // seed the paste burst buffer with everything (grabbed + new) + self.paste_burst.append_char_to_buffer(ch, now); + return (InputResult::None, true); + } + } + _ => unreachable!("on_plain_char_no_hold returned unexpected variant"), + } + } } if let Some(pasted) = self.paste_burst.flush_before_modified_input() { self.handle_paste(pasted); @@ -1263,9 +1299,8 @@ impl ChatComposer { { let has_ctrl_or_alt = has_ctrl_or_alt(modifiers); if !has_ctrl_or_alt { - // Non-ASCII characters (e.g., from IMEs) can arrive in quick bursts and be - // misclassified by paste heuristics. Flush any active burst buffer and insert - // non-ASCII characters directly. + // Non-ASCII characters (e.g., from IMEs) can arrive in quick bursts, so avoid + // holding the first char while still allowing burst detection for paste input. if !ch.is_ascii() { return self.handle_non_ascii_char(input); } @@ -1287,7 +1322,6 @@ impl ChatComposer { if !grab.grabbed.is_empty() { self.textarea.replace_range(grab.start_byte..safe_cur, ""); } - self.paste_burst.begin_with_retro_grabbed(grab.grabbed, now); self.paste_burst.append_char_to_buffer(ch, now); return (InputResult::None, true); } @@ -1524,7 +1558,8 @@ impl ChatComposer { let toggles = matches!(key_event.code, KeyCode::Char('?')) && !has_ctrl_or_alt(key_event.modifiers) - && self.is_empty(); + && self.is_empty() + && !self.is_in_paste_burst(); if !toggles { return false; @@ -2213,13 +2248,46 @@ mod tests { composer.handle_key_event(KeyEvent::new(KeyCode::Char('?'), KeyModifiers::NONE)); assert_eq!(result, InputResult::None); assert!(needs_redraw, "typing should still mark the view dirty"); - std::thread::sleep(ChatComposer::recommended_paste_flush_delay()); - let _ = composer.flush_paste_burst_if_due(); + let _ = flush_after_paste_burst(&mut composer); assert_eq!(composer.textarea.text(), "h?"); assert_eq!(composer.footer_mode, FooterMode::ShortcutSummary); assert_eq!(composer.footer_mode(), FooterMode::ContextOnly); } + #[test] + fn question_mark_does_not_toggle_during_paste_burst() { + use crossterm::event::KeyCode; + use crossterm::event::KeyEvent; + use crossterm::event::KeyModifiers; + + let (tx, _rx) = unbounded_channel::(); + let sender = AppEventSender::new(tx); + let mut composer = ChatComposer::new( + true, + sender, + false, + "Ask Codex to do anything".to_string(), + false, + ); + + // Force an active paste burst so this test doesn't depend on tight timing. + composer + .paste_burst + .begin_with_retro_grabbed(String::new(), Instant::now()); + + for ch in ['h', 'i', '?', 't', 'h', 'e', 'r', 'e'] { + let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Char(ch), KeyModifiers::NONE)); + } + assert!(composer.is_in_paste_burst()); + assert_eq!(composer.textarea.text(), ""); + + let flushed = flush_after_paste_burst(&mut composer); + assert!(flushed, "expected buffered text to flush after stop"); + + assert_eq!(composer.textarea.text(), "hi?there"); + assert_ne!(composer.footer_mode, FooterMode::ShortcutOverlay); + } + #[test] fn shortcut_overlay_persists_while_task_running() { use crossterm::event::KeyCode; @@ -2423,6 +2491,93 @@ mod tests { } } + #[test] + fn non_ascii_burst_handles_newline() { + let test_cases = [ + // triggers on windows + "天地玄黄 宇宙洪荒 +日月盈昃 辰宿列张 +寒来暑往 秋收冬藏 + +你好世界 编码测试 +汉字处理 UTF-8 +终端显示 正确无误 + +风吹竹林 月照大江 +白云千载 青山依旧 +程序员 与 Unicode 同行", + // Simulate pasting "你 好\nhi" with an ideographic space to trigger pastey heuristics. + "你 好\nhi", + ]; + + for test_case in test_cases { + use crossterm::event::KeyCode; + use crossterm::event::KeyEvent; + use crossterm::event::KeyModifiers; + + let (tx, _rx) = unbounded_channel::(); + let sender = AppEventSender::new(tx); + let mut composer = ChatComposer::new( + true, + sender, + false, + "Ask Codex to do anything".to_string(), + false, + ); + + for c in test_case.chars() { + let _ = + composer.handle_key_event(KeyEvent::new(KeyCode::Char(c), KeyModifiers::NONE)); + } + + assert!( + composer.textarea.text().is_empty(), + "non-empty textarea before flush: {test_case}", + ); + let _ = flush_after_paste_burst(&mut composer); + assert_eq!(composer.textarea.text(), test_case); + } + } + + #[test] + fn ascii_burst_treats_enter_as_newline() { + use crossterm::event::KeyCode; + use crossterm::event::KeyEvent; + use crossterm::event::KeyModifiers; + + let (tx, _rx) = unbounded_channel::(); + let sender = AppEventSender::new(tx); + let mut composer = ChatComposer::new( + true, + sender, + false, + "Ask Codex to do anything".to_string(), + false, + ); + + // Force an active burst so this test doesn't depend on tight timing. + composer + .paste_burst + .begin_with_retro_grabbed(String::new(), Instant::now()); + + let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Char('h'), KeyModifiers::NONE)); + let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Char('i'), KeyModifiers::NONE)); + + let (result, _) = + composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + assert!( + matches!(result, InputResult::None), + "Enter during a burst should insert newline, not submit" + ); + + for ch in ['t', 'h', 'e', 'r', 'e'] { + let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Char(ch), KeyModifiers::NONE)); + } + + let _ = flush_after_paste_burst(&mut composer); + assert_eq!(composer.textarea.text(), "hi\nthere"); + } + #[test] fn handle_paste_small_inserts_text() { use crossterm::event::KeyCode; @@ -2725,6 +2880,11 @@ mod tests { } } + fn flush_after_paste_burst(composer: &mut ChatComposer) -> bool { + std::thread::sleep(PasteBurst::recommended_active_flush_delay()); + composer.flush_paste_burst_if_due() + } + #[test] fn slash_init_dispatches_command_and_does_not_submit_literal_text() { use crossterm::event::KeyCode; @@ -3905,8 +4065,7 @@ mod tests { composer.textarea.text().is_empty(), "text should remain empty until flush" ); - std::thread::sleep(ChatComposer::recommended_paste_flush_delay()); - let flushed = composer.flush_paste_burst_if_due(); + let flushed = flush_after_paste_burst(&mut composer); assert!(flushed, "expected buffered text to flush after stop"); assert_eq!(composer.textarea.text(), "a".repeat(count)); assert!( @@ -3939,8 +4098,7 @@ mod tests { // Nothing should appear until we stop and flush assert!(composer.textarea.text().is_empty()); - std::thread::sleep(ChatComposer::recommended_paste_flush_delay()); - let flushed = composer.flush_paste_burst_if_due(); + let flushed = flush_after_paste_burst(&mut composer); assert!(flushed, "expected flush after stopping fast input"); let expected_placeholder = format!("[Pasted Content {count} chars]"); diff --git a/codex-rs/tui2/src/bottom_pane/paste_burst.rs b/codex-rs/tui2/src/bottom_pane/paste_burst.rs index 49377cb21c5..96ed095b8f3 100644 --- a/codex-rs/tui2/src/bottom_pane/paste_burst.rs +++ b/codex-rs/tui2/src/bottom_pane/paste_burst.rs @@ -6,6 +6,12 @@ use std::time::Instant; const PASTE_BURST_MIN_CHARS: u16 = 3; const PASTE_BURST_CHAR_INTERVAL: Duration = Duration::from_millis(8); const PASTE_ENTER_SUPPRESS_WINDOW: Duration = Duration::from_millis(120); +// Slower paste burts have been observed in windows environments, but ideally +// we want to keep this low +#[cfg(not(windows))] +const PASTE_BURST_ACTIVE_IDLE_TIMEOUT: Duration = Duration::from_millis(8); +#[cfg(windows)] +const PASTE_BURST_ACTIVE_IDLE_TIMEOUT: Duration = Duration::from_millis(60); #[derive(Default)] pub(crate) struct PasteBurst { @@ -52,16 +58,14 @@ impl PasteBurst { PASTE_BURST_CHAR_INTERVAL + Duration::from_millis(1) } + #[cfg(test)] + pub(crate) fn recommended_active_flush_delay() -> Duration { + PASTE_BURST_ACTIVE_IDLE_TIMEOUT + Duration::from_millis(1) + } + /// Entry point: decide how to treat a plain char with current timing. pub fn on_plain_char(&mut self, ch: char, now: Instant) -> CharDecision { - match self.last_plain_char_time { - Some(prev) if now.duration_since(prev) <= PASTE_BURST_CHAR_INTERVAL => { - self.consecutive_plain_char_burst = - self.consecutive_plain_char_burst.saturating_add(1) - } - _ => self.consecutive_plain_char_burst = 1, - } - self.last_plain_char_time = Some(now); + self.note_plain_char(now); if self.active { self.burst_window_until = Some(now + PASTE_ENTER_SUPPRESS_WINDOW); @@ -92,6 +96,40 @@ impl PasteBurst { CharDecision::RetainFirstChar } + /// Like on_plain_char(), but never holds the first char. + /// + /// Used for non-ASCII input paths (e.g., IMEs) where holding a character can + /// feel like dropped input, while still allowing burst-based paste detection. + /// + /// Note: This method will only ever return BufferAppend or BeginBuffer. + pub fn on_plain_char_no_hold(&mut self, now: Instant) -> Option { + self.note_plain_char(now); + + if self.active { + self.burst_window_until = Some(now + PASTE_ENTER_SUPPRESS_WINDOW); + return Some(CharDecision::BufferAppend); + } + + if self.consecutive_plain_char_burst >= PASTE_BURST_MIN_CHARS { + return Some(CharDecision::BeginBuffer { + retro_chars: self.consecutive_plain_char_burst.saturating_sub(1), + }); + } + + None + } + + fn note_plain_char(&mut self, now: Instant) { + match self.last_plain_char_time { + Some(prev) if now.duration_since(prev) <= PASTE_BURST_CHAR_INTERVAL => { + self.consecutive_plain_char_burst = + self.consecutive_plain_char_burst.saturating_add(1) + } + _ => self.consecutive_plain_char_burst = 1, + } + self.last_plain_char_time = Some(now); + } + /// Flush the buffered burst if the inter-key timeout has elapsed. /// /// Returns Some(String) when either: @@ -102,9 +140,14 @@ impl PasteBurst { /// /// Returns None if the timeout has not elapsed or there is nothing to flush. pub fn flush_if_due(&mut self, now: Instant) -> FlushResult { + let timeout = if self.is_active_internal() { + PASTE_BURST_ACTIVE_IDLE_TIMEOUT + } else { + PASTE_BURST_CHAR_INTERVAL + }; let timed_out = self .last_plain_char_time - .is_some_and(|t| now.duration_since(t) > PASTE_BURST_CHAR_INTERVAL); + .is_some_and(|t| now.duration_since(t) > timeout); if timed_out && self.is_active_internal() { self.active = false; let out = std::mem::take(&mut self.buffer); diff --git a/codex-rs/tui2/src/bottom_pane/textarea.rs b/codex-rs/tui2/src/bottom_pane/textarea.rs index 2fd415c7f65..4fc673a11de 100644 --- a/codex-rs/tui2/src/bottom_pane/textarea.rs +++ b/codex-rs/tui2/src/bottom_pane/textarea.rs @@ -63,9 +63,10 @@ impl TextArea { pub fn set_text(&mut self, text: &str) { self.text = text.to_string(); self.cursor_pos = self.cursor_pos.clamp(0, self.text.len()); + self.elements.clear(); + self.cursor_pos = self.clamp_pos_to_nearest_boundary(self.cursor_pos); self.wrap_cache.replace(None); self.preferred_col = None; - self.elements.clear(); self.kill_buffer.clear(); } @@ -735,18 +736,36 @@ impl TextArea { .position(|e| pos > e.range.start && pos < e.range.end) } - fn clamp_pos_to_nearest_boundary(&self, mut pos: usize) -> usize { - if pos > self.text.len() { - pos = self.text.len(); + fn clamp_pos_to_char_boundary(&self, pos: usize) -> usize { + let pos = pos.min(self.text.len()); + if self.text.is_char_boundary(pos) { + return pos; + } + let mut prev = pos; + while prev > 0 && !self.text.is_char_boundary(prev) { + prev -= 1; + } + let mut next = pos; + while next < self.text.len() && !self.text.is_char_boundary(next) { + next += 1; + } + if pos.saturating_sub(prev) <= next.saturating_sub(pos) { + prev + } else { + next } + } + + fn clamp_pos_to_nearest_boundary(&self, pos: usize) -> usize { + let pos = self.clamp_pos_to_char_boundary(pos); if let Some(idx) = self.find_element_containing(pos) { let e = &self.elements[idx]; let dist_start = pos.saturating_sub(e.range.start); let dist_end = e.range.end.saturating_sub(pos); if dist_start <= dist_end { - e.range.start + self.clamp_pos_to_char_boundary(e.range.start) } else { - e.range.end + self.clamp_pos_to_char_boundary(e.range.end) } } else { pos @@ -754,6 +773,7 @@ impl TextArea { } fn clamp_pos_for_insertion(&self, pos: usize) -> usize { + let pos = self.clamp_pos_to_char_boundary(pos); // Do not allow inserting into the middle of an element if let Some(idx) = self.find_element_containing(pos) { let e = &self.elements[idx]; @@ -761,9 +781,9 @@ impl TextArea { let dist_start = pos.saturating_sub(e.range.start); let dist_end = e.range.end.saturating_sub(pos); if dist_start <= dist_end { - e.range.start + self.clamp_pos_to_char_boundary(e.range.start) } else { - e.range.end + self.clamp_pos_to_char_boundary(e.range.end) } } else { pos @@ -1041,6 +1061,7 @@ impl TextArea { mod tests { use super::*; // crossterm types are intentionally not imported here to avoid unused warnings + use pretty_assertions::assert_eq; use rand::prelude::*; fn rand_grapheme(rng: &mut rand::rngs::StdRng) -> String { @@ -1133,6 +1154,27 @@ mod tests { assert_eq!(t.cursor(), 5); } + #[test] + fn insert_str_at_clamps_to_char_boundary() { + let mut t = TextArea::new(); + t.insert_str("你"); + t.set_cursor(0); + t.insert_str_at(1, "A"); + assert_eq!(t.text(), "A你"); + assert_eq!(t.cursor(), 1); + } + + #[test] + fn set_text_clamps_cursor_to_char_boundary() { + let mut t = TextArea::new(); + t.insert_str("abcd"); + t.set_cursor(1); + t.set_text("你"); + assert_eq!(t.cursor(), 0); + t.insert_str("a"); + assert_eq!(t.text(), "a你"); + } + #[test] fn delete_backward_and_forward_edges() { let mut t = ta_with("abc");