app/config.py

@@ -64,7 +64,6 @@ class Config(object):
     MAX_INFO_TABS = 5
     MAX_MISSING_FILES_TO_REPORT = 10
     MILLISECOND_SIGFIGS = 0
-    MINIMUM_ROW_HEIGHT = 30
     MYSQL_CONNECT = os.environ.get('MYSQL_CONNECT') or "mysql+mysqldb://musicmuster:musicmuster@localhost/musicmuster_v2"  # noqa E501
     NOTE_TIME_FORMAT = "%H:%M:%S"
     ROOT = os.environ.get('ROOT') or "/home/kae/music"

app/models.py

@@ -428,8 +428,9 @@ class PlaylistRows(Base):
                          plr.note)
 
     @staticmethod
-    def delete_higher_rows(
+    def delete_plrids_not_in_list(
-            session: scoped_session, playlist_id: int, maxrow: int) -> None:
+            session: scoped_session, playlist_id: int,
+            plr_ids: Union[Iterable[int], ValuesView]) -> None:
         """
         Delete rows in given playlist that have a higher row number
         than 'maxrow'
@@ -439,10 +440,11 @@ class PlaylistRows(Base):
             delete(PlaylistRows)
             .where(
                 PlaylistRows.playlist_id == playlist_id,
-                PlaylistRows.row_number > maxrow
+                PlaylistRows.id.not_in(plr_ids)
             )
         )
-        session.flush()
+        # Delete won't take effect until commit()
+        session.commit()
 
     @staticmethod
     def fixup_rownumbers(session: scoped_session, playlist_id: int) -> None:

app/musicmuster.py

@@ -722,20 +722,17 @@ class Window(QMainWindow, Ui_MainWindow):
         # doesn't see player=None and kick off end-of-track actions
         self.playing = False
 
-        # Remove currently playing track colour
-        if (
-            self.current_track and
-            self.current_track.playlist_tab and
-            self.current_track.plr_id
-        ):
-            self.current_track.playlist_tab.reset_plr_row_colour(
-                self.current_track.plr_id)
-
         # Reset PlaylistTrack objects
         if self.current_track.track_id:
             self.previous_track = self.current_track
             self.current_track = PlaylistTrack()
 
+        # Repaint playlist to remove currently playing track colour
+        # What was current track is now previous track
+        with Session() as session:
+            if self.previous_track.playlist_tab:
+                self.previous_track.playlist_tab.update_display(session)
+
         # Reset clocks
         self.frame_fade.setStyleSheet("")
         self.frame_silent.setStyleSheet("")
@@ -859,8 +856,7 @@ class Window(QMainWindow, Ui_MainWindow):
         # Update all displayed playlists
         with Session() as session:
             for i in range(self.tabPlaylist.count()):
-                self.tabPlaylist.widget(i).hide_played_tracks(
+                self.tabPlaylist.widget(i).update_display(session)
-                    self.hide_played_tracks)
 
     def import_track(self) -> None:
         """Import track file"""
@@ -931,6 +927,8 @@ class Window(QMainWindow, Ui_MainWindow):
         """
 
         self.statusbar.showMessage("Imports complete")
+        with Session() as session:
+            playlist_tab.update_display(session)
 
     def insert_header(self) -> None:
         """Show dialog box to enter header text and add to playlist"""
@@ -1251,16 +1249,14 @@ class Window(QMainWindow, Ui_MainWindow):
             self.disable_play_next_controls()
 
             # If previous track playlist is showing and that's not the
-            # current track playlist, we need to reset the current track
+            # current track playlist, we need to update the display to
-            # highlighting
+            # reset the current track highlighting
             if (
                 self.previous_track.playlist_tab == self.visible_playlist_tab()
                 and
                 self.current_track.playlist_tab != self.visible_playlist_tab()
-                and self.previous_track.plr_id
             ):
-                self.previous_track.playlist_tab.reset_plr_row_colour(
+                self.visible_playlist_tab().update_display(session)
-                    self.previous_track.plr_id)
 
             # Update headers
             self.update_headers()

app/playlists.py

@@ -64,6 +64,7 @@ if TYPE_CHECKING:
 
 start_time_re = re.compile(r"@\d\d:\d\d:\d\d")
 HEADER_NOTES_COLUMN = 2
+MINIMUM_ROW_HEIGHT = 30
 
 # Columns
 Column = namedtuple("Column", ['idx', 'heading'])
@@ -100,10 +101,6 @@ class NoSelectDelegate(QStyledItemDelegate):
 
     https://stackoverflow.com/questions/72790705/
     dont-select-text-in-qtablewidget-cell-when-editing/72792962#72792962
-
-    Now this:
-     - increases the height of a row when editing to make editing easier
-     - closes the edit on control-return
     """
 
     def createEditor(self, parent, option, index):
@@ -111,8 +108,7 @@ class NoSelectDelegate(QStyledItemDelegate):
             # Make row just a little bit taller
             row = index.row()
             row_height = self.parent().rowHeight(row)
-            self.parent().setRowHeight(row,
+            self.parent().setRowHeight(row, row_height + MINIMUM_ROW_HEIGHT)
-                                       row_height + Config.MINIMUM_ROW_HEIGHT)
             return QPlainTextEdit(parent)
         return super().createEditor(parent, option, index)
 
@@ -131,9 +127,10 @@ class NoSelectDelegate(QStyledItemDelegate):
 
 class PlaylistTab(QTableWidget):
     # Qt.UserRoles
-    ROW_TRACK_ID = Qt.UserRole
+    ROW_FLAGS = Qt.UserRole
-    ROW_DURATION = Qt.UserRole + 1
+    ROW_TRACK_ID = Qt.UserRole + 1
-    PLAYLISTROW_ID = Qt.UserRole + 2
+    ROW_DURATION = Qt.UserRole + 2
+    PLAYLISTROW_ID = Qt.UserRole + 3
 
     def __init__(self, musicmuster: "Window",
                  session: scoped_session,
@@ -142,9 +139,12 @@ class PlaylistTab(QTableWidget):
         self.musicmuster: Window = musicmuster
         self.playlist_id = playlist_id
 
-        # Set up widget
         self.menu: Optional[QMenu] = None
+
+        # Don't select text on edit
         self.setItemDelegate(NoSelectDelegate(self))
+
+        # Set up widget
         self.setEditTriggers(QAbstractItemView.DoubleClicked)
         self.setAlternatingRowColors(True)
         self.setSelectionMode(QAbstractItemView.ExtendedSelection)
@@ -153,7 +153,7 @@ class PlaylistTab(QTableWidget):
         self.setRowCount(0)
         self.setColumnCount(len(columns))
         self.v_header = self.verticalHeader()
-        self.v_header.setMinimumSectionSize(Config.MINIMUM_ROW_HEIGHT)
+        self.v_header.setMinimumSectionSize(MINIMUM_ROW_HEIGHT)
         self.horizontalHeader().setStretchLastSection(True)
 
         # Header row
@@ -182,7 +182,6 @@ class PlaylistTab(QTableWidget):
         # Qt::CustomContextMenu, and the user has requested a context
         # menu on the widget.
         self.customContextMenuRequested.connect(self._context_menu)
-        # Call self.eventFilter() for events
         self.viewport().installEventFilter(self)
 
         self.itemSelectionChanged.connect(self._select_event)
@@ -192,8 +191,10 @@ class PlaylistTab(QTableWidget):
         self.selecting_in_progress = False
         # Connect signals
         self.horizontalHeader().sectionResized.connect(self._column_resize)
+        # self.horizontalHeader().sectionClicked.connect(self._header_click)
+        # self.setSortingEnabled(True)
 
-        # Load playlist rows
+        # Now load our tracks and notes
         self.populate_display(session, self.playlist_id)
 
     def __repr__(self) -> str:
@@ -246,7 +247,12 @@ class PlaylistTab(QTableWidget):
 
         super().dropEvent(event)
 
-        with Session() as session:
+        log.debug(
+            "playlist.dropEvent(): "
+            f"Moved row(s) {rows} to become row {drop_row}"
+        )
+
+        with Session() as session:       # checked
             self.save_playlist(session)
             self.update_display(session)
 
@@ -343,28 +349,28 @@ class PlaylistTab(QTableWidget):
                                 lambda: self._rescan(row_number, track_id)
                             )
 
+                            self.menu.addSeparator()
+
+                            # Look up in wikipedia
+                            act_wikip = self.menu.addAction("Wikipedia")
+                            act_wikip.triggered.connect(
+                                lambda: self._wikipedia(row_number)
+                            )
+
+                            # Look up in songfacts
+                            act_songfacts = self.menu.addAction("Songfacts")
+                            act_songfacts.triggered.connect(
+                                lambda: self._songfacts(row_number)
+                            )
+
+                            self.menu.addSeparator()
+
                             # Remove track
                             act_remove_track = self.menu.addAction(
                                 'Remove track')
                             act_remove_track.triggered.connect(
                                 lambda: self._remove_track(row_number)
                             )
-                            self.menu.addSeparator()
-
-                        # Look up in wikipedia
-                        act_wikip = self.menu.addAction("Wikipedia")
-                        act_wikip.triggered.connect(
-                            lambda: self._wikipedia(row_number)
-                        )
-
-                        # Look up in songfacts
-                        act_songfacts = self.menu.addAction("Songfacts")
-                        act_songfacts.triggered.connect(
-                            lambda: self._songfacts(row_number)
-                        )
-
-                        self.menu.addSeparator()
-
                     if header_row:
                         # Add track to section header (ie, make this a track
                         # row)
@@ -407,8 +413,10 @@ class PlaylistTab(QTableWidget):
     # 'return' doesn't play the next track.
     #
     # Earlier in this file:
-    #   self.setEditTriggers(QAbstractItemView.DoubleClicked) - triggers
+    #    - self.setEditTriggers(QAbstractItemView.DoubleClicked) - triggers
-    #   editing on double-click
+    #    editing on double-click
+    #    - self.setItemDelegate(NoSelectDelegate(self)) and associated class
+    #    ensure that the text is not selected when editing starts
     #
     # Call sequences:
     #   Start editing:
@@ -599,22 +607,8 @@ class PlaylistTab(QTableWidget):
 
         return [plr for plr in plrs if plr is not None]
 
-    def hide_played_tracks(self, hide: bool) -> None:
+    def insert_header(self, session: scoped_session, note: str,
-        """Hide played tracks if hide is True else show them"""
+                      repaint: bool = True) -> None:
-
-        with Session() as session:
-            played = [
-                p.row_number for p in PlaylistRows.get_played_rows(
-                    session, self.playlist_id)
-            ]
-            for row in range(self.rowCount()):
-                if row in played:
-                    if hide:
-                        self.hideRow(row)
-                    else:
-                        self.showRow(row)
-
-    def insert_header(self, session: scoped_session, note: str) -> None:
         """
         Insert section header into playlist tab.
 
