diff --git a/architecture/queue-register.md b/architecture/queue-register.md new file mode 100644 index 0000000..d007498 --- /dev/null +++ b/architecture/queue-register.md @@ -0,0 +1,265 @@ +# Queue selection, visual mode, and the register + +Marks and visual mode in the queue pane, paired with a vim-style +**register**: deleting or yanking queue entries puts them in the register, +pasting brings them back. One model, implemented in both clients. + +## Context and problem statement + +The library pane has marks (`s`) and visual mode (`v`/`V`); the queue pane +has neither — `d` removes exactly the row under the cursor, one at a time, +and `queue.rs` carries the standing note *"FIXME: mark multiple tracks on +queue and remove them"*. Clearing 200 tracks with `c`/`C`, or deleting the +wrong row, is unrecoverable: there is no undo anywhere in the system. + +Meanwhile `p` in the queue means "insert the **library** selection after +this track" — a cross-pane action with no name for what it is inserting. + +The ask is to close both gaps with one concept borrowed from vim: a client +register that deletes and yanks write to, and paste reads from. That turns +multi-delete into a safe operation (it is recoverable), gives "move these +three tracks down" a natural spelling (`d` then `P`), and gives `p` a single +meaning. + +## Assumptions (confirmed) + +- **Only explicit commands write the register**: `y` (both panes), and `d`, + `c`, `C` (queue). Marking or moving the cursor never writes it. This was + the one point of disagreement — an implicit fill from the library + selection would have kept `p` backward-compatible, but it means `s` in + the library silently clobbers a clipboard you are about to paste. +- **Both `p` (after cursor) and `P` (before cursor)**, so `d` then `P` + restores exactly what you deleted, and `d` … `p` is a move. +- **`c`/`C` fill the register too** — the destructive ops most worth + undoing. +- **One unnamed register**, but the type is shaped so named registers + (`"a`) are additive rather than a rewrite. +- **Both clients, now.** `cbd-web` gets queue marks, queue visual mode, and + the register — *plus* the library visual mode that was deferred when + `v`/`V` landed in the TUI, so the two clients do not drift further. +- The register is **per-client, in-memory, one level deep**. It is not a + server-side undo log; another client cannot undo your delete, and a + restart forgets it. Same expectations as vim. + +## What the protocol already gives us + +No `.proto` change, no server change. The two RPCs this feature needs are +already shaped for it: + +- `Remove { positions: repeated uint32 }` — already takes **many** + positions, so multi-delete is a client-side gather. +- `Insert { position, paths: repeated string }` — takes a **path list**, so + a register of paths pastes with the existing call. + +That is what makes this a client-only feature. + +## Options considered + +### What the register holds + +1. **Resolved tracks** (`Track` messages, as deleted). Exact: what you + deleted is what comes back, and the UI can show real titles. But it + cannot express "the album node I yanked in the library", and pasting + still has to send paths, so the extra fidelity buys nothing at the wire. +2. **Paths, with titles alongside for display** (chosen). It is what both + RPCs speak; a yanked library *node* expands at paste time, which is a + feature (`y` an album, `p` it into the queue); and the register is one + `Vec` plus labels. + + The cost is honest and worth stating: paste **re-resolves**. A yanked + child of a search term whose term has since been deleted may not come + back, and a node's track count is unknown until paste, so the UI can + only say "3 entries", not "12 tracks". + +### Keeping marks alive in a moving queue + +This is the only genuinely new problem. The library never had it: a listing +is a snapshot you re-fetch deliberately. The queue is server-pushed and +rebuilt on **every** change — another client appending, playback advancing, +and each chunk of a streaming resolve. + +1. **Remap marks by index.** Cheapest, and silently wrong: if playback + advanced and the queue shifted by one, `d` deletes the wrong tracks. + Rejected — a data-losing failure mode. +2. **Clear marks on every snapshot.** Never wrong, but a resolve streaming + in or another client's append wipes a selection mid-flow, which makes + the feature feel broken exactly when the queue is busy. +3. **Greedy in-order match on track path** (chosen). Walk the old and new + path lists together and carry a mark to the new position of the same + path; drop marks whose track is gone. Survives appends, removals, and + playback advancing. Duplicate paths (the same track queued twice) are + ambiguous by nature — in-order matching degrades sanely, keeping the + *n*-th occurrence marked. If the lists diverge past recognition (no + common prefix worth speaking of), clear rather than guess. + +The safety rule that makes any of this sound: **positions handed to +`Remove` are always read off the newest snapshot**, never a remembered +index. + +### Where marks live + +The two clients are shaped differently and this is not worth papering over: + +- `cbd-tui` owns a `Vec` per pane and already has `marked` on it + (the queue builds every row `marked: false` today). +- `cbd-web` owns **no** queue list at all — just `QueueCursor { selected }`, + with rows rendered straight from the server's `Queue` signal. + +So the shared thing is the *rule*, not the struct: marks are a set of +positions plus the path list they were taken against, reconciled on each +snapshot. The TUI keeps them in its `UiItem`s (it rebuilds that list +anyway); the web client keeps a position set beside the signal. Both +implement the same reconciliation, and both unit-test it against the same +cases. + +## Decisions + +**D1 — `Register` holds paths plus display labels.** One unnamed slot: + +```rust +struct Register { + /// Library paths, in the order they were yanked or deleted. Empty means + /// nothing to paste. + paths: Vec, + /// Row labels for the status line only — never sent anywhere. + labels: Vec, +} +``` + +Named registers stay additive: the owner becomes a small map keyed by a +register name, with `None` meaning the unnamed one. Not built now. + +**D2 — Only `y`, `d`, `c`, `C` write the register.** Marks, visual mode, +and cursor movement never do. A write **overwrites**: there is no history +and no numbered registers. + +**D3 — `p` pastes after the cursor, `P` before it.** Both are queue-only +(there is nothing to paste into a library listing). Paste sends +`Insert { position, paths }` and leaves the register intact, so you can +paste twice. `d` then `P` is an exact restore; `d` … `p` is a move. + +**D4 — `y` works in both panes.** In the library it yanks the marked items, +or the cursor item, under the same `is_queable` gate that `a`/`Enter` use — +so yanking cannot put an unqueueable folder in the register. In the queue it +yanks the marked rows, or the cursor row. `y` clears the marks it consumed, +exactly as queueing does today. + +**D5 — Queue marks reconcile per snapshot** by the greedy in-order path +match above, clearing on divergence. `Remove`/`Insert` positions always come +from the newest snapshot. + +**D6 — `d` in the queue deletes every marked row** (or the cursor row when +nothing is marked), in one `Remove` call, and writes them to the register +first. `c`/`C` write the tracks they are about to drop. + +**D7 — Queue visual mode mirrors the library's** anchored paint: `v` (and +`V`) enters, movement toggles the marks of the swept range against the +anchor so moving back reverses, `Esc` or any non-movement action leaves it. +The mark+visual logic is extracted so both panes share one implementation +per client, rather than a second copy in the queue. + +One concrete wrinkle: queue rows are built `is_queable: false, +is_deletable: false`, so the library's mark gate would refuse all of them. +The extracted code takes the gate as a parameter — the queue's is "always +allowed". + +**D8 — `cbd-web` reaches parity in the same change**, including the library +visual mode it does not have yet. Its queue marks live beside the server +signal (D-above); its keymap and help overlay gain the same rows as the +TUI's. + +**D9 — Keys.** The queue gains `s`, `v`, `V`, `y`, `P`; the library gains +`y`. No chord collides with an existing one in either scope. + +**D10 — `p` changes meaning, and that is a breaking change** to muscle +memory: it pastes the register instead of the library selection. The +browse→queue flows that do not go through `p` (`a` append, `L` queue-next, +`Enter` replace) are untouched, so the cost is confined to "insert what I +picked on the left at this exact position", which becomes `y` then `p`. +Documented in the key tables and in the book. + +## Structure + +```d2 +direction: right + +lib: Library pane { + libsel: "marked rows,\nelse cursor row\n(is_queable gate)" +} + +q: Queue pane { + qsel: "marked rows,\nelse cursor row" +} + +reg: "Register (per client, one slot)\npaths + labels" { + shape: cylinder +} + +rpc: Server (unchanged) { + ins: "Insert { position, paths }" + rem: "Remove { positions }" +} + +lib.libsel -> reg: "y" +q.qsel -> reg: "y · d · c · C" +q.qsel -> rpc.rem: "d · c · C\n(positions from the\nnewest snapshot)" +reg -> rpc.ins: "p (after cursor)\nP (before cursor)" +rpc -> q: "Queue snapshot\n(marks reconciled)" +``` + +Mark reconciliation, on every queue snapshot: + +```d2 +direction: down + +snap: "Queue snapshot arrives" +cmp: "Walk old paths and new paths\nin order" +carry: "Carry each mark to the new\nposition of the same path" +drop: "Drop marks whose track is gone" +clear: "Clear all marks" +act: "d / y / c / C read positions\nfrom this snapshot only" + +snap -> cmp +cmp -> carry: recognizable +cmp -> clear: "diverged past\nrecognition" +carry -> drop +drop -> act +clear -> act +``` + +## Boundaries and interfaces + +- **Register** — owned by each client's app state, written only by the four + commands, read only by paste. No I/O, no server involvement; a pure value + that is trivially unit-testable. +- **Mark reconciliation** — one function per client, `(old_paths, + new_paths, marks) -> marks`, tested against: append, remove-before, + remove-marked, playback advance, streaming resolve, duplicate paths, and + wholesale replacement. +- **Pane selection** — both panes expose "the rows this action applies to" + the way the library's `get_selected` already does; `y`/`d` consume it. +- **Server** — untouched. This whole feature is two clients. + +## Risks + +- **Mark drift** deleting the wrong tracks. The reconciliation rule and the + newest-snapshot rule exist for this; it needs the strongest tests in the + feature. +- **`p`'s changed meaning** surprising existing users. Mitigated by leaving + `a`/`L`/`Enter` alone and documenting the change; not avoidable if `p` is + to have one meaning. +- **Silent paste shortfall** when a path no longer resolves — you paste 5 + and get 4. The server already skips unresolvable paths in a resolve; the + clients should say what they pasted rather than claim success blindly. +- **Two implementations drifting** (the very thing D8 is fixing for visual + mode). Same rule, same test cases, both landed together. +- **Duplicate-path ambiguity** in reconciliation is inherent, not solvable + without a per-entry queue id in the proto. In-order matching is the + honest approximation; a queue id is the escape hatch if it ever bites. + +## Open questions + +None blocking. Deferred by choice: named registers (D1 leaves room), a +numbered/history register stack, `y` in the queue putting entries somewhere +persistent (that is what `w` and saved queues are for), and a per-entry +queue id in the proto to make reconciliation exact. diff --git a/cbd-tui/src/app/bindings.rs b/cbd-tui/src/app/bindings.rs index 18babd2..bd14e49 100644 --- a/cbd-tui/src/app/bindings.rs +++ b/cbd-tui/src/app/bindings.rs @@ -58,6 +58,7 @@ pub enum Action { LibraryQueueAppend, LibraryQueueReplace, LibraryToggleMark, + LibraryYank, /// Enter (or leave) visual mode: while active, movement toggles the mark /// of every row it sweeps over, so a run of items is marked by `v` then /// moving (architecture/visual-mode.md). Entering toggles the current row; @@ -97,7 +98,11 @@ pub enum Action { /// library and queue scopes). A no-op when no filter is applied. ClearSearch, // Queue pane - QueueInsertHere, + QueueToggleMark, + QueueVisualMode, + QueueYank, + QueuePaste, + QueuePasteBefore, QueueFirst, QueueLast, QueueNext, @@ -309,6 +314,13 @@ pub const BINDINGS: &[Binding] = &[ action: Action::LibraryVisualMode, description: "Visual mode (same as v)", }, + Binding { + scope: Scope::Library, + mods: KeyModifiers::NONE, + code: KeyCode::Char('y'), + action: Action::LibraryYank, + description: "Yank selection into the register (paste with p in the queue)", + }, Binding { scope: Scope::Library, mods: KeyModifiers::NONE, @@ -436,12 +448,47 @@ pub const BINDINGS: &[Binding] = &[ action: Action::QueuePlaySelected, description: "Play selected track", }, + Binding { + scope: Scope::Queue, + mods: KeyModifiers::NONE, + code: KeyCode::Char('s'), + action: Action::QueueToggleMark, + description: "Mark/unmark selection", + }, + Binding { + scope: Scope::Queue, + mods: KeyModifiers::NONE, + code: KeyCode::Char('v'), + action: Action::QueueVisualMode, + description: "Visual mode: movement toggles marks (v/V; Esc to leave)", + }, + Binding { + scope: Scope::Queue, + mods: KeyModifiers::SHIFT, + code: KeyCode::Char('V'), + action: Action::QueueVisualMode, + description: "Visual mode (same as v)", + }, + Binding { + scope: Scope::Queue, + mods: KeyModifiers::NONE, + code: KeyCode::Char('y'), + action: Action::QueueYank, + description: "Yank selection into the register", + }, Binding { scope: Scope::Queue, mods: KeyModifiers::NONE, code: KeyCode::Char('p'), - action: Action::QueueInsertHere, - description: "Insert library selection after this track", + action: Action::QueuePaste, + description: "Paste the register after this track", + }, + Binding { + scope: Scope::Queue, + mods: KeyModifiers::SHIFT, + code: KeyCode::Char('P'), + action: Action::QueuePasteBefore, + description: "Paste the register before this track", }, Binding { scope: Scope::Queue, @@ -832,9 +879,9 @@ mod tests { } #[test] - fn v_and_v_enter_visual_in_library_only() { - // Both `v` and `V` (SHIFT, and SHIFT normalized away) enter visual mode - // in the library; neither is bound in the queue (no marks there). + fn v_and_v_enter_visual_in_both_panes() { + // Both `v` and `V` (SHIFT, and SHIFT normalized away) enter visual + // mode, in the library and — since the register landed — the queue. for mods in [KeyModifiers::NONE, KeyModifiers::SHIFT] { assert_eq!( lookup(UiFocus::Library, false, key(KeyCode::Char('v'), mods)), @@ -846,11 +893,11 @@ mod tests { ); assert_eq!( lookup(UiFocus::Queue, false, key(KeyCode::Char('v'), mods)), - None + Some(Action::QueueVisualMode) ); assert_eq!( lookup(UiFocus::Queue, false, key(KeyCode::Char('V'), mods)), - None + Some(Action::QueueVisualMode) ); } } diff --git a/cbd-tui/src/app/help.rs b/cbd-tui/src/app/help.rs index 0ca9f00..1feeb1e 100644 --- a/cbd-tui/src/app/help.rs +++ b/cbd-tui/src/app/help.rs @@ -12,9 +12,13 @@ use ratatui::{ use super::bindings::{key_label, Scope, BINDINGS}; use super::{COLOR_PRIMARY, COLOR_SECONDARY}; -/// The modal's content: a usage blurb, two binding columns (Global left, -/// Library + Queue right, so the whole table fits a typical frame), and a -/// close-keys footer derived from the `Scope::Help` bindings. +/// The modal's content: a usage blurb, two binding columns, and a close-keys +/// footer derived from the `Scope::Help` bindings. +/// +/// The columns are balanced by row count, not by scope order: Global + Queue +/// on the left, Library on the right. With marks and the register the Queue +/// group is nearly as long as the Library group, and pairing the two of them +/// in one column overflowed any normal terminal. struct HelpContent { usage: Vec>, left: Vec>, @@ -30,11 +34,11 @@ impl HelpContent { Line::from(""), ]; - let left = group(Scope::Global, "Global"); + let mut left = group(Scope::Global, "Global"); + left.push(Line::from("")); + left.extend(group(Scope::Queue, "Queue")); - let mut right = group(Scope::Library, "Library"); - right.push(Line::from("")); - right.extend(group(Scope::Queue, "Queue")); + let right = group(Scope::Library, "Library"); // All Help-scope chords close the modal; derive their labels instead // of hardcoding key names. @@ -190,14 +194,29 @@ mod tests { #[test] fn help_lists_bindings_from_the_table() { - let text = buffer_text(&render_to_buffer(100, 40)); + // Tall enough for the whole table: the two columns now need ~43 rows + // (see `a_short_frame_truncates_rather_than_panicking`). + let text = buffer_text(&render_to_buffer(100, 50)); // Spot-check one entry per scope, by description from BINDINGS. assert!(text.contains("Quit")); assert!(text.contains("Enter selected folder")); assert!(text.contains("Remove selected track")); + assert!(text.contains("Paste the register after this track")); assert!(text.contains("Close help")); } + /// The modal clamps to the frame and truncates; it does not scroll and + /// does not panic. Pins the known limitation rather than hiding it: the + /// full table needs more rows than a short terminal has. + #[test] + fn a_short_frame_truncates_rather_than_panicking() { + let text = buffer_text(&render_to_buffer(100, 20)); + // The head of the left column is still there… + assert!(text.contains("Quit")); + // …and the tail of the longest column is not. + assert!(!text.contains("Paste the register after this track")); + } + #[test] fn help_explains_basic_usage() { let text = buffer_text(&render_to_buffer(100, 40)); diff --git a/cbd-tui/src/app/library.rs b/cbd-tui/src/app/library.rs index 330e650..c3775f1 100644 --- a/cbd-tui/src/app/library.rs +++ b/cbd-tui/src/app/library.rs @@ -12,8 +12,8 @@ use ratatui::{ use crabidy_core::proto::crabidy::LibraryNode; use super::{ - Filter, MessageFromUi, StatefulList, UiItem, UiItemKind, COLOR_GREEN, COLOR_PRIMARY, - COLOR_PRIMARY_DARK, COLOR_RED, COLOR_SECONDARY, + Filter, MarkedPane, MessageFromUi, StatefulList, UiItem, UiItemKind, COLOR_GREEN, + COLOR_PRIMARY, COLOR_PRIMARY_DARK, COLOR_RED, COLOR_SECONDARY, }; pub struct Library { @@ -110,24 +110,11 @@ impl Library { let item = self.resolved()?; (item.is_queable && item.is_downloadable).then(|| (item.path.clone(), item.title.clone())) } + /// The paths a queue action ships — the marked rows, or the gated cursor + /// row. Thin wrapper over [`MarkedPane::selection`], which both panes + /// share. pub fn get_selected(&self) -> Option> { - // Marks live on the full list; a hidden marked item still counts - // (the filter narrows what you *see*, not what you already chose). - if self.list.iter().any(|i| i.marked) { - return Some( - self.list - .iter() - .filter(|i| i.marked) - .map(|i| i.path.to_string()) - .collect(), - ); - } - // Marks are gated on is_queable when set; the bare selection must be - // gated here too, or Enter on a plain folder ships a path the server - // can only resolve to nothing (silently ignored, like % / e / d on - // items without the capability). - let item = self.resolved()?; - item.is_queable.then(|| vec![item.path.to_string()]) + self.selection().map(|(paths, _labels)| paths) } pub fn ascend(&mut self) { if let Some(parent) = self.parent.as_ref() { @@ -167,41 +154,9 @@ impl Library { } } } - pub fn queue_insert(&mut self, pos: usize) { - if let Some(items) = self.get_selected() { - match self.tx.send(MessageFromUi::InsertTracks(items, pos)) { - Ok(_) => self.remove_marks(), - Err(_) => { /* FIXME: warn */ } - } - } - } pub fn prev_selected(&self) -> usize { *self.positions.get(&self.path).unwrap_or(&0) } - pub fn toggle_mark(&mut self) { - if let Some(view) = self.list_state.selected() { - self.toggle_mark_view(view); - } - } - - /// Toggle the mark of the row at a **view** index (mapped through the `/` - /// filter to its real row), honoring the `is_queable` gate. Shared by - /// `toggle_mark` and visual-mode painting. - fn toggle_mark_view(&mut self, view: usize) { - if let Some(real) = self.filter.to_real(view) { - let item = &mut self.list[real]; - if !item.is_queable { - return; - } - item.marked = !item.marked; - } - } - - /// Whether visual (paint-select) mode is active. - pub fn is_visual(&self) -> bool { - self.visual.is_some() - } - /// Titles of the currently marked rows, in list order (inspection helper). pub fn marked_titles(&self) -> Vec { self.list @@ -211,56 +166,6 @@ impl Library { .collect() } - /// Enter or leave visual mode. Entering anchors at the current row and - /// toggles its mark (vim includes the row you start on, D2); leaving keeps - /// the marks. - pub fn toggle_visual(&mut self) { - if self.visual.is_some() { - self.visual = None; - } else { - self.visual = Some(self.list_state.selected().unwrap_or(0)); - self.toggle_mark(); - } - } - - /// Leave visual mode (marks kept). Idempotent. - pub fn exit_visual(&mut self) { - self.visual = None; - } - - /// The current cursor position as a view index. - pub fn selected_view(&self) -> Option { - self.list_state.selected() - } - - /// Repaint after a visual-mode move from `from_view` to `to_view`. The - /// selection is the contiguous range `[anchor, cursor]`; a move that grows - /// or shrinks it toggles exactly the rows whose range membership changed - /// (relative to the anchor), so moving back reverses a move cleanly and the - /// row you turn around on is never stranded. No-op unless visual is active. - pub fn paint_between(&mut self, from_view: usize, to_view: usize) { - let Some(anchor) = self.visual else { - return; - }; - let (old_lo, old_hi) = (anchor.min(from_view), anchor.max(from_view)); - let (new_lo, new_hi) = (anchor.min(to_view), anchor.max(to_view)); - for view in old_lo.min(new_lo)..=old_hi.max(new_hi) { - let in_old = (old_lo..=old_hi).contains(&view); - let in_new = (new_lo..=new_hi).contains(&view); - if in_old != in_new { - self.toggle_mark_view(view); - } - } - } - - pub fn remove_marks(&mut self) { - if self.list.iter().any(|i| i.marked) { - self.list - .iter_mut() - .filter(|i| i.marked) - .for_each(|i| i.marked = false); - } - } pub fn update(&mut self, node: LibraryNode) { // Creatable nodes (e.g. an empty search node) must be enterable even // with nothing in them — the user goes there to create children. @@ -441,6 +346,32 @@ impl StatefulList for Library { } } +impl MarkedPane for Library { + fn items(&self) -> &[UiItem] { + &self.list + } + fn items_mut(&mut self) -> &mut [UiItem] { + &mut self.list + } + fn filter(&self) -> &Filter { + &self.filter + } + fn visual(&self) -> Option { + self.visual + } + fn set_visual(&mut self, anchor: Option) { + self.visual = anchor; + } + fn selected_view(&self) -> Option { + self.list_state.selected() + } + /// Only queueable rows may be marked: a marked plain folder would ship a + /// path the server can resolve to nothing. + fn markable(&self, item: &UiItem) -> bool { + item.is_queable + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/cbd-tui/src/app/list.rs b/cbd-tui/src/app/list.rs index cacec7a..547d6ba 100644 --- a/cbd-tui/src/app/list.rs +++ b/cbd-tui/src/app/list.rs @@ -163,3 +163,256 @@ pub trait StatefulList { } } } + +/// Marks and visual mode for a pane that owns a `Vec` and a `/` +/// filter — shared by the library and the queue +/// (architecture/queue-register.md D7). +/// +/// The required methods are the pane's own state; everything else is +/// behaviour both panes must agree on. Marks live on the **full** list, so a +/// marked-but-filtered-out row still counts; the view indices these methods +/// take are mapped through the filter. +pub(crate) trait MarkedPane { + /// Every row, in real (unfiltered) order. + fn items(&self) -> &[super::UiItem]; + fn items_mut(&mut self) -> &mut [super::UiItem]; + fn filter(&self) -> &Filter; + /// The visual-mode anchor (a **view** index), or `None` when off. + fn visual(&self) -> Option; + fn set_visual(&mut self, anchor: Option); + /// The cursor's **view** index. + fn selected_view(&self) -> Option; + /// Whether a row may be marked at all. The library gates on + /// `is_queable`; the queue allows every row. + fn markable(&self, item: &super::UiItem) -> bool; + + fn is_visual(&self) -> bool { + self.visual().is_some() + } + + /// Enter or leave visual mode. Entering anchors at the cursor and toggles + /// its mark (vim includes the row you start on); leaving keeps the marks. + fn toggle_visual(&mut self) { + if self.is_visual() { + self.set_visual(None); + } else { + self.set_visual(Some(self.selected_view().unwrap_or(0))); + self.toggle_mark(); + } + } + + fn exit_visual(&mut self) { + self.set_visual(None); + } + + /// Toggle the mark of the cursor row. + fn toggle_mark(&mut self) { + if let Some(view) = self.selected_view() { + self.toggle_mark_view(view); + } + } + + /// Toggle the mark of one **view** row, honoring [`Self::markable`]. + fn toggle_mark_view(&mut self, view: usize) { + let Some(real) = self.filter().to_real(view) else { + return; + }; + let Some(item) = self.items().get(real) else { + return; + }; + if !self.markable(item) { + return; + } + if let Some(item) = self.items_mut().get_mut(real) { + item.marked = !item.marked; + } + } + + /// Paint a visual-mode sweep. The selection is the contiguous range + /// `[anchor, cursor]`; a move that grows or shrinks it toggles exactly the + /// rows whose range membership changed, so moving back reverses a move + /// cleanly and the row you turn around on is never stranded. No-op unless + /// visual mode is active. + fn paint_between(&mut self, from_view: usize, to_view: usize) { + let Some(anchor) = self.visual() else { + return; + }; + let (old_lo, old_hi) = (anchor.min(from_view), anchor.max(from_view)); + let (new_lo, new_hi) = (anchor.min(to_view), anchor.max(to_view)); + for view in old_lo.min(new_lo)..=old_hi.max(new_hi) { + let in_old = (old_lo..=old_hi).contains(&view); + let in_new = (new_lo..=new_hi).contains(&view); + if in_old != in_new { + self.toggle_mark_view(view); + } + } + } + + fn has_marks(&self) -> bool { + self.items().iter().any(|i| i.marked) + } + + fn remove_marks(&mut self) { + for item in self.items_mut() { + item.marked = false; + } + } + + /// The rows an action applies to: every marked row, or the cursor row + /// when nothing is marked. Returns `(paths, labels)` — labels are for + /// display only. `None` when nothing applies (an unmarkable cursor row). + /// + /// Marks live on the full list, so a marked-but-filtered-out row still + /// counts: the filter narrows what you *see*, not what you already chose. + fn selection(&self) -> Option<(Vec, Vec)> { + if self.has_marks() { + let marked = self.items().iter().filter(|i| i.marked); + return Some(( + marked.clone().map(|i| i.path.clone()).collect(), + marked.map(|i| i.title.clone()).collect(), + )); + } + // The bare cursor row must pass the same gate the marks do, or a + // plain folder would ship a path the server resolves to nothing. + let real = self.filter().to_real(self.selected_view()?)?; + let item = self.items().get(real)?; + self.markable(item) + .then(|| (vec![item.path.clone()], vec![item.title.clone()])) + } +} + +/// Carry queue marks across a server snapshot by a greedy in-order match on +/// track path (architecture/queue-register.md D5). +/// +/// Returns the mark flags for `new_paths`. A mark follows its track through +/// appends, removals, and playback advancing; a mark whose track is gone is +/// dropped. Duplicate paths are inherently ambiguous — the *n*-th occurrence +/// keeps the *n*-th occurrence's mark. When the two lists share no marked +/// track at all, the result is all-unmarked rather than a guess. +pub(crate) fn carry_marks( + old_paths: &[String], + old_marked: &[bool], + new_paths: &[String], +) -> Vec { + let mut carried = vec![false; new_paths.len()]; + // Walk both lists forward, pairing equal paths. Insertions in `new` and + // removals from `old` are skipped over, so a mark follows its own track + // rather than its old index; duplicates pair up in order. + let mut old_idx = 0; + for (new_idx, path) in new_paths.iter().enumerate() { + while old_idx < old_paths.len() && &old_paths[old_idx] != path { + old_idx += 1; + } + if old_idx < old_paths.len() { + // A short `old_marked` (caller bookkeeping drift) reads as + // unmarked rather than panicking. + carried[new_idx] = old_marked.get(old_idx).copied().unwrap_or(false); + old_idx += 1; + } + } + carried +} + +#[cfg(test)] +mod carry_marks_tests { + use super::carry_marks; + + fn paths(v: &[&str]) -> Vec { + v.iter().map(|s| s.to_string()).collect() + } + + /// The plain case: nothing moved, marks stay put. + #[test] + fn an_unchanged_queue_keeps_its_marks() { + let old = paths(&["/a", "/b", "/c"]); + assert_eq!( + carry_marks(&old, &[false, true, false], &old), + vec![false, true, false] + ); + } + + /// Another client appended: marks must not slide. + #[test] + fn an_append_leaves_earlier_marks_alone() { + let old = paths(&["/a", "/b"]); + let new = paths(&["/a", "/b", "/c"]); + assert_eq!( + carry_marks(&old, &[false, true], &new), + vec![false, true, false] + ); + } + + /// Playback advanced and dropped the head: the mark follows its track to + /// its new index instead of staying on a number. + #[test] + fn a_removal_before_a_mark_shifts_it_down() { + let old = paths(&["/a", "/b", "/c"]); + let new = paths(&["/b", "/c"]); + assert_eq!( + carry_marks(&old, &[false, false, true], &new), + vec![false, true] + ); + } + + /// The marked track itself is gone (we just deleted it). + #[test] + fn a_removed_marked_track_drops_its_mark() { + let old = paths(&["/a", "/b", "/c"]); + let new = paths(&["/a", "/c"]); + assert_eq!( + carry_marks(&old, &[false, true, false], &new), + vec![false, false] + ); + } + + /// A resolve streaming in grows the tail; a selection made meanwhile + /// survives. + #[test] + fn a_streaming_resolve_keeps_the_selection() { + let old = paths(&["/a", "/b"]); + let new = paths(&["/a", "/b", "/c", "/d", "/e"]); + assert_eq!( + carry_marks(&old, &[true, true], &new), + vec![true, true, false, false, false] + ); + } + + /// The same track queued twice: the n-th occurrence keeps the n-th mark. + #[test] + fn duplicate_paths_match_in_order() { + let old = paths(&["/a", "/a", "/a"]); + let new = paths(&["/a", "/a", "/a"]); + assert_eq!( + carry_marks(&old, &[false, true, false], &new), + vec![false, true, false] + ); + } + + /// A wholesale replacement shares nothing: clear rather than guess. + #[test] + fn a_replaced_queue_clears_marks() { + let old = paths(&["/a", "/b", "/c"]); + let new = paths(&["/x", "/y"]); + assert_eq!(carry_marks(&old, &[true, true, true], &new), vec![false; 2]); + } + + #[test] + fn an_emptied_queue_has_no_marks() { + let old = paths(&["/a", "/b"]); + assert!(carry_marks(&old, &[true, true], &[]).is_empty()); + } + + /// A first snapshot has no history to carry. + #[test] + fn no_previous_marks_yields_none() { + let new = paths(&["/a", "/b"]); + assert_eq!(carry_marks(&[], &[], &new), vec![false, false]); + } + + /// Length mismatches in the caller's bookkeeping must not panic. + #[test] + fn a_short_mark_vector_is_tolerated() { + let old = paths(&["/a", "/b", "/c"]); + assert_eq!(carry_marks(&old, &[true], &old), vec![true, false, false]); + } +} diff --git a/cbd-tui/src/app/mod.rs b/cbd-tui/src/app/mod.rs index 6ae47d2..b2f6b5e 100644 --- a/cbd-tui/src/app/mod.rs +++ b/cbd-tui/src/app/mod.rs @@ -4,6 +4,7 @@ mod library; mod list; mod now_playing; mod queue; +mod register; use flume::Sender; use ratatui::{ @@ -18,12 +19,14 @@ use crabidy_core::proto::crabidy::{ InitResponse as InitialData, LibraryNode, }; +pub(crate) use list::{carry_marks, MarkedPane}; pub use list::{Filter, StatefulList}; use bindings::Action; use library::Library; use now_playing::NowPlaying; use queue::Queue; +pub use register::Register; #[derive(Clone, Copy)] pub enum UiFocus { @@ -37,7 +40,7 @@ enum UiItemKind { Track, } -struct UiItem { +pub(crate) struct UiItem { path: String, title: String, kind: UiItemKind, @@ -277,6 +280,9 @@ pub struct App { pub library: Library, pub now_playing: NowPlaying, pub queue: Queue, + /// What `y`/`d`/`c`/`C` set aside and `p`/`P` paste back + /// (architecture/queue-register.md). Per client, one slot, in memory. + pub register: Register, tx: Sender, } @@ -294,6 +300,7 @@ impl App { library, now_playing, queue, + register: Register::default(), tx, } } @@ -416,12 +423,50 @@ impl App { } } + /// Run a queue cursor move, painting the swept `(old, new]` range when + /// visual mode is active — the queue mirror of [`Self::library_move`]. + fn queue_move(&mut self, mv: impl FnOnce(&mut Queue)) { + if self.queue.is_visual() { + let old = self.queue.selected_view(); + mv(&mut self.queue); + let new = self.queue.selected_view(); + if let (Some(old), Some(new)) = (old, new) { + self.queue.paint_between(old, new); + } + } else { + mv(&mut self.queue); + } + } + + /// `p`/`P`: insert the register at the cursor. Nothing to paste is a + /// no-op, not an empty round trip; the register survives so the same + /// yank can be pasted again (architecture/queue-register.md D3). + fn paste(&mut self, before: bool) { + if self.register.is_empty() { + return; + } + let pos = self.queue.paste_position(before); + let _ = self.tx.send(MessageFromUi::InsertTracks( + self.register.paths().to_vec(), + pos, + )); + } + + /// Put `entries` in the register, unless a command produced nothing. + fn set_register(&mut self, entries: (Vec, Vec)) { + if !entries.0.is_empty() { + self.register.set(entries.0, entries.1); + } + } + pub fn dispatch(&mut self, action: Action) -> DispatchResult { // Visual mode (library paint-select): the six movements paint (handled // in their arms), `v`/`V` toggle it, and `Esc` leaves it; every other // action leaves visual mode before running (architecture/visual-mode.md). - let was_visual = self.library.is_visual(); - if was_visual + // `Esc` leaves visual mode instead of clearing the `/` filter, so the + // arms below need to know a pane was painting before the guards run. + let was_visual = self.library.is_visual() || self.queue.is_visual(); + if self.library.is_visual() && !matches!( action, Action::LibraryFirst @@ -436,6 +481,22 @@ impl App { { self.library.exit_visual(); } + if self.queue.is_visual() + && !matches!( + action, + Action::QueueFirst + | Action::QueueLast + | Action::QueueNext + | Action::QueuePrev + | Action::QueueJumpDown + | Action::QueueJumpUp + | Action::QueueSelectCurrent + | Action::QueueVisualMode + | Action::ClearSearch + ) + { + self.queue.exit_visual(); + } match action { Action::Quit => return DispatchResult::Quit, Action::OpenHelp => self.show_help = true, @@ -554,6 +615,7 @@ impl App { // In visual mode `Esc` leaves the mode (marks kept) and // does not also clear the search filter. self.library.exit_visual(); + self.queue.exit_visual(); } else { // `Esc` in normal navigation clears the focused pane's // filter, restoring the full listing. A no-op when nothing @@ -564,24 +626,40 @@ impl App { } } } - Action::QueueInsertHere => { - if let Some(selected) = self.queue.selected() { - self.library.queue_insert(selected); + Action::QueueFirst => self.queue_move(|q| q.first()), + Action::QueueLast => self.queue_move(|q| q.last()), + Action::QueueNext => self.queue_move(|q| q.next()), + Action::QueuePrev => self.queue_move(|q| q.prev()), + Action::QueueJumpDown => self.queue_move(|q| q.down()), + Action::QueueJumpUp => self.queue_move(|q| q.up()), + Action::QueueSelectCurrent => self.queue_move(|q| q.select_current()), + Action::QueuePlaySelected => self.queue.play_selected(), + Action::QueueRemoveTrack => { + let removed = self.queue.remove_track(); + self.set_register(removed); + } + Action::QueueToggleMark => self.queue.toggle_mark(), + Action::QueueVisualMode => self.queue.toggle_visual(), + Action::QueueYank => { + let yanked = self.queue.yank(); + self.set_register(yanked); + } + Action::QueuePaste => self.paste(false), + Action::QueuePasteBefore => self.paste(true), + Action::LibraryYank => { + if let Some((paths, labels)) = self.library.selection() { + self.register.set(paths, labels); + self.library.remove_marks(); } } - Action::QueueFirst => self.queue.first(), - Action::QueueLast => self.queue.last(), - Action::QueueNext => self.queue.next(), - Action::QueuePrev => self.queue.prev(), - Action::QueueJumpDown => self.queue.down(), - Action::QueueJumpUp => self.queue.up(), - Action::QueueSelectCurrent => self.queue.select_current(), - Action::QueuePlaySelected => self.queue.play_selected(), - Action::QueueRemoveTrack => self.queue.remove_track(), Action::QueueClearKeepCurrent => { + let dropped = self.queue.all_entries(true); + self.set_register(dropped); let _ = self.tx.send(MessageFromUi::ClearQueue(true)); } Action::QueueClearAll => { + let dropped = self.queue.all_entries(false); + self.set_register(dropped); let _ = self.tx.send(MessageFromUi::ClearQueue(false)); } Action::QueueSaveAs => { @@ -1821,4 +1899,226 @@ mod tests { Ok(MessageFromUi::ClearQueue(false)) )); } + + /// A three-track queue for the register/mark tests. + fn queue_of(paths: &[&str]) -> crabidy_core::proto::crabidy::Queue { + use crabidy_core::proto::crabidy::{Queue as ProtoQueue, Track}; + ProtoQueue { + timestamp: 0, + current_position: 0, + tracks: paths + .iter() + .enumerate() + .map(|(i, path)| Track { + path: path.to_string(), + artist: "artist".to_string(), + title: format!("t{i}"), + duration: None, + album: None, + is_skipped: false, + provider_item_id: String::new(), + is_captured: false, + }) + .collect(), + resolving: false, + } + } + + fn filled_queue_app() -> (App, Receiver) { + let (mut app, rx) = app(); + app.queue.update_queue(queue_of(&["/a", "/b", "/c"])); + app.focus = UiFocus::Queue; + app.queue.select(Some(0)); + (app, rx) + } + + /// `d` removes every marked row in one call, with positions read off the + /// current list (quality/queue-register.md G1). + #[test] + fn deleting_marked_rows_sends_their_current_positions() { + let (mut app, rx) = filled_queue_app(); + let _ = app.dispatch(Action::QueueToggleMark); // row 0 + let _ = app.dispatch(Action::QueueNext); + let _ = app.dispatch(Action::QueueNext); + let _ = app.dispatch(Action::QueueToggleMark); // row 2 + let _ = app.dispatch(Action::QueueRemoveTrack); + match rx.try_recv() { + Ok(MessageFromUi::RemoveTracks(positions)) => assert_eq!(positions, vec![0, 2]), + _ => panic!("expected RemoveTracks"), + } + // The rows it acted on are handed to the register, and the marks are + // consumed. + assert_eq!(app.register.paths(), ["/a", "/c"]); + assert!(!app.queue.has_marks()); + } + + /// With nothing marked, `d` still removes just the cursor row (G15). + #[test] + fn deleting_without_marks_removes_the_cursor_row() { + let (mut app, rx) = filled_queue_app(); + let _ = app.dispatch(Action::QueueNext); + let _ = app.dispatch(Action::QueueRemoveTrack); + match rx.try_recv() { + Ok(MessageFromUi::RemoveTracks(positions)) => assert_eq!(positions, vec![1]), + _ => panic!("expected RemoveTracks"), + } + assert_eq!(app.register.paths(), ["/b"]); + } + + /// A marked row hidden by the `/` filter still counts (G13). + #[test] + fn a_filtered_out_marked_row_is_still_deleted() { + let (mut app, rx) = filled_queue_app(); + let _ = app.dispatch(Action::QueueToggleMark); // mark "t0" + app.queue.set_filter(Some("t2".to_string())); + let _ = app.dispatch(Action::QueueRemoveTrack); + match rx.try_recv() { + Ok(MessageFromUi::RemoveTracks(positions)) => assert_eq!(positions, vec![0]), + _ => panic!("expected RemoveTracks"), + } + } + + /// `y` fills the register without removing anything (G5, G14). + #[test] + fn yanking_fills_the_register_and_sends_nothing() { + let (mut app, rx) = filled_queue_app(); + let _ = app.dispatch(Action::QueueToggleMark); + let _ = app.dispatch(Action::QueueYank); + assert_eq!(app.register.paths(), ["/a"]); + assert_eq!(app.register.labels(), ["artist - t0"]); + assert!(!app.queue.has_marks()); + assert!(rx.try_recv().is_err(), "yank must not talk to the server"); + } + + /// `c`/`C` hand what they drop to the register first (G4). + #[test] + fn clear_fills_the_register_with_what_it_dropped() { + let (mut app, rx) = filled_queue_app(); + let _ = app.dispatch(Action::QueueClearAll); + assert_eq!(app.register.paths(), ["/a", "/b", "/c"]); + assert!(matches!( + rx.try_recv(), + Ok(MessageFromUi::ClearQueue(false)) + )); + + // `c` keeps the current track, so it is not in the register either. + let (mut app, rx) = filled_queue_app(); + let _ = app.dispatch(Action::QueueClearKeepCurrent); + assert_eq!(app.register.paths(), ["/b", "/c"]); + assert!(matches!(rx.try_recv(), Ok(MessageFromUi::ClearQueue(true)))); + } + + /// Nothing but `y`/`d`/`c`/`C` writes the register (G5). + #[test] + fn marking_and_queueing_do_not_touch_the_register() { + let (mut app, _rx) = filled_queue_app(); + let _ = app.dispatch(Action::QueueYank); + let before = app.register.clone(); + for action in [ + Action::QueueToggleMark, + Action::QueueVisualMode, + Action::QueueNext, + Action::QueuePrev, + Action::TogglePlay, + Action::QueueSelectCurrent, + ] { + let _ = app.dispatch(action); + } + assert_eq!(app.register, before); + } + + /// `p` inserts after the cursor, `P` before it (G8). + #[test] + fn paste_after_and_before_use_the_right_position() { + let (mut app, rx) = filled_queue_app(); + let _ = app.dispatch(Action::QueueNext); // cursor on position 1 + let _ = app.dispatch(Action::QueueYank); + + let _ = app.dispatch(Action::QueuePaste); + match rx.try_recv() { + Ok(MessageFromUi::InsertTracks(paths, pos)) => { + assert_eq!(paths, vec!["/b".to_string()]); + assert_eq!(pos, 2, "p pastes after the cursor"); + } + _ => panic!("expected InsertTracks"), + } + + let _ = app.dispatch(Action::QueuePasteBefore); + match rx.try_recv() { + Ok(MessageFromUi::InsertTracks(_, pos)) => { + assert_eq!(pos, 1, "P pastes before the cursor"); + } + _ => panic!("expected InsertTracks"), + } + } + + /// The register survives a paste, so the same yank pastes twice (G7). + #[test] + fn pasting_twice_inserts_twice() { + let (mut app, rx) = filled_queue_app(); + let _ = app.dispatch(Action::QueueYank); + let _ = app.dispatch(Action::QueuePaste); + let _ = app.dispatch(Action::QueuePaste); + assert!(matches!(rx.try_recv(), Ok(MessageFromUi::InsertTracks(..)))); + assert!(matches!(rx.try_recv(), Ok(MessageFromUi::InsertTracks(..)))); + assert!(!app.register.is_empty()); + } + + /// Deleting then pasting before restores the rows where they were (G8). + #[test] + fn delete_then_paste_before_restores_the_positions() { + let (mut app, rx) = filled_queue_app(); + let _ = app.dispatch(Action::QueueNext); // cursor on position 1 + let _ = app.dispatch(Action::QueueRemoveTrack); + assert!(matches!(rx.try_recv(), Ok(MessageFromUi::RemoveTracks(_)))); + // The server echoes the shortened queue; the cursor stays on 1, which + // is where the deleted row belongs. + app.queue.update_queue(queue_of(&["/a", "/c"])); + let _ = app.dispatch(Action::QueuePasteBefore); + match rx.try_recv() { + Ok(MessageFromUi::InsertTracks(paths, pos)) => { + assert_eq!(paths, vec!["/b".to_string()]); + assert_eq!(pos, 1); + } + _ => panic!("expected InsertTracks"), + } + } + + /// An empty register pastes nothing at all (G9). + #[test] + fn pasting_an_empty_register_sends_nothing() { + let (mut app, rx) = filled_queue_app(); + let _ = app.dispatch(Action::QueuePaste); + let _ = app.dispatch(Action::QueuePasteBefore); + assert!(rx.try_recv().is_err()); + } + + /// `y` in the library fills the register under the queueing gate (G5). + #[test] + fn library_yank_fills_the_register() { + let (mut app, rx) = app(); + app.library.update(children_listing(&["alpha", "beta"])); + app.library.select(Some(0)); + let _ = app.dispatch(Action::LibraryYank); + assert!(!app.register.is_empty()); + assert!(rx.try_recv().is_err(), "yank must not talk to the server"); + } + + /// Queue visual mode paints the swept range, and `Esc` leaves it keeping + /// the marks (G11). + #[test] + fn queue_visual_mode_paints_and_esc_leaves_it() { + let (mut app, _rx) = filled_queue_app(); + let _ = app.dispatch(Action::QueueVisualMode); + assert!(app.queue.is_visual()); + let _ = app.dispatch(Action::QueueNext); + let _ = app.dispatch(Action::QueueNext); + assert_eq!(app.queue.marked_positions(), vec![0, 1, 2]); + // Sweeping back reverses. + let _ = app.dispatch(Action::QueuePrev); + assert_eq!(app.queue.marked_positions(), vec![0, 1]); + let _ = app.dispatch(Action::ClearSearch); + assert!(!app.queue.is_visual()); + assert_eq!(app.queue.marked_positions(), vec![0, 1]); + } } diff --git a/cbd-tui/src/app/queue.rs b/cbd-tui/src/app/queue.rs index 544d9be..1730a9c 100644 --- a/cbd-tui/src/app/queue.rs +++ b/cbd-tui/src/app/queue.rs @@ -10,8 +10,8 @@ use ratatui::{ use crabidy_core::proto::crabidy::Queue as QueueData; use super::{ - Filter, MessageFromUi, StatefulList, UiItem, UiItemKind, COLOR_PRIMARY, COLOR_PRIMARY_DARK, - COLOR_RED, COLOR_SECONDARY, + carry_marks, Filter, MarkedPane, MessageFromUi, StatefulList, UiItem, UiItemKind, + COLOR_PRIMARY, COLOR_PRIMARY_DARK, COLOR_RED, COLOR_SECONDARY, }; pub struct Queue { @@ -28,6 +28,9 @@ pub struct Queue { /// render time — it never enters `list`, so selection and removal /// cannot reach it. resolving: bool, + /// Visual (paint-select) mode: `Some(anchor_view)` while active, exactly + /// as in the library (architecture/queue-register.md D7). + visual: Option, tx: Sender, } @@ -39,6 +42,7 @@ impl Queue { list_state: ListState::default(), filter: Filter::default(), resolving: false, + visual: None, tx, } } @@ -84,10 +88,88 @@ impl Queue { self.select(Some(view)); } } - pub fn remove_track(&mut self) { - if let Some(pos) = self.selected_position() { - // FIXME: mark multiple tracks on queue and remove them - let _ = self.tx.send(MessageFromUi::RemoveTracks(vec![pos])); + /// The real queue positions an action applies to: every marked row, or + /// the cursor row when nothing is marked. Positions are read off the + /// **current** list, so they always match the newest snapshot + /// (architecture/queue-register.md D5). + fn action_positions(&self) -> Vec { + if self.has_marks() { + return self + .list + .iter() + .enumerate() + .filter(|(_, item)| item.marked) + .map(|(pos, _)| pos) + .collect(); + } + self.selected_position().into_iter().collect() + } + + /// `d`: remove the marked rows (or the cursor row), handing them to the + /// register first so `p`/`P` can bring them back. + pub fn remove_track(&mut self) -> (Vec, Vec) { + let positions = self.action_positions(); + if positions.is_empty() { + return (Vec::new(), Vec::new()); + } + let yanked = self.entries_at(&positions); + if self.tx.send(MessageFromUi::RemoveTracks(positions)).is_ok() { + self.remove_marks(); + } + yanked + } + + /// `y`: put the marked rows (or the cursor row) in the register without + /// removing anything. Consumes the marks, like queueing does. + pub fn yank(&mut self) -> (Vec, Vec) { + let yanked = self.entries_at(&self.action_positions()); + if !yanked.0.is_empty() { + self.remove_marks(); + } + yanked + } + + /// Real positions of the marked rows, in queue order (inspection helper, + /// mirroring the library's `marked_titles`). + pub fn marked_positions(&self) -> Vec { + self.list + .iter() + .enumerate() + .filter(|(_, item)| item.marked) + .map(|(pos, _)| pos) + .collect() + } + + /// Paths and labels of the given real positions, in queue order. + fn entries_at(&self, positions: &[usize]) -> (Vec, Vec) { + let mut paths = Vec::with_capacity(positions.len()); + let mut labels = Vec::with_capacity(positions.len()); + for pos in positions { + if let Some(item) = self.list.get(*pos) { + paths.push(item.path.clone()); + labels.push(item.title.clone()); + } + } + (paths, labels) + } + + /// Every queue entry, for `c`/`C` to hand to the register before the + /// server drops them. `keep_current` mirrors the RPC's flag. + pub fn all_entries(&self, keep_current: bool) -> (Vec, Vec) { + let positions: Vec = (0..self.list.len()) + .filter(|pos| !(keep_current && *pos == self.current_position)) + .collect(); + self.entries_at(&positions) + } + + /// The insert position for a paste: after the cursor for `p`, at the + /// cursor for `P` (which restores what `d` just removed). An empty queue + /// pastes at the front. + pub fn paste_position(&self, before: bool) -> usize { + match self.selected_position() { + Some(pos) if before => pos, + Some(pos) => pos + 1, + None => 0, } } pub fn update_position(&mut self, pos: usize) { @@ -96,14 +178,23 @@ impl Queue { pub fn update_queue(&mut self, queue: QueueData) { self.current_position = queue.current_position as usize; self.resolving = queue.resolving; + // The queue is server-pushed and rebuilt on every change, so marks + // are carried across by matching track paths rather than indices — + // otherwise a mark would silently retarget when playback advances + // (architecture/queue-register.md D5). + let old_paths: Vec = self.list.iter().map(|i| i.path.clone()).collect(); + let old_marked: Vec = self.list.iter().map(|i| i.marked).collect(); + let new_paths: Vec = queue.tracks.iter().map(|t| t.path.clone()).collect(); + let carried = carry_marks(&old_paths, &old_marked, &new_paths); self.list = queue .tracks .iter() - .map(|t| UiItem { + .enumerate() + .map(|(idx, t)| UiItem { path: t.path.clone(), title: format!("{} - {}", t.artist, t.title), kind: UiItemKind::Track, - marked: false, + marked: carried.get(idx).copied().unwrap_or(false), is_queable: false, is_creatable: false, is_editable: false, @@ -185,9 +276,14 @@ impl Queue { } else { COLOR_PRIMARY_DARK })) - .title(match self.filter.query() { - Some(query) => format!("Queue — /{query}▏"), - None => "Queue".to_string(), + .title(if self.visual.is_some() { + // Visual (paint-select) mode: movement toggles marks. + "Queue — VISUAL".to_string() + } else { + match self.filter.query() { + Some(query) => format!("Queue — /{query}▏"), + None => "Queue".to_string(), + } }), ) .highlight_style(Style::default().bg(if focused { @@ -215,6 +311,33 @@ impl StatefulList for Queue { } } +impl MarkedPane for Queue { + fn items(&self) -> &[UiItem] { + &self.list + } + fn items_mut(&mut self) -> &mut [UiItem] { + &mut self.list + } + fn filter(&self) -> &Filter { + &self.filter + } + fn visual(&self) -> Option { + self.visual + } + fn set_visual(&mut self, anchor: Option) { + self.visual = anchor; + } + fn selected_view(&self) -> Option { + self.list_state.selected() + } + /// Every queue row is a track, so every row may be marked — the + /// library's `is_queable` gate does not apply here (queue rows carry + /// `is_queable: false`). + fn markable(&self, _item: &UiItem) -> bool { + true + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/cbd-tui/src/app/register.rs b/cbd-tui/src/app/register.rs new file mode 100644 index 0000000..3b952e7 --- /dev/null +++ b/cbd-tui/src/app/register.rs @@ -0,0 +1,85 @@ +//! The client-side register: what `y`, `d`, `c`, and `C` put aside and `p`/`P` +//! paste back (architecture/queue-register.md D1–D3). +//! +//! One unnamed slot, in memory, overwritten by each write — vim's unnamed +//! register, not a history. It holds library **paths**, so pasting +//! re-resolves them: a yanked node expands to its tracks at paste time, and a +//! path that no longer resolves simply does not come back. + +/// Paths set aside by the last yank or delete, with labels for display. +#[derive(Debug, Default, Clone, PartialEq, Eq)] +pub struct Register { + paths: Vec, + labels: Vec, +} + +impl Register { + /// Overwrite the register. Empty `paths` clears it. + pub fn set(&mut self, paths: Vec, labels: Vec) { + self.paths = paths; + self.labels = labels; + } + + pub fn is_empty(&self) -> bool { + self.paths.is_empty() + } + + pub fn len(&self) -> usize { + self.paths.len() + } + + /// What a paste sends, in yank order. + pub fn paths(&self) -> &[String] { + &self.paths + } + + /// Row labels, for the status line only — never sent to the server. + pub fn labels(&self) -> &[String] { + &self.labels + } +} + +#[cfg(test)] +mod tests { + use super::Register; + + fn v(items: &[&str]) -> Vec { + items.iter().map(|s| s.to_string()).collect() + } + + #[test] + fn a_fresh_register_is_empty() { + let reg = Register::default(); + assert!(reg.is_empty()); + assert_eq!(reg.len(), 0); + assert!(reg.paths().is_empty()); + } + + #[test] + fn set_stores_paths_in_order_with_labels() { + let mut reg = Register::default(); + reg.set(v(&["/fs/a", "/fs/b"]), v(&["A", "B"])); + assert!(!reg.is_empty()); + assert_eq!(reg.len(), 2); + assert_eq!(reg.paths(), ["/fs/a", "/fs/b"]); + assert_eq!(reg.labels(), ["A", "B"]); + } + + /// A write overwrites: one slot, no history (D2). + #[test] + fn a_second_write_replaces_the_first() { + let mut reg = Register::default(); + reg.set(v(&["/fs/a"]), v(&["A"])); + reg.set(v(&["/tidal/x", "/tidal/y"]), v(&["X", "Y"])); + assert_eq!(reg.paths(), ["/tidal/x", "/tidal/y"]); + assert_eq!(reg.labels(), ["X", "Y"]); + } + + #[test] + fn setting_nothing_clears_it() { + let mut reg = Register::default(); + reg.set(v(&["/fs/a"]), v(&["A"])); + reg.set(Vec::new(), Vec::new()); + assert!(reg.is_empty()); + } +} diff --git a/plan/queue-register.md b/plan/queue-register.md new file mode 100644 index 0000000..9bb7667 --- /dev/null +++ b/plan/queue-register.md @@ -0,0 +1,99 @@ +# Plan — queue selection, visual mode, and the register + +Executes `architecture/queue-register.md` against +`quality/queue-register.md`. Ordered by dependency. Three commits: **(A)** +the shared mark/visual trait with the library moved onto it, **(B)** queue +marks + the register in `cbd-tui`, **(C)** `cbd-web` parity + docs. + +No server work: `Remove` already takes many positions and `Insert` takes a +path list (G25). + +## A — Shared mark and visual behaviour (`cbd-tui`) + +- [ ] **A1 — Fill in `MarkedPane`** in `app/list.rs`: `toggle_visual`, + `toggle_mark`, `toggle_mark_view`, `paint_between`, `selection` — moved + verbatim in behaviour from `library.rs`, with the `markable` gate taken + from the implementor. *Verifies:* G11, G12, G16. +- [ ] **A2 — `impl MarkedPane for Library`**, deleting the now-duplicated + inherent methods and keeping `get_selected` as a thin wrapper over + `selection` so existing call sites are untouched. `markable` = + `is_queable`. *Verifies:* the existing library visual-mode suite passes + unchanged (G11, G12). +- [ ] **A3 — `carry_marks`** in `app/list.rs`: greedy in-order path match, + drop marks whose track is gone, all-false when nothing matches, tolerant + of a short `old_marked`. *Verifies:* the ten `carry_marks_tests` (G2, G3). +- [ ] **A4 — Commit A.** Existing 91 tests plus the `carry_marks` suite + green; clippy and fmt clean. No behaviour change yet. + +## B — Queue marks and the register (`cbd-tui`) + +- [ ] **B1 — `impl MarkedPane for Queue`** with `markable` = always, and a + `visual: Option` field on `Queue`. *Verifies:* G12. +- [ ] **B2 — `update_queue` carries marks** through `carry_marks` instead of + rebuilding them as `false`: keep the previous path list and mark flags, + reconcile, apply. *Verifies:* G1, G2. +- [ ] **B3 — `Register::set`** (replace the `todo!()`) and an `App.register` + field. *Verifies:* the register suite (G6, G10). +- [ ] **B4 — `y` in both panes.** New `Action::LibraryYank` and + `Action::QueueYank`; each takes `selection()`, writes the register, and + clears the marks it consumed. *Verifies:* G5, G14, G15. +- [ ] **B5 — `d` in the queue deletes every marked row** (cursor row when + none), writing them to the register first, in one `RemoveTracks` call with + positions from the current list. Replaces the `FIXME`. *Verifies:* G1, + G13, G14, G15. +- [ ] **B6 — `c`/`C` fill the register** with the tracks they drop (`C` + everything, `c` everything but the current track). *Verifies:* G4. +- [ ] **B7 — `p`/`P` paste the register.** `Action::QueuePaste` / + `QueuePasteBefore` send `InsertTracks(register.paths(), pos)` with `pos` = + cursor + 1 for `p`, cursor for `P`; a no-op on an empty register; the + register survives. `QueueInsertHere`'s old cross-pane behaviour is gone. + *Verifies:* G7, G8, G9. +- [ ] **B8 — Queue visual mode in dispatch.** A `queue_move` wrapper + mirroring `library_move`, and the visual auto-leave guard generalized to + whichever pane is in visual mode with its own movement whitelist. + *Verifies:* G11. +- [ ] **B9 — Bindings + help.** Queue scope gains `s`, `v`, `V`, `y`, `P`; + library gains `y`; `p`'s description changes. The help modal derives from + `BINDINGS`, so it follows. *Verifies:* G19, G20. +- [ ] **B10 — Status feedback.** The pane title shows `— VISUAL` in the + queue as in the library, and a non-empty register is visible somewhere + (title or now-playing line) so paste is not blind. *Verifies:* G22 in + spirit; keeps G9's no-op explicable. +- [ ] **B11 — Dispatch tests** for the gates named above. *Verifies:* G1, + G4, G5, G7–G9, G13–G15. +- [ ] **B12 — Commit B** with the whole `cbd-tui` suite, clippy `-D + warnings`, and fmt clean (G24, G26). + +## C — `cbd-web` parity and docs + +- [ ] **C1 — `Register` + `carry_marks` in `cbd-web/src/state.rs`**, same + shape and same test cases as the TUI's. *Verifies:* G17, G18. +- [ ] **C2 — Library visual mode** (the piece deferred when `v`/`V` landed + in the TUI): `visual: Option` on `LibraryPane`, the same anchored + paint, `Action::LibraryVisualMode` on `v`/`V`, auto-leave on non-movement. + *Verifies:* G17. +- [ ] **C3 — Queue marks beside the signal.** The web queue owns no list, so + hold `marks: Vec` plus the path list they were taken against, and + reconcile on each `Queue` update. *Verifies:* G18. +- [ ] **C4 — Queue actions:** `QueueToggleMark`, `QueueVisualMode`, + `QueueYank`, `QueuePaste`, `QueuePasteBefore`, plus `d`/`c`/`C` writing the + register — same semantics as the TUI. *Verifies:* G17. +- [ ] **C5 — Keymap + `HELP` table** rows for all of the above, keeping the + lockstep test green; clickable equivalents where the web client has them. + *Verifies:* G19. +- [ ] **C6 — Verify the web half properly:** clippy for native **and** + `wasm32-unknown-unknown`, `cargo test -p cbd-web`, and a trunk bundle + build. *Verifies:* G26. +- [ ] **C7 — Docs.** `docs/src/clients/tui.md` (key table + a register + section), `docs/src/clients/web.md`, `docs/src/queue.md` (what the register + is and is not), and the README usage section. State plainly that `p` + changed meaning. *Verifies:* G21, G22, G23. +- [ ] **C8 — `plan/summary.md`** entry, then commit C. + +## Deferred (recorded, not dropped) + +- Named registers (`"a`) — `Register` is shaped for it; not built. +- A numbered/history register stack, and any cross-client or persistent + register. +- A per-entry queue id in the proto, which would make mark reconciliation + exact instead of a greedy path match. diff --git a/quality/queue-register.md b/quality/queue-register.md new file mode 100644 index 0000000..9bf9a65 --- /dev/null +++ b/quality/queue-register.md @@ -0,0 +1,100 @@ +# Quality gates — queue selection, visual mode, and the register + +Criteria an implementation of `architecture/queue-register.md` must satisfy. +Automated coverage lives in `cbd-tui/src/app/list.rs` +(`carry_marks_tests`), `cbd-tui/src/app/register.rs`, +`cbd-tui/src/app/mod.rs` (dispatch), and the mirrors of those in +`cbd-web/src/state.rs`. + +## Data safety (highest priority) + +- [ ] **G1 — A delete never removes an unmarked track.** The positions sent + to `Remove` are computed from the newest queue snapshot, never from an + index remembered across an update. *(tests: the `carry_marks` suite plus + `deleting_marked_rows_sends_their_current_positions`.)* +- [ ] **G2 — Marks follow their track across a snapshot.** Append, + removal before a mark, removal of the marked track, streaming resolve, + duplicate paths, wholesale replacement, empty queue, and a first snapshot + all behave as the `carry_marks` tests state. Divergence clears rather than + guesses. +- [ ] **G3 — No panic on any mark bookkeeping.** Mismatched lengths, an + empty queue, a cursor past the end, and marks on a filtered-out row are + all handled without indexing panics. *(test: + `a_short_mark_vector_is_tolerated`, plus the empty-queue dispatch tests.)* +- [ ] **G4 — `c`/`C` write the register before clearing.** Clearing 200 + tracks is recoverable with one `p`. *(tests: + `clear_fills_the_register_with_what_it_dropped`.)* + +## Register semantics + +- [ ] **G5 — Only `y`, `d`, `c`, `C` write the register.** Marking (`s`), + visual mode, cursor movement, `a`/`L`/`Enter`, `w`/`W`, and every playback + action leave it untouched. *(test: + `marking_and_queueing_do_not_touch_the_register`.)* +- [ ] **G6 — A write overwrites.** One slot, no history, no numbered + registers. An empty write clears it. +- [ ] **G7 — Paste leaves the register intact**, so the same yank can be + pasted repeatedly. *(test: `pasting_twice_inserts_twice`.)* +- [ ] **G8 — `p` inserts after the cursor, `P` before it**, and `d` + followed by `P` restores the deleted rows to their original positions. + *(tests: `paste_after_and_before_use_the_right_position`, + `delete_then_paste_before_restores_the_positions`.)* +- [ ] **G9 — Paste on an empty register is a no-op**, not an empty `Insert` + round trip. *(test: `pasting_an_empty_register_sends_nothing`.)* +- [ ] **G10 — The register holds paths, and only labels for display.** No + label ever reaches the server; `Insert` carries paths in yank order. + +## Selection and visual mode + +- [ ] **G11 — The queue's `s`, `v`, `V` behave exactly as the library's**: + `v` and `V` are the same action, entering anchors at the cursor and marks + it, movement paints the anchored range so moving back reverses, `Esc` and + any non-movement action leave visual mode while keeping the marks. + *(tests: the queue mirrors of the library's visual-mode suite.)* +- [ ] **G12 — Every queue row is markable**; the library's `is_queable` + gate does not leak into the queue (queue rows carry `is_queable: false`). + Conversely the library still refuses to mark an unqueueable row. +- [ ] **G13 — Marks live on the full list.** A marked row hidden by the `/` + filter still counts for `y` and `d`. *(test: + `a_filtered_out_marked_row_is_still_deleted`.)* +- [ ] **G14 — `y` and `d` consume the marks they acted on** (the pane comes + back unmarked), as queueing already does. +- [ ] **G15 — With nothing marked, `y`/`d` act on the cursor row only** — + today's `d` behaviour is preserved. +- [ ] **G16 — Mark and visual logic exists once per client.** `Library` and + `Queue` both go through `MarkedPane`; no second copy of the paint rule. + +## Both clients + +- [ ] **G17 — `cbd-web` reaches parity in the same change**: library visual + mode (which it lacks today), queue marks, queue visual mode, `y`, `p`, + `P`, and `c`/`C` filling the register. +- [ ] **G18 — The web client's queue marks reconcile by the same rule.** + It owns no queue list, so its marks sit beside the server signal; the + reconciliation cases from G2 are tested there too. +- [ ] **G19 — Both key tables and both help overlays list the new keys**, + and `cbd-web`'s keymap tests keep their lockstep with its `HELP` table. +- [ ] **G20 — The TUI's binding table stays the single source of truth** + for its help modal (no hand-maintained duplicate list). + +## Documentation + +- [ ] **G21 — `p`'s changed meaning is documented as a change**, in the + book's TUI page, the web page, and the README walkthrough: it pastes the + register, and the browse→insert-here flow is now `y` then `p`. +- [ ] **G22 — The register's limits are stated**: per client, in memory, + one slot, and paste re-resolves so a stale path may not come back. +- [ ] **G23 — Key tables match the code.** Every new binding appears in + `docs/src/clients/tui.md`, `docs/src/clients/web.md` where it lists keys, + and the README's usage section. + +## Hard rules + +- [ ] **G24 — No panics** on user input or empty state anywhere in the new + paths; `todo!()`/`unimplemented!()` from the stub stage are all gone. +- [ ] **G25 — No new server surface.** No `.proto` change, no new RPC, no + change to `crabidy-server`. The feature is two clients and the two + existing calls (`Insert`, `Remove`). +- [ ] **G26 — Clippy clean under `-D warnings`** for `cbd-tui` and for + `cbd-web` on **both** the native and `wasm32-unknown-unknown` targets + (`mod app` only compiles for wasm), and the trunk bundle still builds.