From 356411dc67de8c8ae754542db77da4016b1218fe Mon Sep 17 00:00:00 2001 From: Kent Huang Date: Tue, 28 Jul 2026 11:45:44 +0800 Subject: [PATCH 1/3] feat: make long IDs compact and copyable --- Cargo.lock | 1 + README.md | 3 + crates/spacetop/Cargo.toml | 1 + crates/spacetop/src/app.rs | 70 +++++- crates/spacetop/src/app/keys.rs | 3 + crates/spacetop/src/app/mouse.rs | 287 +++++++++++++++++++++- crates/spacetop/src/app/overview.rs | 6 + crates/spacetop/src/lib.rs | 64 ++++- crates/spacetop/src/ui/footer.rs | 23 +- crates/spacetop/src/ui/help.rs | 1 + crates/spacetop/src/ui/list.rs | 108 ++++++-- crates/spacetop/src/ui/mod.rs | 17 +- crates/spacetop/src/ui/tests/chrome.rs | 1 + crates/spacetop/src/ui/tests/task_list.rs | 137 ++++++++++- 14 files changed, 678 insertions(+), 44 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 28ba00a6..452cb411 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2589,6 +2589,7 @@ name = "spacetop" version = "0.2.0" dependencies = [ "anyhow", + "base64", "clap", "crossterm 0.28.1", "ratatui", diff --git a/README.md b/README.md index f4e289a7..64002995 100644 --- a/README.md +++ b/README.md @@ -97,6 +97,9 @@ terminal never keeps swallowing clicks. - **Click** an entity row to select it and open the preview in one action. In the workflow picker, clicking a row opens that workflow. +- **Double-click an entity ID** to copy its full value, even when a long + slug is visually shortened with an ellipsis. This uses OSC 52 and requires + clipboard support from the terminal (or its multiplexer). - **Scroll wheel** scrolls the panel under the cursor: the preview body when hovering the preview, the list selection when hovering the list. - **Drag the divider** between the list and the preview to resize the diff --git a/crates/spacetop/Cargo.toml b/crates/spacetop/Cargo.toml index 551cdc87..7ce20e7b 100644 --- a/crates/spacetop/Cargo.toml +++ b/crates/spacetop/Cargo.toml @@ -6,6 +6,7 @@ edition.workspace = true [dependencies] spacetop-core = { path = "../spacetop-core" } anyhow = "1" +base64 = "0.22" sentry = { version = "0.34", default-features = false, features = ["backtrace", "contexts", "panic", "reqwest", "rustls"] } clap = { version = "4.5", features = ["derive"] } crossterm = "0.28" diff --git a/crates/spacetop/src/app.rs b/crates/spacetop/src/app.rs index 828c9dc4..7616f295 100644 --- a/crates/spacetop/src/app.rs +++ b/crates/spacetop/src/app.rs @@ -1,5 +1,6 @@ use std::cell::Cell; use std::path::{Path, PathBuf}; +use std::time::{Duration, Instant}; use crossterm::event::{KeyCode, KeyEvent, MouseEvent}; @@ -36,6 +37,20 @@ pub use session_activity_worker::{ pub(crate) use keys::ResolvedKeymap; use keys::{handle_overview_key_with_keymap, OverviewKeyAction}; +const COPY_FEEDBACK_DURATION: Duration = Duration::from_secs(2); + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum CopyFeedback { + Succeeded, + Failed, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +struct TimedCopyFeedback { + outcome: CopyFeedback, + expires_at: Instant, +} + #[derive(Debug, Clone, PartialEq)] #[allow(clippy::large_enum_variant)] pub enum AppMode { @@ -213,6 +228,9 @@ pub struct App { pending_overlay_open: bool, pending_open_file: Option, pending_sync: bool, + pending_copy_id: Option, + id_click_candidate: Option, + copy_feedback: Option, } impl App { @@ -355,6 +373,9 @@ impl App { pending_overlay_open: false, pending_open_file: None, pending_sync: false, + pending_copy_id: None, + id_click_candidate: None, + copy_feedback: None, } } @@ -480,6 +501,30 @@ impl App { self.pending_open_file.take() } + /// Drain the full entity ID queued by a mouse double-click. OSC 52 output + /// remains in the terminal event loop rather than app/input state. + pub fn take_pending_copy_id(&mut self) -> Option { + self.pending_copy_id.take() + } + + pub(crate) fn set_copy_feedback_at(&mut self, outcome: CopyFeedback, now: Instant) { + self.copy_feedback = Some(TimedCopyFeedback { + outcome, + expires_at: now + COPY_FEEDBACK_DURATION, + }); + } + + pub(crate) fn copy_feedback(&self) -> Option { + self.copy_feedback_at(Instant::now()) + } + + pub(crate) fn copy_feedback_at(&self, now: Instant) -> Option { + self.copy_feedback + .as_ref() + .filter(|feedback| now < feedback.expires_at) + .map(|feedback| feedback.outcome) + } + /// Record a `Y` keypress intent. The event loop calls /// `take_pending_sync` next tick and runs `git_sync::sync` against the /// active workflow's repo root, synchronously. @@ -948,13 +993,28 @@ impl App { /// the full-pane Definition view handles only wheel scrolling, while /// Search, Timeline, Metrics, Activity, and Relations remain inert. pub fn handle_mouse(&mut self, mouse: MouseEvent) { + self.handle_mouse_at(mouse, Instant::now()); + } + + pub(crate) fn handle_mouse_at(&mut self, mouse: MouseEvent, now: Instant) { if self.help_open { return; } + if matches!( + self.mode, + AppMode::Picker(_) | AppMode::PickerOverlay { .. } + ) { + self.id_click_candidate = None; + self.handle_picker_mouse(mouse); + return; + } let definition_max_scroll = self.definition_max_scroll.get(); let action = match &mut self.mode { - AppMode::Overview(session) => mouse::handle_overview_mouse(session, mouse), + AppMode::Overview(session) => { + mouse::handle_overview_mouse(session, mouse, now, &mut self.id_click_candidate) + } AppMode::Definition { scroll, .. } => { + self.id_click_candidate = None; match mouse.kind { crossterm::event::MouseEventKind::ScrollDown => { definition_scroll_down( @@ -974,11 +1034,10 @@ impl App { } return; } - AppMode::Picker(_) | AppMode::PickerOverlay { .. } => { - self.handle_picker_mouse(mouse); + _ => { + self.id_click_candidate = None; return; } - _ => return, }; self.apply_overview_key_action(action); } @@ -1031,9 +1090,11 @@ impl App { OverviewKeyAction::OpenHelp => self.help_open = true, OverviewKeyAction::Quit => self.should_quit = true, OverviewKeyAction::Switch(workflow_switch) => { + self.id_click_candidate = None; self.pending_switch = Some(workflow_switch); } OverviewKeyAction::OpenPickerOverlay => { + self.id_click_candidate = None; self.pending_overlay_open = true; } OverviewKeyAction::OpenSelectedFile(path) => { @@ -1046,6 +1107,7 @@ impl App { OverviewKeyAction::OpenMetrics => self.open_metrics(), OverviewKeyAction::OpenActivity => self.open_activity(), OverviewKeyAction::OpenRelations => self.open_relations(), + OverviewKeyAction::CopyId(id) => self.pending_copy_id = Some(id), OverviewKeyAction::RequestSync => self.pending_sync = true, } } diff --git a/crates/spacetop/src/app/keys.rs b/crates/spacetop/src/app/keys.rs index 7f173e49..267a50b4 100644 --- a/crates/spacetop/src/app/keys.rs +++ b/crates/spacetop/src/app/keys.rs @@ -277,6 +277,9 @@ pub(crate) enum OverviewKeyAction { OpenMetrics, OpenActivity, OpenRelations, + /// A mouse double-click landed on a rendered entity-ID cell. The full + /// underlying ID is carried to the terminal boundary for OSC 52 output. + CopyId(String), /// `Y` from Overview: request a `git pull --ff-only` against the /// active workflow's repo root. Always emitted when the binding /// fires; the helper classifies availability and reports the result. diff --git a/crates/spacetop/src/app/mouse.rs b/crates/spacetop/src/app/mouse.rs index f9971da9..9b6f00ce 100644 --- a/crates/spacetop/src/app/mouse.rs +++ b/crates/spacetop/src/app/mouse.rs @@ -6,6 +6,9 @@ //! cannot drift from drawn rows by construction. Freshness rides the //! event-loop invariant that `run_terminal` draws before it polls input. +use std::path::PathBuf; +use std::time::{Duration, Instant}; + use crossterm::event::{MouseButton, MouseEvent, MouseEventKind}; use ratatui::layout::{Position, Rect}; @@ -15,6 +18,15 @@ use super::{OverviewSession, PickerState}; /// Rows moved per wheel notch over scrollable body panels. pub(crate) const WHEEL_SCROLL_ROWS: isize = 3; +const ID_DOUBLE_CLICK_WINDOW: Duration = Duration::from_millis(500); + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct IdClickCandidate { + workflow_dir: PathBuf, + entity_id: String, + position: Position, + pressed_at: Instant, +} /// What an overview cell coordinate falls on. #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -50,14 +62,74 @@ pub(crate) fn overview_hit(state: &OverviewState, column: u16, row: u16) -> Over OverviewHit::Chrome } +fn entity_id_at(state: &OverviewState, column: u16, row: u16) -> Option { + let position = Position::new(column, row); + if !state.id_column_rect.get().contains(position) { + return None; + } + let rows = state.list_rows_rect.get(); + if !rows.contains(position) { + return None; + } + let index = state.list_offset.get() + usize::from(row - rows.y); + state + .visible_items() + .get(index) + .map(|entity| entity.id.clone()) +} + +fn track_id_click( + state: &OverviewState, + mouse: MouseEvent, + now: Instant, + candidate: &mut Option, +) -> Option { + if candidate.as_ref().is_some_and(|prior| { + now.saturating_duration_since(prior.pressed_at) > ID_DOUBLE_CLICK_WINDOW + }) { + *candidate = None; + } + + match mouse.kind { + MouseEventKind::Down(MouseButton::Left) => { + let position = Position::new(mouse.column, mouse.row); + if candidate.as_ref().is_some_and(|prior| { + prior.workflow_dir == state.workflow_dir + && prior.position == position + && now.saturating_duration_since(prior.pressed_at) <= ID_DOUBLE_CLICK_WINDOW + }) { + return candidate.take().map(|prior| prior.entity_id); + } + + *candidate = + entity_id_at(state, mouse.column, mouse.row).map(|entity_id| IdClickCandidate { + workflow_dir: state.workflow_dir.clone(), + entity_id, + position, + pressed_at: now, + }); + } + MouseEventKind::Up(_) => {} + MouseEventKind::Down(_) + | MouseEventKind::Drag(_) + | MouseEventKind::ScrollDown + | MouseEventKind::ScrollUp => *candidate = None, + _ => {} + } + None +} + /// Mouse-event peer to `handle_overview_key_with_keymap`, sharing the /// [`OverviewKeyAction`] application path (every current arm returns /// `None`; the enum keeps future mouse actions on the keyboard plumbing). pub(crate) fn handle_overview_mouse( session: &mut OverviewSession, mouse: MouseEvent, + now: Instant, + id_click_candidate: &mut Option, ) -> OverviewKeyAction { let state = session.active_state_mut(); + let copied_id = track_id_click(state, mouse, now, id_click_candidate); match mouse.kind { MouseEventKind::Down(MouseButton::Left) => { match overview_hit(state, mouse.column, mouse.row) { @@ -89,7 +161,9 @@ pub(crate) fn handle_overview_mouse( }, _ => {} } - OverviewKeyAction::None + copied_id + .map(OverviewKeyAction::CopyId) + .unwrap_or(OverviewKeyAction::None) } /// Workflow index under a (column, row) cell in the picker list, mapping @@ -173,7 +247,9 @@ mod tests { use super::*; use crate::app::{App, PreviewPlacement}; - use spacetop_core::domain::{Entity, StageDefinition, WorkflowDefinition, WorkflowSnapshot}; + use spacetop_core::domain::{ + Entity, EntityParseError, StageDefinition, WorkflowDefinition, WorkflowSnapshot, + }; fn entity(id: &str, title: &str, body: &str) -> Entity { Entity { @@ -197,9 +273,15 @@ mod tests { /// App with `n` items; preview initially closed. fn fixture_app(n: usize, body: &str) -> App { + let ids: Vec = (0..n).map(|i| format!("{i:03}")).collect(); + fixture_app_with_ids(&ids, body) + } + + fn fixture_app_with_ids(ids: &[String], body: &str) -> App { let root = PathBuf::from("/tmp/mouse-test"); - let items = (0..n) - .map(|i| entity(&format!("{i:03}"), &format!("Task number {i:03}"), body)) + let items = ids + .iter() + .map(|id| entity(id, &format!("Task {id}"), body)) .collect(); let snapshot = WorkflowSnapshot { definition: WorkflowDefinition { @@ -680,4 +762,201 @@ mod tests { assert!(!state.preview_open()); assert!(app.help_open()); } + + #[test] + fn double_click_copies_full_id_after_first_click_reflows_the_list() { + let full_id = "compact-copyable-slug-ids".to_string(); + let mut app = fixture_app_with_ids(std::slice::from_ref(&full_id), "body"); + draw(&app, 100, 30); + let first_rect = app.as_overview().expect("overview").id_column_rect.get(); + assert_eq!(first_rect.width, 20); + let position = Position::new(first_rect.x + first_rect.width - 1, first_rect.y); + let start = Instant::now(); + + app.handle_mouse_at( + mouse_at( + MouseEventKind::Down(MouseButton::Left), + position.x, + position.y, + ), + start, + ); + assert!(app.as_overview().expect("overview").preview_open()); + assert_eq!(app.take_pending_copy_id(), None); + + // The preview halves the list pane and shrinks the responsive ID + // column, so the original last ID cell is no longer in the new rect. + draw(&app, 100, 30); + let reflowed = app.as_overview().expect("overview").id_column_rect.get(); + assert_eq!(reflowed.width, 19); + assert!(!reflowed.contains(position)); + + // Button-up between presses must not cancel the candidate. + app.handle_mouse_at( + mouse_at( + MouseEventKind::Up(MouseButton::Left), + position.x, + position.y, + ), + start + Duration::from_millis(20), + ); + app.handle_mouse_at( + mouse_at( + MouseEventKind::Down(MouseButton::Left), + position.x, + position.y, + ), + start + Duration::from_millis(100), + ); + + assert_eq!(app.take_pending_copy_id(), Some(full_id)); + } + + #[test] + fn outside_id_cell_and_timeout_do_not_copy() { + let full_id = "compact-copyable-slug-ids".to_string(); + let mut outside = fixture_app_with_ids(std::slice::from_ref(&full_id), "body"); + draw(&outside, 100, 30); + let rows = outside + .as_overview() + .expect("overview") + .list_rows_rect + .get(); + let start = Instant::now(); + let gutter = Position::new(rows.x, rows.y); + outside.handle_mouse_at( + mouse_at(MouseEventKind::Down(MouseButton::Left), gutter.x, gutter.y), + start, + ); + draw(&outside, 100, 30); + outside.handle_mouse_at( + mouse_at(MouseEventKind::Down(MouseButton::Left), gutter.x, gutter.y), + start + Duration::from_millis(100), + ); + assert_eq!(outside.take_pending_copy_id(), None); + + let mut timed_out = fixture_app_with_ids(std::slice::from_ref(&full_id), "body"); + draw(&timed_out, 100, 30); + let id_rect = timed_out + .as_overview() + .expect("overview") + .id_column_rect + .get(); + timed_out.handle_mouse_at( + mouse_at( + MouseEventKind::Down(MouseButton::Left), + id_rect.x, + id_rect.y, + ), + start, + ); + timed_out.handle_mouse_at( + mouse_at( + MouseEventKind::Down(MouseButton::Left), + id_rect.x, + id_rect.y, + ), + start + Duration::from_millis(501), + ); + assert_eq!(timed_out.take_pending_copy_id(), None); + } + + #[test] + fn double_click_maps_through_scroll_offset_and_wheel_cancels_candidate() { + let ids: Vec = (0..40).map(|i| format!("long-slug-{i:03}")).collect(); + let mut app = fixture_app_with_ids(&ids, "body"); + for _ in 0..35 { + app.handle_key(key(KeyCode::Down)); + } + draw(&app, 100, 30); + let state = app.as_overview().expect("overview"); + let id_rect = state.id_column_rect.get(); + let offset = state.list_offset.get(); + assert!(offset > 0); + let expected = state.visible_items()[offset].id.clone(); + let start = Instant::now(); + + app.handle_mouse_at( + mouse_at( + MouseEventKind::Down(MouseButton::Left), + id_rect.x, + id_rect.y, + ), + start, + ); + app.handle_mouse_at( + mouse_at( + MouseEventKind::Down(MouseButton::Left), + id_rect.x, + id_rect.y, + ), + start + Duration::from_millis(100), + ); + assert_eq!(app.take_pending_copy_id(), Some(expected)); + + draw(&app, 100, 30); + let state = app.as_overview().expect("overview"); + let id_rect = state.id_column_rect.get(); + let rows = state.list_rows_rect.get(); + app.handle_mouse_at( + mouse_at( + MouseEventKind::Down(MouseButton::Left), + id_rect.x, + id_rect.y, + ), + start + Duration::from_secs(1), + ); + app.handle_mouse_at( + mouse_at(MouseEventKind::ScrollDown, rows.x, rows.y), + start + Duration::from_millis(1_050), + ); + app.handle_mouse_at( + mouse_at( + MouseEventKind::Down(MouseButton::Left), + id_rect.x, + id_rect.y, + ), + start + Duration::from_millis(1_100), + ); + assert_eq!(app.take_pending_copy_id(), None); + } + + #[test] + fn synthetic_broken_rows_never_produce_id_copy_intents() { + let mut app = fixture_app(1, "body"); + let mut snapshot = app.snapshot(); + snapshot.parse_errors.push(EntityParseError { + path: PathBuf::from("/tmp/mouse-test/broken.md"), + message: "broken.md: malformed frontmatter".to_string(), + line: None, + column: None, + }); + app.reload_from_snapshot(snapshot); + draw(&app, 100, 30); + let state = app.as_overview().expect("overview"); + let id_rect = state.id_column_rect.get(); + let broken_row = id_rect.y + 1; + assert!(!id_rect.contains(Position::new(id_rect.x, broken_row))); + let start = Instant::now(); + + app.handle_mouse_at( + mouse_at( + MouseEventKind::Down(MouseButton::Left), + id_rect.x, + broken_row, + ), + start, + ); + app.handle_mouse_at( + mouse_at( + MouseEventKind::Down(MouseButton::Left), + id_rect.x, + broken_row, + ), + start + Duration::from_millis(100), + ); + + assert_eq!(app.take_pending_copy_id(), None); + assert_eq!(app.as_overview().expect("overview").selected_index(), 1); + } } diff --git a/crates/spacetop/src/app/overview.rs b/crates/spacetop/src/app/overview.rs index c6dca09f..8a47bc85 100644 --- a/crates/spacetop/src/app/overview.rs +++ b/crates/spacetop/src/app/overview.rs @@ -132,6 +132,10 @@ pub struct OverviewState { pub content_rect: Cell, /// Render-fact: the list rows area (after the 1-row section header). pub list_rows_rect: Cell, + /// Render-fact: the entity-ID cells drawn in the list rows. Its width is + /// the responsive ID-column width and its height covers only real entity + /// rows, never synthetic broken rows. + pub id_column_rect: Cell, /// Render-fact: first visible list index, from `ListState::offset()` /// after the stateful render. pub list_offset: Cell, @@ -190,6 +194,7 @@ impl OverviewState { divider_drag: false, content_rect: Cell::new(Rect::default()), list_rows_rect: Cell::new(Rect::default()), + id_column_rect: Cell::new(Rect::default()), list_offset: Cell::new(0), preview_rect: Cell::new(Rect::default()), } @@ -250,6 +255,7 @@ impl OverviewState { divider_drag: false, content_rect: Cell::new(Rect::default()), list_rows_rect: Cell::new(Rect::default()), + id_column_rect: Cell::new(Rect::default()), list_offset: Cell::new(0), preview_rect: Cell::new(Rect::default()), } diff --git a/crates/spacetop/src/lib.rs b/crates/spacetop/src/lib.rs index 6fb9eb6d..b9a1bc94 100644 --- a/crates/spacetop/src/lib.rs +++ b/crates/spacetop/src/lib.rs @@ -10,7 +10,10 @@ use std::sync::mpsc::{Receiver, TryRecvError}; use std::time::{Duration, Instant}; use anyhow::{anyhow, Context}; -use app::{App, AppMode, HistoryWorkerResult, SessionActivityWorkerResult, SyncStatus}; +use app::{ + App, AppMode, CopyFeedback, HistoryWorkerResult, SessionActivityWorkerResult, SyncStatus, +}; +use base64::{engine::general_purpose::STANDARD as BASE64_STANDARD, Engine as _}; use cli::Cli; use crossterm::{ event::{self, DisableMouseCapture, EnableMouseCapture, Event}, @@ -288,7 +291,22 @@ fn run_terminal(mut app: App) -> anyhow::Result<()> { session_activity_worker_state.request_scan(&app); } - // 6. Drain pending "open file in $EDITOR" intent: suspend the TUI, + // 6. Drain pending full-ID copy intent through the active terminal + // backend. Mouse capture stays enabled around the OSC 52 write. + if let Some(id) = app.take_pending_copy_id() { + let outcome = emit_osc52(terminal.backend_mut(), &id) + .and_then(|_| terminal.backend_mut().flush()); + app.set_copy_feedback_at( + if outcome.is_ok() { + CopyFeedback::Succeeded + } else { + CopyFeedback::Failed + }, + Instant::now(), + ); + } + + // 7. Drain pending "open file in $EDITOR" intent: suspend the TUI, // block on the editor process, resume, force a redraw next iter. // Errors are intentionally swallowed — they would otherwise tear // down the TUI for an issue (e.g. editor not installed) that the @@ -601,6 +619,14 @@ fn emit_osc7(w: &mut W, is_tty: bool, cwd: &Path) -> io::Result<()> { Ok(()) } +/// Copy `value` to the terminal clipboard using OSC 52. The caller owns the +/// flush so it can report write and flush failures through one outcome. +fn emit_osc52(w: &mut W, value: &str) -> io::Result<()> { + w.write_all(b"\x1b]52;c;")?; + w.write_all(BASE64_STANDARD.encode(value.as_bytes()).as_bytes())?; + w.write_all(b"\x07") +} + /// Percent-encode `path` per the OSC 7 contract documented on /// [`emit_osc7`]. Operates on raw bytes so non-UTF-8 paths still emit a /// well-formed URL. @@ -947,4 +973,38 @@ mod tests { emit_osc7(&mut buf, false, Path::new("/some/path")).expect("write ok"); assert!(buf.is_empty(), "expected no OSC 7 bytes when not a TTY"); } + + #[test] + fn osc52_writes_exact_full_id_bytes() { + let mut bytes = Vec::new(); + emit_osc52(&mut bytes, "compact-copyable-slug-ids").expect("OSC 52 write"); + assert_eq!(bytes, b"\x1b]52;c;Y29tcGFjdC1jb3B5YWJsZS1zbHVnLWlkcw==\x07"); + } + + #[test] + fn osc52_writes_through_backend_while_mouse_capture_stays_enabled() { + let mut bytes = Vec::new(); + { + let mut backend = CrosstermBackend::new(&mut bytes); + execute!(&mut backend, EnableMouseCapture).expect("enable capture"); + emit_osc52(&mut backend, "compact-copyable-slug-ids").expect("OSC 52 write"); + backend.flush().expect("flush backend"); + execute!(&mut backend, DisableMouseCapture).expect("disable capture"); + } + + let osc52 = b"\x1b]52;c;Y29tcGFjdC1jb3B5YWJsZS1zbHVnLWlkcw==\x07"; + let osc52_start = bytes + .windows(osc52.len()) + .position(|window| window == osc52) + .expect("OSC 52 payload"); + let enable_start = bytes + .windows(8) + .position(|window| window == b"\x1b[?1000h") + .expect("mouse enable sequence"); + let disable_start = bytes + .windows(8) + .position(|window| window == b"\x1b[?1000l") + .expect("mouse disable sequence"); + assert!(enable_start < osc52_start && osc52_start < disable_start); + } } diff --git a/crates/spacetop/src/ui/footer.rs b/crates/spacetop/src/ui/footer.rs index af9ecada..344c1da3 100644 --- a/crates/spacetop/src/ui/footer.rs +++ b/crates/spacetop/src/ui/footer.rs @@ -6,7 +6,7 @@ use ratatui::{ }; use spacetop_core::config::SpacetopConfig; -use crate::app::{OverviewSession, ResolvedKeymap, SyncStatus}; +use crate::app::{CopyFeedback, OverviewSession, ResolvedKeymap, SyncStatus}; /// Marker glyph prefixed to the sync-failed pill label, mirroring the /// `SUCCESS_MARKER` on success so failure and success read symmetrically. @@ -28,8 +28,9 @@ pub(super) fn render_status_footer( keymap: &ResolvedKeymap, warnings: &[String], session: &OverviewSession, + copy_feedback: Option, ) { - let hints = status_footer_hints_with_keymap(session, keymap, warnings); + let hints = status_footer_hints_with_keymap_and_copy(session, keymap, warnings, copy_feedback); let pill_bg = crate::ui::color::footer_bg(config); let sep_style = Style::default(); let mut spans: Vec> = Vec::new(); @@ -60,9 +61,27 @@ pub(crate) fn status_footer_hints_with_keymap( session: &OverviewSession, keymap: &ResolvedKeymap, warnings: &[String], +) -> Vec<(String, Color)> { + status_footer_hints_with_keymap_and_copy(session, keymap, warnings, None) +} + +fn status_footer_hints_with_keymap_and_copy( + session: &OverviewSession, + keymap: &ResolvedKeymap, + warnings: &[String], + copy_feedback: Option, ) -> Vec<(String, Color)> { let preview_open = session.active_state().preview_open(); let mut hints: Vec<(String, Color)> = Vec::new(); + match copy_feedback { + Some(CopyFeedback::Succeeded) => { + hints.push(("\u{2713} ID copied".to_string(), Color::Green)); + } + Some(CopyFeedback::Failed) => { + hints.push(("\u{26A0} ID copy failed".to_string(), Color::Red)); + } + None => {} + } for warning in warnings { hints.push((format!("\u{26A0} {warning}"), Color::Yellow)); } diff --git a/crates/spacetop/src/ui/help.rs b/crates/spacetop/src/ui/help.rs index d93941d8..b7c2ad0e 100644 --- a/crates/spacetop/src/ui/help.rs +++ b/crates/spacetop/src/ui/help.rs @@ -81,6 +81,7 @@ pub(super) fn render_help_popup(frame: &mut Frame<'_>, area: Rect, app: &App) { Style::default().add_modifier(Modifier::BOLD), ))); lines.push(key_line("Click", "select row + open preview")); + lines.push(key_line("Double-click ID", "copy full ID")); lines.push(key_line("Wheel", "scroll panel under cursor")); lines.push(key_line("Drag divider", "resize list/preview split")); lines.push(key_line("Shift+drag", "native terminal text selection")); diff --git a/crates/spacetop/src/ui/list.rs b/crates/spacetop/src/ui/list.rs index 6add76f5..11239954 100644 --- a/crates/spacetop/src/ui/list.rs +++ b/crates/spacetop/src/ui/list.rs @@ -9,6 +9,16 @@ use crate::app::{OverviewState, ViewScope}; use spacetop_core::config::SpacetopConfig; use spacetop_core::domain::{Entity, EntityParseError}; +pub(crate) const ID_COL_MIN: usize = 4; +pub(crate) const ID_COL_MAX: usize = 20; +pub(crate) const TITLE_COL_MIN: usize = 16; + +const GUTTER_WIDTH: usize = 2; +const PHASE_ID_GAP: usize = 1; +const ID_TITLE_GAP: usize = 2; +const ACTIVITY_MARKER_WIDTH: usize = 2; +const WORKTREE_MARKER_WIDTH: usize = 2; + /// Format a phase name into a fixed `width`-character column, preserving the /// user's original casing exactly. Names longer than `width` chars are /// truncated at `width-1` chars and suffixed with `…`; no additional glyphs @@ -45,7 +55,10 @@ pub(super) fn render_task_list( let visible_items = state.visible_items(); let item_count = visible_items.len(); - let items = build_task_list_items(state, &visible_items); + let phase_width = phase_col_width(&visible_items); + let id_width = id_col_width(&visible_items, inner.width, phase_width); + let items = build_task_list_items(state, &visible_items, phase_width, id_width); + state.id_column_rect.set(Rect::default()); // Section header: "Tasks · N" (or "Archived · N") above the list. let section_header_text = format!("{} \u{00B7} {}", title, item_count); @@ -75,6 +88,7 @@ pub(super) fn render_task_list( // No rows drawn this frame: reset the hit-test facts so mouse // events cannot target rows from a previous, larger layout. state.list_rows_rect.set(Rect::default()); + state.id_column_rect.set(Rect::default()); state.list_offset.set(0); return; }; @@ -98,10 +112,69 @@ pub(super) fn render_task_list( // the scroll offset the List widget settled on (only observable after // the stateful render). Same Cell pattern as `task_page_size` above. state.list_rows_rect.set(list_area); - state.list_offset.set(list_state.offset()); + let list_offset = list_state.offset(); + state.list_offset.set(list_offset); + + let visible_entity_rows = visible_items + .len() + .saturating_sub(list_offset) + .min(usize::from(list_area.height)); + if visible_entity_rows > 0 { + state.id_column_rect.set(Rect { + x: list_area + .x + .saturating_add((GUTTER_WIDTH + phase_width + PHASE_ID_GAP) as u16), + y: list_area.y, + width: id_width as u16, + height: visible_entity_rows as u16, + }); + } +} + +fn phase_col_width(items: &[Entity]) -> usize { + items + .iter() + .map(|item| item.status.chars().count()) + .max() + .unwrap_or(4) + .clamp(4, 12) +} + +pub(crate) fn id_col_width(items: &[Entity], pane_width: u16, phase_width: usize) -> usize { + let natural_width = items + .iter() + .map(|item| item.id.chars().count()) + .max() + .unwrap_or(ID_COL_MIN) + .clamp(ID_COL_MIN, ID_COL_MAX); + let fixed_width = GUTTER_WIDTH + + phase_width + + PHASE_ID_GAP + + ID_TITLE_GAP + + ACTIVITY_MARKER_WIDTH + + WORKTREE_MARKER_WIDTH; + let responsive_ceiling = usize::from(pane_width) + .saturating_sub(fixed_width + TITLE_COL_MIN) + .max(ID_COL_MIN); + natural_width.min(responsive_ceiling) } -fn build_task_list_items(state: &OverviewState, items: &[Entity]) -> Vec> { +fn id_col(id: &str, width: usize) -> String { + let char_count = id.chars().count(); + if char_count > width { + let truncated: String = id.chars().take(width - 1).collect(); + format!("{truncated}\u{2026}") + } else { + format!("{id:>width$}") + } +} + +fn build_task_list_items( + state: &OverviewState, + items: &[Entity], + phase_width: usize, + id_width: usize, +) -> Vec> { let scope = state.view_scope(); let broken = state.parse_errors(); if items.is_empty() && broken.is_empty() { @@ -114,33 +187,14 @@ fn build_task_list_items(state: &OverviewState, items: &[Entity]) -> Vec> = items .iter() .enumerate() .map(|(index, item)| { - // Row format: "{gutter} {phase:icw} {title}" + // Row format: "{gutter} {phase:id_width} {title}" // Gutter: "▸ " for selected row, " " otherwise (2 chars). - // Phase column: user casing, pcw-char auto-sized width, ellipsized with "…" if longer. - // ID: icw-char right-aligned, icw = max(4, longest visible ID). + // Phase column: user casing, auto-sized width, ellipsized with "…" if longer. + // ID: responsive 4..=20-char column; over-width values use a trailing "…". // Title: fills remaining width. let is_selected = index == selected_index && !items.is_empty(); @@ -151,8 +205,8 @@ fn build_task_list_items(state: &OverviewState, items: &[Entity]) -> Vecwidth$}", item.id, width = icw); - let phase = phase_col(&item.status, pcw); + let id_str = id_col(&item.id, id_width); + let phase = phase_col(&item.status, phase_width); let id_style = Style::default().add_modifier(Modifier::DIM); let stage_color = diff --git a/crates/spacetop/src/ui/mod.rs b/crates/spacetop/src/ui/mod.rs index 3621103e..31d2372c 100644 --- a/crates/spacetop/src/ui/mod.rs +++ b/crates/spacetop/src/ui/mod.rs @@ -27,7 +27,7 @@ use ratatui::{ }; use spacetop_core::config::SpacetopConfig; -use crate::app::{App, AppMode, OverviewSession, ResolvedKeymap}; +use crate::app::{App, AppMode, CopyFeedback, OverviewSession, ResolvedKeymap}; use graph::render_stage_graph; use layout::{picker_centered, preview_placement, split_content}; @@ -47,6 +47,7 @@ pub fn render_placeholder(frame: &mut Frame<'_>) { pub fn render(frame: &mut Frame<'_>, app: &App) { let warning_messages = app.warning_messages(); + let copy_feedback = app.copy_feedback(); match app.mode() { AppMode::Picker(state) => { // Picker overlays a centered dialog; the dashboard responsive- @@ -62,6 +63,7 @@ pub fn render(frame: &mut Frame<'_>, app: &App) { app.keymap(), &warning_messages, session, + copy_feedback, ); } AppMode::PickerOverlay { underlying, picker } => { @@ -74,6 +76,7 @@ pub fn render(frame: &mut Frame<'_>, app: &App) { app.keymap(), &warning_messages, underlying, + copy_feedback, ); let inner = picker_centered(frame.area(), picker); frame.render_widget(Clear, inner); @@ -95,6 +98,7 @@ pub fn render(frame: &mut Frame<'_>, app: &App) { app.keymap(), &warning_messages, underlying, + copy_feedback, ); search::render_overlay(frame, frame.area(), underlying, state); } @@ -151,6 +155,7 @@ fn render_overview( keymap: &ResolvedKeymap, warnings: &[String], session: &OverviewSession, + copy_feedback: Option, ) { let state = session.active_state(); let show_tabs = session.is_multi(); @@ -197,7 +202,15 @@ fn render_overview( list::render_task_list(frame, content_area, config, state); } - footer::render_status_footer(frame, footer_area, config, keymap, warnings, session); + footer::render_status_footer( + frame, + footer_area, + config, + keymap, + warnings, + session, + copy_feedback, + ); } #[cfg(test)] diff --git a/crates/spacetop/src/ui/tests/chrome.rs b/crates/spacetop/src/ui/tests/chrome.rs index 61deb1f4..9c804077 100644 --- a/crates/spacetop/src/ui/tests/chrome.rs +++ b/crates/spacetop/src/ui/tests/chrome.rs @@ -116,6 +116,7 @@ fn help_popup_documents_mouse_and_shift_drag_convention() { let rendered = buffer_text(terminal.backend().buffer()); assert!(rendered.contains("Mouse"), "help popup needs a Mouse block"); assert!(rendered.contains("Click select row + open preview")); + assert!(rendered.contains("Double-click ID copy full ID")); assert!(rendered.contains("Wheel scroll panel under cursor")); assert!(rendered.contains("Drag divider resize list/preview split")); assert!( diff --git a/crates/spacetop/src/ui/tests/task_list.rs b/crates/spacetop/src/ui/tests/task_list.rs index 65d405e9..089fe8ad 100644 --- a/crates/spacetop/src/ui/tests/task_list.rs +++ b/crates/spacetop/src/ui/tests/task_list.rs @@ -711,9 +711,9 @@ fn task_row_title_aligns_with_slug_ids() { (TitleAlpha at x={alpha_x}, TitleBeta at x={beta_x})" ); - // The long slug ID must render in full in the list pane — the column grew - // to fit it. Scoped to x < list_pane to avoid false-passing on the preview - // pane header, which also renders the ID. + // This slug fits below the responsive 20-cell cap and therefore renders in + // full. Scope to x < list_pane to avoid false-passing on the preview pane + // header, which also renders the ID. let slug_in_list = find_text(buffer, "adversarial-review") .into_iter() .any(|(x, _)| x < list_pane); @@ -1122,3 +1122,134 @@ fn list_rows_rect_and_offset_facts_match_drawn_rows() { ); assert_eq!(preview.x, content.x + (content.width - preview.width)); } + +fn buffer_cells(buffer: &ratatui::buffer::Buffer, rect: ratatui::layout::Rect) -> String { + (rect.x..rect.x + rect.width) + .map(|x| buffer[(x, rect.y)].symbol()) + .collect() +} + +#[test] +fn long_slug_id_column_shrinks_responsively_and_caps_at_twenty_cells() { + let long_id = "compact-copyable-slug-ids"; + let title = "Readable title stays visible"; + + let narrow_app = app_with_items(vec![item(long_id, title, "Body")]); + let mut narrow_terminal = Terminal::new(TestBackend::new(80, 24)).expect("terminal"); + narrow_terminal + .draw(|frame| render(frame, &narrow_app)) + .expect("render narrow"); + let narrow_buffer = narrow_terminal.backend().buffer(); + let narrow_state = narrow_app.as_overview().expect("overview"); + let narrow_id = narrow_state.id_column_rect.get(); + let narrow_rows = narrow_state.list_rows_rect.get(); + assert_eq!(narrow_id.width, 9, "40-cell pane leaves 9 cells for ID"); + assert_eq!(buffer_cells(narrow_buffer, narrow_id), "compact-\u{2026}"); + let title_x = narrow_id.x + narrow_id.width + 2 + 2 + 2; + assert!( + usize::from( + (narrow_rows.x + narrow_rows.width) + .checked_sub(title_x) + .expect("title begins inside list pane") + ) >= 16, + "responsive ID width must reserve at least 16 title cells" + ); + assert_eq!( + buffer_cells( + narrow_buffer, + ratatui::layout::Rect::new(title_x, narrow_id.y, 14, 1) + ), + "Readable title" + ); + + let wide_app = app_with_items(vec![item(long_id, title, "Body")]); + let mut wide_terminal = Terminal::new(TestBackend::new(160, 24)).expect("terminal"); + wide_terminal + .draw(|frame| render(frame, &wide_app)) + .expect("render wide"); + let wide_buffer = wide_terminal.backend().buffer(); + let wide_id = wide_app + .as_overview() + .expect("overview") + .id_column_rect + .get(); + assert_eq!(wide_id.width, 20, "wide panes must still cap the ID column"); + assert_eq!( + buffer_cells(wide_buffer, wide_id), + "compact-copyable-sl\u{2026}" + ); +} + +#[test] +fn short_and_numeric_ids_remain_complete_and_right_aligned() { + let short_app = app_with_items(vec![item("short-slug", "Short slug", "Body")]); + let mut short_terminal = Terminal::new(TestBackend::new(160, 24)).expect("terminal"); + short_terminal + .draw(|frame| render(frame, &short_app)) + .expect("render short slug"); + let short_id = short_app + .as_overview() + .expect("overview") + .id_column_rect + .get(); + assert_eq!(short_id.width, 10); + assert_eq!( + buffer_cells(short_terminal.backend().buffer(), short_id), + "short-slug" + ); + + let numeric_app = app_with_items(vec![item("074", "Numeric ID", "Body")]); + let mut numeric_terminal = Terminal::new(TestBackend::new(80, 24)).expect("terminal"); + numeric_terminal + .draw(|frame| render(frame, &numeric_app)) + .expect("render numeric ID"); + let numeric_id = numeric_app + .as_overview() + .expect("overview") + .id_column_rect + .get(); + assert_eq!(numeric_id.width, 4); + assert_eq!( + buffer_cells(numeric_terminal.backend().buffer(), numeric_id), + " 074" + ); +} + +#[test] +fn footer_shows_copy_success_and_failure_then_expires_feedback() { + let now = std::time::Instant::now(); + + let mut succeeded = app_with_items(vec![item("074", "Copy success", "Body")]); + succeeded.set_copy_feedback_at(crate::app::CopyFeedback::Succeeded, now); + let mut success_terminal = Terminal::new(TestBackend::new(160, 24)).expect("terminal"); + success_terminal + .draw(|frame| render(frame, &succeeded)) + .expect("render success"); + assert!(find_styled_text( + success_terminal.backend().buffer(), + "\u{2713} ID copied", + |style| style.fg == Some(Color::Green) + )); + + let mut failed = app_with_items(vec![item("074", "Copy failure", "Body")]); + failed.set_copy_feedback_at(crate::app::CopyFeedback::Failed, now); + let mut failed_terminal = Terminal::new(TestBackend::new(160, 24)).expect("terminal"); + failed_terminal + .draw(|frame| render(frame, &failed)) + .expect("render failure"); + assert!(find_styled_text( + failed_terminal.backend().buffer(), + "\u{26A0} ID copy failed", + |style| style.fg == Some(Color::Red) + )); + + assert_eq!( + succeeded.copy_feedback_at(now + std::time::Duration::from_millis(1_999)), + Some(crate::app::CopyFeedback::Succeeded) + ); + assert_eq!( + succeeded.copy_feedback_at(now + std::time::Duration::from_secs(2)), + None, + "copy confirmation expires at the two-second boundary" + ); +} From f9bee0997be14f9429f5d5e0a35344e099ac63a7 Mon Sep 17 00:00:00 2001 From: Kent Huang Date: Tue, 28 Jul 2026 12:00:29 +0800 Subject: [PATCH 2/3] fix: measure IDs by terminal cell width --- Cargo.lock | 1 + crates/spacetop/Cargo.toml | 1 + crates/spacetop/src/app/mouse.rs | 46 ++++++++++++++ crates/spacetop/src/ui/list.rs | 23 +++++-- crates/spacetop/src/ui/tests/task_list.rs | 75 +++++++++++++++++++++++ 5 files changed, 140 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 452cb411..1dd4839c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2602,6 +2602,7 @@ dependencies = [ "spacetop-core", "tempfile", "termimad", + "unicode-width 0.2.0", ] [[package]] diff --git a/crates/spacetop/Cargo.toml b/crates/spacetop/Cargo.toml index 7ce20e7b..cd909097 100644 --- a/crates/spacetop/Cargo.toml +++ b/crates/spacetop/Cargo.toml @@ -16,6 +16,7 @@ termimad = "0.34" similar = "2" serde = { version = "1", features = ["derive"] } serde_json = "1" +unicode-width = "0.2" [dev-dependencies] sentry = { version = "0.34", default-features = false, features = ["test"] } diff --git a/crates/spacetop/src/app/mouse.rs b/crates/spacetop/src/app/mouse.rs index 9b6f00ce..d18f4cf2 100644 --- a/crates/spacetop/src/app/mouse.rs +++ b/crates/spacetop/src/app/mouse.rs @@ -812,6 +812,52 @@ mod tests { assert_eq!(app.take_pending_copy_id(), Some(full_id)); } + #[test] + fn double_click_on_wide_unicode_rendered_tail_copies_full_id() { + let full_id = "資料資料資料資料資料".to_string(); + let mut app = fixture_app_with_ids(std::slice::from_ref(&full_id), "body"); + draw(&app, 100, 30); + let id_rect = app.as_overview().expect("overview").id_column_rect.get(); + assert_eq!( + id_rect.width, 20, + "ten wide scalars occupy the full 20-cell cap" + ); + let rendered_width = unicode_width::UnicodeWidthStr::width(full_id.as_str()) as u16; + let rendered_tail = Position::new(id_rect.x + rendered_width - 1, id_rect.y); + assert!( + id_rect.contains(rendered_tail), + "the render-derived hit rectangle must include the visible Unicode tail" + ); + let start = Instant::now(); + + app.handle_mouse_at( + mouse_at( + MouseEventKind::Down(MouseButton::Left), + rendered_tail.x, + rendered_tail.y, + ), + start, + ); + app.handle_mouse_at( + mouse_at( + MouseEventKind::Up(MouseButton::Left), + rendered_tail.x, + rendered_tail.y, + ), + start + Duration::from_millis(10), + ); + app.handle_mouse_at( + mouse_at( + MouseEventKind::Down(MouseButton::Left), + rendered_tail.x, + rendered_tail.y, + ), + start + Duration::from_millis(100), + ); + + assert_eq!(app.take_pending_copy_id(), Some(full_id)); + } + #[test] fn outside_id_cell_and_timeout_do_not_copy() { let full_id = "compact-copyable-slug-ids".to_string(); diff --git a/crates/spacetop/src/ui/list.rs b/crates/spacetop/src/ui/list.rs index 11239954..1d7a926b 100644 --- a/crates/spacetop/src/ui/list.rs +++ b/crates/spacetop/src/ui/list.rs @@ -8,6 +8,7 @@ use ratatui::{ use crate::app::{OverviewState, ViewScope}; use spacetop_core::config::SpacetopConfig; use spacetop_core::domain::{Entity, EntityParseError}; +use unicode_width::UnicodeWidthStr; pub(crate) const ID_COL_MIN: usize = 4; pub(crate) const ID_COL_MAX: usize = 20; @@ -143,7 +144,7 @@ fn phase_col_width(items: &[Entity]) -> usize { pub(crate) fn id_col_width(items: &[Entity], pane_width: u16, phase_width: usize) -> usize { let natural_width = items .iter() - .map(|item| item.id.chars().count()) + .map(|item| UnicodeWidthStr::width(item.id.as_str())) .max() .unwrap_or(ID_COL_MIN) .clamp(ID_COL_MIN, ID_COL_MAX); @@ -160,12 +161,22 @@ pub(crate) fn id_col_width(items: &[Entity], pane_width: u16, phase_width: usize } fn id_col(id: &str, width: usize) -> String { - let char_count = id.chars().count(); - if char_count > width { - let truncated: String = id.chars().take(width - 1).collect(); - format!("{truncated}\u{2026}") + let display_width = UnicodeWidthStr::width(id); + if display_width > width { + let content_width = width.saturating_sub(1); + let prefix_end = id + .char_indices() + .filter_map(|(index, ch)| { + let end = index + ch.len_utf8(); + (UnicodeWidthStr::width(&id[..end]) <= content_width).then_some(end) + }) + .next_back() + .unwrap_or(0); + let prefix = &id[..prefix_end]; + let padding = content_width.saturating_sub(UnicodeWidthStr::width(prefix)); + format!("{prefix}{}\u{2026}", " ".repeat(padding)) } else { - format!("{id:>width$}") + format!("{}{id}", " ".repeat(width - display_width)) } } diff --git a/crates/spacetop/src/ui/tests/task_list.rs b/crates/spacetop/src/ui/tests/task_list.rs index 089fe8ad..080aaf1c 100644 --- a/crates/spacetop/src/ui/tests/task_list.rs +++ b/crates/spacetop/src/ui/tests/task_list.rs @@ -1180,6 +1180,60 @@ fn long_slug_id_column_shrinks_responsively_and_caps_at_twenty_cells() { ); } +#[test] +fn wide_and_combining_ids_use_terminal_cell_width() { + let wide_id = "資料資料資料資料資料"; + let wide_app = app_with_items(vec![item(wide_id, "Wide title", "Body")]); + let mut wide_terminal = Terminal::new(TestBackend::new(80, 24)).expect("terminal"); + wide_terminal + .draw(|frame| render(frame, &wide_app)) + .expect("render wide Unicode"); + let wide_buffer = wide_terminal.backend().buffer(); + let wide_rect = wide_app + .as_overview() + .expect("overview") + .id_column_rect + .get(); + assert_eq!(wide_rect.width, 9); + for (offset, symbol) in [(0, "資"), (2, "料"), (4, "資"), (6, "料"), (8, "\u{2026}")] { + assert_eq!( + wide_buffer[(wide_rect.x + offset, wide_rect.y)].symbol(), + symbol + ); + } + let wide_title_x = wide_rect.x + wide_rect.width + 2 + 2 + 2; + assert_eq!(wide_buffer[(wide_title_x, wide_rect.y)].symbol(), "W"); + + let combining_id = "e\u{301}".repeat(12); + let combining_app = app_with_items(vec![item(&combining_id, "Combining title", "Body")]); + let mut combining_terminal = Terminal::new(TestBackend::new(80, 24)).expect("terminal"); + combining_terminal + .draw(|frame| render(frame, &combining_app)) + .expect("render combining Unicode"); + let combining_buffer = combining_terminal.backend().buffer(); + let combining_rect = combining_app + .as_overview() + .expect("overview") + .id_column_rect + .get(); + assert_eq!(combining_rect.width, 9); + for offset in 0..8 { + assert_eq!( + combining_buffer[(combining_rect.x + offset, combining_rect.y)].symbol(), + "e\u{301}" + ); + } + assert_eq!( + combining_buffer[(combining_rect.x + 8, combining_rect.y)].symbol(), + "\u{2026}" + ); + let combining_title_x = combining_rect.x + combining_rect.width + 2 + 2 + 2; + assert_eq!( + combining_buffer[(combining_title_x, combining_rect.y)].symbol(), + "C" + ); +} + #[test] fn short_and_numeric_ids_remain_complete_and_right_aligned() { let short_app = app_with_items(vec![item("short-slug", "Short slug", "Body")]); @@ -1213,6 +1267,27 @@ fn short_and_numeric_ids_remain_complete_and_right_aligned() { buffer_cells(numeric_terminal.backend().buffer(), numeric_id), " 074" ); + + let combining_short = "e\u{301}e\u{301}e\u{301}"; + let combining_short_app = + app_with_items(vec![item(combining_short, "Combining short", "Body")]); + let mut combining_short_terminal = Terminal::new(TestBackend::new(80, 24)).expect("terminal"); + combining_short_terminal + .draw(|frame| render(frame, &combining_short_app)) + .expect("render short combining ID"); + let combining_short_rect = combining_short_app + .as_overview() + .expect("overview") + .id_column_rect + .get(); + assert_eq!(combining_short_rect.width, 4); + assert_eq!( + buffer_cells( + combining_short_terminal.backend().buffer(), + combining_short_rect + ), + format!(" {combining_short}") + ); } #[test] From 4db7e9eed83d2315887a14b6acecb891db18351c Mon Sep 17 00:00:00 2001 From: Kent Huang Date: Tue, 28 Jul 2026 12:18:53 +0800 Subject: [PATCH 3/3] fix: address PR review feedback --- crates/spacetop/src/app/mouse.rs | 61 +++++++++++++++++++++++++++++++- crates/spacetop/src/ui/list.rs | 22 ++++++------ 2 files changed, 72 insertions(+), 11 deletions(-) diff --git a/crates/spacetop/src/app/mouse.rs b/crates/spacetop/src/app/mouse.rs index d18f4cf2..36bfc8d1 100644 --- a/crates/spacetop/src/app/mouse.rs +++ b/crates/spacetop/src/app/mouse.rs @@ -67,8 +67,12 @@ fn entity_id_at(state: &OverviewState, column: u16, row: u16) -> Option if !state.id_column_rect.get().contains(position) { return None; } + entity_id_at_row(state, row) +} + +fn entity_id_at_row(state: &OverviewState, row: u16) -> Option { let rows = state.list_rows_rect.get(); - if !rows.contains(position) { + if row < rows.y || row >= rows.y.saturating_add(rows.height) { return None; } let index = state.list_offset.get() + usize::from(row - rows.y); @@ -96,6 +100,8 @@ fn track_id_click( if candidate.as_ref().is_some_and(|prior| { prior.workflow_dir == state.workflow_dir && prior.position == position + && entity_id_at_row(state, mouse.row).as_deref() + == Some(prior.entity_id.as_str()) && now.saturating_duration_since(prior.pressed_at) <= ID_DOUBLE_CLICK_WINDOW }) { return candidate.take().map(|prior| prior.entity_id); @@ -812,6 +818,59 @@ mod tests { assert_eq!(app.take_pending_copy_id(), Some(full_id)); } + #[test] + fn double_click_rejects_a_stale_id_after_row_mapping_changes() { + let ids = vec!["first-slug".to_string(), "second-slug".to_string()]; + let app = fixture_app_with_ids(&ids, "body"); + draw(&app, 100, 30); + let state = app.as_overview().expect("overview"); + let id_rect = state.id_column_rect.get(); + let position = Position::new(id_rect.x, id_rect.y); + let start = Instant::now(); + let mut candidate = None; + + assert_eq!( + track_id_click( + state, + mouse_at( + MouseEventKind::Down(MouseButton::Left), + position.x, + position.y, + ), + start, + &mut candidate, + ), + None + ); + assert_eq!( + candidate.as_ref().map(|click| click.entity_id.as_str()), + Some("first-slug") + ); + + // A keyboard selection or refresh can move the list between mouse + // events. The same screen row now identifies a different entity. + state.list_offset.set(1); + assert_eq!( + track_id_click( + state, + mouse_at( + MouseEventKind::Down(MouseButton::Left), + position.x, + position.y, + ), + start + Duration::from_millis(100), + &mut candidate, + ), + None, + "the prior row mapping must not produce a stale copy" + ); + assert_eq!( + candidate.as_ref().map(|click| click.entity_id.as_str()), + Some("second-slug"), + "the second press may begin a fresh gesture for the current row" + ); + } + #[test] fn double_click_on_wide_unicode_rendered_tail_copies_full_id() { let full_id = "資料資料資料資料資料".to_string(); diff --git a/crates/spacetop/src/ui/list.rs b/crates/spacetop/src/ui/list.rs index 1d7a926b..ede2e3d0 100644 --- a/crates/spacetop/src/ui/list.rs +++ b/crates/spacetop/src/ui/list.rs @@ -8,7 +8,7 @@ use ratatui::{ use crate::app::{OverviewState, ViewScope}; use spacetop_core::config::SpacetopConfig; use spacetop_core::domain::{Entity, EntityParseError}; -use unicode_width::UnicodeWidthStr; +use unicode_width::{UnicodeWidthChar, UnicodeWidthStr}; pub(crate) const ID_COL_MIN: usize = 4; pub(crate) const ID_COL_MAX: usize = 20; @@ -164,16 +164,18 @@ fn id_col(id: &str, width: usize) -> String { let display_width = UnicodeWidthStr::width(id); if display_width > width { let content_width = width.saturating_sub(1); - let prefix_end = id - .char_indices() - .filter_map(|(index, ch)| { - let end = index + ch.len_utf8(); - (UnicodeWidthStr::width(&id[..end]) <= content_width).then_some(end) - }) - .next_back() - .unwrap_or(0); + let mut prefix_end = 0; + let mut prefix_width = 0; + for (index, ch) in id.char_indices() { + let char_width = UnicodeWidthChar::width(ch).unwrap_or(0); + if prefix_width + char_width > content_width { + break; + } + prefix_width += char_width; + prefix_end = index + ch.len_utf8(); + } let prefix = &id[..prefix_end]; - let padding = content_width.saturating_sub(UnicodeWidthStr::width(prefix)); + let padding = content_width.saturating_sub(prefix_width); format!("{prefix}{}\u{2026}", " ".repeat(padding)) } else { format!("{}{id}", " ".repeat(width - display_width))