@@ -627,7 +621,7 @@ class PlaylistTab(QTableWidget):
 
         row_number = self.get_new_row_number()
         plr = PlaylistRows(session, self.playlist_id, None, row_number, note)
-        self.insert_row(session, plr)
+        self.insert_row(session, plr, repaint)
         self.save_playlist(session)
 
     def insert_row(self, session: scoped_session, plr: PlaylistRows,
@@ -644,6 +638,7 @@ class PlaylistTab(QTableWidget):
 
         # Add row metadata to userdata column
         userdata_item = QTableWidgetItem()
+        userdata_item.setData(self.ROW_FLAGS, 0)
         userdata_item.setData(self.PLAYLISTROW_ID, plr.id)
         userdata_item.setData(self.ROW_TRACK_ID, plr.track_id)
         self.setItem(row, USERDATA, userdata_item)
@@ -656,6 +651,8 @@ class PlaylistTab(QTableWidget):
                 return
             start_gap_item = self._set_item_text(
                 row, START_GAP, str(start_gap))
+            if start_gap and start_gap >= 500:
+                start_gap_item.setBackground(QColor(Config.COLOUR_LONG_START))
 
             track_title = plr.track.title
             if not track_title:
@@ -682,7 +679,7 @@ class PlaylistTab(QTableWidget):
                 bitrate = ""
             _ = self._set_item_text(row, BITRATE, bitrate)
 
-            # As we have a track_id, any notes should be contained in
+            # As we have track info, any notes should be contained in
             # the notes column
             plr_note = plr.note
             if not plr_note:
@@ -693,9 +690,6 @@ class PlaylistTab(QTableWidget):
             last_played_str = get_relative_date(last_playtime)
             _ = self._set_item_text(row, LASTPLAYED, last_played_str)
 
-            # This is a new track so must be unplayed
-            self._set_row_bold(row)
-
         else:
             # This is a section header so it must have note text
             if plr.note is None:
@@ -704,20 +698,19 @@ class PlaylistTab(QTableWidget):
                 )
                 return
 
