queue: insert at a position, not after it, and show queue marks
Two bugs from the register work, both reported from actual use. **Paste landed one row too far.** `QueueManager::insert_tracks` spliced at `position + 1` — it inserted *after* the given index, while its own CLI help says "insert tracks/subtrees at a position". So `p` (which sent cursor + 1) landed two below the cursor and `P` (cursor) landed one below, exactly as reported. The clients were already computing the right indices for an insert-*at* API. Fixed at the primitive rather than in the clients, because "after" cannot express the front of the queue: the earliest reachable index was 1, so pasting before the first row — and therefore undoing a delete of it — was impossible. `insert_tracks` now inserts **at** `position`, pushing that row down, with 0 the front and past-the-end an append. The two callers that genuinely mean "after" pass `N + 1`: `ResolveKind::InsertAfter` (which keeps its name and its streaming-chunk arithmetic) and `queue_tracks` (play-next, `L`). All existing behaviour is preserved — the whole server suite passes untouched — and three tests pin the new front/interior/play-next cases. `queue insert <POS>` on the CLI shifts by one accordingly, which brings it in line with what its help always claimed. Documented in the proto, the CLI help, and the book. **Queue marks were invisible.** The TUI rendered no mark indicator, so `s` and visual mode had no feedback. Marked rows now carry the library's `*` prefix and the same green bold; the playing row keeps `>` and its red, and a row that is both shows `> * title`. The web client already rendered marks (its `.marked .title` rule), but neither client showed visual mode outside the TUI's pane title — both panes there now get a VISUAL badge in the toolbar. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
95700bf31f
commit
7fe4326923
|
|
@ -76,7 +76,8 @@ pub enum QueueCmd {
|
||||||
Show,
|
Show,
|
||||||
/// Append tracks/subtrees (by library path) to the end of the queue.
|
/// Append tracks/subtrees (by library path) to the end of the queue.
|
||||||
Append { paths: Vec<String> },
|
Append { paths: Vec<String> },
|
||||||
/// Insert tracks/subtrees at a position.
|
/// Insert tracks/subtrees at a position, pushing what was there down
|
||||||
|
/// (0 = front, past the end = append).
|
||||||
Insert { position: u32, paths: Vec<String> },
|
Insert { position: u32, paths: Vec<String> },
|
||||||
/// Replace the whole queue with the given tracks/subtrees.
|
/// Replace the whole queue with the given tracks/subtrees.
|
||||||
Replace { paths: Vec<String> },
|
Replace { paths: Vec<String> },
|
||||||
|
|
|
||||||
|
|
@ -10,7 +10,7 @@ use ratatui::{
|
||||||
use crabidy_core::proto::crabidy::Queue as QueueData;
|
use crabidy_core::proto::crabidy::Queue as QueueData;
|
||||||
|
|
||||||
use super::{
|
use super::{
|
||||||
carry_marks, Filter, MarkedPane, MessageFromUi, StatefulList, UiItem, UiItemKind,
|
carry_marks, Filter, MarkedPane, MessageFromUi, StatefulList, UiItem, UiItemKind, COLOR_GREEN,
|
||||||
COLOR_PRIMARY, COLOR_PRIMARY_DARK, COLOR_RED, COLOR_SECONDARY,
|
COLOR_PRIMARY, COLOR_PRIMARY_DARK, COLOR_RED, COLOR_SECONDARY,
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|
@ -229,13 +229,24 @@ impl Queue {
|
||||||
.map(|(idx, (real, item))| {
|
.map(|(idx, (real, item))| {
|
||||||
let active = real == self.current_position;
|
let active = real == self.current_position;
|
||||||
|
|
||||||
let title = if active {
|
// Markers, in the library's vocabulary: `>` is the playing
|
||||||
format!("> {}", item.title)
|
// track, `*` is a mark (`s`, or painted in visual mode). A
|
||||||
} else {
|
// row can be both.
|
||||||
item.title.to_string()
|
let mut title = String::new();
|
||||||
};
|
if active {
|
||||||
|
title.push_str("> ");
|
||||||
|
}
|
||||||
|
if item.marked {
|
||||||
|
title.push_str("* ");
|
||||||
|
}
|
||||||
|
title.push_str(&item.title);
|
||||||
let mut style = if active {
|
let mut style = if active {
|
||||||
Style::default().fg(COLOR_RED).add_modifier(Modifier::BOLD)
|
Style::default().fg(COLOR_RED).add_modifier(Modifier::BOLD)
|
||||||
|
} else if item.marked {
|
||||||
|
// Same green as a marked library row.
|
||||||
|
Style::default()
|
||||||
|
.fg(COLOR_GREEN)
|
||||||
|
.add_modifier(Modifier::BOLD)
|
||||||
} else if item.is_skipped {
|
} else if item.is_skipped {
|
||||||
// No playable audio: rendered red (not bold — the
|
// No playable audio: rendered red (not bold — the
|
||||||
// playing marker keeps precedence), skipped by
|
// playing marker keeps precedence), skipped by
|
||||||
|
|
@ -455,6 +466,29 @@ mod tests {
|
||||||
|
|
||||||
/// Renders and returns the buffer plus the y of the row containing
|
/// Renders and returns the buffer plus the y of the row containing
|
||||||
/// `needle` and the x of its first character.
|
/// `needle` and the x of its first character.
|
||||||
|
#[test]
|
||||||
|
fn marked_rows_render_a_star_like_the_library() {
|
||||||
|
let (tx, _rx) = flume::unbounded();
|
||||||
|
let mut queue = Queue::new(tx);
|
||||||
|
queue.update_queue(queue_data(&["one", "two"], false));
|
||||||
|
queue.select(Some(1));
|
||||||
|
queue.toggle_mark();
|
||||||
|
let rows = rendered_rows(&mut queue);
|
||||||
|
// The playing row keeps `>`; the marked row gets `*`.
|
||||||
|
assert!(
|
||||||
|
rows.iter().any(|row| row.contains("* artist - two")),
|
||||||
|
"marked row needs a star: {rows:?}"
|
||||||
|
);
|
||||||
|
// And the title says so while visual mode is on.
|
||||||
|
queue.toggle_visual();
|
||||||
|
assert!(
|
||||||
|
rendered_rows(&mut queue)
|
||||||
|
.iter()
|
||||||
|
.any(|row| row.contains("VISUAL")),
|
||||||
|
"visual mode needs a title indicator"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
fn render_and_find(queue: &mut Queue, needle: &str) -> (ratatui::buffer::Buffer, u16, u16) {
|
fn render_and_find(queue: &mut Queue, needle: &str) -> (ratatui::buffer::Buffer, u16, u16) {
|
||||||
let backend = TestBackend::new(40, 8);
|
let backend = TestBackend::new(40, 8);
|
||||||
let mut terminal = Terminal::new(backend).expect("test terminal");
|
let mut terminal = Terminal::new(backend).expect("test terminal");
|
||||||
|
|
|
||||||
|
|
@ -884,6 +884,9 @@ fn LibraryView(store: Store) -> impl IntoView {
|
||||||
"‹"
|
"‹"
|
||||||
</button>
|
</button>
|
||||||
<span class="path" title=pane.path.clone()>{pane.title.clone()}</span>
|
<span class="path" title=pane.path.clone()>{pane.title.clone()}</span>
|
||||||
|
<Show when=move || library.with(|p| p.is_visual())>
|
||||||
|
<span class="mode">"VISUAL"</span>
|
||||||
|
</Show>
|
||||||
<span class="spacer"></span>
|
<span class="spacer"></span>
|
||||||
<Show when=move || library.with(|p| p.is_creatable)>
|
<Show when=move || library.with(|p| p.is_creatable)>
|
||||||
<button
|
<button
|
||||||
|
|
@ -1048,6 +1051,9 @@ fn QueueView(store: Store) -> impl IntoView {
|
||||||
(!reg.is_empty()).then(|| format!(" — register: {}", reg.len()))
|
(!reg.is_empty()).then(|| format!(" — register: {}", reg.len()))
|
||||||
}}
|
}}
|
||||||
</span>
|
</span>
|
||||||
|
<Show when=move || store.queue_cursor.with(|c| c.is_visual())>
|
||||||
|
<span class="mode">"VISUAL"</span>
|
||||||
|
</Show>
|
||||||
<span class="spacer"></span>
|
<span class="spacer"></span>
|
||||||
<button
|
<button
|
||||||
class="ghost"
|
class="ghost"
|
||||||
|
|
|
||||||
|
|
@ -392,6 +392,18 @@ input {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* Visual (paint-select) mode badge in a pane toolbar — the web counterpart of
|
||||||
|
the TUI's "— VISUAL" pane title. */
|
||||||
|
.mode {
|
||||||
|
padding: 0.05rem 0.4rem;
|
||||||
|
border: 1px solid var(--accent);
|
||||||
|
border-radius: 0.6rem;
|
||||||
|
color: var(--accent);
|
||||||
|
font-size: 0.75rem;
|
||||||
|
font-weight: 600;
|
||||||
|
letter-spacing: 0.04em;
|
||||||
|
}
|
||||||
|
|
||||||
.help {
|
.help {
|
||||||
min-inline-size: min(52rem, 94vw);
|
min-inline-size: min(52rem, 94vw);
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -148,6 +148,9 @@ message RemoveRequest {
|
||||||
message RemoveResponse {}
|
message RemoveResponse {}
|
||||||
|
|
||||||
message InsertRequest {
|
message InsertRequest {
|
||||||
|
// Index to insert **at**: the entry currently there, and everything after
|
||||||
|
// it, shifts down. 0 inserts at the front, a position at or past the end
|
||||||
|
// appends. "After entry N" is therefore N + 1.
|
||||||
uint32 position = 1;
|
uint32 position = 1;
|
||||||
repeated string paths = 2;
|
repeated string paths = 2;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -343,9 +343,10 @@ impl PendingResolve {
|
||||||
ResolveKind::InsertAfter(position) => {
|
ResolveKind::InsertAfter(position) => {
|
||||||
// Advance the cursor so this op's next chunk lands right
|
// Advance the cursor so this op's next chunk lands right
|
||||||
// behind this one, keeping the collection contiguous.
|
// behind this one, keeping the collection contiguous.
|
||||||
// `insert_tracks` clamps positions past the end.
|
// `insert_tracks` inserts *at* an index and clamps past the
|
||||||
|
// end, so "after N" is "at N + 1".
|
||||||
self.kind = ResolveKind::InsertAfter(position + tracks.len() as u32);
|
self.kind = ResolveKind::InsertAfter(position + tracks.len() as u32);
|
||||||
queue.insert_tracks(position, tracks)
|
queue.insert_tracks(position + 1, tracks)
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
if started.is_some() {
|
if started.is_some() {
|
||||||
|
|
@ -592,19 +593,24 @@ impl QueueManager {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Inserts `tracks` **at** `position`: whatever sits there, and everything
|
||||||
|
/// after it, shifts down. `0` inserts at the front and a position at or
|
||||||
|
/// past the end appends, so every insertion point is reachable — which is
|
||||||
|
/// what a client needs to paste *before* the first row. An empty queue is
|
||||||
|
/// a replace, which starts playback.
|
||||||
|
///
|
||||||
|
/// Callers wanting "after track N" (play-next, `ResolveKind::InsertAfter`)
|
||||||
|
/// pass `N + 1`.
|
||||||
pub fn insert_tracks(&mut self, position: u32, tracks: &[Track]) -> Option<Track> {
|
pub fn insert_tracks(&mut self, position: u32, tracks: &[Track]) -> Option<Track> {
|
||||||
let len = self.tracks.len();
|
let len = self.tracks.len();
|
||||||
if len == 0 {
|
if len == 0 {
|
||||||
return self.replace_with_tracks(tracks);
|
return self.replace_with_tracks(tracks);
|
||||||
}
|
}
|
||||||
let inserted = tracks.len();
|
let inserted = tracks.len();
|
||||||
let position = (position as usize).min(len - 1);
|
let at = (position as usize).min(len);
|
||||||
let order_additions: Vec<usize> = (len..len + inserted).collect();
|
let order_additions: Vec<usize> = (len..len + inserted).collect();
|
||||||
self.play_order.extend(order_additions);
|
self.play_order.extend(order_additions);
|
||||||
let tail: Vec<Track> = self
|
let tail: Vec<Track> = self.tracks.splice(at.., tracks.to_vec()).collect();
|
||||||
.tracks
|
|
||||||
.splice(position + 1.., tracks.to_vec())
|
|
||||||
.collect();
|
|
||||||
self.tracks.extend(tail);
|
self.tracks.extend(tail);
|
||||||
let mut changed: Vec<usize> = Vec::new();
|
let mut changed: Vec<usize> = Vec::new();
|
||||||
// In shuffle mode we may already have played positions that are
|
// In shuffle mode we may already have played positions that are
|
||||||
|
|
@ -614,7 +620,7 @@ impl QueueManager {
|
||||||
.play_order
|
.play_order
|
||||||
.iter_mut()
|
.iter_mut()
|
||||||
.take(self.current_offset)
|
.take(self.current_offset)
|
||||||
.filter(|i| position < **i)
|
.filter(|i| at <= **i)
|
||||||
{
|
{
|
||||||
*i += inserted;
|
*i += inserted;
|
||||||
changed.push(*i);
|
changed.push(*i);
|
||||||
|
|
@ -636,9 +642,10 @@ impl QueueManager {
|
||||||
None
|
None
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Play-next: insert right after the current track.
|
||||||
pub fn queue_tracks(&mut self, tracks: &[Track]) -> Option<Track> {
|
pub fn queue_tracks(&mut self, tracks: &[Track]) -> Option<Track> {
|
||||||
let pos = self.current_position();
|
let pos = self.current_position();
|
||||||
self.insert_tracks(pos as u32, tracks)
|
self.insert_tracks(pos as u32 + 1, tracks)
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn clear(&mut self, exclude_current: bool) -> bool {
|
pub fn clear(&mut self, exclude_current: bool) -> bool {
|
||||||
|
|
@ -750,6 +757,35 @@ mod tests {
|
||||||
assert_eq!(q.next_track().unwrap().title, "track 0");
|
assert_eq!(q.next_track().unwrap().title, "track 0");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Position 0 puts tracks at the very front — the insertion point a
|
||||||
|
/// client needs to paste *before* the first row (and so to undo deleting
|
||||||
|
/// it). Everything at or after the index shifts down.
|
||||||
|
#[test]
|
||||||
|
fn insert_at_zero_lands_at_the_front() {
|
||||||
|
let mut q = queue_with(2);
|
||||||
|
q.insert_tracks(0, &[track(9)]);
|
||||||
|
assert_eq!(titles(&q), vec!["track 9", "track 0", "track 1"]);
|
||||||
|
assert_eq!(q.play_order.len(), 3);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// An interior insert pushes the row that was there down, rather than
|
||||||
|
/// landing after it.
|
||||||
|
#[test]
|
||||||
|
fn insert_at_a_position_pushes_that_row_down() {
|
||||||
|
let mut q = queue_with(3);
|
||||||
|
q.insert_tracks(1, &[track(9)]);
|
||||||
|
assert_eq!(titles(&q), vec!["track 0", "track 9", "track 1", "track 2"]);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Play-next stays "after the current track" even though the primitive
|
||||||
|
/// now inserts *at* an index.
|
||||||
|
#[test]
|
||||||
|
fn queue_tracks_lands_right_after_the_current_track() {
|
||||||
|
let mut q = queue_with(3); // playing track 0
|
||||||
|
q.queue_tracks(&[track(9)]);
|
||||||
|
assert_eq!(titles(&q), vec!["track 0", "track 9", "track 1", "track 2"]);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn insert_past_end_appends() {
|
fn insert_past_end_appends() {
|
||||||
let mut q = queue_with(2);
|
let mut q = queue_with(2);
|
||||||
|
|
|
||||||
|
|
@ -41,6 +41,8 @@ cbd global volume -- -0.1 # lower the volume
|
||||||
marked. `library create`/`rename`/`delete`, and `library save`/
|
marked. `library create`/`rename`/`delete`, and `library save`/
|
||||||
`capture` (the `w`/`W` equivalents into `/crabidy` — see [The crabidy
|
`capture` (the `w`/`W` equivalents into `/crabidy` — see [The crabidy
|
||||||
store](../store.md)) mutate it.
|
store](../store.md)) mutate it.
|
||||||
|
- `queue insert <POS> <PATH>…` inserts **at** `POS`, pushing the row that
|
||||||
|
was there down; `0` puts tracks at the front.
|
||||||
- `queue show` prints the [queue](../queue.md); `queue append`/`insert`/
|
- `queue show` prints the [queue](../queue.md); `queue append`/`insert`/
|
||||||
`replace <PATH>…`, `queue remove <POS>…`, `queue clear
|
`replace <PATH>…`, `queue remove <POS>…`, `queue clear
|
||||||
[--keep-current]`, `queue set-current <POS>`, `queue save`/`capture
|
[--keep-current]`, `queue set-current <POS>`, `queue save`/`capture
|
||||||
|
|
|
||||||
|
|
@ -145,6 +145,10 @@ limits.
|
||||||
|
|
||||||
Two consequences worth knowing:
|
Two consequences worth knowing:
|
||||||
|
|
||||||
|
- `Insert` takes the index to insert **at**: the row that was there, and
|
||||||
|
everything after it, shifts down. `0` is the front and a position at or
|
||||||
|
past the end appends, so every insertion point is reachable — which is what
|
||||||
|
pasting *before* the first row needs. "Play after track N" is `N + 1`.
|
||||||
- `Remove` takes **positions**, so a client must send positions from the
|
- `Remove` takes **positions**, so a client must send positions from the
|
||||||
queue state it is currently showing. Both clients carry their marks across
|
queue state it is currently showing. Both clients carry their marks across
|
||||||
each pushed queue snapshot by matching track paths, so a mark follows its
|
each pushed queue snapshot by matching track paths, so a mark follows its
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue