Skip to content

Clear a queued song's row without taking the playlist's own - #344

Merged
crmne merged 1 commit into
crmne:mainfrom
MaxFreedomPollard:clear-queue-keeps-the-contexts-copy
Sep 9, 2026
Merged

crmne merged 1 commit into
crmne:mainfrom
MaxFreedomPollard:clear-queue-keeps-the-contexts-copy

Conversation

@MaxFreedomPollard

Copy link
Copy Markdown
Contributor

Why

Play next puts a song at the top of Next up, and the trash beside that heading takes those rows back. Rule 7 in docs/_reference/queue.md says it only removes your songs and the playlist's rows below stay. It takes one of those too when the song you queued is also coming up later in whatever is playing. Queue a song that sits further down the playlist, change your mind, press the trash, and the playlist loses its copy as well. The row is gone from the list until the next fetch of Spotify's queue brings it back, and on local playback the engine has already been told to drop what it held, so the song after this one is not the one the list promised.

What changed

queue_one writes a queued song to two lists. manual_queue is the session's record of what you asked for; pending_queue_adds holds the same song for a moment so a late or stale answer from Spotify cannot make the new row vanish before it is confirmed. clear_queue in src/app.rs counted the two together:

for uri in self
    .manual_queue
    .iter()
    .chain(self.pending_queue_adds.iter().map(|(uri, _)| uri))
{
    *counts.entry(uri.clone()).or_insert(0) += 1;
}

One song queued once is one row in Next up, but it is in both lists, so counts says two. The retain under it walks the whole queue rather than only the rows on top, so the second row it finds with that URI is the playlist's own, and that goes as well.

The two lists count the same wishes, so the rows to take back are as many as the longer of them holds, not as many as both together. Neither list alone is the count either: a pending add outlives its manual_queue entry once the queued song starts playing, and manual_queue drops its oldest entry past a hundred songs. clear_queue now counts each list and keeps the larger count per song, and nothing else in it moves.

clear_queue_takes_the_hand_queued_rows_and_keeps_the_context sets manual_queue by hand, so pending_queue_adds is empty there and the old and new arithmetic agree. That is why it stayed green through this. the_queued_section_covers_only_the_users_own_rows already says the context's own copy of a queued song is not a queued row; clearing now agrees with it.

Verification

clearing_a_just_queued_song_leaves_the_contexts_copy_of_it goes through the actions rather than the fields: with spotify:track:b already among the context's upcoming rows, Action::AddToQueue for b, then Action::ClearQueue. It asserts the queue straight after the add too, so a change that stopped inserting the row could not make it pass by accident.

On main it fails:

assertion `left == right` failed: the queued b goes and the context keeps the b it was going to play
  left: ["spotify:track:ctx1", "spotify:track:ctx2"]
 right: ["spotify:track:ctx1", "spotify:track:b", "spotify:track:ctx2"]

macOS 26.2 on Apple silicon, toolchain 1.98.0 from rust-toolchain.toml. cargo fmt --all --check, cargo clippy --locked --no-default-features --all-targets -- -D warnings, cargo test --locked --no-default-features --all-targets (350 passed, up from 349 by the new test), cargo test --locked --no-default-features --doc, and RUSTDOCFLAGS='-D warnings' cargo doc --locked --no-default-features --no-deps. --no-default-features throughout because this machine has no libprojectM build for MilkDrop; nothing here touches that feature. Not run on Linux or Windows: the change is std collections with no platform-specific code.

No user-visible setting, file, or network access changes, and the queue's documented rules are the ones this restores, so there is no documentation change.

  • I read CONTRIBUTING.md and kept this pull request to one concern.
  • I added or updated tests for behaviour that can regress.
  • I ran formatting, Clippy, and the relevant tests locally.
  • I updated documentation for user-visible behaviour, settings, files, or network access.
  • I understand and take responsibility for every submitted line, including AI-assisted code.

Play next writes a queued song to two lists: `manual_queue`, saved with
the session, and `pending_queue_adds`, which holds the same song for a
moment so a late answer from Spotify cannot make the new row vanish.
`clear_queue` in src/app.rs counted the two together, so one song queued
once asked Next up for two rows. The `retain` below walks the whole list
rather than only the rows on top, so the second row it took was the
playlist's own copy of that song, further down.

Queue a song that is already coming up later, change your mind, press
the trash beside Playing next, and the playlist loses it too. Rule 7 in
docs/_reference/queue.md says clear only removes your songs, and
`the_queued_section_covers_only_the_users_own_rows` already says the
context's own copy is not one of them.

The two lists count the same wishes, so the rows to take back are as
many as the longer of them holds. Neither list alone is the count: a
pending add outlives its `manual_queue` entry once the song starts, and
`manual_queue` drops its oldest entry past a hundred songs.
@crmne
crmne merged commit ccce4cd into crmne:main Sep 9, 2026
6 checks passed
crmne added a commit that referenced this pull request Sep 9, 2026
9didhe pushed a commit to 9didhe/spotifast-prism that referenced this pull request Nov 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants