From 7627449f219c315219a1553c9771b9ca23473205 Mon Sep 17 00:00:00 2001 From: Daniel Edrisian Date: Wed, 20 Aug 2025 14:18:29 -0700 Subject: [PATCH] wip 1 --- codex-rs/tui/src/app.rs | 34 +++- codex-rs/tui/src/bottom_pane/chat_composer.rs | 8 + codex-rs/tui/src/bottom_pane/mod.rs | 9 + codex-rs/tui/src/chatwidget.rs | 103 ++++++++++- codex-rs/tui/src/history_cell.rs | 5 + codex-rs/tui/src/string_utils.rs | 171 ++++++++++++++++-- 6 files changed, 301 insertions(+), 29 deletions(-) diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index db54193148..d00a5cb2c1 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -48,11 +48,21 @@ where crate::clipboard_paste::PasteImageError, >, { - // Treat both Ctrl+V and Cmd+V (SUPER on macOS) as the "paste image" hotkey. + // Treat Ctrl+V as paste everywhere; treat Cmd+V (SUPER) as paste only on macOS. let is_v = matches!(key_event.code, KeyCode::Char('v')); let mods = key_event.modifiers; - let has_paste_modifier = mods.contains(crossterm::event::KeyModifiers::CONTROL) - || mods.contains(crossterm::event::KeyModifiers::SUPER); + let has_cmd_on_macos = { + #[cfg(target_os = "macos")] + { + mods.contains(crossterm::event::KeyModifiers::SUPER) + } + #[cfg(not(target_os = "macos"))] + { + false + } + }; + let has_paste_modifier = + mods.contains(crossterm::event::KeyModifiers::CONTROL) || has_cmd_on_macos; if key_event.kind == KeyEventKind::Press && is_v && has_paste_modifier { match paste_fn() { @@ -799,6 +809,7 @@ mod tests { } #[test] + #[cfg(target_os = "macos")] fn cmd_v_success_attaches_image() { let (tx, rx) = std::sync::mpsc::channel(); let sender = AppEventSender::new(tx); @@ -834,6 +845,23 @@ mod tests { } } + #[test] + #[cfg(not(target_os = "macos"))] + fn super_v_not_handled_on_non_macos() { + let (tx, rx) = std::sync::mpsc::channel(); + let sender = AppEventSender::new(tx); + let key_event = KeyEvent::new(KeyCode::Char('v'), KeyModifiers::SUPER); + let handled = try_handle_ctrl_v_with(&sender, &key_event, || { + Err(crate::clipboard_paste::PasteImageError::NoImage( + "none".into(), + )) + }); + assert!( + !handled, + "SUPER should not be treated as paste on non-macOS" + ); + assert!(rx.try_recv().is_err()); + } #[test] fn ctrl_v_failure_not_consumed() { let (tx, rx) = std::sync::mpsc::channel(); diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index dde690878a..cec2f580fb 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -199,6 +199,14 @@ impl ChatComposer { true } + pub fn current_text(&self) -> String { + self.textarea.text().to_string() + } + + pub fn set_text(&mut self, text: &str) { + self.textarea.set_text(text); + } + pub fn attach_image( &mut self, path: std::path::PathBuf, diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index a212cba99a..2fd69842a8 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -185,6 +185,15 @@ impl BottomPane<'_> { } } + pub(crate) fn current_input_text(&self) -> String { + self.composer.current_text() + } + + pub(crate) fn replace_input_text(&mut self, text: &str) { + self.composer.set_text(text); + self.request_redraw(); + } + pub(crate) fn ctrl_c_quit_hint_visible(&self) -> bool { self.ctrl_c_quit_hint } diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 7a66a14aa9..3893e05d57 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -545,16 +545,44 @@ impl ChatWidget<'_> { match self.bottom_pane.handle_key_event(key_event) { InputResult::Submitted(text) => { - let images = self + let mut images = self .bottom_pane .take_recent_submission_images_with_placeholders(); let display = self .bottom_pane .take_last_submitted_display() .unwrap_or_else(|| text.clone()); + // If there are no images yet, and the submitted text looks like + // a single quoted path or file:// URL to a local image, treat + // it as an image attachment instead of plain text. + if images.is_empty() { + if let Some((path, width, height, fmt)) = + Self::try_interpret_as_single_image_path(&text) + { + // Attach the detected image and clear the agent text. + self.attach_image(path.clone(), width, height, &fmt); + images.push((String::new(), path)); + // Keep display unchanged so the user sees what they typed. + self.submit_user_message_with_display(String::new(), images, display); + return; + } + } + self.submit_user_message_with_display(text, images, display); } - InputResult::None => {} + InputResult::None => { + // Inline detection: if the current input exactly matches a single + // quoted path or file:// URL to a local PNG/JPEG, convert it + // immediately into an image attachment. + let current = self.bottom_pane.current_input_text(); + if let Some((path, width, height, fmt)) = + Self::try_interpret_as_single_image_path(¤t) + { + // Clear the typed path and insert the image placeholder. + self.bottom_pane.replace_input_text(""); + self.attach_image(path, width, height, &fmt); + } + } } } @@ -579,6 +607,65 @@ impl ChatWidget<'_> { self.request_redraw(); } + // Heuristic: interpret the entire submitted text as a single image path/URL + // when it is quoted or looks like a standalone path. Returns (path, w, h, fmt) + // if it resolves to a local PNG/JPEG image. + fn try_interpret_as_single_image_path( + text: &str, + ) -> Option<(std::path::PathBuf, u32, u32, String)> { + let s = text.trim(); + if s.is_empty() || s.contains('\n') { + return None; + } + + // Strip surrounding quotes if present. + let mut candidate = s.to_string(); + if candidate.len() >= 2 + && ((candidate.starts_with('"') && candidate.ends_with('"')) + || (candidate.starts_with('\'') && candidate.ends_with('\''))) + { + candidate = candidate[1..candidate.len() - 1].to_string(); + } + + // Expand ~/ to $HOME + if let Some(rest) = candidate.strip_prefix("~/") { + if let Ok(home) = std::env::var("HOME") { + let mut p = std::path::PathBuf::from(home); + p.push(rest); + candidate = p.to_string_lossy().into_owned(); + } + } + + // Consider multiple representations + let mut try_paths: Vec = Vec::new(); + if let Some(p) = file_url_to_path(&candidate) { + try_paths.push(p); + } + try_paths.push(std::path::PathBuf::from(&candidate)); + let unescaped = unescape_backslashes(&candidate); + if unescaped != candidate { + try_paths.push(std::path::PathBuf::from(unescaped)); + } + + for path in try_paths { + if path.is_file() { + if let Some(ext) = path.extension().and_then(|e| e.to_str()) { + let ext_l = ext.to_ascii_lowercase(); + if matches!(ext_l.as_str(), "png" | "jpg" | "jpeg") { + let (mut w, mut h) = (0u32, 0u32); + if let Ok((dw, dh)) = image::image_dimensions(&path) { + w = dw; + h = dh; + } + let fmt = if ext_l == "png" { "PNG" } else { "JPEG" }.to_string(); + return Some((path, w, h, fmt)); + } + } + } + } + None + } + pub(crate) fn handle_paste(&mut self, text: String) { // First, attempt to interpret the pasted text as a file path to an image // and attach it. This mirrors the logic previously handled at the app level. @@ -607,8 +694,9 @@ impl ChatWidget<'_> { // Trim and strip quotes for the most direct case. let mut s = text.trim().to_string(); if !s.is_empty() { - if (s.starts_with('"') && s.ends_with('"')) - || (s.starts_with('\'') && s.ends_with('\'')) + if s.len() >= 2 + && ((s.starts_with('"') && s.ends_with('"')) + || (s.starts_with('\'') && s.ends_with('\''))) { s = s[1..s.len() - 1].to_string(); } @@ -633,8 +721,9 @@ impl ChatWidget<'_> { 'outer: for raw in candidates { let mut s = raw.trim().to_string(); - if (s.starts_with('"') && s.ends_with('"')) - || (s.starts_with('\'') && s.ends_with('\'')) + if s.len() >= 2 + && ((s.starts_with('"') && s.ends_with('"')) + || (s.starts_with('\'') && s.ends_with('\''))) { s = s[1..s.len() - 1].to_string(); } @@ -758,7 +847,7 @@ impl ChatWidget<'_> { }); } // Show the original display text (with inline image placeholders) in history. - self.add_to_history(&history_cell::new_user_prompt(display_text)); + self.add_to_history(&history_cell::new_user_prompt_with_images(display_text)); } pub(crate) fn handle_codex_event(&mut self, event: Event) { diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index d9ce9ad5e5..4e447c8b8c 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -194,6 +194,11 @@ pub(crate) fn new_user_prompt(message: String) -> PlainHistoryCell { PlainHistoryCell { lines } } +pub(crate) fn new_user_prompt_with_images(message: String) -> PlainHistoryCell { + // Currently identical to new_user_prompt; kept separate for clarity when images are present. + new_user_prompt(message) +} + pub(crate) fn new_active_exec_command( command: Vec, parsed: Vec, diff --git a/codex-rs/tui/src/string_utils.rs b/codex-rs/tui/src/string_utils.rs index c2891eca2d..73a82c4ce7 100644 --- a/codex-rs/tui/src/string_utils.rs +++ b/codex-rs/tui/src/string_utils.rs @@ -29,33 +29,166 @@ pub(crate) fn percent_decode_to_string(input: &str) -> Option { String::from_utf8(out).ok() } -// Convert a file:// URL into a local path (macOS/Unix only, UTF-8). +// Convert a file:// URL into a local path. +// Rules: +// - Accept only empty host or "localhost"; reject remote hosts. +// - On Unix, require absolute paths (leading '/'). +// - On Windows, support forms like file:///C:/path and file://localhost/C:/path. pub(crate) fn file_url_to_path(s: &str) -> Option { - if let Some(rest) = s.strip_prefix("file://") { - // Strip optional host like file://localhost/... - let rest = rest.strip_prefix("localhost").unwrap_or(rest); - let decoded = percent_decode_to_string(rest)?; - let p = std::path::PathBuf::from(decoded); - return Some(p); + let Some(mut rest) = s.strip_prefix("file://") else { + return None; + }; + + // Handle optional host (e.g., file://localhost/...). Only allow empty or localhost. + if let Some(after_host) = rest.strip_prefix("localhost") { + rest = after_host; + } else if rest.starts_with('/') { + // empty host, keep as-is + } else { + // Non-local host is not supported – reject. + return None; + } + + // Percent-decode the path portion + let decoded = percent_decode_to_string(rest)?; + + #[cfg(windows)] + { + // On Windows, URLs often look like file:///C:/path or file://localhost/C:/path + // If the decoded path starts with '/:/', strip the leading slash. + let path = if decoded.len() >= 4 + && decoded.as_bytes()[0] == b'/' + && decoded.as_bytes()[1].is_ascii_alphabetic() + && decoded.as_bytes()[2] == b':' + && decoded.as_bytes()[3] == b'/' + { + decoded[1..].to_string() + } else { + decoded + }; + return Some(std::path::PathBuf::from(path)); + } + + #[cfg(not(windows))] + { + // On Unix, require absolute path. + if !decoded.starts_with('/') { + return None; + } + Some(std::path::PathBuf::from(decoded)) } - None } // Unescape simple bash-style backslash escapes (e.g., spaces, parens). pub(crate) fn unescape_backslashes(s: &str) -> String { - let mut out = String::with_capacity(s.len()); - let mut chars = s.chars(); - while let Some(c) = chars.next() { - if c == '\\' { - if let Some(n) = chars.next() { - out.push(n); + // On Windows, do not unescape backslashes; they are core to paths like C:\Users. + #[cfg(windows)] + { + return s.to_string(); + } + + // On Unix, unescape common shell-escaped characters (e.g., spaces, parens, quotes). + #[cfg(not(windows))] + { + let mut out = String::with_capacity(s.len()); + let mut chars = s.chars(); + while let Some(c) = chars.next() { + if c == '\\' { + if let Some(n) = chars.next() { + // Only unescape a known set of characters typically escaped in shells. + let should_unescape = matches!( + n, + ' ' | '(' + | ')' + | '[' + | ']' + | '{' + | '}' + | '\\' + | '$' + | '&' + | '!' + | '#' + | ';' + | ':' + | '@' + | '=' + | '+' + | ',' + | '~' + | '|' + | '<' + | '>' + | '?' + | '*' + ); + if should_unescape { + out.push(n); + } else { + out.push('\\'); + out.push(n); + } + } else { + // Trailing backslash; keep it. + out.push('\\'); + } } else { - // Trailing backslash; keep it. - out.push('\\'); + out.push(c); } - } else { - out.push(c); } + out + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn percent_decode_basic() { + assert_eq!(percent_decode_to_string("/a%20b").as_deref(), Some("/a b")); + } + + #[test] + #[cfg(not(windows))] + fn file_url_localhost_unix() { + assert_eq!( + file_url_to_path("file://localhost//tmp/foo").unwrap(), + std::path::PathBuf::from("/tmp/foo") + ); + assert_eq!( + file_url_to_path("file:////tmp/foo").unwrap(), + std::path::PathBuf::from("/tmp/foo") + ); + assert!(file_url_to_path("file://host/tmp/foo").is_none()); + assert!(file_url_to_path("file://localhosttmp/foo").is_none()); + } + + #[test] + #[cfg(windows)] + fn file_url_windows_drive() { + assert_eq!( + file_url_to_path("file:///C:/Users/test").unwrap(), + std::path::PathBuf::from("C:/Users/test") + ); + assert_eq!( + file_url_to_path("file://localhost/C:/Users/test").unwrap(), + std::path::PathBuf::from("C:/Users/test") + ); + assert!(file_url_to_path("file://host/C:/Users/test").is_none()); + } + + #[test] + #[cfg(not(windows))] + fn unescape_backslashes_unix() { + assert_eq!(unescape_backslashes("My\\ File(1).png"), "My File(1).png"); + // Leave unknown escapes intact + assert_eq!(unescape_backslashes("abc\\z"), "abc\\z"); + } + + #[test] + #[cfg(windows)] + fn unescape_backslashes_windows_noop() { + assert_eq!(unescape_backslashes("C:\\Users\\test"), "C:\\Users\\test"); } - out }