diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 982f3fd22b..53fecc685c 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -1273,9 +1273,12 @@ See the Codex keymap documentation for supported actions and examples." match &event { TuiEvent::Resize => { self.chat_widget.cancel_owned_screen_split_drag(); + self.cancel_owned_screen_selection(); } TuiEvent::FocusLost => { - if self.chat_widget.cancel_owned_screen_split_drag() { + let split_canceled = self.chat_widget.cancel_owned_screen_split_drag(); + let selection_canceled = self.cancel_owned_screen_selection(); + if split_canceled || selection_canceled { tui.frame_requester().schedule_frame(); } } @@ -1290,7 +1293,9 @@ See the Codex keymap documentation for supported actions and examples." } if self.overlay.is_some() { - if self.chat_widget.cancel_owned_screen_split_drag() { + let split_canceled = self.chat_widget.cancel_owned_screen_split_drag(); + let selection_canceled = self.cancel_owned_screen_selection(); + if split_canceled || selection_canceled { tui.frame_requester().schedule_frame(); } let _ = self.handle_backtrack_overlay_event(tui, event).await?; diff --git a/codex-rs/tui/src/app/conversation_panes.rs b/codex-rs/tui/src/app/conversation_panes.rs index b1bc48152f..6ffff42ea5 100644 --- a/codex-rs/tui/src/app/conversation_panes.rs +++ b/codex-rs/tui/src/app/conversation_panes.rs @@ -130,6 +130,7 @@ pub(crate) struct ConversationPanes { focused: PaneSlot, dispatch: Option, owned_screen_split: OwnedScreenSplitState, + selection_clipboard_lease: Option, } // Rendering and `/side` lifecycle consume the remaining APIs in the next stack layers. @@ -142,6 +143,7 @@ impl ConversationPanes { focused: PaneSlot::Parent, dispatch: None, owned_screen_split: OwnedScreenSplitState::default(), + selection_clipboard_lease: None, }) } @@ -213,6 +215,15 @@ impl ConversationPanes { self.owned_screen_split.cancel_drag() } + pub(super) fn retain_selection_clipboard_lease( + &mut self, + lease: Option, + ) { + if lease.is_some() { + self.selection_clipboard_lease = lease; + } + } + pub(super) fn by_slot(&self, slot: PaneSlot) -> Option<&ConversationPane> { match slot { PaneSlot::Parent => Some(&self.parent), diff --git a/codex-rs/tui/src/app/conversation_panes_tests.rs b/codex-rs/tui/src/app/conversation_panes_tests.rs index d327f557ba..0b3f5b1c28 100644 --- a/codex-rs/tui/src/app/conversation_panes_tests.rs +++ b/codex-rs/tui/src/app/conversation_panes_tests.rs @@ -97,6 +97,21 @@ async fn taking_focused_side_restores_parent_selection() { ); } +#[tokio::test] +async fn selection_clipboard_lease_survives_side_removal() { + let (parent, _parent_rx) = pane_init(PaneSlot::Parent).await; + let (side, _side_rx) = pane_init(PaneSlot::Side).await; + let Ok(mut panes) = ConversationPanes::new_parent(parent) else { + panic!("parent pane should install"); + }; + assert!(panes.install_side(side).is_ok()); + panes.retain_selection_clipboard_lease(Some(crate::clipboard_copy::ClipboardLease::test())); + + drop(panes.take_side().expect("side pane")); + + assert!(panes.selection_clipboard_lease.is_some()); +} + #[tokio::test] async fn only_focused_pane_owns_terminal_title_output() { let (parent, _parent_rx) = pane_init(PaneSlot::Parent).await; diff --git a/codex-rs/tui/src/app/owned_screen.rs b/codex-rs/tui/src/app/owned_screen.rs index 0d81af1805..f40411755a 100644 --- a/codex-rs/tui/src/app/owned_screen.rs +++ b/codex-rs/tui/src/app/owned_screen.rs @@ -116,6 +116,9 @@ impl OwnedScreen { } fn handle_mouse_scroll(&mut self, event: MouseScrollEvent) -> bool { + if self.viewport.selection_is_active() { + return true; + } if !self .last_conversation_area .contains(Position::new(event.column, event.row)) @@ -126,6 +129,29 @@ impl OwnedScreen { true } + fn begin_selection(&mut self, position: Position) -> bool { + self.viewport + .begin_selection(self.last_conversation_area, position) + } + + fn update_selection(&mut self, position: Position) -> bool { + self.viewport + .update_selection(self.last_conversation_area, position) + } + + fn finish_selection(&mut self, position: Position) -> Option { + self.viewport + .finish_selection(self.last_conversation_area, position) + } + + fn cancel_selection(&mut self) { + self.viewport.cancel_selection(); + } + + fn selection_is_active(&self) -> bool { + self.viewport.selection_is_active() + } + fn clear_last_render_areas(&mut self) { self.last_pane_area = Rect::default(); self.last_conversation_area = Rect::default(); @@ -220,10 +246,17 @@ fn render_layout( let divider_active = panes.owned_screen_split_is_dragging(); Clear.render(layout.area(), buffer); for slot in [PaneSlot::Parent, PaneSlot::Side] { + let slot_is_visible = match layout { + OwnedScreenLayout::Single { slot: visible, .. } => slot == visible, + OwnedScreenLayout::Split { .. } => true, + }; if let Some(screen) = panes .by_slot_mut(slot) .and_then(|pane| pane.owned_screen.as_mut()) { + if !slot_is_visible { + screen.cancel_selection(); + } screen.clear_last_render_areas(); } } @@ -333,7 +366,13 @@ impl App { tui: &mut tui::Tui, event: MouseScrollEvent, ) -> bool { - if self.chat_widget.owned_screen_split_is_dragging() { + let selection_is_active = [PaneSlot::Parent, PaneSlot::Side].into_iter().any(|slot| { + self.chat_widget + .by_slot(slot) + .and_then(|pane| pane.owned_screen.as_ref()) + .is_some_and(OwnedScreen::selection_is_active) + }); + if self.chat_widget.owned_screen_split_is_dragging() || selection_is_active { return true; } for slot in [PaneSlot::Parent, PaneSlot::Side] { @@ -358,17 +397,17 @@ impl App { tui: &mut tui::Tui, event: MousePrimaryEvent, ) -> bool { - if self.overlay.is_some() - || !self.chat_widget.has_side() - || !self.chat_widget.no_modal_or_popup_active() - { - if self.chat_widget.cancel_owned_screen_split_drag() { + if self.overlay.is_some() || !self.chat_widget.no_modal_or_popup_active() { + let split_canceled = self.chat_widget.cancel_owned_screen_split_drag(); + let selection_canceled = self.cancel_owned_screen_selection(); + if split_canceled || selection_canceled { tui.frame_requester().schedule_frame(); } return false; } if self.chat_widget.handle_owned_screen_split_mouse(event) { + self.cancel_owned_screen_selection(); match event.kind { MousePrimaryEventKind::Drag => tui .frame_requester() @@ -379,11 +418,51 @@ impl App { } return true; } + let position = Position::new(event.column, event.row); if event.kind != MousePrimaryEventKind::Press { - return false; + let active_slot = [PaneSlot::Parent, PaneSlot::Side].into_iter().find(|slot| { + self.chat_widget + .by_slot(*slot) + .and_then(|pane| pane.owned_screen.as_ref()) + .is_some_and(OwnedScreen::selection_is_active) + }); + let Some(active_slot) = active_slot else { + return false; + }; + let mut copied = None; + let handled = self + .chat_widget + .by_slot_mut(active_slot) + .and_then(|pane| pane.owned_screen.as_mut()) + .is_some_and(|screen| match event.kind { + MousePrimaryEventKind::Drag => screen.update_selection(position), + MousePrimaryEventKind::Release => { + copied = screen.finish_selection(position); + true + } + MousePrimaryEventKind::Press => false, + }); + if let Some(text) = copied + && let Some(pane) = self.chat_widget.by_slot_mut(active_slot) + { + pane.chat_widget.copy_selected_text(&text); + let lease = pane.chat_widget.take_clipboard_lease(); + self.chat_widget.retain_selection_clipboard_lease(lease); + } + if handled { + match event.kind { + MousePrimaryEventKind::Drag => tui + .frame_requester() + .schedule_frame_in(crate::tui::TARGET_FRAME_INTERVAL), + MousePrimaryEventKind::Press | MousePrimaryEventKind::Release => { + tui.frame_requester().schedule_frame(); + } + } + } + return handled; } - let position = Position::new(event.column, event.row); + let selection_canceled = self.cancel_owned_screen_selection(); let target = [PaneSlot::Parent, PaneSlot::Side].into_iter().find(|slot| { self.chat_widget .by_slot(*slot) @@ -391,19 +470,48 @@ impl App { .is_some_and(|screen| screen.last_pane_area.contains(position)) }); let Some(target) = target else { + if selection_canceled { + tui.frame_requester().schedule_frame(); + } return false; }; + let selection_started = self.chat_widget.by_slot_mut(target).is_some_and(|pane| { + pane.chat_widget.no_modal_or_popup_active() + && pane + .owned_screen + .as_mut() + .is_some_and(|screen| screen.begin_selection(position)) + }); let focus_changed = self.chat_widget.focused_slot() != target; let backtrack_was_primed = self.backtrack.primed; if !self.focus_conversation_pane(target) { + if selection_canceled { + tui.frame_requester().schedule_frame(); + } return false; } - if focus_changed || backtrack_was_primed { + if selection_started || selection_canceled || focus_changed || backtrack_was_primed { tui.frame_requester().schedule_frame(); } true } + pub(super) fn cancel_owned_screen_selection(&mut self) -> bool { + let mut canceled = false; + for slot in [PaneSlot::Parent, PaneSlot::Side] { + if let Some(screen) = self + .chat_widget + .by_slot_mut(slot) + .and_then(|pane| pane.owned_screen.as_mut()) + && screen.selection_is_active() + { + screen.cancel_selection(); + canceled = true; + } + } + canceled + } + pub(crate) fn sync_owned_screen_cells(&mut self) { let cells = self.chat_widget.transcript_cells.clone(); if let Some(screen) = &mut self.chat_widget.owned_screen { @@ -453,6 +561,7 @@ impl App { } if !self.chat_widget.no_modal_or_popup_active() { self.chat_widget.cancel_owned_screen_split_drag(); + self.cancel_owned_screen_selection(); } let focused = self.chat_widget.focused_slot(); let has_side = self.chat_widget.has_side(); diff --git a/codex-rs/tui/src/app/owned_screen_tests.rs b/codex-rs/tui/src/app/owned_screen_tests.rs index 31a7ceb765..bc013a1855 100644 --- a/codex-rs/tui/src/app/owned_screen_tests.rs +++ b/codex-rs/tui/src/app/owned_screen_tests.rs @@ -385,6 +385,166 @@ async fn primary_press_focuses_closed_parent_without_enabling_input() { ); } +#[tokio::test] +async fn primary_drag_selects_text_in_a_single_owned_pane() { + let mut app = app_with_owned_parent().await; + seed_pane(&mut app, PaneSlot::Parent, "", &["selectable"]); + let _terminal = render_app(&mut app, /*width*/ 40, /*height*/ 10); + let mut tui = crate::tui::test_support::make_test_tui().expect("create test TUI"); + + assert!( + app.handle_owned_screen_mouse_primary(&mut tui, primary_press(/*column*/ 0, /*row*/ 0),) + ); + assert!(app.handle_owned_screen_mouse_primary( + &mut tui, + primary_event( + MousePrimaryEventKind::Drag, + /*column*/ 4, + /*row*/ 0, + ), + )); + assert!( + app.chat_widget + .owned_screen + .as_ref() + .expect("parent owned screen") + .selection_is_active() + ); + + let selected = render_app(&mut app, /*width*/ 40, /*height*/ 10); + for column in 0..=4 { + assert!( + selected.backend().buffer()[(column, 0)] + .modifier + .contains(ratatui::style::Modifier::REVERSED), + "column {column} should be highlighted" + ); + } +} + +#[tokio::test] +async fn click_release_clears_selection_without_copying() { + let mut app = app_with_owned_parent().await; + seed_pane(&mut app, PaneSlot::Parent, "", &["selectable"]); + let _terminal = render_app(&mut app, /*width*/ 40, /*height*/ 10); + let mut tui = crate::tui::test_support::make_test_tui().expect("create test TUI"); + + assert!( + app.handle_owned_screen_mouse_primary(&mut tui, primary_press(/*column*/ 0, /*row*/ 0),) + ); + assert!(app.handle_owned_screen_mouse_primary( + &mut tui, + primary_event( + MousePrimaryEventKind::Release, + /*column*/ 0, + /*row*/ 0, + ), + )); + assert!( + !app.chat_widget + .owned_screen + .as_ref() + .expect("parent owned screen") + .selection_is_active() + ); +} + +#[tokio::test] +async fn text_drag_crosses_divider_but_divider_press_takes_priority() { + let mut app = app_with_owned_side().await; + seed_pane(&mut app, PaneSlot::Parent, "", &["parent selectable"]); + seed_pane(&mut app, PaneSlot::Side, "", &["side selectable"]); + let _terminal = render_app(&mut app, /*width*/ 120, /*height*/ 12); + let parent = app + .chat_widget + .by_slot(PaneSlot::Parent) + .and_then(|pane| pane.owned_screen.as_ref()) + .expect("parent screen"); + let conversation = parent.last_conversation_area; + let divider_column = parent.last_pane_area.right(); + let mut tui = crate::tui::test_support::make_test_tui().expect("create test TUI"); + + assert!(app.handle_owned_screen_mouse_primary( + &mut tui, + primary_press(conversation.x, conversation.y), + )); + assert!(app.handle_owned_screen_mouse_primary( + &mut tui, + primary_event( + MousePrimaryEventKind::Drag, + divider_column.saturating_add(/*rhs*/ 8), + conversation.y, + ), + )); + assert!(!app.chat_widget.owned_screen_split_is_dragging()); + assert!( + app.chat_widget + .owned_screen + .as_ref() + .expect("parent owned screen") + .selection_is_active() + ); + + assert!(app.handle_owned_screen_mouse_primary( + &mut tui, + primary_press(divider_column, conversation.y), + )); + assert!(app.chat_widget.owned_screen_split_is_dragging()); + assert!( + !app.chat_widget + .owned_screen + .as_ref() + .expect("parent owned screen") + .selection_is_active() + ); + assert!(app.handle_owned_screen_mouse_primary( + &mut tui, + primary_event( + MousePrimaryEventKind::Release, + divider_column, + conversation.y, + ), + )); +} + +#[tokio::test] +async fn focused_only_layout_cancels_selection_in_the_hidden_pane() { + let mut app = app_with_owned_side().await; + seed_pane(&mut app, PaneSlot::Parent, "", &["parent selectable"]); + seed_pane(&mut app, PaneSlot::Side, "", &["side selectable"]); + let _wide = render_app(&mut app, /*width*/ 83, /*height*/ 12); + let conversation = app + .chat_widget + .by_slot(PaneSlot::Parent) + .and_then(|pane| pane.owned_screen.as_ref()) + .expect("parent screen") + .last_conversation_area; + let mut tui = crate::tui::test_support::make_test_tui().expect("create test TUI"); + assert!(app.handle_owned_screen_mouse_primary( + &mut tui, + primary_press(conversation.x, conversation.y), + )); + assert!(app.handle_owned_screen_mouse_primary( + &mut tui, + primary_event( + MousePrimaryEventKind::Drag, + conversation.x.saturating_add(/*rhs*/ 4), + conversation.y, + ), + )); + + assert!(app.chat_widget.focus(PaneSlot::Side)); + let _narrow = render_app(&mut app, /*width*/ 82, /*height*/ 12); + + assert!( + !app.chat_widget + .by_slot(PaneSlot::Parent) + .and_then(|pane| pane.owned_screen.as_ref()) + .expect("parent screen") + .selection_is_active() + ); +} + #[tokio::test] async fn narrow_layout_renders_only_the_focused_side() { let mut app = app_with_owned_side().await; diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index e568436b40..ab6d8264e7 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -1181,7 +1181,6 @@ impl ChatComposer { urls } - #[cfg(test)] pub(crate) fn show_footer_flash(&mut self, line: Line<'static>, duration: Duration) { self.footer.show_flash(line, duration); } diff --git a/codex-rs/tui/src/bottom_pane/chat_composer/footer_state.rs b/codex-rs/tui/src/bottom_pane/chat_composer/footer_state.rs index 113385ff72..dc0da7c97b 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer/footer_state.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer/footer_state.rs @@ -1,5 +1,6 @@ //! Footer and status-row presentation state for the chat composer. +use std::time::Duration; use std::time::Instant; use ratatui::text::Line; @@ -8,8 +9,6 @@ use crate::bottom_pane::footer::CollaborationModeIndicator; use crate::bottom_pane::footer::FooterMode; use crate::bottom_pane::footer::GoalStatusIndicator; use crate::key_hint::KeyBinding; -#[cfg(test)] -use std::time::Duration; pub(super) struct FooterState { pub(super) quit_shortcut_expires_at: Option, @@ -53,7 +52,6 @@ impl FooterState { .is_some_and(|flash| Instant::now() < flash.expires_at) } - #[cfg(test)] pub(super) fn show_flash(&mut self, line: Line<'static>, duration: Duration) { let expires_at = Instant::now() .checked_add(duration) diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index ad30dedf5a..cdac4f9ca6 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -873,6 +873,12 @@ impl BottomPane { self.request_redraw(); } + pub(crate) fn show_footer_flash(&mut self, line: Line<'static>, duration: Duration) { + self.composer.show_footer_flash(line, duration); + self.request_redraw(); + self.request_redraw_in(duration); + } + /// Applies the externally decided Plan-mode nudge visibility to the footer presentation. pub(crate) fn set_plan_mode_nudge_visible(&mut self, visible: bool) { if self.composer.set_plan_mode_nudge_visible(visible) { diff --git a/codex-rs/tui/src/chatwidget/interaction.rs b/codex-rs/tui/src/chatwidget/interaction.rs index ba0aeb0a91..d10a1570bd 100644 --- a/codex-rs/tui/src/chatwidget/interaction.rs +++ b/codex-rs/tui/src/chatwidget/interaction.rs @@ -255,6 +255,36 @@ impl ChatWidget { self.copy_last_agent_markdown_with(crate::clipboard_copy::copy_to_clipboard); } + pub(crate) fn copy_selected_text(&mut self, text: &str) { + self.copy_selected_text_with(text, crate::clipboard_copy::copy_to_clipboard); + } + + pub(crate) fn take_clipboard_lease(&mut self) -> Option { + self.clipboard_lease.take() + } + + pub(super) fn copy_selected_text_with( + &mut self, + text: &str, + copy_fn: impl FnOnce(&str) -> Result, String>, + ) { + match copy_fn(text) { + Ok(lease) => { + self.clipboard_lease = lease; + self.bottom_pane.show_footer_flash( + Line::from("Copied selection").green(), + Duration::from_secs(/*secs*/ 2), + ); + } + Err(error) => { + self.add_to_history(history_cell::new_error_event(format!( + "Copy failed: {error}" + ))); + } + } + self.request_redraw(); + } + pub(crate) fn truncate_agent_copy_history_to_user_turn_count( &mut self, user_turn_count: usize, diff --git a/codex-rs/tui/src/chatwidget/tests/slash_commands.rs b/codex-rs/tui/src/chatwidget/tests/slash_commands.rs index d8b3f8a0cd..c7cfc219c9 100644 --- a/codex-rs/tui/src/chatwidget/tests/slash_commands.rs +++ b/codex-rs/tui/src/chatwidget/tests/slash_commands.rs @@ -1943,6 +1943,47 @@ async fn slash_copy_stores_clipboard_lease_and_preserves_it_on_failure() { ); } +#[tokio::test] +async fn selection_copy_is_exact_shows_footer_feedback_and_preserves_lease_on_failure() { + use ratatui::Terminal; + use ratatui::backend::TestBackend; + + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + let calls = std::cell::Cell::new(0); + chat.copy_selected_text_with("source-backed\ntext", |text| { + calls.set(calls.get() + 1); + assert_eq!(text, "source-backed\ntext"); + Ok(Some(crate::clipboard_copy::ClipboardLease::test())) + }); + + assert_eq!(calls.get(), 1); + assert!(chat.clipboard_lease.is_some()); + assert!( + drain_insert_history(&mut rx).is_empty(), + "successful selection copy should use transient footer feedback" + ); + + let bottom_pane = chat.bottom_pane_renderable(); + let width = 48; + let height = bottom_pane.desired_height(width); + let mut terminal = Terminal::new(TestBackend::new(width, height)).expect("create terminal"); + terminal + .draw(|frame| bottom_pane.render(frame.area(), frame.buffer_mut())) + .expect("render copied selection feedback"); + assert_snapshot!(normalized_backend_snapshot(terminal.backend())); + drop(bottom_pane); + + chat.copy_selected_text_with("source-backed\ntext", |text| { + assert_eq!(text, "source-backed\ntext"); + Err("blocked".to_string()) + }); + + assert!(chat.clipboard_lease.is_some()); + let cells = drain_insert_history(&mut rx); + assert_eq!(cells.len(), 1, "expected one failure message"); + assert!(lines_to_single_string(&cells[0]).contains("Copy failed: blocked")); +} + #[tokio::test] async fn slash_copy_state_is_preserved_during_running_task() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; diff --git a/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__slash_commands__selection_copy_is_exact_shows_footer_feedback_and_preserves_lease_on_failure.snap b/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__slash_commands__selection_copy_is_exact_shows_footer_feedback_and_preserves_lease_on_failure.snap new file mode 100644 index 0000000000..be84b6f3ba --- /dev/null +++ b/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__slash_commands__selection_copy_is_exact_shows_footer_feedback_and_preserves_lease_on_failure.snap @@ -0,0 +1,9 @@ +--- +source: tui/src/chatwidget/tests/slash_commands.rs +expression: normalized_backend_snapshot(terminal.backend()) +--- +" " +" " +"› Ask Codex to do anything " +" " +" Copied selection " diff --git a/codex-rs/tui/src/clipboard_copy.rs b/codex-rs/tui/src/clipboard_copy.rs index 1ad369d24b..b26424df59 100644 --- a/codex-rs/tui/src/clipboard_copy.rs +++ b/codex-rs/tui/src/clipboard_copy.rs @@ -56,10 +56,9 @@ pub(crate) fn copy_to_clipboard(text: &str) -> Result, St /// /// On Linux/X11 and some Wayland compositors, clipboard contents are served by the /// owning process. Dropping the `arboard::Clipboard` before the user pastes causes -/// the content to vanish. Store this lease on the widget that triggered the copy so -/// the handle lives as long as the TUI does. On non-Linux native paths and OSC 52 -/// paths the lease is `None` — those backends do not require process-lifetime -/// ownership. +/// the content to vanish. Store this lease on an owner that lives as long as the TUI; an +/// individual conversation pane may be removed before the user pastes. On non-Linux native paths +/// and OSC 52 paths the lease is `None` — those backends do not require process-lifetime ownership. pub(crate) struct ClipboardLease { #[cfg(target_os = "linux")] _clipboard: Option,