diff --git a/codex-rs/tui/Cargo.toml b/codex-rs/tui/Cargo.toml index 657543f73e..4e0f19cc9a 100644 --- a/codex-rs/tui/Cargo.toml +++ b/codex-rs/tui/Cargo.toml @@ -37,6 +37,7 @@ codex-login = { path = "../login" } codex-ollama = { path = "../ollama" } color-eyre = "0.6.3" crossterm = { version = "0.28.1", features = ["bracketed-paste"] } +diffy = "0.4.2" image = { version = "^0.25.6", default-features = false, features = [ "jpeg", "png", diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index bbe324c12a..e56f5d7bc5 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -31,8 +31,6 @@ use std::sync::mpsc::channel; use std::thread; use std::time::Duration; -use crate::slash_command::SlashCommand; - /// Time window for debouncing redraw requests. const REDRAW_DEBOUNCE: Duration = Duration::from_millis(10); diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index ab1cadaf1e..dc0e4ba70d 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -113,7 +113,7 @@ impl ChatComposer { pub fn cursor_pos(&self, area: Rect) -> Option<(u16, u16)> { let popup_height = match &self.active_popup { - ActivePopup::Command(popup) => popup.calculate_required_height(), + ActivePopup::Slash(popup) => popup.calculate_required_height(), ActivePopup::File(popup) => popup.calculate_required_height(), ActivePopup::None => 1, }; @@ -179,7 +179,7 @@ impl ChatComposer { } else { self.textarea.insert_str(&pasted); } - self.sync_slash_command_popup(); + self.sync_command_popup(); self.sync_file_search_popup(); true } @@ -232,7 +232,7 @@ impl ChatComposer { }; // Update (or hide/show) popup after processing the key. - self.sync_slash_command_popup(); + self.sync_command_popup(); if matches!(self.active_popup, ActivePopup::Slash(_)) { self.dismissed_file_popup_token = None; } else { @@ -355,24 +355,29 @@ impl ChatComposer { match image::image_dimensions(&path_buf) { Ok((w, h)) => { // Remove the current @token (mirror logic from insert_selected_path without inserting text) - let (row, col) = self.textarea.cursor(); - let mut lines: Vec = self.textarea.lines().to_vec(); - if let Some(line) = lines.get_mut(row) { - let cursor_byte_offset = cursor_byte_offset(line, col); - if let Some((start, end)) = - at_token_bounds(line, cursor_byte_offset, true) - { - let mut new_line = - String::with_capacity(line.len() - (end - start)); - new_line.push_str(&line[..start]); - new_line.push_str(&line[end..]); - *line = new_line; - let new_text = lines.join("\n"); - self.textarea.select_all(); - self.textarea.cut(); - let _ = self.textarea.insert_str(new_text); - } - } + // using the flat text and byte-offset cursor API. + let cursor_offset = self.textarea.cursor(); + let text = self.textarea.text(); + let before_cursor = &text[..cursor_offset]; + let after_cursor = &text[cursor_offset..]; + + // Determine token boundaries in the full text. + let start_idx = before_cursor + .char_indices() + .rfind(|(_, c)| c.is_whitespace()) + .map(|(idx, c)| idx + c.len_utf8()) + .unwrap_or(0); + let end_rel_idx = after_cursor + .char_indices() + .find(|(_, c)| c.is_whitespace()) + .map(|(idx, _)| idx) + .unwrap_or(after_cursor.len()); + let end_idx = cursor_offset + end_rel_idx; + + // Remove the token slice; keep a single trailing space for flow. + self.textarea.replace_range(start_idx..end_idx, ""); + self.textarea.insert_str(" "); + let format_label = match Path::new(&sel_path) .extension() .and_then(|e| e.to_str()) @@ -396,7 +401,7 @@ impl ChatComposer { format_label ); // Optionally add a trailing space to keep typing fluid. - let _ = self.textarea.insert_str(" "); + self.textarea.insert_str(" "); } Err(_) => { // Fallback to plain path insertion if metadata read fails. @@ -642,8 +647,8 @@ impl ChatComposer { .. } = input { - // First try image placeholders (any backspace inside one removes it entirely) - if self.try_remove_image_placeholder_on_backspace() { + // Remove image placeholder only when cursor is at the end of it + if self.try_remove_image_placeholder_at_cursor() { return (InputResult::None, true); } // Then try pasted-content placeholders (only when at end) @@ -657,7 +662,7 @@ impl ChatComposer { let text_after = self.textarea.text(); // Start/continue an explicit file-search session when the cursor is on an @token. - if Self::current_at_token_allow_empty(&self.textarea).is_some() { + if Self::current_at_token(&self.textarea).is_some() { self.file_search_mode = true; // Allow popup to show for this token. self.dismissed_file_popup_token = None; @@ -667,6 +672,10 @@ impl ChatComposer { self.pending_pastes .retain(|(placeholder, _)| text_after.contains(placeholder)); + // Keep only attached images whose placeholders still exist in text + self.attached_images + .retain(|(placeholder, _)| text_after.contains(placeholder)); + (InputResult::None, true) } @@ -698,61 +707,38 @@ impl ChatComposer { } } - /// Attempts to remove an attached image placeholder if a backspace occurs *anywhere* inside it. + /// Attempts to remove an attached image placeholder if the cursor is at the end of one. /// Returns true if a placeholder + image mapping was removed. - fn try_remove_image_placeholder_on_backspace(&mut self) -> bool { + fn try_remove_image_placeholder_at_cursor(&mut self) -> bool { if self.attached_images.is_empty() { return false; } + let p = self.textarea.cursor(); + let text = self.textarea.text(); - // Materialize full text and compute global cursor + deletion indices. - let lines: Vec = self.textarea.lines().to_vec(); - let (cursor_row, cursor_col) = self.textarea.cursor(); - - // Compute global char index of cursor (in characters, since placeholders are ASCII). - let mut global_index: usize = 0; - for (i, line) in lines.iter().enumerate() { - if i == cursor_row { - global_index += cursor_col; - break; - } else { - global_index += line.chars().count() + 1; // +1 for the newline that will be joined - } + // Find any image placeholder that ends at the cursor position + if let Some((idx, placeholder)) = + self.attached_images + .iter() + .enumerate() + .find_map(|(i, (ph, _))| { + if p < ph.len() { + return None; + } + let potential_ph_start = p - ph.len(); + if text[potential_ph_start..p] == *ph { + Some((i, ph.clone())) + } else { + None + } + }) + { + self.textarea.replace_range(p - placeholder.len()..p, ""); + self.attached_images.remove(idx); + true + } else { + false } - if global_index == 0 { - return false; - } - let deletion_index = global_index - 1; // char that will be removed by backspace - - let text = lines.join("\n"); - - // Iterate over attached images; search each placeholder occurrence. - for idx in 0..self.attached_images.len() { - let (placeholder, _path) = &self.attached_images[idx]; - let ph_len = placeholder.len(); - let mut search_from = 0; - while let Some(rel_pos) = text[search_from..].find(placeholder) { - let ph_start = search_from + rel_pos; - let ph_end = ph_start + ph_len; // exclusive - if deletion_index >= ph_start && deletion_index < ph_end { - // Deletion inside this placeholder: remove entire placeholder. - let mut new_text = String::with_capacity(text.len() - ph_len); - new_text.push_str(&text[..ph_start]); - new_text.push_str(&text[ph_end..]); - - // Replace textarea contents. - self.textarea.select_all(); - self.textarea.cut(); - let _ = self.textarea.insert_str(new_text); - - // Remove attached image entry. - self.attached_images.remove(idx); - return true; - } - search_from = ph_start + ph_len; // continue searching for additional occurrences - } - } - false } /// Synchronize `self.command_popup` with the current text in the @@ -788,7 +774,7 @@ impl ChatComposer { } // Determine current query (may be empty if user just selected @file and hasn't typed yet). - let query_opt = Self::current_at_token_allow_empty(&self.textarea); + let query_opt = Self::current_at_token(&self.textarea); let Some(query) = query_opt else { // Token removed – end session. self.active_popup = ActivePopup::None; @@ -844,7 +830,7 @@ impl WidgetRef for &ChatComposer { let [textarea_rect, popup_rect] = Layout::vertical([Constraint::Min(0), Constraint::Max(popup_height)]).areas(area); match &self.active_popup { - ActivePopup::Command(popup) => { + ActivePopup::Slash(popup) => { popup.render_ref(popup_rect, buf); } ActivePopup::File(popup) => { @@ -940,57 +926,6 @@ impl WidgetRef for &ChatComposer { // the exact same definition of a *token* (whitespace-delimited) and *@token*. // ----------------------------------------------------------------------------- -/// Convert a cursor column expressed in characters (as provided by tui-textarea) -/// to a byte offset into `line`. -fn cursor_byte_offset(line: &str, cursor_col_chars: usize) -> usize { - line.chars() - .take(cursor_col_chars) - .map(|c| c.len_utf8()) - .sum() -} - -/// Return (start_byte, end_byte) of the token (whitespace-delimited) containing -/// `cursor_byte_offset`. Returns None if there is no non-empty token. -fn token_bounds(line: &str, cursor_byte_offset: usize) -> Option<(usize, usize)> { - if cursor_byte_offset > line.len() { - return None; - } - let before = &line[..cursor_byte_offset]; - let after = &line[cursor_byte_offset..]; - let start = before - .char_indices() - .rfind(|(_, c)| c.is_whitespace()) - .map(|(i, c)| i + c.len_utf8()) - .unwrap_or(0); - let end_rel = after - .char_indices() - .find(|(_, c)| c.is_whitespace()) - .map(|(i, _)| i) - .unwrap_or(after.len()); - let end = cursor_byte_offset + end_rel; - if start >= end { - None - } else { - Some((start, end)) - } -} - -/// Like `token_bounds` but ensures the token starts with '@'. If `allow_empty` -/// is false, requires at least one character after '@'. Returns byte bounds. -fn at_token_bounds( - line: &str, - cursor_byte_offset: usize, - allow_empty: bool, -) -> Option<(usize, usize)> { - let (start, end) = token_bounds(line, cursor_byte_offset)?; - let token = &line[start..end]; - if token.starts_with('@') && (allow_empty || token.len() > 1) { - Some((start, end)) - } else { - None - } -} - #[cfg(test)] mod tests { use super::ActivePopup; @@ -1543,7 +1478,7 @@ mod tests { } #[test] - fn image_placeholder_removed_on_backspace_anywhere() { + fn image_placeholder_backspace_behaves_like_text_placeholder() { use crossterm::event::KeyCode; use crossterm::event::KeyEvent; use crossterm::event::KeyModifiers; @@ -1555,22 +1490,25 @@ mod tests { let placeholder = composer.attached_images[0].0.clone(); // Case 1: backspace at end - composer.textarea.move_cursor(tui_textarea::CursorMove::End); + composer.textarea.move_cursor_to_end_of_line(false); composer.handle_key_event(KeyEvent::new(KeyCode::Backspace, KeyModifiers::NONE)); - assert!(!composer.textarea.lines().join("\n").contains(&placeholder)); + assert!(!composer.textarea.text().contains(&placeholder)); assert!(composer.attached_images.is_empty()); - // Re-add and test backspace in middle + // Re-add and test backspace in middle: should break the placeholder string + // and drop the image mapping (same as text placeholder behavior). assert!(composer.attach_image(path.clone(), 20, 10, "PNG")); let placeholder2 = composer.attached_images[0].0.clone(); // Move cursor to roughly middle of placeholder - let mid = (placeholder2.len() / 2) as u16; - composer - .textarea - .move_cursor(tui_textarea::CursorMove::Jump(0, mid)); - composer.handle_key_event(KeyEvent::new(KeyCode::Backspace, KeyModifiers::NONE)); - assert!(!composer.textarea.lines().join("\n").contains(&placeholder2)); - assert!(composer.attached_images.is_empty()); + if let Some(start_pos) = composer.textarea.text().find(&placeholder2) { + let mid_pos = start_pos + (placeholder2.len() / 2); + composer.textarea.set_cursor(mid_pos); + composer.handle_key_event(KeyEvent::new(KeyCode::Backspace, KeyModifiers::NONE)); + assert!(!composer.textarea.text().contains(&placeholder2)); + assert!(composer.attached_images.is_empty()); + } else { + panic!("Placeholder not found in textarea"); + } } #[test] diff --git a/codex-rs/tui/src/bottom_pane/command_popup.rs b/codex-rs/tui/src/bottom_pane/command_popup.rs index d3093b0146..3a3a6055ea 100644 --- a/codex-rs/tui/src/bottom_pane/command_popup.rs +++ b/codex-rs/tui/src/bottom_pane/command_popup.rs @@ -11,16 +11,14 @@ use crate::slash_command::built_in_slash_commands; use codex_common::fuzzy_match::fuzzy_match; pub(crate) struct CommandPopup { - prefix: char, command_filter: String, all_commands: Vec<(&'static str, SlashCommand)>, state: ScrollState, } impl CommandPopup { - pub(crate) fn new(prefix: char, all_commands: Vec<(&'static str, SlashCommand)>) -> Self { + pub(crate) fn new(_all_commands: Vec<(&'static str, SlashCommand)>) -> Self { Self { - prefix, command_filter: String::new(), all_commands: built_in_slash_commands(), state: ScrollState::new(), @@ -115,7 +113,7 @@ impl CommandPopup { impl CommandPopup { pub(crate) fn slash() -> Self { - CommandPopup::new('/', built_in_slash_commands()) + CommandPopup::new(built_in_slash_commands()) } } @@ -145,7 +143,7 @@ mod tests { #[test] fn filter_includes_init_when_typing_prefix() { - let mut popup = CommandPopup::new('/', built_in_slash_commands()); + let mut popup = CommandPopup::new(built_in_slash_commands()); // Simulate the composer line starting with '/in' so the popup filters // matching commands by prefix. popup.on_composer_text_change("/in".to_string()); @@ -161,7 +159,7 @@ mod tests { #[test] fn selecting_init_by_exact_match() { - let mut popup = CommandPopup::new('/', built_in_slash_commands()); + let mut popup = CommandPopup::new(built_in_slash_commands()); popup.on_composer_text_change("/init".to_string()); // When an exact match exists, the selected command should be that diff --git a/codex-rs/tui/src/bottom_pane/file_search_popup.rs b/codex-rs/tui/src/bottom_pane/file_search_popup.rs index e1c22cd767..f7ee2f6eeb 100644 --- a/codex-rs/tui/src/bottom_pane/file_search_popup.rs +++ b/codex-rs/tui/src/bottom_pane/file_search_popup.rs @@ -33,7 +33,9 @@ impl FileSearchPopup { waiting: true, matches: Vec::new(), state: ScrollState::new(), - } + }; + popup.populate_current_dir_if_empty_query(); + popup } /// Update the query and reset state to *waiting*. @@ -130,7 +132,7 @@ impl FileSearchPopup { } let mut entries: Vec = Vec::new(); if let Ok(read_dir) = fs::read_dir(".") { - for entry in read_dir.flatten().take(MAX_RESULTS) { + for entry in read_dir.flatten().take(MAX_POPUP_ROWS) { if let Ok(file_type) = entry.file_type() { // Skip hidden files (dotfiles) for a cleaner popup. let file_name = entry.file_name(); @@ -150,11 +152,9 @@ impl FileSearchPopup { } } self.matches = entries; - self.selected_idx = if self.matches.is_empty() { - None - } else { - Some(0) - }; + self.state.clamp_selection(self.matches.len()); + self.state + .ensure_visible(self.matches.len(), self.matches.len().min(MAX_POPUP_ROWS)); } } diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 47c14a59a3..8c533be4dd 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -270,10 +270,6 @@ impl ChatWidget<'_> { let _ = (width, height, format_label); // reserved for future use (e.g., status flash) self.bottom_pane .attach_image(path.clone(), width, height, format_label); - // Surface a quick background event so user sees confirmation. - self.add_to_history(HistoryCell::new_background_event(format!( - "[image copied] {width}x{height} {format_label}" - ))); self.request_redraw(); }