diff --git a/architecture/queue-order.md b/architecture/queue-order.md new file mode 100644 index 0000000..cee07cf --- /dev/null +++ b/architecture/queue-order.md @@ -0,0 +1,338 @@ +# Queue order: de-duplication and sorting + +## Context + +The queue is an ordered list of tracks the server owns; clients mutate it +through RPCs and learn the result from the update stream +(`docs/src/queue.md`). Every existing mutation either *adds* tracks +(`Replace`/`Append`/`Queue`/`Insert`), *drops* them (`Remove`/`ClearQueue`), +or moves the cursor (`SetCurrent`). Nothing **reorders** what is already +there, and nothing notices that the same track is in the queue twice. + +Both gaps show up in ordinary use: + +- Queue an artist, then a playlist that contains three of the same tracks, + and the queue plays them twice. Today the only fix is spotting the rows + and pressing `d` on each. +- Queue five albums by appending them, and they play in the order they were + appended, interleaved by however the provider listed them. There is no way + to say "group this by artist" or "shortest first" short of clearing the + queue and re-queueing in a different order. + +So: two new queue-order operations — **dedup** (drop duplicate entries) and +**sort** (reorder by a strategy) — for every client, without re-resolving +anything through providers. + +## Assumptions + +- **A1 — The server owns queue state; clients ask and then observe.** Same + rule as `architecture/seek.md` A2 and `architecture/mpris.md` A2: a client + sends the operation, the playback loop performs it, and the resulting + `Queue` snapshot is the truth. No client predicts the new order. +- **A2 — Order is queue state, shuffle is a modifier.** `tracks` is the + queue order (what every client renders, and what playback follows with + shuffle off); `play_order` is the *play* order shuffle permutes. Dedup and + sort rewrite the former. They are not new modifiers: nothing about them is + remembered, they are one-shot rewrites, and after one runs the queue is + just a queue in a different order. +- **A3 — Both work on metadata already in the queue.** `Track` carries + path, artist, title, album, duration and `provider_item_id`; no provider + round trip, no network, no new provider trait method. +- **A4 — The current track keeps playing.** Neither operation interrupts + audio: dedup never removes the playing track, sort never restarts it. The + playing track's *position* changes, which the existing `QueueTrack` + broadcast already covers. +- **A5 — A queue can be long.** Ten thousand entries after queueing a large + artist is normal (`architecture/progressive-queueing.md`), so both + operations must be O(n log n) with keys computed once per track, not per + comparison, and must not hold the queue lock across an await. + +## Options considered + +### Where the operations run + +**Option A — client-side, on top of the existing RPCs.** A client already +holds the full `Queue` snapshot, so it could find duplicate positions itself +and send `Remove`. Sorting would mean sending `Replace` with the paths in the +new order — which re-resolves every path through its provider (slow, and a +provider that has since changed its listing returns something else), loses +the playing track (a replace restarts playback at the head), and would make +each client reimplement the comparators. And the `Remove` positions are +computed from a snapshot that a still-running resolve can invalidate before +the request lands. + +**Option B — server-side RPCs *(chosen)*.** Both run on the playback loop, +the single writer of queue state: atomic against resolve chunks and against +other clients, no re-resolution, the current track is identified by *index* +and not by re-lookup, and the result reaches every client (and the persister) +through the one broadcast site that already exists. + +**Option C — a general `ReorderQueue(permutation)` RPC.** The client decides +the order and the server applies it; sort strategies would then be a purely +client-side concern, and the same RPC would later serve drag-and-drop and +"move this track up". More general, but a permutation is an argument about +*positions*, and positions shift under a running resolve — a stale +permutation is not merely a no-op, it scrambles the queue. It also puts five +comparators in every client. Kept as **Deferred**: a move/reorder RPC wants +a different argument shape (identify rows by path, not index) and is its own +feature. + +**Decision: Option B.** Two RPCs, `DedupQueue` and `SortQueue`, handled on +the playback loop like every other queue verb. + +### What counts as a duplicate + +1. **Same `path`.** Exactly the same library entry queued twice. +2. **Same provider item** — `provider_item_id` scoped to its provider, with + `path` as the fallback when the id is empty. Catches the same track + reached by two routes: through an album, through a playlist, through a + search result. +3. **Same artist and title**, normalized. Would also catch the same song + from two different providers. + +**Decision: 2, falling back to 1** (D3). 3 is rejected: identical +artist/title is routinely a *different recording* — a live take, a remaster, +a radio edit, the studio version — and the server cannot tell which. Dropping +one would be a silent, unrecoverable edit of the user's queue, and the false +positives land exactly on the collections (greatest-hits, live albums) where +a user is most deliberate. A fuzzy dedup belongs behind a client-side preview +where the user confirms each pair; that is Deferred. + +## Decisions + +- **D1 — Two RPCs, both `QueueOwner`.** `DedupQueue` and `SortQueue` mutate + the queue, so they sit with `Remove`/`ClearQueue`/`SetCurrent` in the + rights matrix (`architecture/roles-auth.md`), not with the one appender + verb. The pinned method-list test in `auth.rs` keeps a new RPC from + reaching the wire unmapped. +- **D2 — `DedupQueue` answers with a count.** Every other queue verb + answers with an empty message because the update stream carries the truth. + Dedup is the exception: "how many did that remove?" cannot be recovered + from the new snapshot (a client would have to diff against a snapshot it + may never have had), and `0` is the answer a user most needs — it says + *there were no duplicates*, as opposed to *nothing happened*. So + `DedupQueueResponse.removed`, produced on the loop and returned through a + bounded result channel, the way `SaveQueue` already reports. + `SortQueueResponse` stays empty: the new order *is* the answer, and it + arrives on the stream. +- **D3 — Duplicate identity is `(provider, provider_item_id)`, or the whole + path.** The provider is the first path segment; ids are provider-internal + (the content store keys them the same way, `by_provider_id(provider, id)`), + so leaving them unscoped would let two providers' numeric ids collide and + merge two unrelated tracks. When the id is empty — most providers, and + every local file — the key is the full path, which is exact. The + consequence worth stating: a captured copy under `/crabidy` and its + streaming original are **not** duplicates, because they are different + providers. Conservative on purpose: a missed duplicate is a keypress, a + wrong merge is lost queue state. +- **D4 — The survivor is the current track, else the earliest.** Within a + group of duplicates, the entry at the current position survives if it is + in the group; otherwise the earliest one does, and every later copy goes. + "Keep the first" alone would remove the playing track whenever the playing + copy was a later one — a dedup that stops the music is a bug, not a + policy (A4). +- **D5 — Dedup is expressed as a removal.** It computes the positions to + drop and hands them to `QueueManager::remove_tracks`, which already + maintains `play_order`, shifts `current_offset`, and ignores out-of-range + positions. Since the current track is never in that list, `remove_tracks` + reports no successor to start and playback is untouched — the same code + path a client's `d` takes, so there is one removal implementation, not two. +- **D6 — Five sort strategies, on a proto enum.** `ARTIST`, `ALBUM`, + `TITLE`, `DURATION` and `REVERSE`, plus a `descending` flag. + `UNSPECIFIED` (the proto3 default, i.e. a client that forgot the field) is + `InvalidArgument`, never a silent default. `REVERSE` reverses the order + the queue is in and ignores `descending` — it is not a key, and reversing + descendingly is the same thing. +- **D7 — The sort is stable, and the keys are compound.** `ARTIST` sorts by + (artist, album) and `ALBUM` by (album) alone; within an equal key, queue + order survives — which for a queued album *is* its track order, and is + much closer to what a user means than sorting an album's tracks + alphabetically by title. A stable sort is what makes "sort by album, then + by artist" compose across two presses, too. +- **D8 — Unknown sorts last, in both directions.** An empty artist/album/ + title and an absent duration go to the end ascending *and* descending. A + length-less web radio stream is not "the longest track", and a missing + album is not alphabetically first. One rule for every key, so a user never + has to remember which end the blanks pile up at. +- **D9 — Text comparison is case-insensitive and locale-naive.** Keys are + lowercased once per track (decorate–sort–undecorate, A5), then compared as + Unicode strings. No collation, no article stripping: "The Beatles" sorts + under T. Locale-aware collation would need a collation library and a + locale the server does not have (it has no user, only clients); doing it + half-way — special-casing English articles — would be wrong for every + other language in a music library. +- **D10 — Sorting reorders `tracks`; what plays next depends on shuffle.** + With shuffle **off**, `play_order` is rebuilt as the identity over the new + order and `current_offset` follows the current track: the queue order *is* + the play order, so a sort changes what plays next — the point of sorting. + With shuffle **on**, `play_order` is remapped through the sort permutation, + so the shuffled play sequence and the position within it are preserved + exactly: the user asked for a random order, and sorting the *display* must + not silently reshuffle. Either way the current track stays current and + keeps playing (A4). +- **D11 — Both are legal while a resolve is in flight, and neither waits for + it.** They apply to what the queue holds at that moment; chunks still + being resolved land afterwards at their insert index or at the end, + unsorted. Refusing (`FailedPrecondition`) would make both operations + flaky exactly on the big queues that need them, and waiting would block + the loop. Clients already render the `resolving` indicator, so "more is + still arriving" is visible; a second press settles the rest. +- **D12 — Both persist through the existing broadcast site.** + `broadcast_queue` hands the snapshot to the persister and to the stream, + so a dedup or a sort survives a server restart with no new persistence + code (`architecture/queue-persistence.md`). +- **D13 — Pure key logic lives in its own module.** `queue_order.rs` holds + the duplicate key, the sort keys, and the permutation — no state, no locks, + unit-testable directly. `QueueManager::dedup`/`sort` keep the + `play_order`/`current_offset` bookkeeping, because that invariant is + theirs. The split is what keeps D3/D7/D8 testable as data instead of + through a queue. +- **D14 — TUI: `u` dedups, `S` opens a sort menu.** `u` is "unique" and is + free in the queue scope. Sorting needs a choice, so `S` opens a modal + overlay listing the five strategies with their letters (`a` artist, `l` + album, `t` title, `d` duration, `r` reverse; the capital of each sorts + descending), `Esc` closes. A menu rather than a chord sequence: the + strategies are discoverable in the overlay instead of only in the help + modal, and the app already has three modal overlays (help, input, search) + to follow. It is modal in the same strict sense — while it is open, the + bindings table is unreachable. +- **D15 — The dedup count is reported in the queue pane's title, briefly.** + `Queue — removed 7 duplicates`, for a few seconds, then back to normal. + The title already multiplexes VISUAL, the `/` query and the register count, + so no layout changes and no new region; and the message is queue-scoped, + which is where the user is looking after pressing `u`. The count travels + as a typed `MessageToUi` variant, not a preformatted string — the wording + is the UI layer's business. +- **D16 — The web client keeps the same keys and adds the two buttons a mouse + needs.** `u` dedups and `S` opens the sort menu as a dialog, because the + keymap is deliberately the TUI's (`architecture/web-client.md`); the menu's + rows are clickable as well as typeable. The queue toolbar gains a **sort** + button that opens that same dialog and a **dedup** button — without them + both operations would be invisible to a mouse, and routing the button + through the same menu keeps the strategy list in exactly one place (a + second widget listing five strategies is a second thing to keep in step). + The dedup count goes to the existing toast rather than a pane title. +- **D17 — CLI: `cbd queue dedup` and `cbd queue sort [--desc]`.** The + strategy is a `clap` `ValueEnum`, so the shell completes it and a typo is + a parse error rather than an `InvalidArgument` round trip. `dedup` prints + the count it got back (D2). + +## Structure + +```d2 +direction: right + +clients: Clients { + tui: cbd-tui\nu / S menu + web: cbd-web\nu / S / select + cli: cbd queue\ndedup / sort +} + +rpc: gRPC (QueueOwner) { + dedup: DedupQueue\n-> removed: u32 + sort: SortQueue\n(strategy, descending) +} + +loop: playback loop\n(single writer) { + cmd: PlaybackCommand\nDedupQueue / SortQueue + qm: QueueManager\ntracks + play_order + keys: queue_order\nduplicate key, sort keys +} + +out: One broadcast site { + stream: Queue update\n(every client) + persist: persister\n(latest wins) +} + +clients.tui -> rpc.dedup +clients.web -> rpc.dedup +clients.cli -> rpc.dedup +clients.cli -> rpc.sort +clients.tui -> rpc.sort +clients.web -> rpc.sort +rpc.dedup -> loop.cmd +rpc.sort -> loop.cmd +loop.cmd -> loop.qm: mutate +loop.qm -> loop.keys: keys / permutation +loop.qm -> out.stream +loop.qm -> out.persist +rpc.dedup -> clients.cli: removed count +``` + +The sort's effect on the two orders (D10) — the same permutation, applied +differently depending on shuffle: + +```d2 +shape: sequence_diagram + +client: client +loop: playback loop +off: QueueManager\nshuffle off +on: QueueManager\nshuffle on +stream: update stream + +client -> loop: SortQueue(ARTIST, asc) +loop -> off: sort(ARTIST, asc) +off -> off: tracks := sorted +off -> off: play_order := identity +off -> off: offset := the current track's new index +loop -> on: sort(ARTIST, asc) +on -> on: tracks := sorted +on -> on: play_order := remapped through the permutation +on -> on: offset unchanged +off -> stream: Queue (new order, new position) +on -> stream: Queue (new order, new position) +loop -> client: SortQueueResponse {} +``` + +## Boundaries + +- **`crabidy-server::queue_order`** — pure functions: the duplicate key of a + track, the sort key of a track, and the permutation for a strategy. Knows + nothing about locks, play order or clients. +- **`crabidy-server::QueueManager`** — gains `dedup()` and `sort()`. Owns + the `play_order`/`current_offset` invariants; `dedup` delegates the actual + removal to `remove_tracks` (D5). +- **`crabidy-server::playback`** — two new `PlaybackCommand` arms, each a + lock–mutate–broadcast on the loop. `DedupQueue` carries a result channel + for the count. +- **`crabidy-server::rpc` / `auth`** — the two methods, their argument + validation (`UNSPECIFIED` → `InvalidArgument`), and their row in the rights + matrix. +- **Clients** (`cbd-tui`, `cbd-web`, `cbd-cli`) — bindings/commands, the + sort-menu modal, and rendering the count. No client computes an order. +- **Unchanged**: providers, the content store, the resolve pipeline, and + every existing queue RPC. + +## Risks + +- **A pending insert's index goes stale.** Dedup shifts positions and sort + moves everything, so the remaining chunks of an in-flight `InsertAt` op + land somewhere else than the user pointed at. This is pre-existing — a + plain `Remove` while resolving does the same — and bounded by D11's + "settle it with a second press", but it is real. +- **Dedup is per-provider (D3), so the obvious cross-provider duplicate — a + captured track and its streaming source — stays.** It is the conservative + end of a trade-off, and it will read as a bug to someone. +- **A sort with shuffle on changes nothing audible** (D10) and may look + broken. The clients show shuffle state, and the queue visibly reorders. +- **Very large queues copy their tracks once per sort.** A `Vec` + permutation on ten thousand tracks is a handful of milliseconds on the + loop; well under the loop's other work (a resolve chunk), but it is work + done while no other command is served. + +## Deferred + +- **Fuzzy dedup** (artist/title matching) behind a client-side confirmation + view — the only safe home for it (see the options above). +- **`ReorderQueue`/move**: drag-and-drop in the web client and `K`/`J` row + moves in the TUI, on an RPC that identifies rows by path rather than + index (Option C). +- **Sort by release year.** `Album.release_date` is a provider string and + not always ISO 8601, so a year needs the same parse-or-drop treatment the + TUI notification does; worth doing once that parse lives somewhere shared. +- **A remembered sort** ("keep the queue sorted by artist as tracks + arrive") — a modifier, which is a different feature from a one-shot + rewrite (A2), and one that fights progressive queueing. +- **Sort within a marked range only**, the visual-mode analogue of a partial + sort. diff --git a/cbd-cli/src/client.rs b/cbd-cli/src/client.rs index b0f8d07..b945f5f 100644 --- a/cbd-cli/src/client.rs +++ b/cbd-cli/src/client.rs @@ -17,14 +17,15 @@ use tonic::{Request, Status}; use crabidy_core::proto::crabidy::{ crabidy_service_client::CrabidyServiceClient, AppendRequest, CaptureLibraryNodeRequest, - ChangeVolumeRequest, ClearQueueRequest, CreateLibraryNodeRequest, DeleteLibraryNodeRequest, - GetLibraryNodeRequest, InitRequest, InsertRequest, LibraryNode, NextRequest, PrevRequest, - Queue, RemoveRequest, RenameLibraryNodeRequest, ReplaceRequest, RestartTrackRequest, - SaveQueueRequest, SeekRequest, SetCurrentRequest, StopRequest, ToggleMuteRequest, - TogglePlayRequest, ToggleRepeatRequest, ToggleShuffleRequest, Track, + ChangeVolumeRequest, ClearQueueRequest, CreateLibraryNodeRequest, DedupQueueRequest, + DeleteLibraryNodeRequest, GetLibraryNodeRequest, InitRequest, InsertRequest, LibraryNode, + NextRequest, PrevRequest, Queue, QueueSort, RemoveRequest, RenameLibraryNodeRequest, + ReplaceRequest, RestartTrackRequest, SaveQueueRequest, SeekRequest, SetCurrentRequest, + SortQueueRequest, StopRequest, ToggleMuteRequest, TogglePlayRequest, ToggleRepeatRequest, + ToggleShuffleRequest, Track, }; -use crate::{Connection, GlobalCmd, LibraryCmd, QueueCmd, RemoteCmd}; +use crate::{Connection, GlobalCmd, LibraryCmd, QueueCmd, RemoteCmd, SortKey}; /// The reserved library path the live queue mirrors into; `queue capture` /// captures it (architecture/crabidy-store.md). @@ -167,6 +168,29 @@ async fn run_library( Ok(()) } +/// The wire strategy a CLI `sort ` names (architecture/queue-order.md D17). +fn wire_sort(key: SortKey) -> QueueSort { + match key { + SortKey::Artist => QueueSort::Artist, + SortKey::Album => QueueSort::Album, + SortKey::Title => QueueSort::Title, + SortKey::Duration => QueueSort::Duration, + SortKey::Reverse => QueueSort::Reverse, + } +} + +/// The CLI spelling of a strategy, so the confirmation line names it the way +/// the user typed it. +fn sort_label(key: SortKey) -> &'static str { + match key { + SortKey::Artist => "artist", + SortKey::Album => "album", + SortKey::Title => "title", + SortKey::Duration => "duration", + SortKey::Reverse => "reverse", + } +} + async fn run_queue(client: &mut Client, cmd: QueueCmd) -> Result<(), Box> { match cmd { QueueCmd::Show => { @@ -217,6 +241,29 @@ async fn run_queue(client: &mut Client, cmd: QueueCmd) -> Result<(), Box { + let response = client + .dedup_queue(DedupQueueRequest {}) + .await + .map_err(rpc_error)?; + // 0 is a real answer — "there were no duplicates" — so it is + // printed like any other count (architecture/queue-order.md D2). + println!("removed {} duplicate(s)", response.into_inner().removed); + } + QueueCmd::Sort { key, desc } => { + client + .sort_queue(SortQueueRequest { + sort: wire_sort(key) as i32, + descending: desc, + }) + .await + .map_err(rpc_error)?; + match key { + SortKey::Reverse => println!("reversed the queue"), + _ if desc => println!("sorted the queue by {} (descending)", sort_label(key)), + _ => println!("sorted the queue by {}", sort_label(key)), + } + } QueueCmd::SetCurrent { position } => { client .set_current(SetCurrentRequest { position }) diff --git a/cbd-cli/src/lib.rs b/cbd-cli/src/lib.rs index ac58bfe..bdc76a6 100644 --- a/cbd-cli/src/lib.rs +++ b/cbd-cli/src/lib.rs @@ -70,6 +70,23 @@ pub enum LibraryCmd { } /// Queue operations against a running server. +/// How `queue sort` orders the queue — the CLI spelling of the wire's +/// `QueueSort` (architecture/queue-order.md D17). A `ValueEnum` so the shell +/// completes it and a typo is a parse error instead of a round trip. +#[derive(Clone, Copy, Debug, PartialEq, Eq, ValueEnum)] +pub enum SortKey { + /// Artist, then album; queue order within an album. + Artist, + /// Album title. + Album, + /// Track title. + Title, + /// Duration; tracks with no known length sort last. + Duration, + /// Reverse the order the queue is in now. + Reverse, +} + #[derive(Debug, Subcommand)] pub enum QueueCmd { /// Print the current queue. @@ -88,6 +105,18 @@ pub enum QueueCmd { #[arg(long)] keep_current: bool, }, + /// Drop duplicate entries, keeping one copy of each track. Prints how + /// many were removed; never removes the playing track. + Dedup, + /// Reorder the queue by one strategy (architecture/queue-order.md). + Sort { + #[arg(value_enum)] + key: SortKey, + /// Largest/last first. Ignored by `reverse`; blanks and unknown + /// durations sort last either way. + #[arg(long)] + desc: bool, + }, /// Jump to a queue position. SetCurrent { position: u32 }, /// Link-save the current queue as `/crabidy/`. @@ -401,6 +430,47 @@ mod tests { } } + #[test] + fn queue_dedup_takes_no_arguments() { + let cli = TuiCli::try_parse_from(["cbd-tui", "queue", "dedup"]).expect("parse"); + assert!(matches!( + cli.command, + Some(TuiCommand::Queue(QueueCmd::Dedup)) + )); + } + + /// The strategy is a value enum, so it is spelled lowercase, completes in + /// a shell, and a typo fails at parse time rather than as a server + /// `InvalidArgument` (architecture/queue-order.md D17). + #[test] + fn queue_sort_parses_a_strategy_and_an_optional_direction() { + let cli = TuiCli::try_parse_from(["cbd-tui", "queue", "sort", "artist"]).expect("parse"); + match cli.command { + Some(TuiCommand::Queue(QueueCmd::Sort { key, desc })) => { + assert_eq!(key, SortKey::Artist); + assert!(!desc, "ascending unless asked"); + } + other => panic!("unexpected: {other:?}"), + } + let cli = TuiCli::try_parse_from(["cbd-tui", "queue", "sort", "duration", "--desc"]) + .expect("parse"); + match cli.command { + Some(TuiCommand::Queue(QueueCmd::Sort { key, desc })) => { + assert_eq!(key, SortKey::Duration); + assert!(desc); + } + other => panic!("unexpected: {other:?}"), + } + assert!( + TuiCli::try_parse_from(["cbd-tui", "queue", "sort", "artiste"]).is_err(), + "a misspelled strategy must not reach the server" + ); + assert!( + TuiCli::try_parse_from(["cbd-tui", "queue", "sort"]).is_err(), + "the strategy is required" + ); + } + #[test] fn global_volume_parses_a_signed_delta() { let cli = diff --git a/cbd-tui/src/app/bindings.rs b/cbd-tui/src/app/bindings.rs index ddd101b..fc58309 100644 --- a/cbd-tui/src/app/bindings.rs +++ b/cbd-tui/src/app/bindings.rs @@ -122,6 +122,15 @@ pub enum Action { QueueRemoveTrack, QueueClearKeepCurrent, QueueClearAll, + /// Drop duplicate entries from the queue, server-side. The playing track + /// is never the one removed, so it cannot interrupt playback; the number + /// removed appears in the pane title for a few seconds + /// (architecture/queue-order.md). + QueueDedup, + /// Open the sort menu: one keypress there reorders the whole queue + /// (architecture/queue-order.md D14). Modal — while it is open, this + /// table is unreachable. + QueueSortMenu, /// Open the input overlay asking for a name to save the queue under /// (a link save at `/crabidy/`). No-op while the queue is empty. QueueSaveAs, @@ -552,6 +561,20 @@ pub const BINDINGS: &[Binding] = &[ action: Action::QueueClearAll, description: "Clear entire queue", }, + Binding { + scope: Scope::Queue, + mods: KeyModifiers::NONE, + code: KeyCode::Char('u'), + action: Action::QueueDedup, + description: "Unique: drop duplicate tracks from the queue", + }, + Binding { + scope: Scope::Queue, + mods: KeyModifiers::SHIFT, + code: KeyCode::Char('S'), + action: Action::QueueSortMenu, + description: "Sort the queue (opens a menu of strategies)", + }, Binding { scope: Scope::Queue, mods: KeyModifiers::NONE, @@ -1056,6 +1079,45 @@ mod tests { } } + /// The queue-order keys are queue-scoped: `u` must not shadow anything in + /// the library, and `Ctrl-u` (jump up) must stay itself + /// (architecture/queue-order.md D14). + #[test] + fn queue_order_keys_are_bound_in_the_queue_only() { + assert_eq!( + lookup( + UiFocus::Queue, + false, + key(KeyCode::Char('u'), KeyModifiers::NONE) + ), + Some(Action::QueueDedup) + ); + assert_eq!( + lookup( + UiFocus::Queue, + false, + key(KeyCode::Char('S'), KeyModifiers::SHIFT) + ), + Some(Action::QueueSortMenu) + ); + for code in [KeyCode::Char('u'), KeyCode::Char('S')] { + assert_eq!( + lookup(UiFocus::Library, false, key(code, KeyModifiers::NONE)), + None, + "{code:?} belongs to the queue" + ); + } + assert_eq!( + lookup( + UiFocus::Queue, + false, + key(KeyCode::Char('u'), KeyModifiers::CONTROL) + ), + Some(Action::QueueJumpUp), + "Ctrl-u still jumps" + ); + } + #[test] fn key_labels_are_human_readable() { assert_eq!(key_label(KeyModifiers::NONE, KeyCode::Char('q')), "q"); diff --git a/cbd-tui/src/app/mod.rs b/cbd-tui/src/app/mod.rs index a1059b6..bda0263 100644 --- a/cbd-tui/src/app/mod.rs +++ b/cbd-tui/src/app/mod.rs @@ -5,6 +5,7 @@ mod list; mod now_playing; mod queue; mod register; +mod sort; use flume::Sender; use ratatui::{ @@ -16,7 +17,7 @@ use ratatui::{ use crabidy_core::proto::crabidy::{ get_update_stream_response::Update as StreamUpdate, CaptureProgress, - InitResponse as InitialData, LibraryNode, + InitResponse as InitialData, LibraryNode, QueueSort, }; pub(crate) use list::{carry_marks, MarkedPane}; @@ -106,6 +107,12 @@ pub enum MessageToUi { Init(InitialData), ReplaceLibraryNode(LibraryNode), Update(StreamUpdate), + /// A `DedupQueue` finished and removed this many entries — the count the + /// queue pane shows for a few seconds (architecture/queue-order.md D15). + /// Typed rather than a preformatted string: the wording belongs to the UI. + QueueDeduped { + removed: u32, + }, } // FIXME: Rename this @@ -147,6 +154,15 @@ pub enum MessageFromUi { RemoveTracks(Vec), ReplaceQueue(Vec), ClearQueue(bool), + /// Drop duplicate queue entries; the reply comes back as + /// [`MessageToUi::QueueDeduped`] (architecture/queue-order.md). + DedupQueue, + /// Reorder the queue by one strategy. Fire-and-forget: the new order + /// arrives on the update stream like every other queue change. + SortQueue { + sort: QueueSort, + descending: bool, + }, NextTrack, PrevTrack, RestartTrack, @@ -306,6 +322,10 @@ pub struct App { pub input: Option, /// `Some` while a `/` search input is open; modal like the others. pub search: Option, + /// True while the queue sort menu is open. Modal like the others: keys go + /// to [`Self::handle_sort_key`] and the bindings table is unreachable + /// (architecture/queue-order.md D14). + pub sort_menu: bool, /// Progress of running (and recently finished) captures, rendered as /// status lines at the bottom of the library pane. pub captures: CaptureBoard, @@ -328,6 +348,7 @@ impl App { show_help: false, input: None, search: None, + sort_menu: false, captures: CaptureBoard::default(), library, now_playing, @@ -337,6 +358,31 @@ impl App { } } + /// Handles one key while the sort menu is open (`sort_menu`). + /// + /// A key the menu claims sends the sort and closes; `Esc`, `q` and `S` + /// close without sorting; anything else is ignored and the menu stays up, + /// so a mistyped key does not silently drop back into the bindings table + /// where it would *do* something (architecture/queue-order.md D14). + pub fn handle_sort_key(&mut self, key: crossterm::event::KeyEvent) { + use crossterm::event::KeyCode; + match key.code { + KeyCode::Esc => self.sort_menu = false, + KeyCode::Char(c) => { + if let Some((sort, descending)) = sort::choose(c) { + let _ = self.tx.send(MessageFromUi::SortQueue { sort, descending }); + self.sort_menu = false; + } else if matches!(c, 'q' | 'S') { + // The keys that got here (and the one that opened it). + self.sort_menu = false; + } + } + // Every other key is swallowed: the menu stays up rather than + // letting a stray keypress reach the pane bindings. + _ => {} + } + } + /// Handles one key while a `/` search input is open /// (`search.is_some()`). Typing filters the focused pane live; /// `Enter` keeps the filter and returns to navigation; `Esc` clears @@ -700,6 +746,21 @@ impl App { self.set_register(dropped); let _ = self.tx.send(MessageFromUi::ClearQueue(false)); } + Action::QueueDedup => { + // Server-side (architecture/queue-order.md D1): the count + // comes back as `MessageToUi::QueueDeduped`. Nothing to do on + // an empty queue. + if !self.queue.is_empty() { + let _ = self.tx.send(MessageFromUi::DedupQueue); + } + } + Action::QueueSortMenu => { + // Opening on an empty queue would offer five ways to sort + // nothing. + if !self.queue.is_empty() { + self.sort_menu = true; + } + } Action::QueueSaveAs => { // Nothing to save from an empty queue; silently ignored // like the other capability-gated openers. @@ -821,7 +882,12 @@ impl App { } } - // The help modal renders last so it overlays every pane. + // The sort menu and the help modal render last so they overlay every + // pane. Both cannot be open at once (each is modal), so the order + // between them is arbitrary. + if self.sort_menu { + sort::render(f); + } if self.show_help { help::render(f); } @@ -1754,6 +1820,83 @@ mod tests { assert_eq!(input.buffer, ""); } + // ---- queue order (architecture/queue-order.md) --------------------- + + #[test] + fn dedup_asks_the_server_only_with_a_non_empty_queue() { + let (mut app, rx) = app(); + assert_eq!(app.dispatch(Action::QueueDedup), DispatchResult::Continue); + assert!(rx.try_recv().is_err(), "nothing to dedup in an empty queue"); + + app.queue.update_queue(one_track_queue()); + let _ = app.dispatch(Action::QueueDedup); + assert!(matches!(rx.try_recv(), Ok(MessageFromUi::DedupQueue))); + } + + #[test] + fn the_sort_menu_opens_only_with_a_non_empty_queue() { + let (mut app, _rx) = app(); + let _ = app.dispatch(Action::QueueSortMenu); + assert!(!app.sort_menu, "no menu for an empty queue"); + + app.queue.update_queue(one_track_queue()); + let _ = app.dispatch(Action::QueueSortMenu); + assert!(app.sort_menu); + } + + #[test] + fn a_sort_menu_key_sends_the_strategy_and_closes_the_menu() { + let (mut app, rx) = app(); + app.queue.update_queue(one_track_queue()); + let _ = app.dispatch(Action::QueueSortMenu); + + app.handle_sort_key(key(crossterm::event::KeyCode::Char('A'))); + assert!(!app.sort_menu, "picking a strategy closes the menu"); + match rx.try_recv() { + Ok(MessageFromUi::SortQueue { sort, descending }) => { + assert_eq!(sort, QueueSort::Artist); + assert!(descending, "the capital sorts descending"); + } + other => panic!("expected a sort message, got {:?}", other.is_ok()), + } + } + + #[test] + fn escape_closes_the_sort_menu_without_sorting() { + let (mut app, rx) = app(); + app.queue.update_queue(one_track_queue()); + for closer in [ + crossterm::event::KeyCode::Esc, + crossterm::event::KeyCode::Char('q'), + crossterm::event::KeyCode::Char('S'), + ] { + let _ = app.dispatch(Action::QueueSortMenu); + app.handle_sort_key(key(closer)); + assert!(!app.sort_menu, "{closer:?} must close the menu"); + assert!(rx.try_recv().is_err(), "{closer:?} must not sort"); + } + } + + /// A key the menu does not offer keeps it open and does nothing. Falling + /// back to the bindings table would let a stray `c` clear the queue while + /// the user thinks they are choosing a sort + /// (architecture/queue-order.md D14). + #[test] + fn an_unknown_key_leaves_the_sort_menu_open() { + let (mut app, rx) = app(); + app.queue.update_queue(one_track_queue()); + let _ = app.dispatch(Action::QueueSortMenu); + for stray in [ + crossterm::event::KeyCode::Char('c'), + crossterm::event::KeyCode::Char('x'), + crossterm::event::KeyCode::Enter, + ] { + app.handle_sort_key(key(stray)); + assert!(app.sort_menu, "{stray:?} must not close the menu"); + assert!(rx.try_recv().is_err(), "{stray:?} must not do anything"); + } + } + #[test] fn queue_download_capture_targets_the_current_queue() { let (mut app, rx) = app(); diff --git a/cbd-tui/src/app/queue.rs b/cbd-tui/src/app/queue.rs index 6d99d91..bfa6305 100644 --- a/cbd-tui/src/app/queue.rs +++ b/cbd-tui/src/app/queue.rs @@ -31,9 +31,16 @@ pub struct Queue { /// Visual (paint-select) mode: `Some(anchor_view)` while active, exactly /// as in the library (architecture/queue-register.md D7). visual: Option, + /// The result of the last dedup and when it arrived: shown in the pane + /// title for [`NOTICE_LINGER`], then gone + /// (architecture/queue-order.md D15). + notice: Option<(String, std::time::Instant)>, tx: Sender, } +/// How long a dedup result stays in the pane title. +const NOTICE_LINGER: std::time::Duration = std::time::Duration::from_secs(4); + impl Queue { pub fn new(tx: Sender) -> Self { Self { @@ -43,10 +50,33 @@ impl Queue { filter: Filter::default(), resolving: false, visual: None, + notice: None, tx, } } + /// Shows how many entries a dedup removed, in the pane title, for a few + /// seconds (architecture/queue-order.md D15). `0` is worth saying: it + /// means the queue held no duplicates. + pub fn show_dedup_result(&mut self, removed: u32) { + let text = match removed { + 0 => "no duplicates".to_string(), + 1 => "removed 1 duplicate".to_string(), + n => format!("removed {n} duplicates"), + }; + self.notice = Some((text, std::time::Instant::now())); + } + + /// The title note to render right now, or `None` once it has expired. + /// Separate from [`Self::show_dedup_result`] so the expiry is checked at + /// render time rather than on a timer. + fn notice(&self) -> Option<&str> { + self.notice + .as_ref() + .filter(|(_, at)| at.elapsed() < NOTICE_LINGER) + .map(|(text, _)| text.as_str()) + } + /// The real queue position under the cursor, mapped through the /// active filter — this is what the server-facing ops send. fn selected_position(&self) -> Option { @@ -290,15 +320,18 @@ impl Queue { } else { COLOR_PRIMARY_DARK })) - .title(if self.visual.is_some() { - // Visual (paint-select) mode: movement toggles marks. - "Queue — VISUAL".to_string() - } else { - match (self.filter.query(), register_len) { - (Some(query), _) => format!("Queue — /{query}▏"), + // One title, several claimants: a mode the user is *in* + // (visual, an active search) outranks a result they have + // already been shown, which outranks the standing register + // count (architecture/queue-order.md D15). + .title(match (self.visual.is_some(), self.filter.query()) { + (true, _) => "Queue — VISUAL".to_string(), + (false, Some(query)) => format!("Queue — /{query}▏"), + (false, None) => match (self.notice(), register_len) { + (Some(notice), _) => format!("Queue — {notice}"), (None, 0) => "Queue".to_string(), (None, n) => format!("Queue — register: {n}"), - } + }, }), ) .highlight_style(Style::default().bg(if focused { @@ -562,6 +595,62 @@ mod tests { assert_eq!(queue.filter_query(), Some("beta")); } + /// The dedup result is shown in the pane title, and `0` is shown too — it + /// is the answer "no duplicates" (architecture/queue-order.md D15). + #[test] + fn the_dedup_result_appears_in_the_title() { + let (tx, _rx) = flume::unbounded(); + let mut queue = Queue::new(tx); + queue.update_queue(queue_data(&["one", "two"], false)); + queue.show_dedup_result(7); + let title = rendered_rows(&mut queue).remove(0); + assert!(title.contains('7'), "title: {title:?}"); + // Zero is an answer, not a non-event: it has to say so in words a + // user can act on, not read as a dropped keypress. + queue.show_dedup_result(0); + let title = rendered_rows(&mut queue).remove(0); + assert!( + title.contains("no duplicates"), + "a zero count is still an answer: {title:?}" + ); + } + + #[test] + fn the_dedup_result_expires() { + let (tx, _rx) = flume::unbounded(); + let mut queue = Queue::new(tx); + queue.update_queue(queue_data(&["one"], false)); + queue.show_dedup_result(3); + assert!(queue.notice().is_some()); + // Backdate the stamp past the linger; the expiry is checked at render + // time, so nothing else has to run. + queue.notice = queue.notice.take().map(|(text, at)| { + ( + text, + at - NOTICE_LINGER - std::time::Duration::from_millis(1), + ) + }); + assert!(queue.notice().is_none()); + let title = rendered_rows(&mut queue).remove(0); + assert!( + !title.contains('3'), + "expired notice still shown: {title:?}" + ); + } + + /// The title has one slot and several claimants; a mode the user is *in* + /// outranks a result they have already seen. + #[test] + fn an_active_search_outranks_the_dedup_result() { + let (tx, _rx) = flume::unbounded(); + let mut queue = Queue::new(tx); + queue.update_queue(queue_data(&["alpha", "beta"], false)); + queue.set_filter(Some("bet".to_string())); + queue.show_dedup_result(2); + let title = rendered_rows(&mut queue).remove(0); + assert!(title.contains("/bet"), "title: {title:?}"); + } + #[test] fn colored_rows_darken_under_the_focused_selection_bar() { // A red (skipped) row under the light focused selection bar was diff --git a/cbd-tui/src/app/sort.rs b/cbd-tui/src/app/sort.rs new file mode 100644 index 0000000..95f9024 --- /dev/null +++ b/cbd-tui/src/app/sort.rs @@ -0,0 +1,222 @@ +//! The queue sort menu: a modal overlay that turns one keypress into a sort +//! strategy (architecture/queue-order.md D14). +//! +//! Opened with `S` in the queue pane. While it is open the bindings table is +//! unreachable, exactly like the help, input and search overlays: keys answer +//! *this* menu. The strategies live in one table so the overlay lists what it +//! accepts and nothing can drift out of it. + +use ratatui::{ + layout::Rect, + style::{Modifier, Style}, + text::{Line, Span}, + widgets::{Block, BorderType, Borders, Clear, Paragraph}, + Frame, +}; + +use crabidy_core::proto::crabidy::QueueSort; + +use super::{COLOR_PRIMARY, COLOR_SECONDARY}; + +/// One row of the menu: the key that picks it, what it does, and the wire +/// strategy it maps to. +pub struct SortChoice { + /// Lowercase key; its uppercase form sorts descending (except + /// [`QueueSort::Reverse`], which has no direction). + pub key: char, + /// Shown verbatim in the overlay. Imperative, no trailing period. + pub description: &'static str, + pub sort: QueueSort, +} + +/// The menu, in display order. `l` is album — `a` is taken by artist, and a +/// menu key only has to be unambiguous *inside the menu*, where the overlay +/// spells it out. +pub const SORT_MENU: &[SortChoice] = &[ + SortChoice { + key: 'a', + description: "Artist, then album", + sort: QueueSort::Artist, + }, + SortChoice { + key: 'l', + description: "Album", + sort: QueueSort::Album, + }, + SortChoice { + key: 't', + description: "Title", + sort: QueueSort::Title, + }, + SortChoice { + key: 'd', + description: "Duration", + sort: QueueSort::Duration, + }, + SortChoice { + key: 'r', + description: "Reverse the current order", + sort: QueueSort::Reverse, + }, +]; + +/// The strategy and direction a key picks, or `None` for a key the menu does +/// not claim (the caller then leaves the menu open). +/// +/// The uppercase form of a key sorts descending. `QueueSort::Reverse` ignores +/// the direction on the wire (D6), so `R` and `r` are the same request. +pub fn choose(key: char) -> Option<(QueueSort, bool)> { + let lower = key.to_ascii_lowercase(); + let entry = SORT_MENU.iter().find(|entry| entry.key == lower)?; + // Reverse has no direction on the wire, so `R` must not send a + // "descending reverse" the server would then ignore (D6). + let descending = key.is_ascii_uppercase() && entry.sort != QueueSort::Reverse; + Some((entry.sort, descending)) +} + +/// The footer line: how to sort descending, and how to leave. +const FOOTER: &str = "Capitals sort descending · Esc/q/S closes"; + +/// The menu's lines: a key column, the descriptions, and the footer. +fn lines() -> Vec> { + let mut lines: Vec> = SORT_MENU + .iter() + .map(|entry| { + Line::from(vec![ + Span::styled( + format!(" {} ", entry.key), + Style::default() + .fg(COLOR_PRIMARY) + .add_modifier(Modifier::BOLD), + ), + Span::from(format!(" {}", entry.description)), + ]) + }) + .collect(); + lines.push(Line::from("")); + lines.push(Line::from(Span::styled( + FOOTER, + Style::default().fg(COLOR_SECONDARY), + ))); + lines +} + +/// Renders the menu as a `Clear`-backed popup centered in the frame, above +/// everything else in it. +pub fn render(f: &mut Frame) { + let area = popup_area(f.area()); + f.render_widget(Clear, area); + let block = Block::default() + .borders(Borders::ALL) + .border_type(BorderType::Rounded) + .border_style(Style::default().fg(COLOR_PRIMARY)) + .title("Sort queue"); + let inner = block.inner(area); + f.render_widget(block, area); + f.render_widget(Paragraph::new(lines()), inner); +} + +/// The centered popup rectangle, sized to the menu but clamped to `frame`. +/// Overflowing lines are truncated rather than scrolled — the help modal's +/// rule, and this content is five rows. +fn popup_area(frame: Rect) -> Rect { + let content_width = lines() + .iter() + .map(|line| line.width() as u16) + .max() + .unwrap_or(0) + .max(FOOTER.chars().count() as u16); + let width = content_width.saturating_add(2).min(frame.width); + let height = (lines().len() as u16).saturating_add(2).min(frame.height); + Rect::new( + frame.x + (frame.width.saturating_sub(width)) / 2, + frame.y + (frame.height.saturating_sub(height)) / 2, + width, + height, + ) +} + +#[cfg(test)] +mod tests { + use super::*; + use ratatui::{backend::TestBackend, Terminal}; + + #[test] + fn a_lowercase_key_sorts_ascending_and_its_capital_descending() { + assert_eq!(choose('a'), Some((QueueSort::Artist, false))); + assert_eq!(choose('A'), Some((QueueSort::Artist, true))); + assert_eq!(choose('l'), Some((QueueSort::Album, false))); + assert_eq!(choose('t'), Some((QueueSort::Title, false))); + assert_eq!(choose('D'), Some((QueueSort::Duration, true))); + } + + /// Reverse has no direction on the wire (D6), so both cases are the same + /// request — not a "descending reverse" that would silently be a no-op. + #[test] + fn reverse_ignores_the_case() { + assert_eq!(choose('r'), Some((QueueSort::Reverse, false))); + assert_eq!(choose('R'), Some((QueueSort::Reverse, false))); + } + + #[test] + fn keys_the_menu_does_not_offer_are_not_claimed() { + for key in ['x', 'j', '1', ' ', 'q'] { + assert_eq!(choose(key), None, "{key:?} is not a strategy"); + } + } + + /// Every table row must be reachable by its own key, and no two rows may + /// claim the same one. + #[test] + fn the_table_is_consistent() { + let mut seen = Vec::new(); + for entry in SORT_MENU { + assert!( + entry.key.is_ascii_lowercase(), + "{:?} must be a lowercase key so its capital is free for descending", + entry.key + ); + assert!(!seen.contains(&entry.key), "duplicate key {:?}", entry.key); + assert!(!entry.description.trim().is_empty()); + assert_eq!(choose(entry.key), Some((entry.sort, false))); + seen.push(entry.key); + } + } + + #[test] + fn the_overlay_lists_every_strategy_with_its_key() { + let backend = TestBackend::new(60, 16); + let mut terminal = Terminal::new(backend).expect("test terminal"); + terminal.draw(render).expect("draw must not panic"); + let buffer = terminal.backend().buffer().clone(); + let text: String = (0..buffer.area.height) + .map(|y| { + (0..buffer.area.width) + .map(|x| buffer[(x, y)].symbol().to_string()) + .collect::() + }) + .collect::>() + .join("\n"); + for entry in SORT_MENU { + assert!( + text.contains(entry.description), + "{:?} missing from the overlay:\n{text}", + entry.description + ); + } + } + + /// A frame smaller than the menu must clamp, not panic or draw outside it + /// — the help modal's rule. + #[test] + fn a_tiny_frame_clamps_the_popup() { + for (w, h) in [(80, 24), (20, 6), (4, 2), (1, 1)] { + let frame = Rect::new(0, 0, w, h); + let area = popup_area(frame); + assert!(area.width <= frame.width && area.height <= frame.height); + let backend = TestBackend::new(w, h); + let mut terminal = Terminal::new(backend).expect("test terminal"); + terminal.draw(render).expect("draw must not panic"); + } + } +} diff --git a/cbd-tui/src/lib.rs b/cbd-tui/src/lib.rs index 1a44ce9..f440aad 100644 --- a/cbd-tui/src/lib.rs +++ b/cbd-tui/src/lib.rs @@ -218,6 +218,22 @@ async fn poll( MessageFromUi::ClearQueue(exclude_current) => { rpc_client.clear_queue(exclude_current).await? } + MessageFromUi::DedupQueue => { + // The count is the whole point (architecture/queue-order.md + // D2); a failure is logged and the queue simply stays as it + // is — it must not tear down the poll loop. + match rpc_client.dedup_queue().await { + Ok(removed) => { + let _ = tx.send(MessageToUi::QueueDeduped { removed }); + } + Err(err) => error!("failed to dedup the queue: {err}"), + } + } + MessageFromUi::SortQueue { sort, descending } => { + if let Err(err) = rpc_client.sort_queue(sort, descending).await { + error!(?sort, descending, "failed to sort the queue: {err}"); + } + } MessageFromUi::SaveQueue(name) => { // A rejected save (bad name, empty queue) must not tear // down the poll loop; the server logs the cause. @@ -334,6 +350,9 @@ fn run_ui( app.now_playing.update_spectrum(frame.bins); } }, + MessageToUi::QueueDeduped { removed } => { + app.queue.show_dedup_result(removed); + } } } @@ -356,6 +375,8 @@ fn run_ui( app.handle_search_key(key); } else if app.input.is_some() { app.handle_input_key(key); + } else if app.sort_menu { + app.handle_sort_key(key); } else if let Some(action) = bindings::lookup(app.focus, app.show_help, key) { if app.dispatch(action) == DispatchResult::Quit { break; diff --git a/cbd-tui/src/rpc.rs b/cbd-tui/src/rpc.rs index 0656cc4..98a4eec 100644 --- a/cbd-tui/src/rpc.rs +++ b/cbd-tui/src/rpc.rs @@ -1,11 +1,11 @@ use crabidy_core::proto::crabidy::{ crabidy_service_client::CrabidyServiceClient, AppendRequest, CaptureLibraryNodeRequest, - ChangeVolumeRequest, ClearQueueRequest, CreateLibraryNodeRequest, DeleteLibraryNodeRequest, - GetLibraryNodeRequest, GetUpdateStreamRequest, GetUpdateStreamResponse, InitRequest, - InitResponse, InsertRequest, LibraryNode, NextRequest, PrevRequest, QueueRequest, - RemoveRequest, RenameLibraryNodeRequest, ReplaceRequest, RestartTrackRequest, SaveQueueRequest, - SeekRequest, SetCurrentRequest, StopRequest, ToggleMuteRequest, TogglePlayRequest, - ToggleRepeatRequest, ToggleShuffleRequest, + ChangeVolumeRequest, ClearQueueRequest, CreateLibraryNodeRequest, DedupQueueRequest, + DeleteLibraryNodeRequest, GetLibraryNodeRequest, GetUpdateStreamRequest, + GetUpdateStreamResponse, InitRequest, InitResponse, InsertRequest, LibraryNode, NextRequest, + PrevRequest, QueueRequest, QueueSort, RemoveRequest, RenameLibraryNodeRequest, ReplaceRequest, + RestartTrackRequest, SaveQueueRequest, SeekRequest, SetCurrentRequest, SortQueueRequest, + StopRequest, ToggleMuteRequest, TogglePlayRequest, ToggleRepeatRequest, ToggleShuffleRequest, }; use std::{collections::HashMap, error::Error, fmt, time::Duration}; @@ -280,6 +280,33 @@ impl RpcClient { Ok(()) } + /// Drops duplicate queue entries and returns how many went + /// (architecture/queue-order.md D2) — the one queue verb with an answer + /// worth showing, because `0` means "no duplicates", not "nothing + /// happened". + pub async fn dedup_queue(&mut self) -> Result> { + let response = self + .client + .dedup_queue(Request::new(DedupQueueRequest {})) + .await?; + Ok(response.into_inner().removed) + } + + /// Reorders the queue server-side; the new order arrives on the update + /// stream like any other queue change. + pub async fn sort_queue( + &mut self, + sort: QueueSort, + descending: bool, + ) -> Result<(), Box> { + let request = Request::new(SortQueueRequest { + sort: sort as i32, + descending, + }); + self.client.sort_queue(request).await?; + Ok(()) + } + pub async fn save_queue(&mut self, name: String) -> Result<(), Box> { let save_queue_request = Request::new(SaveQueueRequest { name }); self.client.save_queue(save_queue_request).await?; diff --git a/cbd-web/src/app.rs b/cbd-web/src/app.rs index eb072c8..18aaf26 100644 --- a/cbd-web/src/app.rs +++ b/cbd-web/src/app.rs @@ -601,9 +601,45 @@ impl Store { })); } } + Action::QueueDedup => { + if self.queue.with_untracked(|q| !q.is_empty()) { + let this = *self; + let Some(mut rpc) = self.rpc() else { return }; + spawn_local(async move { + match rpc.dedup_queue().await { + // 0 is an answer, not a non-event + // (architecture/queue-order.md D2). + Ok(removed) => this.notify(format!("removed {removed} duplicate(s)")), + Err(status) => this.fail(status), + } + }); + } + } + Action::QueueSortMenu => { + // Five ways to sort nothing is not a menu. + if self.queue.with_untracked(|q| !q.is_empty()) { + self.dialog.set(Some(Dialog::Sort)); + } + } + Action::CloseSortMenu => self.dialog.set(None), + Action::QueueSort { sort, descending } => { + self.dialog.set(None); + self.call(async move |mut rpc: Rpc| rpc.sort_queue(sort, descending).await) + } } } + /// Shows a plain notice in the same toast errors use — a result the user + /// asked for, not a failure (architecture/queue-order.md D16). + fn notify(&self, message: String) { + let toast = self.toast; + toast.set(Some(message)); + spawn_local(async move { + gloo_timers::future::TimeoutFuture::new(TOAST_MS).await; + toast.set(None); + }); + } + /// Submits the name dialog (Enter) — the TUI's `handle_input_key` /// submit arm. fn submit_name(&self, purpose: NamePurpose, title: String) { @@ -759,18 +795,24 @@ pub fn App() -> impl IntoView { // their own keys), and browser defaults for handled chords are // suppressed so Space does not scroll or Tab move focus. let _handle = window_event_listener(leptos::ev::keydown, move |ev| { - let dialog_open = store.dialog.with_untracked(Option::is_some); - let help_open = matches!(store.dialog.get_untracked(), Some(Dialog::Help)); - if dialog_open && !help_open { + let dialog = store.dialog.get_untracked(); + let help_open = matches!(dialog, Some(Dialog::Help)); + // Help and the sort menu are the two dialogs that *answer* keys; the + // rest (text input, login) own their fields. + let sort_open = matches!(dialog, Some(Dialog::Sort)); + if dialog.is_some() && !help_open && !sort_open { return; } if ev.alt_key() || ev.meta_key() { return; } let key = ev.key(); - if let Some(action) = + let action = if sort_open { + keymap::sort_menu_key(&key) + } else { keymap::lookup(store.focus.get_untracked(), help_open, &key, ev.ctrl_key()) - { + }; + if let Some(action) = action { ev.prevent_default(); store.dispatch(action); } @@ -1093,6 +1135,24 @@ fn QueueView(store: Store) -> impl IntoView { "VISUAL" + // The two order operations. Buttons as well as keys: `u` and + // `S` are invisible to a mouse (architecture/queue-order.md + // D16), and "sort" opens the same menu the key does, so the + // strategy list exists exactly once. + +