From 1ce64804fb0e0986428ff79809d1fb190d2a5a28 Mon Sep 17 00:00:00 2001 From: Keith Edmunds Date: Sun, 28 Apr 2024 12:54:32 +0100 Subject: [PATCH] Fix moving rows Also fix associated tests. Fixes #234 --- app/playlistmodel.py | 15 ++++++++++++--- tests/test_playlistmodel.py | 10 +++++----- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/app/playlistmodel.py b/app/playlistmodel.py index 1d1af4f..e81b662 100644 --- a/app/playlistmodel.py +++ b/app/playlistmodel.py @@ -856,15 +856,23 @@ class PlaylistModel(QAbstractTableModel): # Build a {current_row_number: new_row_number} dictionary row_map: dict[int, int] = {} + # The destination row number will need to be reduced by the + # number of rows being move from above the destination row + # otherwise rows below the destination row will end up above the + # moved rows. + adjusted_to_row = to_row_number - len([a for a in from_rows if a <= to_row_number]) + # Put the from_row row numbers into the row_map. Ultimately the # total number of elements in the playlist doesn't change, so # check that adding the moved rows starting at to_row won't # overshoot the end of the playlist. - if to_row_number + len(from_rows) > len(self.playlist_rows): + if adjusted_to_row + len(from_rows) > len(self.playlist_rows): next_to_row = len(self.playlist_rows) - len(from_rows) else: - next_to_row = to_row_number + next_to_row = adjusted_to_row + # zip iterates from_row and to_row simultaneously from the + # respective sequences inside zip() for from_row, to_row in zip( from_rows, range(next_to_row, next_to_row + len(from_rows)) ): @@ -873,7 +881,8 @@ class PlaylistModel(QAbstractTableModel): # Move the remaining rows to the row_map. We want to fill it # before (if there are gaps) and after (likewise) the rows that # are moving. - # This iterates old_row and new_row simultaneously. + # zip iterates old_row and new_row simultaneously from the + # respective sequences inside zip() for old_row, new_row in zip( [x for x in self.playlist_rows.keys() if x not in from_rows], [y for y in range(len(self.playlist_rows)) if y not in row_map.values()], diff --git a/tests/test_playlistmodel.py b/tests/test_playlistmodel.py index d164750..2a45ead 100644 --- a/tests/test_playlistmodel.py +++ b/tests/test_playlistmodel.py @@ -159,9 +159,9 @@ class TestMMMiscRowMove(unittest.TestCase): elif row == 3: assert self.model.playlist_rows[row].note == str(4) elif row == 4: - assert self.model.playlist_rows[row].note == str(5) - elif row == 5: assert self.model.playlist_rows[row].note == str(3) + elif row == 5: + assert self.model.playlist_rows[row].note == str(5) def test_move_rows_test3(self): # move row 4 to row 3 @@ -208,7 +208,7 @@ class TestMMMiscRowMove(unittest.TestCase): assert row in self.model.playlist_rows assert self.model.playlist_rows[row].plr_rownum == row new_order.append(int(self.model.playlist_rows[row].note)) - assert new_order == [0, 2, 3, 6, 7, 8, 9, 1, 4, 5, 10] + assert new_order == [0, 2, 3, 6, 7, 1, 4, 5, 10, 8, 9] def test_move_rows_test6(self): # move rows [3, 6] → 5 @@ -221,7 +221,7 @@ class TestMMMiscRowMove(unittest.TestCase): assert row in self.model.playlist_rows assert self.model.playlist_rows[row].plr_rownum == row new_order.append(int(self.model.playlist_rows[row].note)) - assert new_order == [0, 1, 2, 4, 5, 3, 6, 7, 8, 9, 10] + assert new_order == [0, 1, 2, 4, 3, 6, 5, 7, 8, 9, 10] def test_move_rows_test7(self): # move rows [3, 5, 6] → 8 @@ -234,7 +234,7 @@ class TestMMMiscRowMove(unittest.TestCase): assert row in self.model.playlist_rows assert self.model.playlist_rows[row].plr_rownum == row new_order.append(int(self.model.playlist_rows[row].note)) - assert new_order == [0, 1, 2, 4, 7, 8, 9, 10, 3, 5, 6] + assert new_order == [0, 1, 2, 4, 7, 3, 5, 6, 8, 9, 10] def test_move_rows_test8(self): # move rows [7, 8, 10] → 5