From 0794f061ee022079560eba43591a69955c64dfa6 Mon Sep 17 00:00:00 2001 From: Keith Edmunds Date: Sun, 19 Feb 2023 20:09:00 +0000 Subject: [PATCH] WIP: playlists.py refactor --- app/config.py | 1 + app/models.py | 10 +-- app/playlists.py | 203 ++++++++++++++++++----------------------------- 3 files changed, 84 insertions(+), 130 deletions(-) diff --git a/app/config.py b/app/config.py index 0c8246a..fe3590f 100644 --- a/app/config.py +++ b/app/config.py @@ -64,6 +64,7 @@ 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" diff --git a/app/models.py b/app/models.py index efe6f45..a538d37 100644 --- a/app/models.py +++ b/app/models.py @@ -428,9 +428,8 @@ class PlaylistRows(Base): plr.note) @staticmethod - def delete_plrids_not_in_list( - session: scoped_session, playlist_id: int, - plr_ids: Union[Iterable[int], ValuesView]) -> None: + def delete_higher_rows( + session: scoped_session, playlist_id: int, maxrow: int) -> None: """ Delete rows in given playlist that have a higher row number than 'maxrow' @@ -440,11 +439,10 @@ class PlaylistRows(Base): delete(PlaylistRows) .where( PlaylistRows.playlist_id == playlist_id, - PlaylistRows.id.not_in(plr_ids) + PlaylistRows.row_number > maxrow ) ) - # Delete won't take effect until commit() - session.commit() + session.flush() @staticmethod def fixup_rownumbers(session: scoped_session, playlist_id: int) -> None: diff --git a/app/playlists.py b/app/playlists.py index b8421df..1c4eeae 100644 --- a/app/playlists.py +++ b/app/playlists.py @@ -64,7 +64,6 @@ 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']) @@ -101,6 +100,10 @@ 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): @@ -108,7 +111,8 @@ class NoSelectDelegate(QStyledItemDelegate): # Make row just a little bit taller row = index.row() row_height = self.parent().rowHeight(row) - self.parent().setRowHeight(row, row_height + MINIMUM_ROW_HEIGHT) + self.parent().setRowHeight(row, + row_height + Config.MINIMUM_ROW_HEIGHT) return QPlainTextEdit(parent) return super().createEditor(parent, option, index) @@ -127,10 +131,9 @@ class NoSelectDelegate(QStyledItemDelegate): class PlaylistTab(QTableWidget): # Qt.UserRoles - ROW_FLAGS = Qt.UserRole - ROW_TRACK_ID = Qt.UserRole + 1 - ROW_DURATION = Qt.UserRole + 2 - PLAYLISTROW_ID = Qt.UserRole + 3 + ROW_TRACK_ID = Qt.UserRole + ROW_DURATION = Qt.UserRole + 1 + PLAYLISTROW_ID = Qt.UserRole + 2 def __init__(self, musicmuster: "Window", session: scoped_session, @@ -139,12 +142,9 @@ class PlaylistTab(QTableWidget): self.musicmuster: Window = musicmuster self.playlist_id = playlist_id - self.menu: Optional[QMenu] = None - - # Don't select text on edit - self.setItemDelegate(NoSelectDelegate(self)) - # Set up widget + self.menu: Optional[QMenu] = None + self.setItemDelegate(NoSelectDelegate(self)) 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(MINIMUM_ROW_HEIGHT) + self.v_header.setMinimumSectionSize(Config.MINIMUM_ROW_HEIGHT) self.horizontalHeader().setStretchLastSection(True) # Header row @@ -182,6 +182,7 @@ 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) @@ -191,10 +192,8 @@ 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) - # Now load our tracks and notes + # Load playlist rows self.populate_display(session, self.playlist_id) def __repr__(self) -> str: @@ -247,12 +246,7 @@ class PlaylistTab(QTableWidget): super().dropEvent(event) - log.debug( - "playlist.dropEvent(): " - f"Moved row(s) {rows} to become row {drop_row}" - ) - - with Session() as session: # checked + with Session() as session: self.save_playlist(session) self.update_display(session) @@ -349,28 +343,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) @@ -413,10 +407,8 @@ class PlaylistTab(QTableWidget): # 'return' doesn't play the next track. # # Earlier in this file: - # - self.setEditTriggers(QAbstractItemView.DoubleClicked) - triggers - # editing on double-click - # - self.setItemDelegate(NoSelectDelegate(self)) and associated class - # ensure that the text is not selected when editing starts + # self.setEditTriggers(QAbstractItemView.DoubleClicked) - triggers + # editing on double-click # # Call sequences: # Start editing: @@ -607,8 +599,7 @@ class PlaylistTab(QTableWidget): return [plr for plr in plrs if plr is not None] - def insert_header(self, session: scoped_session, note: str, - repaint: bool = True) -> None: + def insert_header(self, session: scoped_session, note: str) -> None: """ Insert section header into playlist tab. @@ -621,7 +612,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, repaint) + self.insert_row(session, plr) self.save_playlist(session) def insert_row(self, session: scoped_session, plr: PlaylistRows, @@ -638,7 +629,6 @@ 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) @@ -651,8 +641,6 @@ 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: @@ -679,7 +667,7 @@ class PlaylistTab(QTableWidget): bitrate = "" _ = self._set_item_text(row, BITRATE, bitrate) - # As we have track info, any notes should be contained in + # As we have a track_id, any notes should be contained in # the notes column plr_note = plr.note if not plr_note: @@ -698,19 +686,20 @@ class PlaylistTab(QTableWidget): ) return - # Make empty items (row background won't be coloured without - # items present). Any notes should displayed starting in - # column 2 for now - bug in Qt means that when row size is - # set, spanned columns are ignored, so put notes in col2 - # (typically title). + # In order to colour the row, we need items in every column. + # Bug in PyQt5 means that required height of row considers + # text to be wrapped in one column and ignores any spanned + # columns, hence putting notes in HEADER_NOTES_COLUMN which + # is typically reasonably wide and thus minimises + # unneccessary row height increases. for i in range(1, len(columns)): - if i == 2: + if i == HEADER_NOTES_COLUMN: 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 (no) track_id + # Save (or clear) track_id userdata_item.setData(self.ROW_TRACK_ID, 0) if repaint: @@ -763,10 +752,7 @@ 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 """ @@ -780,12 +766,11 @@ class PlaylistTab(QTableWidget): ) return - search_from = current_row + 1 - # Mark current row as played self._set_played_row(session, current_row) - next_row = self._find_next_track_row(session, search_from) + # Set next track + next_row = self._find_next_track_row(session, current_row + 1) if next_row: self._set_next(session, next_row) @@ -827,14 +812,9 @@ 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 populated + # that it's processed after list is 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""" @@ -858,34 +838,20 @@ 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()): - row_plr[row].row_number = row - row_plr[row].playlist_id = self.playlist_id + plr = self._get_playlistrow_object(session, row) + if not plr: + continue + plr.row_number = row + plr.playlist_id = self.playlist_id - # Any rows in the database for this playlist that have a plr id - # that's not in the displayed playlist need to be deleted. - - # Ensure changes flushed + # Any rows in the database for this playlist that has a row + # number equal to or greater than the row count needs to be + # removed. session.flush() - PlaylistRows.delete_plrids_not_in_list( - session, self.playlist_id, - display_plr_ids.values()) + PlaylistRows.delete_higher_rows( + session, self.playlist_id, self.rowCount() - 1) def scroll_current_to_top(self) -> None: """Scroll currently-playing row to top""" @@ -931,9 +897,6 @@ 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: @@ -947,14 +910,14 @@ class PlaylistTab(QTableWidget): row = 0 # Don't select section headers - wrapped: bool = False + wrapped = 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-notes + # non-headers return row = 0 wrapped = True @@ -968,9 +931,6 @@ 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: @@ -985,7 +945,7 @@ class PlaylistTab(QTableWidget): row = last_row # Don't select section headers - wrapped: bool = False + wrapped = False track_id = self._get_row_track_id(row) while not track_id: row -= 1 @@ -1004,11 +964,9 @@ class PlaylistTab(QTableWidget): """Sets the select track as next to play""" row = self._get_selected_row() - if row is None: - return None - - with Session() as session: - self._set_next(session, row) + if row is not None: + with Session() as session: + self._set_next(session, row) def tab_visible(self) -> None: """Called when tab becomes visible""" @@ -1240,6 +1198,10 @@ 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() @@ -1261,8 +1223,6 @@ 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) @@ -1384,14 +1344,12 @@ 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 ( - row not in track_rows or - row in played_rows or - not file_is_readable(plr.track.path) - ): + if not file_is_readable(plr.track.path): continue else: return row @@ -1446,11 +1404,7 @@ class PlaylistTab(QTableWidget): row: int) -> Optional[PlaylistRows]: """Return the playlistrow object associated with this row""" - userdata_item = self.item(row, USERDATA) - if not userdata_item: - return None - - playlistrow_id = userdata_item.data(self.PLAYLISTROW_ID) + playlistrow_id = self._get_playlistrow_id(row) if not playlistrow_id: return None @@ -1602,7 +1556,9 @@ 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() @@ -1615,6 +1571,7 @@ 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, @@ -1700,7 +1657,7 @@ class PlaylistTab(QTableWidget): # We can't have null text if not plr.note: plr.note = Config.TEXT_NO_TRACK_NO_NOTE - session.commit() + session.flush() # Clear track text items for i in range(2, len(columns)): @@ -1757,10 +1714,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 padding_required == 0: - break if self.isRowHidden(i): continue + if padding_required == 0: + break top_row = i padding_required -= 1 @@ -1846,7 +1803,6 @@ 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)}") @@ -1893,6 +1849,7 @@ 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( @@ -1928,7 +1885,7 @@ class PlaylistTab(QTableWidget): return plr.played = True - session.commit() + session.flush() def _set_row_bold(self, row: int, bold: bool = True) -> None: """ @@ -1952,8 +1909,6 @@ class PlaylistTab(QTableWidget): Set or reset row background colour """ - column: int - if colour: brush = QBrush(colour) else: