From 001abcc35c345993c9f46b91d89af1de772dda25 Mon Sep 17 00:00:00 2001 From: Test User Date: Wed, 29 Jul 2026 21:50:17 +0200 Subject: [PATCH] queue: de-duplicate and sort the queue, server-side MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two verbs that rewrite what the queue already holds instead of adding to it: DedupQueue and SortQueue, on the playback loop, for every client. Client-side was the tempting shortcut and the wrong one — a client-computed Remove races the resolve stream, and "sorting" as a Replace of paths re-resolves everything through providers and restarts playback at the head. A duplicate is the same provider item id *scoped to its provider* (the content store keys content the same way, so unscoped ids let two providers' numeric ids collide), or the whole path when a provider reports no id. Deliberately not artist+title: identical metadata is routinely a different recording, and a wrong merge is unrecoverable queue state. Within a group the playing entry survives, else the earliest — "keep the first" stops the music whenever the playing copy is a later one — and the removal itself goes through remove_tracks, so one code path maintains play_order. Sorting permutes `tracks` and then treats the play order by mode: with shuffle off it rebuilds it as the identity with the cursor on the current track (the sort decides what plays next), with shuffle on it remaps it through the permutation so the shuffled sequence and the position in it survive. Five stable strategies (artist→album, album, title, duration, reverse), keys built once per track, unknown sorting last in both directions, and UNSPECIFIED refused as InvalidArgument. DedupQueue answers with a count, alone among the queue verbs: 0 cannot be recovered from the new snapshot and is what a user needs to hear. It crosses as a number — the wording is each client's. TUI: `u` and a modal `S` sort menu whose unclaimed keys are swallowed, plus the count in the queue pane title. Web: the same keys, clickable menu rows, toolbar buttons, count in a toast. CLI: `queue dedup` and `queue sort [--desc]` with the strategy as a ValueEnum. Full dev-flow: architecture/queue-order.md, quality/queue-order.md, plan/queue-order.md, plan/summary.md. Co-Authored-By: Claude Opus 5 (1M context) --- architecture/queue-order.md | 338 +++++++++++++++++++++ cbd-cli/src/client.rs | 59 +++- cbd-cli/src/lib.rs | 70 +++++ cbd-tui/src/app/bindings.rs | 62 ++++ cbd-tui/src/app/mod.rs | 147 ++++++++- cbd-tui/src/app/queue.rs | 103 ++++++- cbd-tui/src/app/sort.rs | 222 ++++++++++++++ cbd-tui/src/lib.rs | 21 ++ cbd-tui/src/rpc.rs | 39 ++- cbd-web/src/app.rs | 112 ++++++- cbd-web/src/keymap.rs | 156 ++++++++++ cbd-web/src/rpc.rs | 32 +- cbd-web/src/state.rs | 4 + cbd-web/style.css | 26 ++ crabidy-core/crabidy/v1/crabidy.proto | 52 ++++ crabidy-server/src/auth.rs | 10 +- crabidy-server/src/lib.rs | 255 +++++++++++++++- crabidy-server/src/playback.rs | 178 +++++++++++ crabidy-server/src/queue_order.rs | 418 ++++++++++++++++++++++++++ crabidy-server/src/rpc.rs | 141 ++++++++- docs/src/clients/cli.md | 6 + docs/src/clients/tui.md | 2 + docs/src/queue.md | 55 ++++ plan/queue-order.md | 137 +++++++++ plan/summary.md | 57 ++++ quality/queue-order.md | 119 ++++++++ 26 files changed, 2775 insertions(+), 46 deletions(-) create mode 100644 architecture/queue-order.md create mode 100644 cbd-tui/src/app/sort.rs create mode 100644 crabidy-server/src/queue_order.rs create mode 100644 plan/queue-order.md create mode 100644 quality/queue-order.md 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. + +