Refactor set track times

app/playlistmodel.py

@@ -94,6 +94,39 @@ class _PlaylistRowData:
             f"note='{self.note}', title='{self.title}', artist='{self.artist}'>"
         )
 
+    def set_start(
+        self, modified_rows: list[int], start: Optional[dt.datetime]
+    ) -> Optional[dt.datetime]:
+        """
+        Set start time for this row
+
+        Update passed modified rows list if we changed the row.
+
+        Return new start time
+        """
+
+        changed = False
+
+        if self.start_time != start:
+            self.start_time = start
+            changed = True
+        if start is None:
+            if self.end_time is not None:
+                self.end_time = None
+                changed = True
+            new_start_time = None
+        else:
+            end_time = start + dt.timedelta(milliseconds=self.duration)
+            new_start_time = end_time
+            if self.end_time != end_time:
+                self.end_time = end_time
+                changed = True
+
+        if changed and self.plr_rownum not in modified_rows:
+            modified_rows.append(self.plr_rownum)
+
+        return new_start_time
+
 
 class PlaylistModel(QAbstractTableModel):
     """
@@ -310,7 +343,9 @@ class PlaylistModel(QAbstractTableModel):
 
             session.commit()
 
-    def data(self, index: QModelIndex, role: int = Qt.ItemDataRole.DisplayRole) -> QVariant:
+    def data(
+        self, index: QModelIndex, role: int = Qt.ItemDataRole.DisplayRole
+    ) -> QVariant:
         """Return data to view"""
 
         if not index.isValid() or not (0 <= index.row() < len(self.playlist_rows)):
@@ -640,84 +675,15 @@ class PlaylistModel(QAbstractTableModel):
         Process possible section timing directives embeded in header
         """
 
-        count: int = 0
-        unplayed_count: int = 0
-        duration: int = 0
-
         if prd.note.endswith("+"):
-            # This header is the start of a timed section
+            return self.start_of_timed_section_header(prd)
-            for row_number in range(prd.plr_rownum + 1, len(self.playlist_rows)):
+
-                row_prd = self.playlist_rows[row_number]
-                if self.is_header_row(row_number):
-                    if row_prd.note.endswith("-"):
-                        return (
-                            f"{prd.note[:-1].strip()} "
-                            f"[{count} tracks, {ms_to_mmss(duration)} unplayed]"
-                        )
-                    else:
-                        continue
-                else:
-                    count += 1
-                    if not row_prd.played:
-                        unplayed_count += 1
-                        duration += row_prd.duration
-            return (
-                f"{prd.note[:-1].strip()} "
-                f"[{count} tracks, {ms_to_mmss(duration, none='none')} "
-                "unplayed (to end of playlist)]"
-            )
         elif prd.note.endswith("="):
-            # Show subtotal
+            return self.section_subtotal_header(prd)
-            for row_number in range(prd.plr_rownum - 1, -1, -1):
-                row_prd = self.playlist_rows[row_number]
-                if self.is_header_row(row_number):
-                    if row_prd.note.endswith("-"):
-                        # There was no start of section
-                        return prd.note
-                    if row_prd.note.endswith(("+", "=")):
-                        # If we are playing this section, also
-                        # calculate end time if all tracks are played.
-                        end_time_str = ""
-                        if (
-                            track_sequence.current
-                            and track_sequence.current.end_time
-                            and (
-                                row_number
-                                < track_sequence.current.row_number
-                                < prd.plr_rownum
-                            )
-                        ):
-                            section_end_time = (
-                                track_sequence.current.end_time
-                                + dt.timedelta(milliseconds=duration)
-                            )
-                            end_time_str = (
-                                ", section end time "
-                                + section_end_time.strftime(Config.TRACK_TIME_FORMAT)
-                            )
-                        stripped_note = prd.note[:-1].strip()
-                        if stripped_note:
-                            return (
-                                f"{stripped_note} ["
-                                f"{unplayed_count}/{count} track{'s' if count > 1 else ''} "
-                                f"({ms_to_mmss(duration)}) unplayed{end_time_str}]"
-                            )
-                        else:
-                            return (
-                                f"[{unplayed_count}/{count} track{'s' if count > 1 else ''} "
-                                f"({ms_to_mmss(duration)}) unplayed{end_time_str}]"
-                            )
-                    else:
-                        continue
-                else:
-                    count += 1
-                    if not row_prd.played:
-                        unplayed_count += 1
-                        duration += row_prd.duration
 
         elif prd.note == "-":
             # If the hyphen is the only thing on the line, echo the note
-            # tha started the section without the trailing "+".
+            # that started the section without the trailing "+".
             for row_number in range(prd.plr_rownum - 1, -1, -1):
                 row_prd = self.playlist_rows[row_number]
                 if self.is_header_row(row_number):
@@ -909,6 +875,7 @@ class PlaylistModel(QAbstractTableModel):
 
         # Update display
         self.reset_track_sequence_row_numbers()
