diff --git a/CLAUDE.md b/CLAUDE.md index a38654a..5bfbfe4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -103,6 +103,16 @@ persistence) → GUI (Qt widgets that read the manager and connect to its signal so the desktop's media keys / now-playing popup control playback. Spacebar and arrow keys are handled locally via `MainWindow.eventFilter`. +- **`lintunes/trash.py`** — freedesktop.org Trash spec 1.0, hand-rolled (no new + dep). The trash is **per-filesystem**: music usually lives on a mounted volume, + so the file belongs in `/.Trash-` with a *topdir-relative*, + percent-encoded `Path=`, not in `~/.local/share/Trash` (which would be a + cross-device copy the file manager can't "Restore"). The `.trashinfo` is + created with `O_EXCL` **first** to claim the name atomically, then the file is + renamed in; a failed rename unlinks the info file so there's never a + half-trashed pair. Raises `TrashError` without touching the file, so a caller + can treat failure as "not deleted". Only `LibraryManager` calls it. + - **`lintunes/device_sync.py`** — one-way playlist sync to the Rabbit R1 (Device menu). The Rabbit mounts via **MTP/gvfs** (a FUSE path under `/run/user//gvfs`), not mass storage — so plain file I/O, but never @@ -152,8 +162,11 @@ persistence) → GUI (Qt widgets that read the manager and connect to its signal inside `LibraryManager.organize_root()` (`/Music`) to keep the tree organized iTunes-style (`_maybe_move_file`; undoable; files outside the root are never moved; the new path syncs cross-machine via the `location` - newest-wins merge in `conflict_resolver`). Nothing else may move or rewrite - music files. The library JSON is the source of truth for everything else. + newest-wins merge in `conflict_resolver`). Since Round 33 the *only* other + path is an explicit user delete (`LibraryManager.delete_tracks`), which moves + the file to the desktop trash via `trash.py` — never `unlink`, so it stays + recoverable. Nothing else may move, rewrite or remove music files. The library + JSON is the source of truth for everything else. - **Versioning & self-update:** `__version__` in `lintunes/__init__.py` is the single source of truth (`setup.py` regex-reads it, never imports the package). Claude bumps minor for feature rounds and patch for fix-only diff --git a/TASKS.md b/TASKS.md index 205611f..9e8a8fe 100644 --- a/TASKS.md +++ b/TASKS.md @@ -7,7 +7,6 @@ When a round closes, move its finished items to `tasks-done.md`. # tasks -- [ ] ability to delete songs from library - [ ] archive the done tasks in here to another file, this is crufty.... diff --git a/lintunes/__init__.py b/lintunes/__init__.py index 6f7440a..ca87981 100644 --- a/lintunes/__init__.py +++ b/lintunes/__init__.py @@ -1,3 +1,3 @@ """LinTunes — iTunes-style music library manager and player for Linux.""" -__version__ = "0.5.2" +__version__ = "0.6.0" diff --git a/lintunes/gui/main_window.py b/lintunes/gui/main_window.py index 7ecf231..bbe221b 100644 --- a/lintunes/gui/main_window.py +++ b/lintunes/gui/main_window.py @@ -171,7 +171,13 @@ class MainWindow(QMainWindow): view.show_in_playlist_requested.connect(self._show_track_in_playlist) view.table.tracks_changed.connect(self._update_totals) view.table.download_art_requested.connect(self._download_album_art) + view.table.remove_from_library_requested.connect( + lambda ids: self._delete_tracks(ids, delete_files=False)) + view.table.delete_from_library_requested.connect( + lambda ids: self._delete_tracks(ids, delete_files=True)) self._playlist_view.files_dropped.connect(self._import_files_to_playlist) + manager.tracks_removed.connect(self._on_tracks_removed) + manager.tracks_restored.connect(self._on_tracks_restored) self.player.error_occurred.connect( lambda msg: QMessageBox.warning(self, "Can't play track", msg)) self.player.track_missing.connect(self._on_track_missing) @@ -589,6 +595,79 @@ class MainWindow(QMainWindow): def _import_files_to_playlist(self, paths, pid, position=None): self.import_files([Path(p) for p in paths], pid, position) + # ---- deleting tracks ---- + + def _delete_tracks(self, track_ids: list[int], delete_files: bool): + tracks = [self._manager.library.tracks[tid] for tid in track_ids + if tid in self._manager.library.tracks] + if not tracks or not self._confirm_delete(tracks, delete_files): + return + removed, failures = self._manager.delete_tracks( + [t.track_id for t in tracks], delete_files=delete_files) + + if removed: + where = "moved to Trash" if delete_files else "file kept" + self.statusBar().showMessage( + f"Removed {len(removed)} song(s) from your library " + f"({where}) — undo with Ctrl+Z", 8000) + if failures: + QMessageBox.warning( + self, "Couldn't delete", + f"{len(failures)} file(s) could not be moved to the Trash, so " + "those songs are still in your library:\n\n" + + "\n".join(f"• “{name}”: {err}" for name, err in failures[:5])) + + def _confirm_delete(self, tracks: list, delete_files: bool) -> bool: + if len(tracks) == 1: + track = tracks[0] + who = f" by {track.artist}" if track.artist else "" + question = f"Delete “{track.name}”{who} from your library?" + else: + question = f"Delete {len(tracks)} songs from your library?" + + if delete_files: + fate = ("The file will be moved to your Trash, so you can put it " + "back later." if len(tracks) == 1 else + "The files will be moved to your Trash, so you can put " + "them back later.") + else: + fate = ("The file stays where it is on disk." + if len(tracks) == 1 else + "The files stay where they are on disk.") + + doomed = {t.track_id for t in tracks} + playlists = {pid for pid, _name in + [entry for t in tracks + for entry in self._manager.playlists_containing(t.track_id)]} + lines = [question, fate] + if playlists: + lines.append(f"{'It' if len(doomed) == 1 else 'They'} will also be " + f"removed from {len(playlists)} playlist(s).") + + box = QMessageBox(self) + box.setIcon(QMessageBox.Icon.Warning) + box.setWindowTitle("Delete from Library") + box.setText("\n".join(lines)) + delete_button = box.addButton( + "Delete Song" if len(tracks) == 1 else "Delete Songs", + QMessageBox.ButtonRole.DestructiveRole) + cancel = box.addButton("Cancel", QMessageBox.ButtonRole.RejectRole) + box.setDefaultButton(cancel) + box.exec() + return box.clickedButton() is delete_button + + def _on_tracks_removed(self, track_ids: list): + """Tracks left the library (deleted here, or undone/redone). Keep the + player off dead ids and rebuild the library view.""" + self.player.drop_tracks(set(track_ids)) + self._on_library_reloaded() + self._update_totals() + + def _on_tracks_restored(self, track_ids: list): + """An undo put tracks back — show them again.""" + self._on_library_reloaded() + self._update_totals() + # ---- album art download ---- def _download_album_art(self, track_ids: list[int]): diff --git a/lintunes/gui/track_table.py b/lintunes/gui/track_table.py index 0a17fdb..77de7be 100644 --- a/lintunes/gui/track_table.py +++ b/lintunes/gui/track_table.py @@ -426,6 +426,8 @@ class TrackTableView(QTableView): show_in_playlist_requested = pyqtSignal(int, str) # track_id, playlist pid rating_edited = pyqtSignal(int, int) # track_id, new rating 0-100 download_art_requested = pyqtSignal(list) # selected track ids + remove_from_library_requested = pyqtSignal(list) # track ids, file kept + delete_from_library_requested = pyqtSignal(list) # track ids, file trashed def __init__(self, parent=None, playlist_mode=False): super().__init__(parent) @@ -701,6 +703,14 @@ class TrackTableView(QTableView): if self._playlist_mode and self._content_editable: menu.addSeparator() remove_action = menu.addAction("Remove from Playlist\tDel") + + # Library removal is offered in both modes (unlike the playlist-only + # remove above) and has no keyboard shortcut — Del stays "remove from + # playlist", and touching files shouldn't be one keystroke away. + menu.addSeparator() + unlist_action = menu.addAction("Remove from Library") + delete_action = menu.addAction("Remove from Library and Delete File") + chosen = menu.exec(self.viewport().mapToGlobal(pos)) if chosen is None: return @@ -722,6 +732,10 @@ class TrackTableView(QTableView): self.show_in_playlist_requested.emit(selected[0], chosen.data()) elif remove_action is not None and chosen is remove_action: self.remove_requested.emit(self.selected_source_rows()) + elif chosen is unlist_action: + self.remove_from_library_requested.emit(self.selected_track_ids()) + elif chosen is delete_action: + self.delete_from_library_requested.emit(self.selected_track_ids()) # ---- rating hover / click ---- diff --git a/lintunes/library_manager.py b/lintunes/library_manager.py index 521c763..69baf86 100644 --- a/lintunes/library_manager.py +++ b/lintunes/library_manager.py @@ -7,7 +7,7 @@ from PyQt6.QtCore import QObject, QTimer, pyqtSignal log = logging.getLogger(__name__) -from lintunes import smart, tagging +from lintunes import smart, tagging, trash from lintunes.importers.file_importer import organized_destination, unique_path from lintunes.models import Library, Playlist, PlaylistType from lintunes.perf import timed @@ -39,6 +39,8 @@ class LibraryManager(QObject): conflict_resolved = pyqtSignal(list) # list[ConflictSummary] just merged tag_write_failed = pyqtSignal(str, str) # track name, error text file_move_failed = pyqtSignal(str, str) # track name, error text + tracks_removed = pyqtSignal(list) # track ids gone from the library + tracks_restored = pyqtSignal(list) # track ids back after an undo def __init__(self, library: Library, data_dir: Path, parent=None): super().__init__(parent) @@ -328,6 +330,137 @@ class LibraryManager(QObject): self._touch_smart(None) return track.track_id + def delete_tracks(self, track_ids: list[int], + delete_files: bool = False) -> tuple[list[int], list[tuple[str, str]]]: + """Remove tracks from the library (and every playlist holding them), + optionally moving their files to the desktop trash. + + Returns ``(removed_ids, failures)`` where a failure is + ``(track name, error)``. A track whose file can't be trashed is **kept** + — a library entry pointing at a file still on disk is recoverable, a + silently-orphaned one is not. + + Playlist cleanup happens here rather than lazily on display: playlist + edits address tracks by row index into ``track_ids``, so an id left + dangling would shift the stored rows out of step with the displayed ones + and a later removal would hit the wrong track. + """ + tracks = [self.library.tracks[tid] for tid in dict.fromkeys(track_ids) + if tid in self.library.tracks] + if not tracks: + return [], [] + + failures: list[tuple[str, str]] = [] + trashed: list[tuple[object, Path]] = [] + if delete_files: + doomed = [] + for track in tracks: + try: + trashed.append((track, self._trash_file(track))) + doomed.append(track) + except trash.TrashError as e: + log.warning("Could not trash %s: %s", track.location, e) + failures.append((track.name or str(track.location), str(e))) + self.file_move_failed.emit(track.name or str(track.location), str(e)) + tracks = doomed + if not tracks: + return [], failures + + snapshots = self._playlist_snapshots([t.track_id for t in tracks]) + self._remove_tracks(tracks, snapshots) + + # The trashed paths live in a mutable cell: a redo re-trashes the files + # and may land on different collision-suffixed names than the first pass. + state = {"trashed": trashed} + + def undo(): + for track, path in state["trashed"]: + try: + trash.restore_from_trash(path, Path(track.location)) + except trash.TrashError as e: + # Trash emptied behind our back. Restore the library entry + # anyway — a track pointing at a missing file is something + # the user can see and fix; a silently dropped one isn't. + log.warning("Could not restore %s: %s", track.location, e) + self.file_move_failed.emit(track.name or str(path), str(e)) + self._restore_tracks(tracks, snapshots) + + def redo(): + state["trashed"] = [] + if delete_files: + for track in tracks: + try: + state["trashed"].append((track, self._trash_file(track))) + except trash.TrashError as e: + log.warning("Could not re-trash %s: %s", track.location, e) + self.file_move_failed.emit( + track.name or str(track.location), str(e)) + self._remove_tracks(tracks, snapshots) + + self.undo_stack.push(Command( + "Delete from Library" if delete_files else "Remove from Library", + undo=undo, redo=redo)) + return [t.track_id for t in tracks], failures + + def _trash_file(self, track) -> Path: + """Send a track's file to the trash and tidy up the folders it emptied.""" + if not track.location: + raise trash.TrashError("This track has no file on disk.") + src = Path(track.location) + dest = trash.send_to_trash(src) + root = self.organize_root() + if root is not None and root in src.parents: + self._prune_empty_dirs(src.parent, root) + return dest + + def _playlist_snapshots(self, track_ids: list[int]) -> dict[str, list[int]]: + """``{pid: track_ids before removal}`` for each non-smart playlist holding + any of these tracks — enough for undo to restore original positions.""" + doomed = set(track_ids) + return {p.persistent_id: list(p.track_ids) + for p in self.library.playlists.values() + if not p.is_smart and doomed.intersection(p.track_ids)} + + def _remove_tracks(self, tracks: list, snapshots: dict[str, list[int]]): + doomed = {t.track_id for t in tracks} + for track in tracks: + self.library.tracks.pop(track.track_id, None) + self._invalidate_artwork(track) + for pid in snapshots: + playlist = self.library.playlists.get(pid) + if playlist is not None: + self._set_track_ids( + pid, [tid for tid in playlist.track_ids if tid not in doomed]) + self._dirty_tracks = True + self._dirty_metadata = True + self._schedule_save() + self._touch_smart(None) + self.tracks_removed.emit(sorted(doomed)) + + def _restore_tracks(self, tracks: list, snapshots: dict[str, list[int]]): + for track in tracks: + self.library.tracks[track.track_id] = track + self._max_track_id = max(self._max_track_id, track.track_id) + for pid, ids in snapshots.items(): + if pid in self.library.playlists: + self._set_track_ids(pid, list(ids)) + self._dirty_tracks = True + self._dirty_metadata = True + self._schedule_save() + self._touch_smart(None) + self.tracks_restored.emit(sorted(t.track_id for t in tracks)) + self.playlists_changed.emit() + + @staticmethod + def _invalidate_artwork(track): + """Drop the track's exported art. Imported lazily: mpris pulls in QtDBus, + which core library code shouldn't need just to delete a song.""" + try: + from lintunes.mpris import invalidate_artwork + invalidate_artwork(track) + except Exception: # no D-Bus bindings here — the cache just ages out + pass + def update_track_fields(self, track_id: int, fields: dict): """In-memory-only field update (no file write, not undoable).""" track = self.library.tracks.get(track_id) diff --git a/lintunes/player.py b/lintunes/player.py index d1cb199..7a65a3e 100644 --- a/lintunes/player.py +++ b/lintunes/player.py @@ -400,6 +400,28 @@ class Player(QObject): self._index = self._queue.index(current_id) self._reshuffle() + def drop_tracks(self, track_ids): + """Forget tracks that no longer exist in the library. + + Stops if one of them is playing; otherwise the current track keeps its + place in the shortened queue. Without this the queue keeps dead ids and + ``_load_current`` stops the whole thing with an error dialog when the + walk eventually reaches one. + """ + doomed = set(track_ids) + if not doomed: + return + if self._current_track is not None and self._current_track.track_id in doomed: + self.stop() + current_id = None + else: + current_id = (self._queue[self._index] + if 0 <= self._index < len(self._queue) else None) + self._queue = [tid for tid in self._queue if tid not in doomed] + self._index = (self._queue.index(current_id) + if current_id is not None and current_id in self._queue else -1) + self._reshuffle() + # ---- shuffle ---- def is_shuffle(self) -> bool: diff --git a/lintunes/trash.py b/lintunes/trash.py new file mode 100644 index 0000000..c29bd00 --- /dev/null +++ b/lintunes/trash.py @@ -0,0 +1,179 @@ +"""Move files to the desktop trash, per the freedesktop.org Trash spec 1.0. + +Music usually lives on whatever volume the user mounted it from, not on the home +filesystem — so the trash that matters is the one at the top of the *file's own* +mount (``/.Trash-``), not ``~/.local/share/Trash``. Getting that +wrong would mean a slow cross-device copy and, worse, a trashed file the file +manager's "Restore" can't put back, because the recorded original path would sit +on a different filesystem than the trash holding it. + +Nothing here knows about the library; ``LibraryManager`` is the only caller. +""" + +import logging +import os +import stat +from datetime import datetime +from pathlib import Path +from urllib.parse import quote, unquote + +log = logging.getLogger(__name__) + + +class TrashError(OSError): + """Raised when a file could not be trashed. The file is left where it is.""" + + +def _home_trash() -> Path: + base = os.environ.get("XDG_DATA_HOME") or str(Path.home() / ".local" / "share") + return Path(base) / "Trash" + + +def _topdir(path: Path) -> Path: + """The mount point the path lives on.""" + current = path if path.is_dir() else path.parent + current = current.resolve() + while not os.path.ismount(current) and current != current.parent: + current = current.parent + return current + + +def trash_dir_for(path: Path) -> tuple[Path, Path | None]: + """Where ``path`` should be trashed. + + Returns ``(trash_dir, topdir)``. ``topdir`` is None for the home trash — the + spec wants an absolute ``Path=`` there and a topdir-relative one in a volume + trash, so the caller needs to know which it got. + """ + home_trash = _home_trash() + top = _topdir(path) + + # Same filesystem as the home trash? Then that's the right one, whether or + # not it already exists. Compare devices rather than paths: /home is often + # its own mount, which would otherwise look like a "volume". + try: + home_dev = os.stat(_first_existing(home_trash)).st_dev + if os.stat(path).st_dev == home_dev: + return home_trash, None + except OSError: # no readable home — fall through to the volume trash + pass + + # Spec: an admin-provided /.Trash must be a sticky, non-symlink dir, + # and we get our own uid-named subdir inside it. Otherwise we make our own. + uid = os.getuid() + admin = top / ".Trash" + try: + st = os.lstat(admin) + if stat.S_ISDIR(st.st_mode) and (st.st_mode & stat.S_ISVTX): + return admin / str(uid), top + except OSError: + pass + return top / f".Trash-{uid}", top + + +def _first_existing(path: Path) -> Path: + """Nearest existing ancestor (inclusive) — for stat'ing a dir we may create.""" + current = path + while not current.exists() and current != current.parent: + current = current.parent + return current + + +def _ensure_trash(trash_dir: Path) -> tuple[Path, Path]: + files_dir, info_dir = trash_dir / "files", trash_dir / "info" + try: + for directory in (trash_dir, files_dir, info_dir): + directory.mkdir(mode=0o700, parents=True, exist_ok=True) + except OSError as e: + raise TrashError(f"Could not prepare the trash at {trash_dir}: {e}") from e + return files_dir, info_dir + + +def _claim(info_dir: Path, name: str) -> tuple[Path, str]: + """Reserve a name in the trash by creating its .trashinfo exclusively. + + Creating the info file is what makes the claim atomic against another app + trashing a same-named file at the same moment; the audio file is moved in + afterwards, under the name we just won. + """ + stem, suffix = Path(name).stem, Path(name).suffix + for i in range(0, 1000): + candidate = name if i == 0 else f"{stem} {i}{suffix}" + info_path = info_dir / f"{candidate}.trashinfo" + try: + fd = os.open(info_path, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600) + except FileExistsError: + continue + except OSError as e: + raise TrashError(f"Could not write {info_path}: {e}") from e + os.close(fd) + return info_path, candidate + raise TrashError(f"Could not find a free name in {info_dir} for {name}") + + +def send_to_trash(path: Path) -> Path: + """Move ``path`` into the trash for its filesystem; return its new location. + + Raises ``TrashError`` without touching the file if anything goes wrong, so a + caller can treat a failure as "this was not deleted". + """ + path = Path(path) + if not path.exists(): + raise TrashError(f"{path} does not exist") + + trash_dir, top = trash_dir_for(path) + files_dir, info_dir = _ensure_trash(trash_dir) + info_path, name = _claim(info_dir, path.name) + + # Path= is relative to the topdir in a volume trash, absolute in the home + # one, and percent-encoded either way. + original = path.resolve() + recorded = original.relative_to(top) if top is not None else original + info_path.write_text( + "[Trash Info]\n" + f"Path={quote(str(recorded))}\n" + f"DeletionDate={datetime.now().strftime('%Y-%m-%dT%H:%M:%S')}\n", + encoding="utf-8") + + dest = files_dir / name + try: + os.rename(original, dest) + except OSError as e: + info_path.unlink(missing_ok=True) # never leave a half-trashed pair + raise TrashError(f"Could not move {path} to the trash: {e}") from e + log.info("Trashed %s -> %s", original, dest) + return dest + + +def restore_from_trash(trashed: Path, original: Path) -> None: + """Move a trashed file back and drop its .trashinfo. Raises TrashError.""" + trashed, original = Path(trashed), Path(original) + if not trashed.exists(): + raise TrashError(f"{trashed} is no longer in the trash") + try: + original.parent.mkdir(parents=True, exist_ok=True) + os.rename(trashed, original) + except OSError as e: + raise TrashError(f"Could not restore {trashed} to {original}: {e}") from e + info_path = trashed.parent.parent / "info" / f"{trashed.name}.trashinfo" + info_path.unlink(missing_ok=True) + + +def original_path(trashed: Path) -> Path | None: + """The path a trashed file came from, per its .trashinfo — None if unreadable. + + Only used by tests and diagnostics; restoring uses the location the library + already knows. + """ + info_path = Path(trashed).parent.parent / "info" / f"{Path(trashed).name}.trashinfo" + try: + text = info_path.read_text(encoding="utf-8") + except OSError: + return None + for line in text.splitlines(): + if line.startswith("Path="): + recorded = Path(unquote(line[len("Path="):])) + if recorded.is_absolute(): + return recorded + return _topdir(Path(trashed)) / recorded + return None diff --git a/tasks-done.md b/tasks-done.md index 3f21413..cfb58da 100644 --- a/tasks-done.md +++ b/tasks-done.md @@ -1,5 +1,53 @@ ## Done +### Round 33 (2026-08-14) — Delete songs from the library (v0.6.0) + +An iTunes 12 habit LinTunes had no answer for: getting rid of a song you don't +want to keep. `LibraryManager` had `add_track` but no inverse, and nothing in the +app had ever deleted a local music file. Deletion goes to the desktop trash +rather than `unlink`, so it's recoverable both by Ctrl+Z and from the file +manager afterwards. + +Two constraints shaped it. The trash is **per-filesystem** — the music sits on a +mounted volume, so the right destination is `/.Trash-`, and using +the home trash would mean a cross-device copy plus a recorded original path +"Restore" can't reach. And playlist cleanup has to be **synchronous with the +removal**: playlist edits address tracks by row index into `track_ids` while the +view skips ids missing from the library, so a dangling id would desync the two +and make a later "Remove from Playlist" hit the wrong track. + +- [x] `lintunes/trash.py` — freedesktop Trash spec 1.0 by hand, no new + dependency (a new pip dep would need a manual `pip install -e .` on the + other machine). Picks the trash by the file's own mount, writes the + percent-encoded `.trashinfo` with `O_EXCL` first to claim the name, then + renames the file in; a failed rename cleans up the info file. `TrashError` + leaves the file untouched. +- [x] `LibraryManager.delete_tracks(ids, delete_files=)` — the single funnel. + Removes from `library.tracks` and every non-smart playlist, prunes the + dirs it emptied, invalidates the exported artwork, and pushes one undo + Command covering the whole selection. A file that can't be trashed + **keeps its track** rather than leaving an orphaned entry. New + `tracks_removed` / `tracks_restored` signals. +- [x] Undo restores the file from the trash *and* the track's original position + in each playlist; redo re-trashes (re-recording paths, since a redo can + land on a different collision-suffixed name). If the trash was emptied + behind us, the library entry is restored anyway — a visible broken track + beats a silent second loss. +- [x] `Player.drop_tracks()` — stops if the deleted track is playing, otherwise + keeps the current track's place in the shortened queue. Previously a dead + id in the queue made `_load_current` stop everything with an error dialog + once the walk reached it. +- [x] Context menu gains "Remove from Library" and "Remove from Library and + Delete File", in **both** the library and playlist views. No keyboard + shortcut — `Del` still means "remove from playlist". Confirmation uses the + existing destructive-dialog idiom (DestructiveRole button, Cancel as + default) and names the playlists affected. +- [x] `tests/test_round33.py` (26 tests) covers the spec details (volume vs home + trash, percent-encoding, collision suffixes, no orphaned info file on + failure), the manager (playlist cleanup, pruning, partial failure, undo of + position, emptied-trash undo) and `drop_tracks`. Verified for real against + `/run/media/trav/tummult/.Trash-1000` with a scratch file. + ### Round 32 (2026-08-14) — Custom start times survive a track change (v0.5.2) trav reported "Pretty Girls is a Motherfucker" starting at 0:00 despite its diff --git a/tests/test_round33.py b/tests/test_round33.py new file mode 100644 index 0000000..ffbf7f3 --- /dev/null +++ b/tests/test_round33.py @@ -0,0 +1,386 @@ +"""Round 33: deleting songs from the library. + +Right-clicking a track (or a multi-selection) now offers "Remove from Library" +and "Remove from Library and Delete File". The second moves the file to the +desktop trash rather than unlinking it, so it stays recoverable both by Ctrl+Z +in-app and by "Restore" from the file manager afterwards. + +Three things this covers that are easy to get wrong: + +1. **The trash is per-filesystem.** Music usually sits on a mounted volume, not + the home filesystem, so the file belongs in ``/.Trash-``. Putting + it in ``~/.local/share/Trash`` would be a cross-device copy and would record + an original path the desktop can't restore to. +2. **Playlist cleanup has to be synchronous.** Playlist edits address tracks by + row index into ``track_ids`` while the view skips ids missing from the + library, so a dangling id would desync the two and make a later "Remove from + Playlist" hit the wrong track. +3. **A file that can't be trashed must keep its track.** Better a song you have + to delete again than a library entry orphaned from a file still on disk. +""" + +import os +from pathlib import Path +from unittest.mock import MagicMock + +import pytest + +from lintunes import trash +from lintunes.library_manager import LibraryManager +from lintunes.models import Library, Playlist, PlaylistType, Track + + +# -------------------------------------------------------------------------- +# 1. lintunes/trash.py — the freedesktop Trash spec +# -------------------------------------------------------------------------- + +@pytest.fixture +def home_trash(tmp_path, monkeypatch): + """Point XDG_DATA_HOME at tmp_path so the 'home' trash is under test.""" + monkeypatch.setenv("XDG_DATA_HOME", str(tmp_path / "xdg")) + return tmp_path / "xdg" / "Trash" + + +def _info_for(trashed: Path) -> dict: + text = (trashed.parent.parent / "info" / f"{trashed.name}.trashinfo").read_text() + assert text.splitlines()[0] == "[Trash Info]" + return dict(line.split("=", 1) for line in text.splitlines()[1:] if "=" in line) + + +class TestTrashModule: + def test_file_lands_in_trash_with_info(self, tmp_path, home_trash): + song = tmp_path / "song.mp3" + song.write_bytes(b"audio") + + dest = trash.send_to_trash(song) + + assert not song.exists() + assert dest == home_trash / "files" / "song.mp3" + assert dest.read_bytes() == b"audio" + info = _info_for(dest) + # Home trash records an absolute path. + assert info["Path"] == str(song.resolve()) + assert "T" in info["DeletionDate"] + + def test_path_is_percent_encoded(self, tmp_path, home_trash): + song = tmp_path / "a song & more.mp3" + song.write_bytes(b"x") + info = _info_for(trash.send_to_trash(song)) + assert "%20" in info["Path"] and " " not in info["Path"] + assert trash.original_path( + home_trash / "files" / "a song & more.mp3") == song.resolve() + + def test_volume_trash_used_for_other_filesystem(self, tmp_path, monkeypatch): + """A file on its own mount goes to /.Trash- with a + topdir-relative Path, not to the home trash.""" + volume = tmp_path / "volume" + (volume / "music").mkdir(parents=True) + song = volume / "music" / "song.mp3" + song.write_bytes(b"audio") + + monkeypatch.setattr(os.path, "ismount", lambda p: Path(p) == volume) + real_stat = os.stat + monkeypatch.setattr( + os, "stat", + lambda p, *a, **k: (MagicMock(st_dev=99) if Path(p) == song + else real_stat(p, *a, **k))) + + dest = trash.send_to_trash(song) + + assert dest == volume / f".Trash-{os.getuid()}" / "files" / "song.mp3" + assert _info_for(dest)["Path"] == "music/song.mp3" # relative to topdir + + def test_name_collision_gets_a_suffix(self, tmp_path, home_trash): + first = tmp_path / "song.mp3" + first.write_bytes(b"one") + trash.send_to_trash(first) + + (tmp_path / "other").mkdir() + second = tmp_path / "other" / "song.mp3" + second.write_bytes(b"two") + dest = trash.send_to_trash(second) + + assert dest.name == "song 1.mp3" + assert dest.read_bytes() == b"two" + assert (home_trash / "files" / "song.mp3").read_bytes() == b"one" + + def test_restore_round_trip(self, tmp_path, home_trash): + song = tmp_path / "song.mp3" + song.write_bytes(b"audio") + dest = trash.send_to_trash(song) + + trash.restore_from_trash(dest, song) + + assert song.read_bytes() == b"audio" + assert not dest.exists() + # The .trashinfo must go too, or the name stays claimed forever. + assert not (home_trash / "info" / "song.mp3.trashinfo").exists() + + def test_failed_move_leaves_no_orphan_info(self, tmp_path, home_trash, + monkeypatch): + song = tmp_path / "song.mp3" + song.write_bytes(b"audio") + + def boom(*a, **k): + raise OSError("disk on fire") + monkeypatch.setattr(os, "rename", boom) + + with pytest.raises(trash.TrashError): + trash.send_to_trash(song) + + assert song.exists() # caller can still treat it as un-deleted + assert list((home_trash / "info").iterdir()) == [] + + def test_missing_file_raises(self, tmp_path, home_trash): + with pytest.raises(trash.TrashError): + trash.send_to_trash(tmp_path / "nope.mp3") + + +# -------------------------------------------------------------------------- +# 2. LibraryManager.delete_tracks +# -------------------------------------------------------------------------- + +def _library(tmp_path, count=3): + """A library whose tracks are real files under an organized music tree.""" + music = tmp_path / "music" + library = Library(music_folder=str(music)) + for tid in range(1, count + 1): + path = music / "Music" / f"Artist{tid}" / "Album" / f"T{tid}.mp3" + path.parent.mkdir(parents=True, exist_ok=True) + path.write_bytes(b"audio" + str(tid).encode()) + library.tracks[tid] = Track(track_id=tid, name=f"T{tid}", + artist=f"Artist{tid}", location=str(path)) + return library + + +def _manager(tmp_path, library): + return LibraryManager(library, tmp_path / "data") + + +def _playlist(library, pid, track_ids, name="Mix"): + library.playlists[pid] = Playlist( + name=name, persistent_id=pid, playlist_type=PlaylistType.REGULAR, + track_ids=list(track_ids)) + return library.playlists[pid] + + +class TestDeleteTracks: + def test_removes_from_library_and_playlists(self, qapp, tmp_path, home_trash): + library = _library(tmp_path) + playlist = _playlist(library, "AAAA1111", [1, 2, 3]) + manager = _manager(tmp_path, library) + + removed, failures = manager.delete_tracks([2], delete_files=True) + + assert removed == [2] and failures == [] + assert 2 not in manager.library.tracks + # No dangling id: stored rows must match what the view will show. + assert playlist.track_ids == [1, 3] + + def test_file_goes_to_the_trash(self, qapp, tmp_path, home_trash): + library = _library(tmp_path) + path = Path(library.tracks[1].location) + manager = _manager(tmp_path, library) + + manager.delete_tracks([1], delete_files=True) + + assert not path.exists() + assert (home_trash / "files" / "T1.mp3").read_bytes() == b"audio1" + + def test_keep_file_variant_leaves_it_alone(self, qapp, tmp_path, home_trash): + library = _library(tmp_path) + path = Path(library.tracks[1].location) + manager = _manager(tmp_path, library) + + manager.delete_tracks([1], delete_files=False) + + assert 1 not in manager.library.tracks + assert path.exists() + assert not (home_trash / "files").exists() + + def test_emptied_dirs_pruned_but_not_the_root(self, qapp, tmp_path, home_trash): + library = _library(tmp_path) + album = Path(library.tracks[1].location).parent + manager = _manager(tmp_path, library) + + manager.delete_tracks([1], delete_files=True) + + assert not album.exists() and not album.parent.exists() + assert manager.organize_root().exists() + + def test_trash_failure_keeps_the_track(self, qapp, tmp_path, home_trash, + monkeypatch): + library = _library(tmp_path) + manager = _manager(tmp_path, library) + monkeypatch.setattr(trash, "send_to_trash", + MagicMock(side_effect=trash.TrashError("nope"))) + + removed, failures = manager.delete_tracks([1], delete_files=True) + + assert removed == [] + assert 1 in manager.library.tracks # never orphan the entry + assert failures and failures[0][0] == "T1" + + def test_partial_failure_still_deletes_the_rest(self, qapp, tmp_path, + home_trash, monkeypatch): + library = _library(tmp_path) + manager = _manager(tmp_path, library) + real = trash.send_to_trash + + def flaky(path): + if Path(path).name == "T1.mp3": + raise trash.TrashError("nope") + return real(path) + monkeypatch.setattr(trash, "send_to_trash", flaky) + + removed, failures = manager.delete_tracks([1, 2], delete_files=True) + + assert removed == [2] and len(failures) == 1 + assert 1 in manager.library.tracks and 2 not in manager.library.tracks + + def test_unknown_ids_are_ignored(self, qapp, tmp_path, home_trash): + manager = _manager(tmp_path, _library(tmp_path)) + assert manager.delete_tracks([999], delete_files=True) == ([], []) + + def test_emits_tracks_removed(self, qapp, tmp_path, home_trash): + manager = _manager(tmp_path, _library(tmp_path)) + seen = [] + manager.tracks_removed.connect(seen.append) + + manager.delete_tracks([1, 3], delete_files=False) + + assert seen == [[1, 3]] + + +class TestDeleteUndo: + def test_undo_restores_track_file_and_playlist_position( + self, qapp, tmp_path, home_trash): + library = _library(tmp_path) + playlist = _playlist(library, "AAAA1111", [1, 2, 3]) + path = Path(library.tracks[2].location) + manager = _manager(tmp_path, library) + + manager.delete_tracks([2], delete_files=True) + manager.undo_stack.undo() + + assert 2 in manager.library.tracks + assert path.read_bytes() == b"audio2" + # Back in the middle, not appended to the end. + assert playlist.track_ids == [1, 2, 3] + + def test_redo_deletes_again(self, qapp, tmp_path, home_trash): + library = _library(tmp_path) + playlist = _playlist(library, "AAAA1111", [1, 2, 3]) + path = Path(library.tracks[2].location) + manager = _manager(tmp_path, library) + + manager.delete_tracks([2], delete_files=True) + manager.undo_stack.undo() + manager.undo_stack.redo() + + assert 2 not in manager.library.tracks + assert not path.exists() + assert playlist.track_ids == [1, 3] + + def test_undo_label_reflects_the_variant(self, qapp, tmp_path, home_trash): + manager = _manager(tmp_path, _library(tmp_path)) + manager.delete_tracks([1], delete_files=True) + assert manager.undo_stack.undo_label() == "Delete from Library" + + manager.delete_tracks([2], delete_files=False) + assert manager.undo_stack.undo_label() == "Remove from Library" + + def test_undo_restores_entry_even_if_trash_was_emptied( + self, qapp, tmp_path, home_trash): + library = _library(tmp_path) + manager = _manager(tmp_path, library) + manager.delete_tracks([1], delete_files=True) + (home_trash / "files" / "T1.mp3").unlink() # user emptied the trash + + manager.undo_stack.undo() + + # The file is unrecoverable, but losing the library entry too would be + # a second, silent loss. + assert 1 in manager.library.tracks + + def test_deleted_track_is_gone_from_saved_library(self, qapp, tmp_path, + home_trash): + library = _library(tmp_path) + manager = _manager(tmp_path, library) + manager.delete_tracks([1], delete_files=True) + manager.flush() + + import json + saved = json.loads((tmp_path / "data" / "library.json").read_text()) + assert "1" not in saved and "2" in saved + + +# -------------------------------------------------------------------------- +# 3. Player.drop_tracks +# -------------------------------------------------------------------------- + +def _player(qapp, track_ids): + from lintunes.player import Player + library = Library() + for tid in track_ids: + library.tracks[tid] = Track(track_id=tid, name=f"T{tid}") + manager = MagicMock() + manager.library = library + player = Player(manager) + player._queue = list(track_ids) + return player + + +class TestPlayerDropTracks: + def test_stops_when_the_playing_track_is_deleted(self, qapp): + player = _player(qapp, [1, 2, 3]) + player._index = 1 + player._current_track = player._manager.library.tracks[2] + player._sink = MagicMock() + + player.drop_tracks({2}) + + assert player.current_track is None + assert player._queue == [1, 3] + + def test_current_track_keeps_its_place(self, qapp): + player = _player(qapp, [1, 2, 3]) + player._index = 2 + player._current_track = player._manager.library.tracks[3] + + player.drop_tracks({1}) + + # Track 3 moved from index 2 to index 1; a stale index would make + # "next" replay the wrong song. + assert player._queue == [2, 3] + assert player._index == 1 + assert player.current_track.track_id == 3 + + def test_empty_set_is_a_no_op(self, qapp): + player = _player(qapp, [1, 2, 3]) + player._index = 1 + player.drop_tracks(set()) + assert player._queue == [1, 2, 3] and player._index == 1 + + +# -------------------------------------------------------------------------- +# 4. The context-menu signals +# -------------------------------------------------------------------------- + +class TestContextMenuSignals: + @pytest.mark.parametrize("playlist_mode", [False, True]) + def test_both_actions_exist_in_both_modes(self, qapp, playlist_mode): + """Unlike "Remove from Playlist", library removal is offered in the + library view too.""" + from lintunes.gui.track_table import TrackTableView + table = TrackTableView(playlist_mode=playlist_mode) + assert hasattr(table, "remove_from_library_requested") + assert hasattr(table, "delete_from_library_requested") + + def test_signals_carry_track_ids(self, qapp): + from lintunes.gui.track_table import TrackTableView + table = TrackTableView() + seen = [] + table.delete_from_library_requested.connect(seen.append) + table.delete_from_library_requested.emit([7, 9]) + assert seen == [[7, 9]]