diff --git a/NEWS b/NEWS index 839e79835..7a62ffa57 100644 --- a/NEWS +++ b/NEWS @@ -4,6 +4,7 @@ ver 0.23 (not yet released) - show the audio format in "playlistinfo" - support "listfiles" with arbitrary storage plugins - support relative positions in "addid" + - fix relative positions in "move" and "moveid" * database - proxy: require MPD 0.20 or later - proxy: require libmpdclient 2.11 or later diff --git a/doc/protocol.rst b/doc/protocol.rst index 28977c487..e0a1ce1a0 100644 --- a/doc/protocol.rst +++ b/doc/protocol.rst @@ -739,14 +739,22 @@ Whenever possible, ids should be used. at ``START:END`` [#since_0_15]_ to ``TO`` in the playlist. + If ``TO`` starts with ``+`` or ``-``, then it is relative to the + current song; e.g. ``+0`` moves to right after the current song + and ``-0`` moves to right before the current song (i.e. zero songs + between the current song and the moved range). + .. _command_moveid: :command:`moveid {FROM} {TO}` Moves the song with ``FROM`` (songid) to ``TO`` (playlist index) in the - playlist. If ``TO`` is negative, it - is relative to the current song in the playlist (if - there is one). + playlist. + + If ``TO`` starts with ``+`` or ``-``, then it is relative to the + current song; e.g. ``+0`` moves to right after the current song + and ``-0`` moves to right before the current song (i.e. zero songs + between the current song and the moved song). .. _command_playlist: diff --git a/src/Partition.hxx b/src/Partition.hxx index 5bf6b7243..b90a01101 100644 --- a/src/Partition.hxx +++ b/src/Partition.hxx @@ -147,14 +147,10 @@ struct Partition final : QueueListener, PlayerListener, MixerListener { playlist.Shuffle(pc, range); } - void MoveRange(RangeArg range, int to) { + void MoveRange(RangeArg range, unsigned to) { playlist.MoveRange(pc, range, to); } - void MoveId(unsigned id, int to) { - playlist.MoveId(pc, id, to); - } - void SwapPositions(unsigned song1, unsigned song2) { playlist.SwapPositions(pc, song1, song2); } diff --git a/src/command/QueueCommands.cxx b/src/command/QueueCommands.cxx index e80acb4ab..3ab7a9143 100644 --- a/src/command/QueueCommands.cxx +++ b/src/command/QueueCommands.cxx @@ -363,8 +363,62 @@ handle_prioid(Client &client, Request args, [[maybe_unused]] Response &r) return CommandResult::OK; } +static unsigned +ParseMoveDestination(const char *s, const RangeArg range, + const playlist &p) +{ + assert(!range.IsEmpty()); + assert(!range.IsOpenEnded()); + + const unsigned queue_length = p.queue.GetLength(); + + if (*s == '+') { + /* after the current song */ + + unsigned current = RequireCurrentPosition(p); + assert(current < queue_length); + if (range.Contains(current)) + /* no-op */ + return range.start; + + if (current >= range.end) + current -= range.Count(); + + return current + 1 + + ParseCommandArgUnsigned(s + 1, + queue_length - current - range.Count()); + } else if (*s == '-') { + /* before the current song */ + + unsigned current = RequireCurrentPosition(p); + assert(current < queue_length); + if (range.Contains(current)) + /* no-op */ + return range.start; + + if (current >= range.end) + current -= range.Count(); + + return current - + ParseCommandArgUnsigned(s + 1, + queue_length - current - range.Count()); + } else + /* absolute position */ + return ParseCommandArgUnsigned(s, + queue_length - range.Count()); +} + +static CommandResult +handle_move(Partition &partition, RangeArg range, const char *to) +{ + partition.MoveRange(range, + ParseMoveDestination(to, range, + partition.playlist)); + return CommandResult::OK; +} + CommandResult -handle_move(Client &client, Request args, [[maybe_unused]] Response &r) +handle_move(Client &client, Request args, Response &r) { RangeArg range = args.ParseRange(0); if (range.IsOpenEnded()) { @@ -372,18 +426,22 @@ handle_move(Client &client, Request args, [[maybe_unused]] Response &r) return CommandResult::ERROR; } - int to = args.ParseInt(1); - client.GetPartition().MoveRange(range, to); - return CommandResult::OK; + return handle_move(client.GetPartition(), range, args[1]); } CommandResult handle_moveid(Client &client, Request args, [[maybe_unused]] Response &r) { - unsigned id = args.ParseUnsigned(0); - int to = args.ParseInt(1); - client.GetPartition().MoveId(id, to); - return CommandResult::OK; + auto &partition = client.GetPartition(); + const auto &queue = partition.playlist.queue; + + const int position = queue.IdToPosition(args.ParseUnsigned(0)); + if (position < 0) { + r.Error(ACK_ERROR_NO_EXIST, "No such song"); + return CommandResult::ERROR; + } + + return handle_move(partition, RangeArg::Single(position), args[1]); } CommandResult diff --git a/src/queue/Playlist.hxx b/src/queue/Playlist.hxx index 69cd7d8d2..5e13d0370 100644 --- a/src/queue/Playlist.hxx +++ b/src/queue/Playlist.hxx @@ -258,9 +258,7 @@ public: void Shuffle(PlayerControl &pc, RangeArg range); - void MoveRange(PlayerControl &pc, RangeArg range, int to); - - void MoveId(PlayerControl &pc, unsigned id, int to); + void MoveRange(PlayerControl &pc, RangeArg range, unsigned to); void SwapPositions(PlayerControl &pc, unsigned song1, unsigned song2); diff --git a/src/queue/PlaylistEdit.cxx b/src/queue/PlaylistEdit.cxx index 8816d4a0e..c9b5c630c 100644 --- a/src/queue/PlaylistEdit.cxx +++ b/src/queue/PlaylistEdit.cxx @@ -315,51 +315,31 @@ playlist::StaleSong(PlayerControl &pc, const char *uri) noexcept } void -playlist::MoveRange(PlayerControl &pc, RangeArg range, int to) +playlist::MoveRange(PlayerControl &pc, RangeArg range, unsigned to) { if (!queue.IsValidPosition(range.start) || !queue.IsValidPosition(range.end - 1)) throw PlaylistError::BadRange(); - if ((to >= 0 && to + range.Count() - 1 >= GetLength()) || - (to < 0 && unsigned(std::abs(to)) > GetLength())) + if (to + range.Count() - 1 >= GetLength()) throw PlaylistError::BadRange(); - if ((int)range.start == to) + if (range.start == to) /* nothing happens */ return; const DetachedSong *const queued_song = GetQueuedSong(); - /* - * (to < 0) => move to offset from current song - * (-playlist.length == to) => move to position BEFORE current song - */ - const int currentSong = GetCurrentPosition(); - if (to < 0) { - if (currentSong < 0) - /* can't move relative to current song, - because there is no current song */ - throw PlaylistError::BadRange(); - - if (range.Contains(currentSong)) - /* no-op, can't be moved to offset of itself */ - return; - to = (currentSong + std::abs(to)) % GetLength(); - if (range.start < (unsigned)to) - to -= range.Count(); - } - queue.MoveRange(range.start, range.end, to); if (!queue.random && current >= 0) { /* update current */ if (range.Contains(current)) - current += unsigned(to) - range.start; + current += to - range.start; else if (unsigned(current) >= range.end && - unsigned(current) <= unsigned(to)) + unsigned(current) <= to) current -= range.Count(); - else if (unsigned(current) >= unsigned(to) && + else if (unsigned(current) >= to && unsigned(current) < range.start) current += range.Count(); } @@ -368,16 +348,6 @@ playlist::MoveRange(PlayerControl &pc, RangeArg range, int to) OnModified(); } -void -playlist::MoveId(PlayerControl &pc, unsigned id1, int to) -{ - int song = queue.IdToPosition(id1); - if (song < 0) - throw PlaylistError::NoSuchSong(); - - MoveRange(pc, RangeArg::Single(song), to); -} - void playlist::Shuffle(PlayerControl &pc, RangeArg range) {