+        self.update_track_times()
         self.invalidate_rows(list(row_map.keys()))
 
     def move_rows_between_playlists(
@@ -1185,7 +1152,68 @@ class PlaylistModel(QAbstractTableModel):
 
         return len(self.playlist_rows)
 
-    def selection_is_sortable(self, row_numbers: List[int]) -> bool:
+    def section_subtotal_header(self, prd: _PlaylistRowData) -> str:
+        """
+        Process this row as subtotal within a timed section and
+        return display text for this row
+        """
+
+        count: int = 0
+        unplayed_count: int = 0
+        duration: int = 0
+
+        # Show subtotal
+        for row_number in range(prd.plr_rownum - 1, -1, -1):
+            row_prd = self.playlist_rows[row_number]
+            if self.is_header_row(row_number):
+                if row_prd.note.endswith("-"):
+                    # There was no start of section
+                    return prd.note
+                if row_prd.note.endswith(("+", "=")):
+                    # If we are playing this section, also
+                    # calculate end time if all tracks are played.
+                    end_time_str = ""
+                    if (
+                        track_sequence.current
+                        and track_sequence.current.end_time
+                        and (
+                            row_number
+                            < track_sequence.current.row_number
+                            < prd.plr_rownum
+                        )
+                    ):
+                        section_end_time = (
+                            track_sequence.current.end_time
+                            + dt.timedelta(milliseconds=duration)
+                        )
+                        end_time_str = (
+                            ", section end time "
+                            + section_end_time.strftime(Config.TRACK_TIME_FORMAT)
+                        )
+                    stripped_note = prd.note[:-1].strip()
+                    if stripped_note:
+                        return (
+                            f"{stripped_note} ["
+                            f"{unplayed_count}/{count} track{'s' if count > 1 else ''} "
+                            f"({ms_to_mmss(duration)}) unplayed{end_time_str}]"
+                        )
+                    else:
+                        return (
+                            f"[{unplayed_count}/{count} track{'s' if count > 1 else ''} "
+                            f"({ms_to_mmss(duration)}) unplayed{end_time_str}]"
+                        )
+                else:
+                    continue
+            else:
+                count += 1
+                if not row_prd.played:
+                    unplayed_count += 1
+                    duration += row_prd.duration
+
+        # Should never get here
+        return f"Error calculating subtotal ({row_prd.note})"
+
+    def selection_is_sortable(self, row_numbers: list[int]) -> bool:
         """
         Return True if the selection is sortable. That means:
         - at least two rows selected
@@ -1376,6 +1404,37 @@ class PlaylistModel(QAbstractTableModel):
 
         self.sort_by_attribute(row_numbers, "title")
 
+    def start_of_timed_section_header(self, prd: _PlaylistRowData) -> str:
+        """
+        Process this row as the start of a timed section and
+        return display text for this row
+        """
+
+        count: int = 0
+        unplayed_count: int = 0
+        duration: int = 0
+
+        for row_number in range(prd.plr_rownum + 1, len(self.playlist_rows)):
+            row_prd = self.playlist_rows[row_number]
+            if self.is_header_row(row_number):
+                if row_prd.note.endswith("-"):
+                    return (
+                        f"{prd.note[:-1].strip()} "
+                        f"[{count} tracks, {ms_to_mmss(duration)} unplayed]"
+                    )
+                else:
+                    continue
+            else:
+                count += 1
+                if not row_prd.played:
+                    unplayed_count += 1
+                    duration += row_prd.duration
+        return (
+            f"{prd.note[:-1].strip()} "
+            f"[{count} tracks, {ms_to_mmss(duration, none='none')} "
+            "unplayed (to end of playlist)]"
+        )
+
     def supportedDropActions(self) -> Qt.DropAction:
         return Qt.DropAction.MoveAction | Qt.DropAction.CopyAction
 
@@ -1408,51 +1467,36 @@ class PlaylistModel(QAbstractTableModel):
         log.debug("update_track_times()")
 
         next_start_time: Optional[dt.datetime] = None
-        update_rows: List[int] = []
+        update_rows: list[int] = []
-        playlist_length = len(self.playlist_rows)
+        row_count = len(self.playlist_rows)
-        if not playlist_length:
-            return
 
-        for row_number in range(playlist_length):
+        current_track_row = None
+        next_track_row = None
+        if track_sequence.current:
+            current_track_row = track_sequence.current.row_number
+            # Update current track details now so that they are available
+            # when we deal with next track row which may be above current
+            # track row.
+            self.playlist_rows[
+                current_track_row
+            ].start_time = track_sequence.current.start_time
+            self.playlist_rows[
+                current_track_row
+            ].end_time = track_sequence.current.end_time
+            update_rows.append(current_track_row)
+        if track_sequence.next:
+            next_track_row = track_sequence.next.row_number
+
+        for row_number in range(row_count):
             prd = self.playlist_rows[row_number]
 
-            # Reset start_time if this is the current row
+            # Don't update times for tracks that have been played, for
-            if track_sequence.current:
+            # unreadable tracks or for the current track, handled above.
-                if row_number == track_sequence.current.row_number:
-                    prd.start_time = track_sequence.current.start_time
-                    prd.end_time = track_sequence.current.end_time
-                    update_rows.append(row_number)
-                    if not next_start_time:
-                        next_start_time = prd.end_time
-                    continue
-
-                # Set start time for next row if we have a current track
-                if track_sequence.next and track_sequence.current.end_time:
-                    if row_number == track_sequence.next.row_number:
-                        prd.start_time = track_sequence.current.end_time
-                        prd.end_time = prd.start_time + dt.timedelta(
-                            milliseconds=prd.duration
-                        )
-                        next_start_time = prd.end_time
-                        update_rows.append(row_number)
-                        continue
-
-            # Don't update times for tracks that have been played
-            if prd.played:
-                continue
-
-            # If we're between the current and next row, zero out
-            # times
             if (
-                track_sequence.current
+                prd.played
-                and track_sequence.next
+                or (prd.path and file_is_unreadable(prd.path))
-                and track_sequence.current.row_number
+                or (current_track_row and row_number == current_track_row)
-                < row_number
-                < track_sequence.next.row_number
             ):
-                prd.start_time = None
-                prd.end_time = None
-                update_rows.append(row_number)
                 continue
 
             # Reset start time if timing in header
@@ -1462,28 +1506,24 @@ class PlaylistModel(QAbstractTableModel):
                     next_start_time = header_time
                 continue
 
-            # This is an unplayed track
+            # Set start time for next row if we have a current track
-            # Don't schedule unplayable tracks
+            if (
-            if file_is_unreadable(prd.path):
+                next_track_row
+                and row_number == next_track_row
+                and track_sequence.current
+                and track_sequence.current.end_time
+            ):
+                next_start_time = prd.set_start(update_rows, track_sequence.current.end_time)
                 continue
 
-            # Set start/end if we have a start time
+            # If we're between the current and next row, zero out
-            if next_start_time is None:
+            # times
+            if (current_track_row or row_count) < row_number < (next_track_row or 0):
+                prd.set_start(update_rows, None)
                 continue
 
-            # Update start time of this row if it's incorrect
+            # Set start/end
-            if prd.start_time != next_start_time:
+            next_start_time = prd.set_start(update_rows, next_start_time)
-                prd.start_time = next_start_time
-                update_rows.append(row_number)
-
-            # Calculate next start time
-            next_start_time += dt.timedelta(milliseconds=prd.duration)
-
-            # Update end time of this row if it's incorrect
-            if prd.end_time != next_start_time:
-                prd.end_time = next_start_time
-                if row_number not in update_rows:
-                    update_rows.append(row_number)
 
         # Update start/stop times of rows that have changed
         for updated_row in update_rows:
@@ -1524,7 +1564,8 @@ class PlaylistProxyModel(QSortFilterProxyModel):
                 # Don't hide current track
                 if (
                     track_sequence.current
-                    and track_sequence.current.playlist_id == self.source_model.playlist_id
+                    and track_sequence.current.playlist_id
+                    == self.source_model.playlist_id
                     and track_sequence.current.row_number == source_row
                 ):
                     return True
@@ -1540,7 +1581,8 @@ class PlaylistProxyModel(QSortFilterProxyModel):
                 # Handle previous track
                 if track_sequence.previous:
                     if (
-                        track_sequence.previous.playlist_id != self.source_model.playlist_id
+                        track_sequence.previous.playlist_id
+                        != self.source_model.playlist_id
                         or track_sequence.previous.row_number != source_row
                     ):
                         # This row isn't our previous track: hide it
@@ -1549,11 +1591,10 @@ class PlaylistProxyModel(QSortFilterProxyModel):
                         # This row is our previous track. Don't hide it
                         # until HIDE_AFTER_PLAYING_OFFSET milliseconds
                         # after current track has started
-                        if (
+                        if track_sequence.current.start_time and dt.datetime.now() > (
                             track_sequence.current.start_time
-                            and dt.datetime.now() > (
+                            + dt.timedelta(
-                                track_sequence.current.start_time
+                                milliseconds=Config.HIDE_AFTER_PLAYING_OFFSET
-                                + dt.timedelta(milliseconds=Config.HIDE_AFTER_PLAYING_OFFSET)
                             )
                         ):
                             return False
@@ -1565,9 +1606,7 @@ class PlaylistProxyModel(QSortFilterProxyModel):
                             # true next time through.
                             QTimer.singleShot(
                                 Config.HIDE_AFTER_PLAYING_OFFSET + 100,
-                                lambda: self.source_model.invalidate_row(
+                                lambda: self.source_model.invalidate_row(source_row),
-                                    source_row
-                                ),
                             )
                             return True
                     # Next track not playing yet so don't hide previous