-            # In order to colour the row, we need items in every column.
+            # Make empty items (row background won't be coloured without
-            # Bug in PyQt5 means that required height of row considers
+            # items present). Any notes should displayed starting in
-            # text to be wrapped in one column and ignores any spanned
+            # column 2 for now - bug in Qt means that when row size is
-            # columns, hence putting notes in HEADER_NOTES_COLUMN which
+            # set, spanned columns are ignored, so put notes in col2
-            # is typically reasonably wide and thus minimises
+            # (typically title).
-            # unneccessary row height increases.
             for i in range(1, len(columns)):
-                if i == HEADER_NOTES_COLUMN:
+                if i == 2:
                     continue
                 self.setItem(row, i, QTableWidgetItem())
             self.setSpan(row, HEADER_NOTES_COLUMN, 1, len(columns) - 1)
             _ = self._set_item_text(row, HEADER_NOTES_COLUMN, plr.note)
 
-            # Save (or clear) track_id
+            # Save (no) track_id
             userdata_item.setData(self.ROW_TRACK_ID, 0)
 
         if repaint:
@@ -770,7 +763,10 @@ class PlaylistTab(QTableWidget):
         Notification from musicmuster that track has started playing.
 
         Actions required:
+        - Note start time
+        - Mark next-track row as current
         - Mark current row as played
+        - Scroll to put next track as required
         - Set next track
         - Update display
         """
@@ -784,11 +780,12 @@ class PlaylistTab(QTableWidget):
                       )
             return
 
+        search_from = current_row + 1
+
         # Mark current row as played
         self._set_played_row(session, current_row)
 
-        # Set next track
+        next_row = self._find_next_track_row(session, search_from)
-        next_row = self._find_next_track_row(session, current_row + 1)
         if next_row:
             self._set_next(session, next_row)
 
@@ -830,9 +827,14 @@ class PlaylistTab(QTableWidget):
         self._set_column_widths(session)
 
         # Needed to wrap notes column correctly - add to event queue so
-        # that it's processed after list is populated
+        # that it's processed after list populated
         QTimer.singleShot(0, self.tab_visible)
 
+        # We possibly don't need to save the playlist here, but row
+        # numbers may have changed during population, and it's cheap to do
+        # self.save_playlist(session)
+        self.update_display(session)
+
     def remove_rows(self, row_numbers: List[int]) -> None:
         """Remove passed rows from display"""
 
@@ -848,15 +850,6 @@ class PlaylistTab(QTableWidget):
         # Reset drag mode
         self.setDragEnabled(False)
 
-    def reset_plr_row_colour(self, plr_id: int) -> None:
-        """Reset background of row pointed to by plr_id"""
-
-        row = self._plrid_to_row_number(plr_id)
-        if not row:
-            return
-
-        self._set_row_colour(row, None)
-
     def save_playlist(self, session: scoped_session) -> None:
         """
         Get the PlaylistRow objects for each row in the display. Correct
