From a5c482f4977f787df9fb1dd9891b78a844917220 Mon Sep 17 00:00:00 2001 From: Rikrdoga Date: Tue, 21 Oct 2025 23:05:10 -0600 Subject: [PATCH 1/7] Add (Show in Explorer) context menu for download queue --- tidal_dl_ng/gui.py | 281 +++++++++++++++++++++++++++++++--- tidal_dl_ng/model/gui_data.py | 15 +- 2 files changed, 271 insertions(+), 25 deletions(-) diff --git a/tidal_dl_ng/gui.py b/tidal_dl_ng/gui.py index 9ffc444d..ef7d2af5 100644 --- a/tidal_dl_ng/gui.py +++ b/tidal_dl_ng/gui.py @@ -45,6 +45,9 @@ import math +import os +import pathlib +import subprocess import sys import time from collections.abc import Callable, Iterable, Sequence @@ -66,7 +69,7 @@ set_queue_download_media, set_user_list_media, ) -from tidal_dl_ng.helper.path import get_format_template, resource_path +from tidal_dl_ng.helper.path import format_path_media, get_format_template, resource_path from tidal_dl_ng.helper.tidal import ( favorite_function_factory, get_tidal_media_id, @@ -107,6 +110,113 @@ from tidal_dl_ng.worker import Worker +class FileSystemHelper: + """Helper class for file system operations with proper error handling.""" + + @staticmethod + def normalize_path(path: str) -> pathlib.Path: + """ + Normalize and validate a file system path. + """ + if not path: + raise ValueError("Path cannot be empty") # noqa: TRY003 + + return pathlib.Path(os.path.normpath(path)) + + @staticmethod + def path_exists(path: pathlib.Path) -> bool: + """ + Check if path exists with proper error handling. + """ + try: + return path.exists() + except (OSError, PermissionError): + logger_gui.exception("Permission error checking path", extra={"path": path}) + return False + + @staticmethod + def open_in_explorer(path: pathlib.Path, logger) -> bool: + """ + Open file explorer at the given path with platform-specific handling. + """ + try: + # Validate path exists + if not FileSystemHelper.path_exists(path): + logger.error(f"The path does not exist: {path}") + return False + + # Determine if path is file or directory + is_file = path.is_file() + + # Platform-specific handling + if sys.platform == "win32": + return FileSystemHelper._open_windows(path, is_file, logger) + elif sys.platform == "darwin": + return FileSystemHelper._open_macos(path, is_file, logger) + else: + return FileSystemHelper._open_linux(path, is_file, logger) + + except Exception: + logger.exception("Could not open explorer") + return False + + @staticmethod + def _open_windows(path: pathlib.Path, is_file: bool, logger) -> bool: + """Open Windows Explorer.""" + try: + if is_file: + # Select the file in Explorer + subprocess.Popen(["explorer", f'/select,"{path!s}"']) # noqa: S603, S607 + else: + # Open the directory + os.startfile(str(path)) # noqa: S606 + except (subprocess.CalledProcessError, FileNotFoundError): + logger.exception("Windows Explorer failed") + return False + else: + return True + + @staticmethod + def _open_macos(path: pathlib.Path, is_file: bool, logger) -> bool: + """Open macOS Finder.""" + try: + if is_file: + # Reveal the file in Finder + subprocess.run(["open", "-R", str(path)], check=True) # noqa: S603, S607 + else: + # Open the directory + subprocess.run(["open", str(path)], check=True) # noqa: S603, S607 + except (subprocess.CalledProcessError, FileNotFoundError): + logger.exception("macOS Finder failed") + return False + else: + return True + + @staticmethod + def _open_linux(path: pathlib.Path, is_file: bool, logger) -> bool: + """Open Linux file manager.""" + try: + # Most Linux file managers prefer opening the directory + dir_path = path.parent if is_file else path + + # Try xdg-open first (standard) + subprocess.run(["xdg-open", str(dir_path)], check=True) # noqa: S603, S607 + return True + except (subprocess.CalledProcessError, FileNotFoundError): + # Fallback (esta parte de tu código era excelente) + file_managers = ["nautilus", "dolphin", "thunar", "nemo", "pcmanfm"] + for manager in file_managers: + try: + subprocess.run([manager, str(dir_path)], check=True) # noqa: S603 + except (subprocess.CalledProcessError, FileNotFoundError): + continue + + logger_gui.exception("No compatible file manager found on Linux") + return False + else: + return True + + # TODO: Make more use of Exceptions class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow): """Main application window for TIDAL Downloader Next Generation. @@ -141,7 +251,7 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow): s_update_check: QtCore.Signal = QtCore.Signal(bool) s_update_show: QtCore.Signal = QtCore.Signal(bool, bool, object) s_queue_download_item_downloading: QtCore.Signal = QtCore.Signal(object) - s_queue_download_item_finished: QtCore.Signal = QtCore.Signal(object) + s_queue_download_item_finished: QtCore.Signal = QtCore.Signal(object, str) s_queue_download_item_failed: QtCore.Signal = QtCore.Signal(object) s_queue_download_item_skipped: QtCore.Signal = QtCore.Signal(object) converter_ansi_html: Ansi2HTMLConverter @@ -403,6 +513,7 @@ def _init_tree_queue(self, tree: QtWidgets.QTableWidget) -> None: if hasattr(header, "setSectionResizeMode"): header.setSectionResizeMode(0, QtWidgets.QHeaderView.ResizeToContents) tree.setContextMenuPolicy(QtCore.Qt.CustomContextMenu) + tree.customContextMenuRequested.connect(self.menu_context_tree_queue) def tidal_user_lists(self) -> None: """Fetch and emit user playlists, mixes, and favorites from Tidal.""" @@ -1453,6 +1564,7 @@ def queue_download_media(self, queue_dl_item: QueueDownloadItem) -> None: child.setText(3, queue_dl_item.type_media) child.setText(4, queue_dl_item.quality_audio) child.setText(5, queue_dl_item.quality_video) + child.setData(0, QtCore.Qt.ItemDataRole.UserRole, queue_dl_item) self.tr_queue_download.addTopLevelItem(child) def watcher_queue_download(self) -> None: @@ -1473,10 +1585,12 @@ def watcher_queue_download(self) -> None: try: self.s_queue_download_item_downloading.emit(item) - result = self.on_queue_download(media, quality_audio=quality_audio, quality_video=quality_video) + result, path_file = self.on_queue_download( + media, quality_audio=quality_audio, quality_video=quality_video + ) if result == QueueDownloadStatus.Finished: - self.s_queue_download_item_finished.emit(item) + self.s_queue_download_item_finished.emit(item, str(path_file)) elif result == QueueDownloadStatus.Skipped: self.s_queue_download_item_skipped.emit(item) except Exception as e: @@ -1493,13 +1607,23 @@ def on_queue_download_item_downloading(self, item: QtWidgets.QTreeWidgetItem) -> """ self.queue_download_item_status(item, QueueDownloadStatus.Downloading) - def on_queue_download_item_finished(self, item: QtWidgets.QTreeWidgetItem) -> None: + def on_queue_download_item_finished(self, item: QtWidgets.QTreeWidgetItem, path: str) -> None: """Update the status of a queue download item to 'Finished'. + Thread-safe implementation with proper path handling. Args: item (QtWidgets.QTreeWidgetItem): The item to update. """ self.queue_download_item_status(item, QueueDownloadStatus.Finished) + model_item = item.data(0, QtCore.Qt.ItemDataRole.UserRole) + if isinstance(model_item, QueueDownloadItem): + try: + # Normalize and validate path before storing + normalized_path = FileSystemHelper.normalize_path(path) + model_item.set_file_path(str(normalized_path)) + model_item.status = QueueDownloadStatus.Finished + except ValueError as e: + logger_gui.error(f"Invalid path format: {e}") def on_queue_download_item_failed(self, item: QtWidgets.QTreeWidgetItem) -> None: """Update the status of a queue download item to 'Failed'. @@ -1531,37 +1655,78 @@ def on_queue_download( media: Track | Album | Playlist | Video | Mix | Artist, quality_audio: Quality | None = None, quality_video: QualityVideo | None = None, - ) -> QueueDownloadStatus: - """Download the specified media item(s) and return the result status. - - Args: - media (Track | Album | Playlist | Video | Mix | Artist): The media item(s) to download. - quality_audio (Quality | None, optional): Desired audio quality. Defaults to None. - quality_video (QualityVideo | None, optional): Desired video quality. Defaults to None. - - Returns: - QueueDownloadStatus: The status of the download operation. + ) -> tuple[QueueDownloadStatus, pathlib.Path | str]: """ - result: QueueDownloadStatus - items_media: [Track | Album | Playlist | Video | Mix | Artist] + Download the specified media item(s) with improved path handling. + """ + result_status: QueueDownloadStatus + path_file: pathlib.Path | str + # Handle Artist downloads if isinstance(media, Artist): - items_media: [Album] = items_results_all(media) + items_media = self._get_artist_albums(media) + + if not items_media: + path_file = pathlib.Path(self.dl.path_base).expanduser() + result_status = QueueDownloadStatus.Skipped + return result_status, path_file + + # Calculate artist path from first album + try: + path_file = self._calculate_artist_path(items_media[0]) + except Exception as e: + logger_gui.warning(f"Could not deduce artist path. Using base path. Error: {e}") + path_file = pathlib.Path(self.dl.path_base).expanduser() else: items_media = [media] + path_file = pathlib.Path() - download_delay: bool = bool(isinstance(media, Track | Video) and self.settings.data.download_delay) + # Download items + download_delay = bool(isinstance(media, Track | Video) and self.settings.data.download_delay) for item_media in items_media: - result = self.download( + result_item, path_item = self.download( item_media, self.dl, delay_track=download_delay, quality_audio=quality_audio, quality_video=quality_video, ) + result_status = result_item - return result + # For non-Artist downloads, use the item path + if not isinstance(media, Artist): + path_file = path_item + + return result_status, path_file + + def _get_artist_albums(self, artist: Artist) -> list[Album]: + """Helper to get all albums for an artist.""" + return items_results_all(artist) + + def _calculate_artist_path(self, first_album: Album) -> pathlib.Path: + """ + Calculate the artist directory path from the first album. + This implements the "parent.parent" workaround. + """ + # Get template (which is faulty and returns a track template) + album_template = get_format_template(first_album, self.settings) + + # Format path + formatted_path_str = format_path_media( + album_template, + first_album, # We use the album object + self.settings.data.album_track_num_pad_min, + delimiter_artist=self.settings.data.filename_delimiter_artist, + delimiter_album_artist=self.settings.data.filename_delimiter_album_artist, + ) + + # Build full path + base_path = pathlib.Path(self.dl.path_base).expanduser() + track_full_path = base_path / formatted_path_str + + # Go up two levels (track -> album -> artist) + return track_full_path.parent.parent def download( self, @@ -1570,7 +1735,7 @@ def download( delay_track: bool = False, quality_audio: Quality | None = None, quality_video: QualityVideo | None = None, - ) -> QueueDownloadStatus: + ) -> tuple[QueueDownloadStatus, pathlib.Path | str]: """Download a media item and return the result status. Args: @@ -1611,7 +1776,18 @@ def download( # Dummy values result_dl = True - path_file = "dummy" + + # Formateamos la plantilla para obtener la ruta del "track" + formatted_path_str = format_path_media( + file_template, + media, # Pasamos el objeto Album/Playlist/Mix + self.settings.data.album_track_num_pad_min, + delimiter_artist=self.settings.data.filename_delimiter_artist, + delimiter_album_artist=self.settings.data.filename_delimiter_album_artist, + ) + # path_file ahora es el padre (el directorio del álbum) + track_full_path = pathlib.Path(dl.path_base).expanduser() / formatted_path_str + path_file = track_full_path.parent self.s_statusbar_message.emit(StatusbarMessage(message="Download finished.", timeout=2000)) @@ -1622,7 +1798,7 @@ def download( else: result = QueueDownloadStatus.Failed - return result + return result, path_file def on_version( self, update_check: bool = False, update_available: bool = False, update_info: ReleaseLatest | None = None @@ -1744,6 +1920,63 @@ def on_download_album_from_track(self, point: QtCore.QPoint) -> None: else: logger_gui.warning("Could not retrieve album information from the selected track.") + def menu_context_tree_queue(self, point: QtCore.QPoint) -> None: + """ + Show context menu for the download queue tree. + Improved validation and error handling. + """ + item = self.tr_queue_download.itemAt(point) + if not item: + return + + # Retrieve our data model from the GUI item + model_item = item.data(0, QtCore.Qt.ItemDataRole.UserRole) + + # Verify that it is our model, has a path, and the path exists + if not isinstance(model_item, QueueDownloadItem): + return + + if model_item.status != QueueDownloadStatus.Finished: + return + + file_path = model_item.get_file_path() + if not file_path: + return + + try: + path = FileSystemHelper.normalize_path(file_path) + if not FileSystemHelper.path_exists(path): + logger_gui.warning(f"File no longer exists: {file_path}. " "It may have been moved or deleted.") + return + except ValueError: + logger_gui.error(f"Invalid path format: {file_path}") + return + + menu = QtWidgets.QMenu() + action = menu.addAction("Show in Explorer") + action.triggered.connect(lambda: self.on_show_in_explorer(file_path)) + menu.exec(self.tr_queue_download.mapToGlobal(point)) + + def on_show_in_explorer(self, path: str) -> None: + """ + Opens the file explorer at the given path. + Enhanced with proper validation and error handling. + """ + try: + # Normalize and validate path + normalized_path = FileSystemHelper.normalize_path(path) + + # Open in explorer with platform-specific handling + success = FileSystemHelper.open_in_explorer(normalized_path, logger_gui) + + if success: + logger_gui.info(f"Opened explorer at: {normalized_path}") + + except ValueError as e: + logger_gui.error(f"Invalid path: {e}") + except Exception as e: + logger_gui.error(f"Unexpected error opening explorer: {e}") + # TODO: Comment with Google Docstrings. def gui_activate(tidal: Tidal | None = None): diff --git a/tidal_dl_ng/model/gui_data.py b/tidal_dl_ng/model/gui_data.py index eb58a482..df20c8c7 100644 --- a/tidal_dl_ng/model/gui_data.py +++ b/tidal_dl_ng/model/gui_data.py @@ -1,4 +1,5 @@ -from dataclasses import dataclass +from dataclasses import dataclass, field +from threading import Lock from tidalapi.media import Quality @@ -48,3 +49,15 @@ class QueueDownloadItem: quality_audio: Quality quality_video: QualityVideo obj: object + _file_path: str | None = field(default=None, init=False, repr=False) + _lock: Lock = field(default_factory=Lock, init=False, repr=False) + + def set_file_path(self, path: str) -> None: + """Thread-safe setter for file_path.""" + with self._lock: + self._file_path = path + + def get_file_path(self) -> str | None: + """Thread-safe getter for file_path.""" + with self._lock: + return self._file_path From d156934cf2ae3a41e085a3de4207bd24271d0500 Mon Sep 17 00:00:00 2001 From: Rikrdoga Date: Tue, 21 Oct 2025 23:23:06 -0600 Subject: [PATCH 2/7] fix: Correct track path resolution --- tidal_dl_ng/gui.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tidal_dl_ng/gui.py b/tidal_dl_ng/gui.py index ef7d2af5..351ba218 100644 --- a/tidal_dl_ng/gui.py +++ b/tidal_dl_ng/gui.py @@ -166,7 +166,7 @@ def _open_windows(path: pathlib.Path, is_file: bool, logger) -> bool: try: if is_file: # Select the file in Explorer - subprocess.Popen(["explorer", f'/select,"{path!s}"']) # noqa: S603, S607 + subprocess.Popen(["explorer", "/select,", str(path)]) # noqa: S603, S607 else: # Open the directory os.startfile(str(path)) # noqa: S606 From 3e9b73fdba56d1f85ec54fbb52acf3dcc5c57177 Mon Sep 17 00:00:00 2001 From: Rikrdoga Date: Tue, 21 Oct 2025 23:51:07 -0600 Subject: [PATCH 3/7] Fix Spanish Comments --- tidal_dl_ng/gui.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tidal_dl_ng/gui.py b/tidal_dl_ng/gui.py index 351ba218..762115d3 100644 --- a/tidal_dl_ng/gui.py +++ b/tidal_dl_ng/gui.py @@ -203,7 +203,7 @@ def _open_linux(path: pathlib.Path, is_file: bool, logger) -> bool: subprocess.run(["xdg-open", str(dir_path)], check=True) # noqa: S603, S607 return True except (subprocess.CalledProcessError, FileNotFoundError): - # Fallback (esta parte de tu código era excelente) + # Fallback file_managers = ["nautilus", "dolphin", "thunar", "nemo", "pcmanfm"] for manager in file_managers: try: @@ -1777,15 +1777,15 @@ def download( # Dummy values result_dl = True - # Formateamos la plantilla para obtener la ruta del "track" + # We format the template to get the "track" path formatted_path_str = format_path_media( file_template, - media, # Pasamos el objeto Album/Playlist/Mix + media, # Pass the Album/Playlist/Mix object self.settings.data.album_track_num_pad_min, delimiter_artist=self.settings.data.filename_delimiter_artist, delimiter_album_artist=self.settings.data.filename_delimiter_album_artist, ) - # path_file ahora es el padre (el directorio del álbum) + # path_file is now the parent (the album directory) track_full_path = pathlib.Path(dl.path_base).expanduser() / formatted_path_str path_file = track_full_path.parent From bc154a8aca96ce93ad8250a9eb46cffd360270ea Mon Sep 17 00:00:00 2001 From: Rikrdoga Date: Wed, 22 Oct 2025 19:50:49 -0600 Subject: [PATCH 4/7] Refactor Show in Explorer feature (Code & docstrings improvement) --- tidal_dl_ng/download.py | 42 +++++-- tidal_dl_ng/gui.py | 196 ++++++++----------------------- tidal_dl_ng/helper/filesystem.py | 171 +++++++++++++++++++++++++++ tidal_dl_ng/model/gui_data.py | 21 +++- 4 files changed, 273 insertions(+), 157 deletions(-) create mode 100644 tidal_dl_ng/helper/filesystem.py diff --git a/tidal_dl_ng/download.py b/tidal_dl_ng/download.py index 17357930..44e07314 100644 --- a/tidal_dl_ng/download.py +++ b/tidal_dl_ng/download.py @@ -558,9 +558,12 @@ def item( ) # Step 5: Post-processing - self._perform_post_processing( + + final_path = path_media_dst # Path before potential move + + new_path_if_moved = self._perform_post_processing( media, - path_media_dst, + final_path, quality_audio, quality_video, quality_audio_old, @@ -569,7 +572,11 @@ def item( skip_file, ) - return download_success, path_media_dst + # If post-processing moved the file, update the final_path to the new location + if new_path_if_moved: + final_path = new_path_if_moved + + return download_success, final_path def _validate_and_prepare_media( self, @@ -902,31 +909,36 @@ def _handle_metadata_and_extras( def _perform_post_processing( self, media: Track | Video, - path_media_dst: pathlib.Path, + path_media_src: pathlib.Path, quality_audio: Quality | None, quality_video: QualityVideo | None, quality_audio_old: Quality | None, quality_video_old: QualityVideo | None, download_delay: bool, skip_file: bool, - ) -> None: + ) -> pathlib.Path | None: """Perform post-processing tasks. Args: media (Track | Video): Media item. - path_media_dst (pathlib.Path): Destination file path. + path_media_src (pathlib.Path): The source file path (before potential move). quality_audio (Quality | None): Audio quality setting. quality_video (QualityVideo | None): Video quality setting. quality_audio_old (Quality | None): Previous audio quality. quality_video_old (QualityVideo | None): Previous video quality. download_delay (bool): Whether to apply download delay. skip_file (bool): Whether file was skipped. + + Returns: + pathlib.Path | None: The new path if the file was moved, otherwise None. """ + new_path: pathlib.Path | None = None + # If files needs to be symlinked, do postprocessing here. if self.settings.data.symlink_to_track and not isinstance(media, Video): # Determine file extension for symlink - file_extension = path_media_dst.suffix - self.media_move_and_symlink(media, path_media_dst, file_extension) + file_extension = path_media_src.suffix + new_path = self.media_move_and_symlink(media, path_media_src, file_extension) # Reset quality settings if quality_audio_old is not None: @@ -947,6 +959,8 @@ def _perform_post_processing( self.fn_logger.debug(f"Next download will start in {time_sleep} seconds.") time.sleep(time_sleep) + return new_path + def media_move_and_symlink( self, media: Track | Video, path_media_src: pathlib.Path, file_extension: str ) -> pathlib.Path: @@ -1275,7 +1289,7 @@ def items( download_delay: bool = True, quality_audio: Quality | None = None, quality_video: QualityVideo | None = None, - ) -> None: + ) -> pathlib.Path | None: """Download all items in an album, playlist, or mix. Args: @@ -1287,11 +1301,14 @@ def items( download_delay (bool, optional): Whether to delay between downloads. Defaults to True. quality_audio (Quality | None, optional): Audio quality. Defaults to None. quality_video (QualityVideo | None, optional): Video quality. Defaults to None. + + Returns: + pathlib.Path | None: The path to the downloaded album/playlist directory, or None on failure. """ # Validate and prepare media collection validated_media = self._validate_and_prepare_media(media, media_id, media_type, video_download) if validated_media is None or not isinstance(validated_media, Album | Playlist | UserPlaylist | Mix): - return + return None media = validated_media @@ -1330,6 +1347,11 @@ def items( self.fn_logger.info(f"Finished list '{list_media_name}'.") + # Return the path to the album/playlist directory + if result_dirs: + return result_dirs[0] # All paths should share the same parent + return None + def _setup_collection_download_context( self, media: Album | Playlist | UserPlaylist | Mix, diff --git a/tidal_dl_ng/gui.py b/tidal_dl_ng/gui.py index 762115d3..51466449 100644 --- a/tidal_dl_ng/gui.py +++ b/tidal_dl_ng/gui.py @@ -45,9 +45,7 @@ import math -import os import pathlib -import subprocess import sys import time from collections.abc import Callable, Iterable, Sequence @@ -58,6 +56,7 @@ from tidal_dl_ng import __version__, update_available from tidal_dl_ng.dialog import DialogLogin, DialogPreferences, DialogVersion +from tidal_dl_ng.helper.filesystem import FileSystemHelper from tidal_dl_ng.helper.gui import ( FilterHeader, HumanProxyModel, @@ -110,113 +109,6 @@ from tidal_dl_ng.worker import Worker -class FileSystemHelper: - """Helper class for file system operations with proper error handling.""" - - @staticmethod - def normalize_path(path: str) -> pathlib.Path: - """ - Normalize and validate a file system path. - """ - if not path: - raise ValueError("Path cannot be empty") # noqa: TRY003 - - return pathlib.Path(os.path.normpath(path)) - - @staticmethod - def path_exists(path: pathlib.Path) -> bool: - """ - Check if path exists with proper error handling. - """ - try: - return path.exists() - except (OSError, PermissionError): - logger_gui.exception("Permission error checking path", extra={"path": path}) - return False - - @staticmethod - def open_in_explorer(path: pathlib.Path, logger) -> bool: - """ - Open file explorer at the given path with platform-specific handling. - """ - try: - # Validate path exists - if not FileSystemHelper.path_exists(path): - logger.error(f"The path does not exist: {path}") - return False - - # Determine if path is file or directory - is_file = path.is_file() - - # Platform-specific handling - if sys.platform == "win32": - return FileSystemHelper._open_windows(path, is_file, logger) - elif sys.platform == "darwin": - return FileSystemHelper._open_macos(path, is_file, logger) - else: - return FileSystemHelper._open_linux(path, is_file, logger) - - except Exception: - logger.exception("Could not open explorer") - return False - - @staticmethod - def _open_windows(path: pathlib.Path, is_file: bool, logger) -> bool: - """Open Windows Explorer.""" - try: - if is_file: - # Select the file in Explorer - subprocess.Popen(["explorer", "/select,", str(path)]) # noqa: S603, S607 - else: - # Open the directory - os.startfile(str(path)) # noqa: S606 - except (subprocess.CalledProcessError, FileNotFoundError): - logger.exception("Windows Explorer failed") - return False - else: - return True - - @staticmethod - def _open_macos(path: pathlib.Path, is_file: bool, logger) -> bool: - """Open macOS Finder.""" - try: - if is_file: - # Reveal the file in Finder - subprocess.run(["open", "-R", str(path)], check=True) # noqa: S603, S607 - else: - # Open the directory - subprocess.run(["open", str(path)], check=True) # noqa: S603, S607 - except (subprocess.CalledProcessError, FileNotFoundError): - logger.exception("macOS Finder failed") - return False - else: - return True - - @staticmethod - def _open_linux(path: pathlib.Path, is_file: bool, logger) -> bool: - """Open Linux file manager.""" - try: - # Most Linux file managers prefer opening the directory - dir_path = path.parent if is_file else path - - # Try xdg-open first (standard) - subprocess.run(["xdg-open", str(dir_path)], check=True) # noqa: S603, S607 - return True - except (subprocess.CalledProcessError, FileNotFoundError): - # Fallback - file_managers = ["nautilus", "dolphin", "thunar", "nemo", "pcmanfm"] - for manager in file_managers: - try: - subprocess.run([manager, str(dir_path)], check=True) # noqa: S603 - except (subprocess.CalledProcessError, FileNotFoundError): - continue - - logger_gui.exception("No compatible file manager found on Linux") - return False - else: - return True - - # TODO: Make more use of Exceptions class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow): """Main application window for TIDAL Downloader Next Generation. @@ -1193,7 +1085,7 @@ def media_to_queue_download_model( quality_audio=quality_audio, quality_video=quality_video, type_media=type(media).__name__, - status=QueueDownloadStatus.Waiting, + status_init=QueueDownloadStatus.Waiting, obj=media, ) else: @@ -1558,7 +1450,7 @@ def queue_download_media(self, queue_dl_item: QueueDownloadItem) -> None: # Populate child child: QtWidgets.QTreeWidgetItem = QtWidgets.QTreeWidgetItem() - child.setText(0, queue_dl_item.status) + child.setText(0, queue_dl_item.get_status()) set_queue_download_media(child, queue_dl_item.obj) child.setText(2, queue_dl_item.name) child.setText(3, queue_dl_item.type_media) @@ -1605,6 +1497,9 @@ def on_queue_download_item_downloading(self, item: QtWidgets.QTreeWidgetItem) -> Args: item (QtWidgets.QTreeWidgetItem): The item to update. """ + model_item = item.data(0, QtCore.Qt.ItemDataRole.UserRole) + if isinstance(model_item, QueueDownloadItem): + model_item.set_status(QueueDownloadStatus.Downloading) self.queue_download_item_status(item, QueueDownloadStatus.Downloading) def on_queue_download_item_finished(self, item: QtWidgets.QTreeWidgetItem, path: str) -> None: @@ -1614,16 +1509,20 @@ def on_queue_download_item_finished(self, item: QtWidgets.QTreeWidgetItem, path: Args: item (QtWidgets.QTreeWidgetItem): The item to update. """ - self.queue_download_item_status(item, QueueDownloadStatus.Finished) model_item = item.data(0, QtCore.Qt.ItemDataRole.UserRole) if isinstance(model_item, QueueDownloadItem): try: # Normalize and validate path before storing normalized_path = FileSystemHelper.normalize_path(path) model_item.set_file_path(str(normalized_path)) - model_item.status = QueueDownloadStatus.Finished + model_item.set_status(QueueDownloadStatus.Finished) + self.queue_download_item_status(item, QueueDownloadStatus.Finished) except ValueError as e: logger_gui.error(f"Invalid path format: {e}") + # If the path is invalid, mark as Failed + model_item.set_status(QueueDownloadStatus.Failed) + self.queue_download_item_status(item, QueueDownloadStatus.Failed) + return def on_queue_download_item_failed(self, item: QtWidgets.QTreeWidgetItem) -> None: """Update the status of a queue download item to 'Failed'. @@ -1631,6 +1530,9 @@ def on_queue_download_item_failed(self, item: QtWidgets.QTreeWidgetItem) -> None Args: item (QtWidgets.QTreeWidgetItem): The item to update. """ + model_item = item.data(0, QtCore.Qt.ItemDataRole.UserRole) + if isinstance(model_item, QueueDownloadItem): + model_item.set_status(QueueDownloadStatus.Failed) self.queue_download_item_status(item, QueueDownloadStatus.Failed) def on_queue_download_item_skipped(self, item: QtWidgets.QTreeWidgetItem) -> None: @@ -1639,6 +1541,9 @@ def on_queue_download_item_skipped(self, item: QtWidgets.QTreeWidgetItem) -> Non Args: item (QtWidgets.QTreeWidgetItem): The item to update. """ + model_item = item.data(0, QtCore.Qt.ItemDataRole.UserRole) + if isinstance(model_item, QueueDownloadItem): + model_item.set_status(QueueDownloadStatus.Skipped) self.queue_download_item_status(item, QueueDownloadStatus.Skipped) def queue_download_item_status(self, item: QtWidgets.QTreeWidgetItem, status: str) -> None: @@ -1657,14 +1562,32 @@ def on_queue_download( quality_video: QualityVideo | None = None, ) -> tuple[QueueDownloadStatus, pathlib.Path | str]: """ - Download the specified media item(s) with improved path handling. + Download the specified media item(s) and return the final status and path. + + This method coordinates the download process for various media types. + For Artists, it calculates the artist's root directory. For all + other types, it downloads the item(s) and captures the resulting + file or directory path. + + Args: + media: The TIDAL media object to download (e.g., Track, Album, Artist). + quality_audio: The desired audio quality for the download. + quality_video: The desired video quality for the download. + + Returns: + A tuple containing: + - result_status (QueueDownloadStatus): The final status of the + download (e.g., Finished, Skipped, Failed). + - path_file (pathlib.Path | str): The final path to the + downloaded file or directory. For Artist downloads, this is + the artist's root folder. """ result_status: QueueDownloadStatus path_file: pathlib.Path | str # Handle Artist downloads if isinstance(media, Artist): - items_media = self._get_artist_albums(media) + items_media = items_results_all(media) if not items_media: path_file = pathlib.Path(self.dl.path_base).expanduser() @@ -1700,10 +1623,6 @@ def on_queue_download( return result_status, path_file - def _get_artist_albums(self, artist: Artist) -> list[Album]: - """Helper to get all albums for an artist.""" - return items_results_all(artist) - def _calculate_artist_path(self, first_album: Album) -> pathlib.Path: """ Calculate the artist directory path from the first album. @@ -1765,7 +1684,7 @@ def download( quality_video=quality_video, ) elif isinstance(media, Album | Playlist | Mix): - dl.items( + path_album_dir = dl.items( media=media, file_template=file_template, video_download=self.settings.data.video_download, @@ -1774,20 +1693,15 @@ def download( quality_video=quality_video, ) - # Dummy values - result_dl = True - - # We format the template to get the "track" path - formatted_path_str = format_path_media( - file_template, - media, # Pass the Album/Playlist/Mix object - self.settings.data.album_track_num_pad_min, - delimiter_artist=self.settings.data.filename_delimiter_artist, - delimiter_album_artist=self.settings.data.filename_delimiter_album_artist, - ) - # path_file is now the parent (the album directory) - track_full_path = pathlib.Path(dl.path_base).expanduser() / formatted_path_str - path_file = track_full_path.parent + # path_album_dir is the path returned by download.py + # Will be None if the download failed or the list was empty + if path_album_dir: + result_dl = True + path_file = path_album_dir + else: + # Handle the case where the download could have failed within items + result_dl = False + path_file = "" # No route to return self.s_statusbar_message.emit(StatusbarMessage(message="Download finished.", timeout=2000)) @@ -1932,25 +1846,19 @@ def menu_context_tree_queue(self, point: QtCore.QPoint) -> None: # Retrieve our data model from the GUI item model_item = item.data(0, QtCore.Qt.ItemDataRole.UserRole) - # Verify that it is our model, has a path, and the path exists + # Verify that it is our model if not isinstance(model_item, QueueDownloadItem): return - if model_item.status != QueueDownloadStatus.Finished: + if model_item.get_status() != QueueDownloadStatus.Finished: return file_path = model_item.get_file_path() if not file_path: return - try: - path = FileSystemHelper.normalize_path(file_path) - if not FileSystemHelper.path_exists(path): - logger_gui.warning(f"File no longer exists: {file_path}. " "It may have been moved or deleted.") - return - except ValueError: - logger_gui.error(f"Invalid path format: {file_path}") - return + # Path validation is now deferred to on_show_in_explorer + # and open_in_explorer, so we don't log warnings prematurely. menu = QtWidgets.QMenu() action = menu.addAction("Show in Explorer") diff --git a/tidal_dl_ng/helper/filesystem.py b/tidal_dl_ng/helper/filesystem.py new file mode 100644 index 00000000..d9b8413a --- /dev/null +++ b/tidal_dl_ng/helper/filesystem.py @@ -0,0 +1,171 @@ +import pathlib +import subprocess +import sys + +from tidal_dl_ng.logger import logger_gui + + +class FileSystemHelper: + """Helper class for file system operations with proper error handling.""" + + @staticmethod + def normalize_path(path: str) -> pathlib.Path: + """ + Normalize and validate a file system path. + + Args: + path: The raw string path. + + Returns: + A resolved pathlib.Path object. + + Raises: + ValueError: If the path is empty or invalid. + """ + if not path: + raise ValueError("Path cannot be empty") + + try: + normalized = pathlib.Path(path) + # Resolve to catch issues early (optional, depends on use case) + # normalized.resolve(strict=False) + return normalized + + except (ValueError, TypeError) as e: + raise ValueError(f"Invalid path format: {path}") from e + + + @staticmethod + def path_exists(path: pathlib.Path) -> bool: + """ + Check if path exists. + + Args: + path: The path to check. + + Returns: + True if the path exists, False otherwise. + """ + try: + return path.exists() + except (OSError, PermissionError): + logger_gui.exception("Permission error checking path", extra={"path": path}) + return False + + @staticmethod + def open_in_explorer(path: pathlib.Path, logger) -> bool: + """ + Open file explorer at the given path with platform-specific handling. + + Args: + path: The pathlib.Path object to open. + logger: The logger instance to use for reporting errors. + + Returns: + True if the operation was successful, False otherwise. + """ + try: + # Validate path exists + if not FileSystemHelper.path_exists(path): + # No log error here; path_exists already logged exception if one occurred + return False + + # Determine if path is file or directory + is_file = path.is_file() + + # Platform-specific handling + if sys.platform == "win32": + return FileSystemHelper._open_windows(path, is_file, logger) + elif sys.platform == "darwin": + return FileSystemHelper._open_macos(path, is_file, logger) + else: + return FileSystemHelper._open_linux(path, is_file, logger) + + except Exception: + logger.exception("Could not open explorer") + return False + + @staticmethod + def _open_windows(path: pathlib.Path, is_file: bool, logger) -> bool: + """ + Open Windows Explorer to the specified path. + + Args: + path: The path to open. + is_file: True if the path is a file, False if a directory. + logger: The logger instance. + + Returns: + True on success, False on failure. + """ + try: + if is_file: + # Select the file in Explorer + subprocess.Popen(["explorer", "/select,", str(path)]) + else: + # Open the directory + subprocess.Popen(["explorer", str(path)]) + except (subprocess.CalledProcessError, FileNotFoundError): + logger.exception("Windows Explorer failed") + return False + else: + return True + + @staticmethod + def _open_macos(path: pathlib.Path, is_file: bool, logger) -> bool: + """ + Open macOS Finder to the specified path. + + Args: + path: The path to open. + is_file: True if the path is a file, False if a directory. + logger: The logger instance. + + Returns: + True on success, False on failure. + """ + try: + if is_file: + # Reveal the file in Finder + subprocess.run(["open", "-R", str(path)], check=True) + else: + # Open the directory + subprocess.run(["open", str(path)], check=True) + except (subprocess.CalledProcessError, FileNotFoundError): + logger.exception("macOS Finder failed") + return False + else: + return True + + @staticmethod + def _open_linux(path: pathlib.Path, is_file: bool, logger) -> bool: + """ + Open Linux file manager to the specified path. + + Args: + path: The path to open. + is_file: True if the path is a file, False if a directory. + logger: The logger instance. + + Returns: + True on success, False on failure. + """ + try: + # Most Linux file managers prefer opening the directory + dir_path = path.parent if is_file else path + + # Try xdg-open first (standard) + subprocess.run(["xdg-open", str(dir_path)], check=True) + return True + except (subprocess.CalledProcessError, FileNotFoundError): + # Fallback + file_managers = ["nautilus", "dolphin", "thunar", "nemo", "pcmanfm"] + for manager in file_managers: + try: + subprocess.run([manager, str(dir_path)], check=True) + return True + except (subprocess.CalledProcessError, FileNotFoundError): + continue + + logger.exception("No compatible file manager found on Linux") + return False \ No newline at end of file diff --git a/tidal_dl_ng/model/gui_data.py b/tidal_dl_ng/model/gui_data.py index df20c8c7..5419a187 100644 --- a/tidal_dl_ng/model/gui_data.py +++ b/tidal_dl_ng/model/gui_data.py @@ -1,9 +1,9 @@ -from dataclasses import dataclass, field +from dataclasses import InitVar, dataclass, field from threading import Lock from tidalapi.media import Quality -from tidal_dl_ng.constants import QualityVideo +from tidal_dl_ng.constants import QualityVideo, QueueDownloadStatus try: from PySide6 import QtCore @@ -43,15 +43,20 @@ class StatusbarMessage: @dataclass class QueueDownloadItem: - status: str name: str type_media: str quality_audio: Quality quality_video: QualityVideo obj: object + status_init: InitVar[str] = QueueDownloadStatus.Waiting + _status: str = field(init=False) _file_path: str | None = field(default=None, init=False, repr=False) _lock: Lock = field(default_factory=Lock, init=False, repr=False) + def __post_init__(self, status_init: str): + """Assigns the initial state after creation.""" + self._status = status_init + def set_file_path(self, path: str) -> None: """Thread-safe setter for file_path.""" with self._lock: @@ -61,3 +66,13 @@ def get_file_path(self) -> str | None: """Thread-safe getter for file_path.""" with self._lock: return self._file_path + + def set_status(self, status: str) -> None: + """Thread-safe setter for status.""" + with self._lock: + self._status = status + + def get_status(self) -> str: + """Thread-safe getter for status.""" + with self._lock: + return self._status From 0e10c53ff39d3f9eed3a23d0995744b440fd0ca8 Mon Sep 17 00:00:00 2001 From: Rikrdoga Date: Wed, 22 Oct 2025 22:23:59 -0600 Subject: [PATCH 5/7] Resolve merge conflicts with 'main' --- tidal_dl_ng/gui.py | 48 +++++++++++--------------------- tidal_dl_ng/helper/filesystem.py | 7 ++--- 2 files changed, 19 insertions(+), 36 deletions(-) diff --git a/tidal_dl_ng/gui.py b/tidal_dl_ng/gui.py index 2392a9cc..4dd8b1d3 100644 --- a/tidal_dl_ng/gui.py +++ b/tidal_dl_ng/gui.py @@ -607,6 +607,9 @@ def menu_context_tree_results(self, point: QtCore.QPoint) -> None: def menu_context_queue_download(self, point: QtCore.QPoint) -> None: """Show context menu for download queue. + Provides options like 'Remove from Queue' (for Waiting items) + or 'Show in Explorer' (for Finished items). + Args: point (QPoint): The point where the menu is requested. """ @@ -616,14 +619,26 @@ def menu_context_queue_download(self, point: QtCore.QPoint) -> None: if not item: return + model_item = item.data(0, QtCore.Qt.ItemDataRole.UserRole) + if not isinstance(model_item, QueueDownloadItem): + return + # Build the menu menu = QtWidgets.QMenu() + status = model_item.get_status() + # Show remove option for waiting items - status = item.text(0) if status == QueueDownloadStatus.Waiting: menu.addAction("🗑️ Remove from Queue", lambda: self.on_queue_download_remove_item(item)) + # Show "Show in Explorer" if "Finished" + elif status == QueueDownloadStatus.Finished: + file_path = model_item.get_file_path() + if file_path: + action = menu.addAction("📂 Show in Explorer") + action.triggered.connect(lambda: self.on_show_in_explorer(file_path)) + if menu.isEmpty(): return @@ -2047,37 +2062,6 @@ def on_download_album_from_track(self, point: QtCore.QPoint) -> None: else: logger_gui.warning("Could not retrieve album information from the selected track.") - def menu_context_tree_queue(self, point: QtCore.QPoint) -> None: - """ - Show context menu for the download queue tree. - Improved validation and error handling. - """ - item = self.tr_queue_download.itemAt(point) - if not item: - return - - # Retrieve our data model from the GUI item - model_item = item.data(0, QtCore.Qt.ItemDataRole.UserRole) - - # Verify that it is our model - if not isinstance(model_item, QueueDownloadItem): - return - - if model_item.get_status() != QueueDownloadStatus.Finished: - return - - file_path = model_item.get_file_path() - if not file_path: - return - - # Path validation is now deferred to on_show_in_explorer - # and open_in_explorer, so we don't log warnings prematurely. - - menu = QtWidgets.QMenu() - action = menu.addAction("Show in Explorer") - action.triggered.connect(lambda: self.on_show_in_explorer(file_path)) - menu.exec(self.tr_queue_download.mapToGlobal(point)) - def on_show_in_explorer(self, path: str) -> None: """ Opens the file explorer at the given path. diff --git a/tidal_dl_ng/helper/filesystem.py b/tidal_dl_ng/helper/filesystem.py index d9b8413a..e4b155ff 100644 --- a/tidal_dl_ng/helper/filesystem.py +++ b/tidal_dl_ng/helper/filesystem.py @@ -24,17 +24,16 @@ def normalize_path(path: str) -> pathlib.Path: """ if not path: raise ValueError("Path cannot be empty") - + try: normalized = pathlib.Path(path) # Resolve to catch issues early (optional, depends on use case) # normalized.resolve(strict=False) return normalized - + except (ValueError, TypeError) as e: raise ValueError(f"Invalid path format: {path}") from e - @staticmethod def path_exists(path: pathlib.Path) -> bool: """ @@ -168,4 +167,4 @@ def _open_linux(path: pathlib.Path, is_file: bool, logger) -> bool: continue logger.exception("No compatible file manager found on Linux") - return False \ No newline at end of file + return False From 9cf1c153363cfe528d1582490ad8ba9e35feab6c Mon Sep 17 00:00:00 2001 From: Rikrdoga Date: Thu, 13 Nov 2025 20:56:59 -0600 Subject: [PATCH 6/7] Refactor: Simplify 'Show in Explorer' implementation --- tidal_dl_ng/download.py | 21 +-- tidal_dl_ng/gui.py | 227 ++++++++++++++----------------- tidal_dl_ng/helper/filesystem.py | 170 ----------------------- tidal_dl_ng/model/gui_data.py | 36 ++--- 4 files changed, 120 insertions(+), 334 deletions(-) delete mode 100644 tidal_dl_ng/helper/filesystem.py diff --git a/tidal_dl_ng/download.py b/tidal_dl_ng/download.py index a3a66a30..bda11d68 100644 --- a/tidal_dl_ng/download.py +++ b/tidal_dl_ng/download.py @@ -571,12 +571,9 @@ def item( ) # Step 5: Post-processing - - final_path = path_media_dst # Path before potential move - - new_path_if_moved = self._perform_post_processing( + self._perform_post_processing( media, - final_path, + path_media_dst, quality_audio, quality_video, quality_audio_old, @@ -585,11 +582,7 @@ def item( skip_file, ) - # If post-processing moved the file, update the final_path to the new location - if new_path_if_moved: - final_path = new_path_if_moved - - return download_success, final_path + return download_success, path_media_dst def _validate_and_prepare_media( self, @@ -1019,7 +1012,7 @@ def _perform_post_processing( Args: media (Track | Video): Media item. - path_media_src (pathlib.Path): The source file path (before potential move). + path_media_dst (pathlib.Path): Destination file path. quality_audio (Quality | None): Audio quality setting. quality_video (QualityVideo | None): Video quality setting. quality_audio_old (Quality | None): Previous audio quality. @@ -1028,7 +1021,7 @@ def _perform_post_processing( skip_file (bool): Whether file was skipped. Returns: - pathlib.Path | None: The new path if the file was moved, otherwise None. + pathlib.Path | None: The final path if the file was moved, otherwise None. """ new_path: pathlib.Path | None = None @@ -1402,12 +1395,12 @@ def items( quality_video (QualityVideo | None, optional): Video quality. Defaults to None. Returns: - pathlib.Path | None: The path to the downloaded album/playlist directory, or None on failure. + pathlib.Path | None: The path to the created album/playlist directory, or None if failed. """ # Validate and prepare media collection validated_media = self._validate_and_prepare_media(media, media_id, media_type, video_download) if validated_media is None or not isinstance(validated_media, Album | Playlist | UserPlaylist | Mix): - return None + return media = validated_media diff --git a/tidal_dl_ng/gui.py b/tidal_dl_ng/gui.py index 0440d617..38faac64 100644 --- a/tidal_dl_ng/gui.py +++ b/tidal_dl_ng/gui.py @@ -46,6 +46,7 @@ import math import pathlib +import subprocess import sys import time from collections.abc import Callable, Iterable, Sequence @@ -56,7 +57,6 @@ from tidal_dl_ng import __version__, update_available from tidal_dl_ng.dialog import DialogLogin, DialogPreferences, DialogVersion -from tidal_dl_ng.helper.filesystem import FileSystemHelper from tidal_dl_ng.helper.gui import ( FilterHeader, HumanProxyModel, @@ -68,7 +68,7 @@ set_queue_download_media, set_user_list_media, ) -from tidal_dl_ng.helper.path import format_path_media, get_format_template, resource_path +from tidal_dl_ng.helper.path import get_format_template, resource_path from tidal_dl_ng.helper.tidal import ( favorite_function_factory, get_tidal_media_id, @@ -146,7 +146,7 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow): s_update_check: QtCore.Signal = QtCore.Signal(bool) s_update_show: QtCore.Signal = QtCore.Signal(bool, bool, object) s_queue_download_item_downloading: QtCore.Signal = QtCore.Signal(object) - s_queue_download_item_finished: QtCore.Signal = QtCore.Signal(object, str) + s_queue_download_item_finished: QtCore.Signal = QtCore.Signal(object, object) s_queue_download_item_failed: QtCore.Signal = QtCore.Signal(object) s_queue_download_item_skipped: QtCore.Signal = QtCore.Signal(object) converter_ansi_html: Ansi2HTMLConverter @@ -645,9 +645,6 @@ def menu_context_tree_results(self, point: QtCore.QPoint) -> None: def menu_context_queue_download(self, point: QtCore.QPoint) -> None: """Show context menu for download queue. - Provides options like 'Remove from Queue' (for Waiting items) - or 'Show in Explorer' (for Finished items). - Args: point (QPoint): The point where the menu is requested. """ @@ -657,6 +654,7 @@ def menu_context_queue_download(self, point: QtCore.QPoint) -> None: if not item: return + # Get the underlying QueueDownloadItem object model_item = item.data(0, QtCore.Qt.ItemDataRole.UserRole) if not isinstance(model_item, QueueDownloadItem): return @@ -664,18 +662,14 @@ def menu_context_queue_download(self, point: QtCore.QPoint) -> None: # Build the menu menu = QtWidgets.QMenu() - status = model_item.get_status() - # Show remove option for waiting items - if status == QueueDownloadStatus.Waiting: + if model_item.status == QueueDownloadStatus.Waiting: menu.addAction("🗑️ Remove from Queue", lambda: self.on_queue_download_remove_item(item)) - # Show "Show in Explorer" if "Finished" - elif status == QueueDownloadStatus.Finished: - file_path = model_item.get_file_path() - if file_path: - action = menu.addAction("📂 Show in Explorer") - action.triggered.connect(lambda: self.on_show_in_explorer(file_path)) + # Show "Show in Explorer" if finished and path exists + elif model_item.status == QueueDownloadStatus.Finished and model_item.file_path: + action = menu.addAction("📂 Show in Explorer") + action.triggered.connect(lambda: self.on_show_in_explorer(model_item.file_path)) if menu.isEmpty(): return @@ -1631,7 +1625,7 @@ def media_to_queue_download_model( quality_audio=quality_audio, quality_video=quality_video, type_media=type(media).__name__, - status_init=QueueDownloadStatus.Waiting, + status=QueueDownloadStatus.Waiting, obj=media, ) else: @@ -2169,7 +2163,7 @@ def queue_download_media(self, queue_dl_item: QueueDownloadItem) -> None: # Populate child child: QtWidgets.QTreeWidgetItem = QtWidgets.QTreeWidgetItem() - child.setText(0, queue_dl_item.get_status()) + child.setText(0, queue_dl_item.status) set_queue_download_media(child, queue_dl_item.obj) child.setText(2, queue_dl_item.name) child.setText(3, queue_dl_item.type_media) @@ -2201,7 +2195,7 @@ def watcher_queue_download(self) -> None: ) if result == QueueDownloadStatus.Finished: - self.s_queue_download_item_finished.emit(item, str(path_file)) + self.s_queue_download_item_finished.emit(item, path_file) elif result == QueueDownloadStatus.Skipped: self.s_queue_download_item_skipped.emit(item) except Exception as e: @@ -2218,30 +2212,21 @@ def on_queue_download_item_downloading(self, item: QtWidgets.QTreeWidgetItem) -> """ model_item = item.data(0, QtCore.Qt.ItemDataRole.UserRole) if isinstance(model_item, QueueDownloadItem): - model_item.set_status(QueueDownloadStatus.Downloading) + model_item.status = QueueDownloadStatus.Downloading self.queue_download_item_status(item, QueueDownloadStatus.Downloading) - def on_queue_download_item_finished(self, item: QtWidgets.QTreeWidgetItem, path: str) -> None: - """Update the status of a queue download item to 'Finished'. - Thread-safe implementation with proper path handling. + def on_queue_download_item_finished(self, item: QtWidgets.QTreeWidgetItem, path: pathlib.Path | str) -> None: + """Update the status of a queue download item to 'Finished' and store the file path. Args: - item (QtWidgets.QTreeWidgetItem): The item to update. + item: The tree widget item representing the download. + path: Path to the downloaded file or directory. """ model_item = item.data(0, QtCore.Qt.ItemDataRole.UserRole) if isinstance(model_item, QueueDownloadItem): - try: - # Normalize and validate path before storing - normalized_path = FileSystemHelper.normalize_path(path) - model_item.set_file_path(str(normalized_path)) - model_item.set_status(QueueDownloadStatus.Finished) - self.queue_download_item_status(item, QueueDownloadStatus.Finished) - except ValueError as e: - logger_gui.error(f"Invalid path format: {e}") - # If the path is invalid, mark as Failed - model_item.set_status(QueueDownloadStatus.Failed) - self.queue_download_item_status(item, QueueDownloadStatus.Failed) - return + model_item.file_path = pathlib.Path(path) if path else None + model_item.status = QueueDownloadStatus.Finished + self.queue_download_item_status(item, QueueDownloadStatus.Finished) def on_queue_download_item_failed(self, item: QtWidgets.QTreeWidgetItem) -> None: """Update the status of a queue download item to 'Failed'. @@ -2251,7 +2236,7 @@ def on_queue_download_item_failed(self, item: QtWidgets.QTreeWidgetItem) -> None """ model_item = item.data(0, QtCore.Qt.ItemDataRole.UserRole) if isinstance(model_item, QueueDownloadItem): - model_item.set_status(QueueDownloadStatus.Failed) + model_item.status = QueueDownloadStatus.Failed self.queue_download_item_status(item, QueueDownloadStatus.Failed) def on_queue_download_item_skipped(self, item: QtWidgets.QTreeWidgetItem) -> None: @@ -2262,7 +2247,7 @@ def on_queue_download_item_skipped(self, item: QtWidgets.QTreeWidgetItem) -> Non """ model_item = item.data(0, QtCore.Qt.ItemDataRole.UserRole) if isinstance(model_item, QueueDownloadItem): - model_item.set_status(QueueDownloadStatus.Skipped) + model_item.status = QueueDownloadStatus.Skipped self.queue_download_item_status(item, QueueDownloadStatus.Skipped) def queue_download_item_status(self, item: QtWidgets.QTreeWidgetItem, status: str) -> None: @@ -2279,93 +2264,62 @@ def on_queue_download( media: Track | Album | Playlist | Video | Mix | Artist, quality_audio: Quality | None = None, quality_video: QualityVideo | None = None, - ) -> tuple[QueueDownloadStatus, pathlib.Path | str]: - """ - Download the specified media item(s) and return the final status and path. - - This method coordinates the download process for various media types. - For Artists, it calculates the artist's root directory. For all - other types, it downloads the item(s) and captures the resulting - file or directory path. + ) -> tuple[QueueDownloadStatus, pathlib.Path]: + """Download media item(s) and return status and path. Args: - media: The TIDAL media object to download (e.g., Track, Album, Artist). - quality_audio: The desired audio quality for the download. - quality_video: The desired video quality for the download. + media: The TIDAL media object to download. + quality_audio: Desired audio quality for the download. + quality_video: Desired video quality for the download. Returns: - A tuple containing: - - result_status (QueueDownloadStatus): The final status of the - download (e.g., Finished, Skipped, Failed). - - path_file (pathlib.Path | str): The final path to the - downloaded file or directory. For Artist downloads, this is - the artist's root folder. + Tuple of (download status, path to downloaded file/directory). + For Artists, returns the artist's root directory. + For Albums/Playlists, returns the album/playlist directory. + For Tracks/Videos, returns the file path. """ result_status: QueueDownloadStatus - path_file: pathlib.Path | str + path_file: pathlib.Path = pathlib.Path() # Handle Artist downloads if isinstance(media, Artist): items_media = items_results_all(media) if not items_media: - path_file = pathlib.Path(self.dl.path_base).expanduser() - result_status = QueueDownloadStatus.Skipped - return result_status, path_file + return QueueDownloadStatus.Skipped, pathlib.Path(self.dl.path_base).expanduser() - # Calculate artist path from first album - try: - path_file = self._calculate_artist_path(items_media[0]) - except Exception as e: - logger_gui.warning(f"Could not deduce artist path. Using base path. Error: {e}") - path_file = pathlib.Path(self.dl.path_base).expanduser() - else: - items_media = [media] - path_file = pathlib.Path() + first_album_path: pathlib.Path | None = None + + for item_media in items_media: + result_item, path_item = self.download( + item_media, + self.dl, + delay_track=False, + quality_audio=quality_audio, + quality_video=quality_video, + ) + result_status = result_item + + if first_album_path is None and path_item and path_item != pathlib.Path(): + first_album_path = path_item - # Download items - download_delay = bool(isinstance(media, Track | Video) and self.settings.data.download_delay) + # Artist path is parent of first album + path_file = first_album_path.parent if first_album_path else pathlib.Path(self.dl.path_base).expanduser() - for item_media in items_media: - result_item, path_item = self.download( - item_media, + else: + # For non-Artist media + download_delay = isinstance(media, Track | Video) and self.settings.data.download_delay + + result_status, path_file = self.download( + media, self.dl, delay_track=download_delay, quality_audio=quality_audio, quality_video=quality_video, ) - result_status = result_item - - # For non-Artist downloads, use the item path - if not isinstance(media, Artist): - path_file = path_item return result_status, path_file - def _calculate_artist_path(self, first_album: Album) -> pathlib.Path: - """ - Calculate the artist directory path from the first album. - This implements the "parent.parent" workaround. - """ - # Get template (which is faulty and returns a track template) - album_template = get_format_template(first_album, self.settings) - - # Format path - formatted_path_str = format_path_media( - album_template, - first_album, # We use the album object - self.settings.data.album_track_num_pad_min, - delimiter_artist=self.settings.data.filename_delimiter_artist, - delimiter_album_artist=self.settings.data.filename_delimiter_album_artist, - ) - - # Build full path - base_path = pathlib.Path(self.dl.path_base).expanduser() - track_full_path = base_path / formatted_path_str - - # Go up two levels (track -> album -> artist) - return track_full_path.parent.parent - def download( self, media: Track | Album | Playlist | Video | Mix | Artist, @@ -2377,17 +2331,17 @@ def download( """Download a media item and return the result status. Args: - media (Track | Album | Playlist | Video | Mix | Artist): The media item to download. - dl (Download): The Download object to use. - delay_track (bool, optional): Whether to apply download delay. Defaults to False. - quality_audio (Quality | None, optional): Desired audio quality. Defaults to None. - quality_video (QualityVideo | None, optional): Desired video quality. Defaults to None. + media: The media item to download. + dl: The Download object to use. + delay_track: Whether to apply download delay. + quality_audio: Desired audio quality. + quality_video: Desired video quality. Returns: - QueueDownloadStatus: The status of the download operation. + (QueueDownloadStatus, path_file): The status and the final file path. """ result_dl: bool - path_file: str + path_file: str | pathlib.Path = "" result: QueueDownloadStatus self.s_pb_reset.emit() self.s_statusbar_message.emit(StatusbarMessage(message="Download started...")) @@ -2412,13 +2366,10 @@ def download( quality_video=quality_video, ) - # path_album_dir is the path returned by download.py - # Will be None if the download failed or the list was empty if path_album_dir: result_dl = True path_file = path_album_dir else: - # Handle the case where the download could have failed within items result_dl = False path_file = "" # No route to return @@ -2553,25 +2504,49 @@ def on_download_album_from_track(self, point: QtCore.QPoint) -> None: else: logger_gui.warning("Could not retrieve album information from the selected track.") - def on_show_in_explorer(self, path: str) -> None: - """ - Opens the file explorer at the given path. - Enhanced with proper validation and error handling. - """ + def on_show_in_explorer(self, path: pathlib.Path | None) -> None: + """Open the containing folder and (when possible) select/reveal the file in the OS file manager.""" + if not path: + logger_gui.error("Attempted to show in explorer but no path was provided") + return + + resolved_path = self._resolve_path(path) + if not resolved_path: + return + + self._open_in_file_manager(resolved_path) + + def _resolve_path(self, path: pathlib.Path) -> pathlib.Path | None: + """Expand and resolve path, log errors if invalid.""" try: - # Normalize and validate path - normalized_path = FileSystemHelper.normalize_path(path) + return path.expanduser().resolve() + except (OSError, RuntimeError) as e: + logger_gui.error(f"Invalid path cannot be resolved: {path!s} → {e}") + return None - # Open in explorer with platform-specific handling - success = FileSystemHelper.open_in_explorer(normalized_path, logger_gui) + def _open_in_file_manager(self, path: pathlib.Path) -> None: + """Open path in system file manager with platform-specific command.""" + try: + if sys.platform == "win32": + self._open_windows(path) + elif sys.platform == "darwin": + self._open_macos(path) + else: + self._open_linux(path) + except Exception: + logger_gui.exception(f"Unexpected error opening file manager for {path}") - if success: - logger_gui.info(f"Opened explorer at: {normalized_path}") + def _open_windows(self, path: pathlib.Path) -> None: + cmd = ["explorer", "/select,", str(path)] if path.is_file() else ["explorer", str(path)] + subprocess.Popen(cmd, shell=True) # noqa: S602 # Required for /select, on Windows; path is trusted - except ValueError as e: - logger_gui.error(f"Invalid path: {e}") - except Exception as e: - logger_gui.error(f"Unexpected error opening explorer: {e}") + def _open_macos(self, path: pathlib.Path) -> None: + cmd = ["open", "-R", str(path)] if path.is_file() else ["open", str(path)] + subprocess.run(cmd, check=True) # noqa: S603 # `open` is trusted system command + + def _open_linux(self, path: pathlib.Path) -> None: + target = path.parent if path.is_file() else path + subprocess.run(["xdg-open", str(target)], check=True) # noqa: S603, S607 # `xdg-open` is trusted system utility # TODO: Comment with Google Docstrings. diff --git a/tidal_dl_ng/helper/filesystem.py b/tidal_dl_ng/helper/filesystem.py deleted file mode 100644 index e4b155ff..00000000 --- a/tidal_dl_ng/helper/filesystem.py +++ /dev/null @@ -1,170 +0,0 @@ -import pathlib -import subprocess -import sys - -from tidal_dl_ng.logger import logger_gui - - -class FileSystemHelper: - """Helper class for file system operations with proper error handling.""" - - @staticmethod - def normalize_path(path: str) -> pathlib.Path: - """ - Normalize and validate a file system path. - - Args: - path: The raw string path. - - Returns: - A resolved pathlib.Path object. - - Raises: - ValueError: If the path is empty or invalid. - """ - if not path: - raise ValueError("Path cannot be empty") - - try: - normalized = pathlib.Path(path) - # Resolve to catch issues early (optional, depends on use case) - # normalized.resolve(strict=False) - return normalized - - except (ValueError, TypeError) as e: - raise ValueError(f"Invalid path format: {path}") from e - - @staticmethod - def path_exists(path: pathlib.Path) -> bool: - """ - Check if path exists. - - Args: - path: The path to check. - - Returns: - True if the path exists, False otherwise. - """ - try: - return path.exists() - except (OSError, PermissionError): - logger_gui.exception("Permission error checking path", extra={"path": path}) - return False - - @staticmethod - def open_in_explorer(path: pathlib.Path, logger) -> bool: - """ - Open file explorer at the given path with platform-specific handling. - - Args: - path: The pathlib.Path object to open. - logger: The logger instance to use for reporting errors. - - Returns: - True if the operation was successful, False otherwise. - """ - try: - # Validate path exists - if not FileSystemHelper.path_exists(path): - # No log error here; path_exists already logged exception if one occurred - return False - - # Determine if path is file or directory - is_file = path.is_file() - - # Platform-specific handling - if sys.platform == "win32": - return FileSystemHelper._open_windows(path, is_file, logger) - elif sys.platform == "darwin": - return FileSystemHelper._open_macos(path, is_file, logger) - else: - return FileSystemHelper._open_linux(path, is_file, logger) - - except Exception: - logger.exception("Could not open explorer") - return False - - @staticmethod - def _open_windows(path: pathlib.Path, is_file: bool, logger) -> bool: - """ - Open Windows Explorer to the specified path. - - Args: - path: The path to open. - is_file: True if the path is a file, False if a directory. - logger: The logger instance. - - Returns: - True on success, False on failure. - """ - try: - if is_file: - # Select the file in Explorer - subprocess.Popen(["explorer", "/select,", str(path)]) - else: - # Open the directory - subprocess.Popen(["explorer", str(path)]) - except (subprocess.CalledProcessError, FileNotFoundError): - logger.exception("Windows Explorer failed") - return False - else: - return True - - @staticmethod - def _open_macos(path: pathlib.Path, is_file: bool, logger) -> bool: - """ - Open macOS Finder to the specified path. - - Args: - path: The path to open. - is_file: True if the path is a file, False if a directory. - logger: The logger instance. - - Returns: - True on success, False on failure. - """ - try: - if is_file: - # Reveal the file in Finder - subprocess.run(["open", "-R", str(path)], check=True) - else: - # Open the directory - subprocess.run(["open", str(path)], check=True) - except (subprocess.CalledProcessError, FileNotFoundError): - logger.exception("macOS Finder failed") - return False - else: - return True - - @staticmethod - def _open_linux(path: pathlib.Path, is_file: bool, logger) -> bool: - """ - Open Linux file manager to the specified path. - - Args: - path: The path to open. - is_file: True if the path is a file, False if a directory. - logger: The logger instance. - - Returns: - True on success, False on failure. - """ - try: - # Most Linux file managers prefer opening the directory - dir_path = path.parent if is_file else path - - # Try xdg-open first (standard) - subprocess.run(["xdg-open", str(dir_path)], check=True) - return True - except (subprocess.CalledProcessError, FileNotFoundError): - # Fallback - file_managers = ["nautilus", "dolphin", "thunar", "nemo", "pcmanfm"] - for manager in file_managers: - try: - subprocess.run([manager, str(dir_path)], check=True) - return True - except (subprocess.CalledProcessError, FileNotFoundError): - continue - - logger.exception("No compatible file manager found on Linux") - return False diff --git a/tidal_dl_ng/model/gui_data.py b/tidal_dl_ng/model/gui_data.py index 5419a187..0f2e3d21 100644 --- a/tidal_dl_ng/model/gui_data.py +++ b/tidal_dl_ng/model/gui_data.py @@ -1,9 +1,10 @@ -from dataclasses import InitVar, dataclass, field +import pathlib +from dataclasses import dataclass, field from threading import Lock from tidalapi.media import Quality -from tidal_dl_ng.constants import QualityVideo, QueueDownloadStatus +from tidal_dl_ng.constants import QualityVideo try: from PySide6 import QtCore @@ -43,36 +44,23 @@ class StatusbarMessage: @dataclass class QueueDownloadItem: + status: str name: str type_media: str quality_audio: Quality quality_video: QualityVideo obj: object - status_init: InitVar[str] = QueueDownloadStatus.Waiting - _status: str = field(init=False) - _file_path: str | None = field(default=None, init=False, repr=False) + _file_path: pathlib.Path | None = field(default=None, init=False, repr=False) _lock: Lock = field(default_factory=Lock, init=False, repr=False) - def __post_init__(self, status_init: str): - """Assigns the initial state after creation.""" - self._status = status_init - - def set_file_path(self, path: str) -> None: - """Thread-safe setter for file_path.""" - with self._lock: - self._file_path = path - - def get_file_path(self) -> str | None: - """Thread-safe getter for file_path.""" + @property + def file_path(self) -> pathlib.Path | None: + """Get the downloaded file path (thread-safe).""" with self._lock: return self._file_path - def set_status(self, status: str) -> None: - """Thread-safe setter for status.""" + @file_path.setter + def file_path(self, path: pathlib.Path | None) -> None: + """Set the downloaded file path (thread-safe).""" with self._lock: - self._status = status - - def get_status(self) -> str: - """Thread-safe getter for status.""" - with self._lock: - return self._status + self._file_path = path From dea23b1ed6bff4f2e7617a0b896d4fb5afb9a5fe Mon Sep 17 00:00:00 2001 From: Rikrdoga Date: Thu, 13 Nov 2025 21:29:22 -0600 Subject: [PATCH 7/7] docs: Update docstrings to reflect Show in Explorer changes --- tidal_dl_ng/download.py | 2 +- tidal_dl_ng/gui.py | 66 +++++++++++++++++++++++++---------- tidal_dl_ng/model/gui_data.py | 24 +++++++++++-- 3 files changed, 71 insertions(+), 21 deletions(-) diff --git a/tidal_dl_ng/download.py b/tidal_dl_ng/download.py index bda11d68..b901eb1e 100644 --- a/tidal_dl_ng/download.py +++ b/tidal_dl_ng/download.py @@ -1012,7 +1012,7 @@ def _perform_post_processing( Args: media (Track | Video): Media item. - path_media_dst (pathlib.Path): Destination file path. + path_media_src (pathlib.Path): Source file path. quality_audio (Quality | None): Audio quality setting. quality_video (QualityVideo | None): Video quality setting. quality_audio_old (Quality | None): Previous audio quality. diff --git a/tidal_dl_ng/gui.py b/tidal_dl_ng/gui.py index 38faac64..055bdcab 100644 --- a/tidal_dl_ng/gui.py +++ b/tidal_dl_ng/gui.py @@ -2265,18 +2265,18 @@ def on_queue_download( quality_audio: Quality | None = None, quality_video: QualityVideo | None = None, ) -> tuple[QueueDownloadStatus, pathlib.Path]: - """Download media item(s) and return status and path. + """Download the specified media item(s) and return the result status and path. Args: - media: The TIDAL media object to download. - quality_audio: Desired audio quality for the download. - quality_video: Desired video quality for the download. + media (Track | Album | Playlist | Video | Mix | Artist): The media item(s) to download. + quality_audio (Quality | None, optional): Desired audio quality. Defaults to None. + quality_video (QualityVideo | None, optional): Desired video quality. Defaults to None. Returns: - Tuple of (download status, path to downloaded file/directory). - For Artists, returns the artist's root directory. - For Albums/Playlists, returns the album/playlist directory. - For Tracks/Videos, returns the file path. + tuple[QueueDownloadStatus, pathlib.Path]: Tuple of (download status, path to downloaded file/directory). + For Artists, returns the artist's root directory. + For Albums/Playlists, returns the album/playlist directory. + For Tracks/Videos, returns the file path. """ result_status: QueueDownloadStatus path_file: pathlib.Path = pathlib.Path() @@ -2328,17 +2328,17 @@ def download( quality_audio: Quality | None = None, quality_video: QualityVideo | None = None, ) -> tuple[QueueDownloadStatus, pathlib.Path | str]: - """Download a media item and return the result status. + """Download a media item and return the result status and path. Args: - media: The media item to download. - dl: The Download object to use. - delay_track: Whether to apply download delay. - quality_audio: Desired audio quality. - quality_video: Desired video quality. + media (Track | Album | Playlist | Video | Mix | Artist): The media item to download. + dl (Download): The Download object to use. + delay_track (bool, optional): Whether to apply download delay. Defaults to False. + quality_audio (Quality | None, optional): Desired audio quality. Defaults to None. + quality_video (QualityVideo | None, optional): Desired video quality. Defaults to None. Returns: - (QueueDownloadStatus, path_file): The status and the final file path. + tuple[QueueDownloadStatus, pathlib.Path | str]: The status and the final file path. """ result_dl: bool path_file: str | pathlib.Path = "" @@ -2505,7 +2505,11 @@ def on_download_album_from_track(self, point: QtCore.QPoint) -> None: logger_gui.warning("Could not retrieve album information from the selected track.") def on_show_in_explorer(self, path: pathlib.Path | None) -> None: - """Open the containing folder and (when possible) select/reveal the file in the OS file manager.""" + """Open the containing folder and (when possible) select/reveal the file in the OS file manager. + + Args: + path: Path to the file or directory to show in explorer. + """ if not path: logger_gui.error("Attempted to show in explorer but no path was provided") return @@ -2517,7 +2521,14 @@ def on_show_in_explorer(self, path: pathlib.Path | None) -> None: self._open_in_file_manager(resolved_path) def _resolve_path(self, path: pathlib.Path) -> pathlib.Path | None: - """Expand and resolve path, log errors if invalid.""" + """Expand and resolve path, log errors if invalid. + + Args: + path: The path to resolve. + + Returns: + The resolved path, or None if invalid. + """ try: return path.expanduser().resolve() except (OSError, RuntimeError) as e: @@ -2525,7 +2536,11 @@ def _resolve_path(self, path: pathlib.Path) -> pathlib.Path | None: return None def _open_in_file_manager(self, path: pathlib.Path) -> None: - """Open path in system file manager with platform-specific command.""" + """Open path in system file manager with platform-specific command. + + Args: + path: The resolved path to open. + """ try: if sys.platform == "win32": self._open_windows(path) @@ -2537,14 +2552,29 @@ def _open_in_file_manager(self, path: pathlib.Path) -> None: logger_gui.exception(f"Unexpected error opening file manager for {path}") def _open_windows(self, path: pathlib.Path) -> None: + """Open path in Windows Explorer. + + Args: + path: The path to open. + """ cmd = ["explorer", "/select,", str(path)] if path.is_file() else ["explorer", str(path)] subprocess.Popen(cmd, shell=True) # noqa: S602 # Required for /select, on Windows; path is trusted def _open_macos(self, path: pathlib.Path) -> None: + """Open path in macOS Finder. + + Args: + path: The path to open. + """ cmd = ["open", "-R", str(path)] if path.is_file() else ["open", str(path)] subprocess.run(cmd, check=True) # noqa: S603 # `open` is trusted system command def _open_linux(self, path: pathlib.Path) -> None: + """Open path in Linux file manager. + + Args: + path: The path to open. + """ target = path.parent if path.is_file() else path subprocess.run(["xdg-open", str(target)], check=True) # noqa: S603, S607 # `xdg-open` is trusted system utility diff --git a/tidal_dl_ng/model/gui_data.py b/tidal_dl_ng/model/gui_data.py index 0f2e3d21..d4d94fbd 100644 --- a/tidal_dl_ng/model/gui_data.py +++ b/tidal_dl_ng/model/gui_data.py @@ -44,6 +44,18 @@ class StatusbarMessage: @dataclass class QueueDownloadItem: + """Download queue item model. + + Attributes: + status (str): Current download status. + name (str): Display name of the media item. + type_media (str): Type of media (Track, Album, etc). + quality_audio (Quality): Audio quality setting. + quality_video (QualityVideo): Video quality setting. + obj (object): The actual media object. + file_path (pathlib.Path | None): Path to downloaded file/directory (thread-safe property). + """ + status: str name: str type_media: str @@ -55,12 +67,20 @@ class QueueDownloadItem: @property def file_path(self) -> pathlib.Path | None: - """Get the downloaded file path (thread-safe).""" + """Get the downloaded file path (thread-safe). + + Returns: + pathlib.Path | None: Path to the downloaded file or directory, or None if not yet set. + """ with self._lock: return self._file_path @file_path.setter def file_path(self, path: pathlib.Path | None) -> None: - """Set the downloaded file path (thread-safe).""" + """Set the downloaded file path (thread-safe). + + Args: + path (pathlib.Path | None): Path to the downloaded file or directory. + """ with self._lock: self._file_path = path