@@ -865,20 +858,34 @@ class PlaylistTab(QTableWidget):
         the display.
         """
 
+        # Build a dictionary of
+        #   {display_row_number: display_row_plr_id}
+        display_plr_ids = {row_number: self._get_playlistrow_id(row_number)
+                           for row_number in range(self.rowCount())}
+
+        # Now build a dictionary of
+        #   {display_row_number: display_row_plr}
+        plr_dict_by_id = PlaylistRows.indexed_by_id(
+            session, display_plr_ids.values())
+
+        # Finally a dictionary of
+        #   {display_row_number: plr}
+        row_plr = {row_number: plr_dict_by_id[display_plr_ids[row_number]]
+                   for row_number in range(self.rowCount())}
+
         # Ensure all row plrs have correct row number and playlist_id
         for row in range(self.rowCount()):
-            plr = self._get_playlistrow_object(session, row)
+            row_plr[row].row_number = row
-            if not plr:
+            row_plr[row].playlist_id = self.playlist_id
-                continue
-            plr.row_number = row
-            plr.playlist_id = self.playlist_id
 
-        # Any rows in the database for this playlist that has a row
+        # Any rows in the database for this playlist that have a plr id
-        # number equal to or greater than the row count needs to be
+        # that's not in the displayed playlist need to be deleted.
-        # removed.
+
+        # Ensure changes flushed
         session.flush()
-        PlaylistRows.delete_higher_rows(
+        PlaylistRows.delete_plrids_not_in_list(
-            session, self.playlist_id, self.rowCount() - 1)
+            session, self.playlist_id,
+            display_plr_ids.values())
 
     def scroll_current_to_top(self) -> None:
         """Scroll currently-playing row to top"""
@@ -924,6 +931,9 @@ class PlaylistTab(QTableWidget):
         Wrap at last row.
         """
 
+        row: int
+        selected_rows: List[int]
+
         selected_rows = self._get_selected_rows()
         # we will only handle zero or one selected rows
         if len(selected_rows) > 1:
@@ -937,14 +947,14 @@ class PlaylistTab(QTableWidget):
                 row = 0
 
         # Don't select section headers
-        wrapped = False
+        wrapped: bool = False
         track_id = self._get_row_track_id(row)
         while not track_id:
             row += 1
             if row >= self.rowCount():
                 if wrapped:
                     # we're already wrapped once, so there are no
-                    # non-headers
+                    # non-notes
                     return
                 row = 0
                 wrapped = True
@@ -958,6 +968,9 @@ class PlaylistTab(QTableWidget):
         Wrap at first row.
         """
 
+        row: int
+        selected_rows: List[int]
+
         selected_rows = self._get_selected_rows()
         # we will only handle zero or one selected rows
         if len(selected_rows) > 1:
@@ -972,7 +985,7 @@ class PlaylistTab(QTableWidget):
                 row = last_row
 
         # Don't select section headers
-        wrapped = False
+        wrapped: bool = False
         track_id = self._get_row_track_id(row)
         while not track_id:
             row -= 1
@@ -991,9 +1004,11 @@ class PlaylistTab(QTableWidget):
         """Sets the select track as next to play"""
 
         row = self._get_selected_row()
-        if row is not None:
+        if row is None:
-            with Session() as session:
+            return None
-                self._set_next(session, row)
+
+        with Session() as session:
+            self._set_next(session, row)
 
     def tab_visible(self) -> None:
         """Called when tab becomes visible"""
@@ -1225,10 +1240,6 @@ class PlaylistTab(QTableWidget):
             if not plr:
                 return
 
-            # Don't add track if there's already a track there
-            if plr.track_id is not None:
-                return
-
             plr.track_id = track.id
             session.flush()
 
@@ -1250,6 +1261,8 @@ class PlaylistTab(QTableWidget):
 
             start_gap_item = self._set_item_text(row, START_GAP,
                                                  str(track.start_gap))
+            if track.start_gap and track.start_gap >= 500:
+                start_gap_item.setBackground(QColor(Config.COLOUR_LONG_START))
 
             self._update_row(session, row, track)
 
@@ -1371,12 +1384,14 @@ class PlaylistTab(QTableWidget):
                 session, self.playlist_id)
         ]
         for row in range(starting_row, self.rowCount()):
-            if row not in track_rows or row in played_rows:
-                continue
             plr = self._get_playlistrow_object(session, row)
             if not plr:
                 continue
-            if not file_is_readable(plr.track.path):
+            if (
+                    row not in track_rows or
+                    row in played_rows or
+                    not file_is_readable(plr.track.path)
+            ):
                 continue
             else:
                 return row
@@ -1431,7 +1446,11 @@ class PlaylistTab(QTableWidget):
                                 row: int) -> Optional[PlaylistRows]:
         """Return the playlistrow object associated with this row"""
 
-        playlistrow_id = self._get_playlistrow_id(row)
+        userdata_item = self.item(row, USERDATA)
+        if not userdata_item:
+            return None
+
+        playlistrow_id = userdata_item.data(self.PLAYLISTROW_ID)
         if not playlistrow_id:
             return None
 
@@ -1583,9 +1602,7 @@ class PlaylistTab(QTableWidget):
         """
 
         self.musicmuster.clear_next()
-        self.clear_selection()
         with Session() as session:
-            # TODO: or just reset row background
             self.update_display(session)
         self.musicmuster.update_headers()
 
@@ -1598,7 +1615,6 @@ class PlaylistTab(QTableWidget):
             session.add(plr)
             plr.played = False
             session.flush()
-            # TODO: or just reset row to bold
             self.update_display(session)
 
     def _move_row(self, session: scoped_session, plr: PlaylistRows,
@@ -1684,7 +1700,7 @@ class PlaylistTab(QTableWidget):
             # We can't have null text
             if not plr.note:
                 plr.note = Config.TEXT_NO_TRACK_NO_NOTE
-            session.flush()
+            session.commit()
 
             # Clear track text items
             for i in range(2, len(columns)):
@@ -1741,10 +1757,10 @@ class PlaylistTab(QTableWidget):
             # row 0.
             # target_row = max(0, row - Config.SCROLL_TOP_MARGIN + 1)
             for i in range(row - 1, -1, -1):
-                if self.isRowHidden(i):
-                    continue
                 if padding_required == 0:
                     break
+                if self.isRowHidden(i):
+                    continue
                 top_row = i
                 padding_required -= 1
 
@@ -1830,6 +1846,7 @@ class PlaylistTab(QTableWidget):
         for row in selected_rows:
             ms += self._get_row_duration(row)
 
+        # Only paint message if there are selected track rows
         if ms > 0:
             self.musicmuster.lblSumPlaytime.setText(
                 f"Selected duration: {ms_to_mmss(ms)}")
@@ -1876,7 +1893,6 @@ class PlaylistTab(QTableWidget):
         - Update display
         """
 
-        # Check row has a track
         track_id = self._get_row_track_id(row_number)
         if not track_id:
             log.error(
@@ -1912,7 +1928,7 @@ class PlaylistTab(QTableWidget):
             return
 
         plr.played = True
-        session.flush()
+        session.commit()
 
     def _set_row_bold(self, row: int, bold: bool = True) -> None:
         """
@@ -1936,6 +1952,8 @@ class PlaylistTab(QTableWidget):
         Set or reset row background colour
         """
 
+        column: int
+
         if colour:
             brush = QBrush(colour)
         else: