From 956b30a4f2713e4ed6984f47a9d8fd9d69f04e28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Tue, 18 Aug 2026 23:08:04 +0100 Subject: [PATCH 1/3] Replace do_query with model-specific logic This allows to define a proper return type for input_select_objects. --- beets/ui/__init__.py | 2 +- beets/ui/commands/modify.py | 46 ++++++------ beets/ui/commands/move.py | 122 ++++++++++++++++++-------------- beets/ui/commands/remove.py | 102 +++++++++++++------------- beets/ui/commands/update.py | 17 +++-- beets/ui/commands/write.py | 4 +- beetsplug/edit.py | 4 +- test/ui/commands/test_move.py | 55 +++++++------- test/ui/commands/test_remove.py | 14 ++-- test/ui/commands/test_update.py | 11 +-- test/ui/commands/test_utils.py | 54 -------------- 11 files changed, 190 insertions(+), 241 deletions(-) delete mode 100644 test/ui/commands/test_utils.py diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index 3c6792f780..9951838491 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -399,7 +399,7 @@ def input_select_objects( objs: Sequence[T], rep: Callable[[T], Any], prompt_all: str | None = None, -) -> Any: +) -> Sequence[T]: """Prompt to user to choose all, none, or some of the given objects. Return the list of selected objects. diff --git a/beets/ui/commands/modify.py b/beets/ui/commands/modify.py index 03ec5e12af..60d9f5880d 100644 --- a/beets/ui/commands/modify.py +++ b/beets/ui/commands/modify.py @@ -4,17 +4,16 @@ from typing import TYPE_CHECKING, NamedTuple, Protocol -from beets import library, ui +from beets import ui from beets.dbcore import types from beets.exceptions import UserError +from beets.library import Album, Item from beets.util.deprecation import maybe_replace_legacy_field -from .utils import do_query - if TYPE_CHECKING: from collections.abc import Sequence - from beets.library import LibModel, Library + from beets.library import AlbumOrItem, LibModel, Library class ModifyCLIOpts(Protocol): @@ -59,34 +58,28 @@ def _check_modify_operations( ) -def modify_items( +def modify_objects( + model_cls: type[AlbumOrItem], + objs: Sequence[AlbumOrItem], lib: Library, mods: dict[str, ModifyOperation], dels: Sequence[str], - query: Sequence[str], write: bool, move: bool, - album: bool, confirm: bool, inherit: bool, ) -> None: - """Modifies matching items according to user-specified assignments and + """Modifies albums or items according to user-specified assignments and deletions. - `mods` is a dictionary of field and value pairse indicating + `mods` is a dictionary of field and value pairs indicating assignments. `dels` is a list of fields to be deleted. """ # Parse key=value specifications into a dictionary. - model_cls = library.Album if album else library.Item _check_modify_operations(model_cls, mods) - - # Get the items to modify. - items, albums = do_query(lib, query, album, False) - objs = albums if album else items - # Apply changes *temporarily*, preview them, and collect modified # objects. - ui.print_(f"Modifying {len(objs)} {'album' if album else 'item'}s.") + ui.print_(f"Modifying {len(objs)} {model_cls.__name__.lower()}s.") changed = [] for obj in objs: obj_mods = { @@ -106,6 +99,7 @@ def modify_items( return # Confirm action. + selected_changes: Sequence[AlbumOrItem] if confirm: if write and move: extra = ", move and write tags" @@ -116,18 +110,28 @@ def modify_items( else: extra = "" - changed = ui.input_select_objects( + selected_changes = ui.input_select_objects( f"Really modify{extra}", changed, lambda o: print_and_modify(o, mods, dels), ) + else: + selected_changes = changed # Apply changes to database and files with lib.transaction(): - for obj in changed: + for obj in selected_changes: obj.try_sync(write, move, inherit) +def modify_items(lib: Library, query: Sequence[str], *args, **kwargs) -> None: + modify_objects(Item, list(lib.items(query)), lib, *args, **kwargs) + + +def modify_albums(lib: Library, query: Sequence[str], *args, **kwargs) -> None: + modify_objects(Album, list(lib.albums(query)), lib, *args, **kwargs) + + def print_and_modify( obj: LibModel, mods: dict[str, list[str]] | dict[str, ModifyOperation], @@ -180,14 +184,14 @@ def modify_func(lib: Library, opts: ModifyCLIOpts, args: list[str]) -> None: query, mods, dels = modify_parse_args(args, is_album=opts.album) if not mods and not dels: raise UserError("no modifications specified") - modify_items( + method = modify_albums if opts.album else modify_items + method( lib, + query, mods, dels, - query, ui.should_write(opts.write), ui.should_move(opts.move), - opts.album, not opts.yes, opts.inherit, ) diff --git a/beets/ui/commands/move.py b/beets/ui/commands/move.py index 02d8662179..aea4a2656c 100644 --- a/beets/ui/commands/move.py +++ b/beets/ui/commands/move.py @@ -3,21 +3,17 @@ from __future__ import annotations import os -from typing import TYPE_CHECKING, Protocol - -from typing_extensions import TypeIs +from typing import TYPE_CHECKING, Literal, Protocol from beets import logging, ui from beets.exceptions import UserError from beets.util import MoveOperation, displayable_path, normpath, syspath from beets.util.diff import colordiff -from .utils import do_query - if TYPE_CHECKING: - from collections.abc import Iterable, Sequence + from collections.abc import Callable, Iterable, Sequence - from beets.library import Album, Item, Library + from beets.library import Album, AlbumOrItem, Item, Library # Global logger. log = logging.getLogger("beets") @@ -73,42 +69,22 @@ def show_path_changes(path_changes: Iterable[tuple[bytes, bytes]]) -> None: ui.print_(f"{color_source} {' ' * pad} -> {color_dest}") -def is_album_selection( - objects: list[Item] | list[Album], album: bool -) -> TypeIs[list[Album]]: - return album - - -def move_items( - lib: Library, +def move_objects( + objs: Sequence[AlbumOrItem], + entity: Literal["album", "item"], dest: bytes | None, - query: Sequence[str], + get_paths: Callable[[Iterable[AlbumOrItem]], list[tuple[bytes, bytes]]], + *, copy: bool, - album: bool, pretend: bool, confirm: bool = False, export: bool = False, ) -> None: - """Moves or copies items to a new base directory, given by dest. If + """Move or copy albums or items to a new base directory, given by dest. If dest is None, then the library's base directory is used, making the command "consolidate" files. """ - items, albums = do_query(lib, query, album, False) - objs = albums if album else items num_objs = len(objs) - - # Filter out files that don't need to be moved. - def isitemmoved(item: Item) -> bool: - return item.path != item.destination(basedir=dest) - - def isalbummoved(album: Album) -> bool: - return any(isitemmoved(i) for i in album.items()) - - if is_album_selection(objs, album): - objs = list(filter(isalbummoved, objs)) - else: - objs = list(filter(isitemmoved, objs)) - num_unmoved = num_objs - len(objs) # Report unmoved files that match the query. unmoved_msg = "" @@ -118,7 +94,6 @@ def isalbummoved(album: Album) -> bool: copy = copy or export # Exporting always copies. action = "Copying" if copy else "Moving" act = "copy" if copy else "move" - entity = "album" if album else "item" log.info( "{} {} {}{}{}.", action, @@ -131,29 +106,21 @@ def isalbummoved(album: Album) -> bool: return if pretend: - if is_album_selection(objs, album): - show_path_changes( - [ - (item.path, item.destination(basedir=dest)) - for obj in objs - for item in obj.items() - ] - ) - else: - show_path_changes( - [(obj.path, obj.destination(basedir=dest)) for obj in objs] - ) + show_path_changes(get_paths(objs)) else: + selected_objs: Sequence[AlbumOrItem] if confirm: - objs = ui.input_select_objects( + selected_objs = ui.input_select_objects( f"Really {act}", objs, lambda o: show_path_changes( [(o.path, o.destination(basedir=dest))] ), ) + else: + selected_objs = objs - for obj in objs: + for obj in selected_objs: log.debug("moving: {.filepath}", obj) if export: @@ -169,21 +136,66 @@ def isalbummoved(album: Album) -> bool: obj.move(operation=MoveOperation.MOVE, basedir=dest) +def isitemmoved(dest: bytes | None, item: Item) -> bool: + """Filter out files that don't need to be moved.""" + return item.path != item.destination(basedir=dest) + + +def isalbummoved(dest: bytes | None, album: Album) -> bool: + return any(isitemmoved(dest, i) for i in album.items()) + + +def move_items( + lib: Library, query: Sequence[str], dest: bytes | None, *args, **kwargs +) -> None: + def get_paths(objs: Iterable[Item]) -> list[tuple[bytes, bytes]]: + return [(obj.path, obj.destination(basedir=dest)) for obj in objs] + + move_objects( + [i for i in lib.items(query) if isitemmoved(dest, i)], + "item", + dest, + get_paths, + *args, + **kwargs, + ) + + +def move_albums( + lib: Library, query: Sequence[str], dest: bytes | None, *args, **kwargs +) -> None: + def get_paths(objs: Iterable[Album]) -> list[tuple[bytes, bytes]]: + return [ + (item.path, item.destination(basedir=dest)) + for obj in objs + for item in obj.items() + ] + + move_objects( + [i for i in lib.albums(query) if isalbummoved(dest, i)], + "album", + dest, + get_paths, + *args, + **kwargs, + ) + + def move_func(lib: Library, opts: MoveCLIOpts, args: list[str]) -> None: dest = normpath(opts.dest) if opts.dest else None if dest is not None: if not os.path.isdir(syspath(dest)): raise UserError(f"no such directory: {displayable_path(dest)}") - move_items( + method = move_albums if opts.album else move_items + method( lib, - dest, args, - opts.copy, - opts.album, - opts.pretend, - opts.timid, - opts.export, + dest, + copy=opts.copy, + pretend=opts.pretend, + confirm=opts.timid, + export=opts.export, ) diff --git a/beets/ui/commands/remove.py b/beets/ui/commands/remove.py index ef0053ffe4..f4d99a53da 100644 --- a/beets/ui/commands/remove.py +++ b/beets/ui/commands/remove.py @@ -2,18 +2,15 @@ from __future__ import annotations -from functools import singledispatch +from functools import partial from typing import TYPE_CHECKING, Protocol from beets import ui -from beets.library import Album, Item - -from .utils import do_query if TYPE_CHECKING: - from collections.abc import Sequence + from collections.abc import Callable, Sequence - from beets.library import LibModel, Library + from beets.library import Album, AlbumOrItem, Item, Library class RemoveCLIOpts(Protocol): @@ -22,75 +19,82 @@ class RemoveCLIOpts(Protocol): force: bool -def remove_items( - lib: Library, query: Sequence[str], album: bool, delete: bool, force: bool +def remove_objects( + objs: Sequence[AlbumOrItem], + fmt_obj: Callable[[str, AlbumOrItem], None], + suffix: str, + lib: Library, + delete: bool, + force: bool, ) -> None: - """Remove items matching query from lib. If album, then match and - remove whole albums. If delete, also remove files from disk. - """ - # Get the matching items. - items, albums = do_query(lib, query, album) - objs = albums if album else items - # Confirm file removal if not forcing removal. - if not force: - # Prepare confirmation with user. - album_str = ( - f" in {len(albums)} album{'s' if len(albums) > 1 else ''}" - if album - else "" - ) - + if force: + selected_objs = objs + else: if delete: fmt = "$path - $title" prompt = "Really DELETE" - prompt_all = ( - "Really DELETE" - f" {len(items)} file{'s' if len(items) > 1 else ''}{album_str}" - ) + prompt_all = f"Really DELETE {len(objs)} file{suffix}" else: fmt = "" prompt = "Really remove from the library?" prompt_all = ( - "Really remove" - f" {len(items)} item{'s' if len(items) > 1 else ''}{album_str}" - " from the library?" + f"Really remove {len(objs)} item{suffix} from the library?" ) - @singledispatch - def fmt_obj(obj: LibModel) -> None: - raise NotImplementedError - - @fmt_obj.register - def _item(t: Item) -> None: - ui.print_(format(t, fmt)) - - @fmt_obj.register - def _album(a: Album) -> None: - ui.print_() - for i in a.items(): - fmt_obj(i) + _fmt = partial(fmt_obj, fmt) # Show all the items. for o in objs: - fmt_obj(o) + _fmt(o) # Confirm with user. - objs = ui.input_select_objects( - prompt, objs, fmt_obj, prompt_all=prompt_all + selected_objs = ui.input_select_objects( + prompt, objs, _fmt, prompt_all=prompt_all ) - if not objs: + if not selected_objs: return # Remove (and possibly delete) items. with lib.transaction(): - for obj in objs: + for obj in selected_objs: obj.remove(delete) +def fmt_item(fmt: str, t: Item) -> None: + ui.print_(format(t, fmt)) + + +def fmt_album(fmt: str, a: Album) -> None: + ui.print_() + for i in a.items(): + fmt_item(fmt, i) + + +def remove_items(lib: Library, query: Sequence[str], *args, **kwargs) -> None: + """Remove items matching query from lib.""" + items: Sequence[Item] = list(lib.items(query)) + suffix = "s" if len(items) > 1 else "" + + remove_objects(items, fmt_item, suffix, lib, *args, **kwargs) + + +def remove_albums(lib: Library, query: Sequence[str], *args, **kwargs) -> None: + """Remove albums matching query from lib.""" + albums = list(lib.albums(query)) + items = [i for a in albums for i in a.items()] + suffix = "s" if len(items) > 1 else "" + album_str = f" in {len(albums)} album{'s' if len(albums) > 1 else ''}" + + remove_objects( + albums, fmt_album, f"{suffix}{album_str}", lib, *args, **kwargs + ) + + def remove_func(lib: Library, opts: RemoveCLIOpts, args: list[str]) -> None: - remove_items(lib, args, opts.album, opts.delete, opts.force) + method = remove_albums if opts.album else remove_items + method(lib, args, opts.delete, opts.force) remove_cmd = ui.Subcommand( diff --git a/beets/ui/commands/update.py b/beets/ui/commands/update.py index f2d50ed5af..40cecbd695 100644 --- a/beets/ui/commands/update.py +++ b/beets/ui/commands/update.py @@ -9,12 +9,10 @@ from beets.util import ancestry, syspath from beets.util.color import colorize -from .utils import do_query - if TYPE_CHECKING: - from collections.abc import Collection, Sequence + from collections.abc import Collection, Iterable, Sequence - from beets.library import Library + from beets.library import Item, Library # Global logger. @@ -31,8 +29,7 @@ class UpdateCLIOpts(Protocol): def update_items( lib: Library, - query: Sequence[str], - is_album: bool, + items: Iterable[Item], move: bool, pretend: bool, fields: list[str] | None, @@ -47,7 +44,6 @@ def update_items( """ item_fields: Collection[str] with lib.transaction(): - items, _ = do_query(lib, query, is_album) if move and fields is not None and "path" not in fields: # Special case: if an item needs to be moved, the path field has to # updated; otherwise the new path will not be reflected in the @@ -172,10 +168,13 @@ def update_func(lib: Library, opts: UpdateCLIOpts, args: list[str]) -> None: ui.print_(os.fsdecode(lib.directory)) if not ui.input_yn("Are you sure you want to continue (y/n)?", True): return + if opts.album: + items = [i for a in lib.albums(args) for i in a.items()] + else: + items = list(lib.items(args)) update_items( lib, - args, - opts.album, + items, ui.should_move(opts.move), opts.pretend, opts.fields, diff --git a/beets/ui/commands/write.py b/beets/ui/commands/write.py index 78e8dbaf97..63733b6461 100644 --- a/beets/ui/commands/write.py +++ b/beets/ui/commands/write.py @@ -8,8 +8,6 @@ from beets import library, logging, ui from beets.util import syspath -from .utils import do_query - if TYPE_CHECKING: from collections.abc import Sequence @@ -31,7 +29,7 @@ def write_items( """Write tag information from the database to the respective files in the filesystem. """ - items, _ = do_query(lib, query, False, False) + items = lib.items(query) for item in items: # Item deleted? diff --git a/beetsplug/edit.py b/beetsplug/edit.py index e9dcf2b768..b1af19eac8 100644 --- a/beetsplug/edit.py +++ b/beetsplug/edit.py @@ -17,7 +17,6 @@ from beets.exceptions import UserError from beets.importer import Action from beets.library import Album, Item -from beets.ui.commands.utils import do_query from beets.util import PromptChoice if TYPE_CHECKING: @@ -186,8 +185,7 @@ def _edit_command( ) -> None: """The CLI command function for the `beet edit` command.""" # Get the objects to edit. - items, albums = do_query(lib, args, opts.album, False) - objs = albums if opts.album else items + objs = (lib.albums if opts.album else lib.items)(args) if not objs: ui.print_("Nothing to edit.") return diff --git a/test/ui/commands/test_move.py b/test/ui/commands/test_move.py index b9ec6ef792..df397296c1 100644 --- a/test/ui/commands/test_move.py +++ b/test/ui/commands/test_move.py @@ -3,7 +3,7 @@ from beets import library from beets.test.helper import BeetsTestCase -from beets.ui.commands.move import move_items +from beets.ui.commands.move import move_albums, move_items class MoveTest(BeetsTestCase): @@ -21,91 +21,86 @@ def setUp(self): # Alternate destination directory. self.otherdir = self.temp_path / "testotherdir" - def _move( - self, - query=(), - dest=None, - copy=False, - album=False, - pretend=False, - export=False, - ): + def _move_items(self, dest=None, query=(), **kwargs): + kwargs.setdefault("pretend", False) + kwargs.setdefault("copy", False) move_items( - self.lib, - os.fsencode(dest) if dest else None, - query, - copy, - album, - pretend, - export=export, + self.lib, query, os.fsencode(dest) if dest else None, **kwargs + ) + + def _move_albums(self, dest=None, query=(), **kwargs): + kwargs.setdefault("pretend", False) + kwargs.setdefault("copy", False) + move_albums( + self.lib, query, os.fsencode(dest) if dest else None, **kwargs ) def test_move_item(self): - self._move() + self._move_items() self.i.load() assert b"libdir" in self.i.path assert self.i.filepath.exists() assert not self.initial_item_path.exists() def test_copy_item(self): - self._move(copy=True) + self._move_items(copy=True) self.i.load() assert b"libdir" in self.i.path assert self.i.filepath.exists() assert self.initial_item_path.exists() def test_move_album(self): - self._move(album=True) + self._move_albums() self.i.load() assert b"libdir" in self.i.path assert self.i.filepath.exists() assert not self.initial_item_path.exists() def test_copy_album(self): - self._move(copy=True, album=True) + self._move_albums(copy=True) self.i.load() assert b"libdir" in self.i.path assert self.i.filepath.exists() assert self.initial_item_path.exists() def test_move_item_custom_dir(self): - self._move(dest=self.otherdir) + self._move_items(dest=self.otherdir) self.i.load() assert b"testotherdir" in self.i.path assert self.i.filepath.exists() assert not self.initial_item_path.exists() def test_move_album_custom_dir(self): - self._move(dest=self.otherdir, album=True) + self._move_albums(dest=self.otherdir) self.i.load() assert b"testotherdir" in self.i.path assert self.i.filepath.exists() assert not self.initial_item_path.exists() def test_pretend_move_item(self): - self._move(dest=self.otherdir, pretend=True) + self._move_items(dest=self.otherdir, pretend=True) self.i.load() assert self.i.filepath == self.initial_item_path def test_pretend_move_album(self): - self._move(album=True, pretend=True) + self._move_albums(pretend=True) self.i.load() assert self.i.filepath == self.initial_item_path def test_export_item_custom_dir(self): - self._move(dest=self.otherdir, export=True) + self._move_items(dest=self.otherdir, export=True) self.i.load() assert self.i.filepath == self.initial_item_path assert self.otherdir.exists() def test_export_album_custom_dir(self): - self._move(dest=self.otherdir, album=True, export=True) + self._move_albums(dest=self.otherdir, export=True) self.i.load() assert self.i.filepath == self.initial_item_path assert self.otherdir.exists() def test_pretend_export_item(self): - self._move(dest=self.otherdir, pretend=True, export=True) + self._move_items(dest=self.otherdir, pretend=True, export=True) self.i.load() assert self.i.filepath == self.initial_item_path assert not self.otherdir.exists() @@ -114,7 +109,7 @@ def test_move_missing_singleton_continues(self): self.i.load() old_path = self.i.filepath old_path.unlink() - self._move() + self._move_items() self.i.load() assert self.i.filepath == old_path @@ -130,7 +125,7 @@ def test_move_album_with_missing_track(self): i2.album_id = self.album.id i2.store() - self._move(album=True) + self._move_albums() self.i.load() i2.load() assert self.i.filepath == old_i_path diff --git a/test/ui/commands/test_remove.py b/test/ui/commands/test_remove.py index 33c52ac242..1c7551fd0e 100644 --- a/test/ui/commands/test_remove.py +++ b/test/ui/commands/test_remove.py @@ -1,6 +1,6 @@ from beets import library from beets.test.helper import BeetsTestCase, IOMixin -from beets.ui.commands.remove import remove_items +from beets.ui.commands.remove import remove_albums, remove_items from beets.util import MoveOperation @@ -15,26 +15,26 @@ def setUp(self): def test_remove_items_no_delete(self): self.io.addinput("y") - remove_items(self.lib, "", False, False, False) + remove_items(self.lib, "", False, False) items = self.lib.items() assert len(list(items)) == 0 assert self.i.filepath.exists() def test_remove_items_with_delete(self): self.io.addinput("y") - remove_items(self.lib, "", False, True, False) + remove_items(self.lib, "", True, False) items = self.lib.items() assert len(list(items)) == 0 assert not self.i.filepath.exists() def test_remove_items_with_force_no_delete(self): - remove_items(self.lib, "", False, False, True) + remove_items(self.lib, "", False, True) items = self.lib.items() assert len(list(items)) == 0 assert self.i.filepath.exists() def test_remove_items_with_force_delete(self): - remove_items(self.lib, "", False, True, True) + remove_items(self.lib, "", True, True) items = self.lib.items() assert len(list(items)) == 0 assert not self.i.filepath.exists() @@ -46,7 +46,7 @@ def test_remove_items_select_with_delete(self): for s in ("s", "y", "n"): self.io.addinput(s) - remove_items(self.lib, "", False, True, False) + remove_items(self.lib, "", True, False) items = self.lib.items() assert len(list(items)) == 1 # There is probably no guarantee that the items are queried in any @@ -68,7 +68,7 @@ def test_remove_albums_select_with_delete(self): for s in ("s", "y", "n"): self.io.addinput(s) - remove_items(self.lib, "", True, True, False) + remove_albums(self.lib, "", True, False) items = self.lib.items() assert len(list(items)) == 2 # incl. the item from setUp() # See test_remove_items_select_with_delete() diff --git a/test/ui/commands/test_update.py b/test/ui/commands/test_update.py index 6a6681560e..43541e57db 100644 --- a/test/ui/commands/test_update.py +++ b/test/ui/commands/test_update.py @@ -32,13 +32,7 @@ def setUp(self): artfile.unlink() def _update( - self, - query=(), - album=False, - move=False, - reset_mtime=True, - fields=None, - exclude_fields=None, + self, move=False, reset_mtime=True, fields=None, exclude_fields=None ): self.io.addinput("y") if reset_mtime: @@ -46,8 +40,7 @@ def _update( self.i.store() update_items( self.lib, - query, - album, + list(self.lib.items()), move, False, fields=fields, diff --git a/test/ui/commands/test_utils.py b/test/ui/commands/test_utils.py deleted file mode 100644 index a6ef11f461..0000000000 --- a/test/ui/commands/test_utils.py +++ /dev/null @@ -1,54 +0,0 @@ -import shutil - -import pytest - -from beets import library -from beets.exceptions import UserError -from beets.test import _common -from beets.test.helper import BeetsTestCase -from beets.ui.commands.utils import do_query - - -class QueryTest(BeetsTestCase): - def add_item(self): - itempath = self.lib_path / "srcfile" - shutil.copy(_common.RSRC / "full.mp3", itempath) - item = library.Item.from_path(itempath) - self.lib.add(item) - return item - - def add_album(self, items): - return self.lib.add_album(items) - - def check_do_query( - self, num_items, num_albums, q=(), album=False, also_items=True - ): - items, albums = do_query(self.lib, q, album, also_items) - assert len(items) == num_items - assert len(albums) == num_albums - - def test_query_empty(self): - with pytest.raises(UserError): - do_query(self.lib, (), False) - - def test_query_empty_album(self): - with pytest.raises(UserError): - do_query(self.lib, (), True) - - def test_query_item(self): - self.add_item() - self.check_do_query(1, 0, album=False) - self.add_item() - self.check_do_query(2, 0, album=False) - - def test_query_album(self): - item = self.add_item() - self.add_album([item]) - self.check_do_query(1, 1, album=True) - self.check_do_query(0, 1, album=True, also_items=False) - - item = self.add_item() - item2 = self.add_item() - self.add_album([item, item2]) - self.check_do_query(3, 2, album=True) - self.check_do_query(0, 2, album=True, also_items=False) From 8193b1d9b91fa705d80daff48d0c03c9cc662a69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Tue, 18 Aug 2026 22:20:07 +0100 Subject: [PATCH 2/3] remove: fix ui formatting and test it --- beets/ui/commands/remove.py | 19 +++++++++++-------- test/ui/commands/test_remove.py | 9 ++++++--- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/beets/ui/commands/remove.py b/beets/ui/commands/remove.py index f4d99a53da..47d7267e88 100644 --- a/beets/ui/commands/remove.py +++ b/beets/ui/commands/remove.py @@ -22,7 +22,8 @@ class RemoveCLIOpts(Protocol): def remove_objects( objs: Sequence[AlbumOrItem], fmt_obj: Callable[[str, AlbumOrItem], None], - suffix: str, + file_count: int, + album_str: str, lib: Library, delete: bool, force: bool, @@ -31,15 +32,18 @@ def remove_objects( if force: selected_objs = objs else: + file_suffix = "s" if file_count > 1 else "" if delete: fmt = "$path - $title" prompt = "Really DELETE" - prompt_all = f"Really DELETE {len(objs)} file{suffix}" + prompt_all = ( + f"Really DELETE {file_count} file{file_suffix}{album_str}" + ) else: fmt = "" prompt = "Really remove from the library?" prompt_all = ( - f"Really remove {len(objs)} item{suffix} from the library?" + f"Really remove {len(objs)} item{album_str} from the library?" ) _fmt = partial(fmt_obj, fmt) @@ -75,20 +79,19 @@ def fmt_album(fmt: str, a: Album) -> None: def remove_items(lib: Library, query: Sequence[str], *args, **kwargs) -> None: """Remove items matching query from lib.""" items: Sequence[Item] = list(lib.items(query)) - suffix = "s" if len(items) > 1 else "" + album_str = "" - remove_objects(items, fmt_item, suffix, lib, *args, **kwargs) + remove_objects(items, fmt_item, len(items), album_str, lib, *args, **kwargs) def remove_albums(lib: Library, query: Sequence[str], *args, **kwargs) -> None: """Remove albums matching query from lib.""" albums = list(lib.albums(query)) items = [i for a in albums for i in a.items()] - suffix = "s" if len(items) > 1 else "" - album_str = f" in {len(albums)} album{'s' if len(albums) > 1 else ''}" + album_str = f" and {len(albums)} album{'s' if len(albums) > 1 else ''}" remove_objects( - albums, fmt_album, f"{suffix}{album_str}", lib, *args, **kwargs + albums, fmt_album, len(items), album_str, lib, *args, **kwargs ) diff --git a/test/ui/commands/test_remove.py b/test/ui/commands/test_remove.py index 1c7551fd0e..971e1c7430 100644 --- a/test/ui/commands/test_remove.py +++ b/test/ui/commands/test_remove.py @@ -1,6 +1,6 @@ from beets import library from beets.test.helper import BeetsTestCase, IOMixin -from beets.ui.commands.remove import remove_albums, remove_items +from beets.ui.commands.remove import remove_items from beets.util import MoveOperation @@ -46,7 +46,8 @@ def test_remove_items_select_with_delete(self): for s in ("s", "y", "n"): self.io.addinput(s) - remove_items(self.lib, "", True, False) + output = self.run_with_output("remove", "-d") + assert "Really DELETE 2 files?" in output items = self.lib.items() assert len(list(items)) == 1 # There is probably no guarantee that the items are queried in any @@ -68,7 +69,9 @@ def test_remove_albums_select_with_delete(self): for s in ("s", "y", "n"): self.io.addinput(s) - remove_albums(self.lib, "", True, False) + output = self.run_with_output("remove", "-a", "-d") + assert "Really DELETE 2 files and 2 albums?" in output + items = self.lib.items() assert len(list(items)) == 2 # incl. the item from setUp() # See test_remove_items_select_with_delete() From 725ef3e5839d6ca2f57c723345c1d634684721ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Tue, 18 Aug 2026 19:18:07 +0100 Subject: [PATCH 3/3] modify: fix selecting objects --- beets/ui/commands/modify.py | 8 ++------ docs/changelog.rst | 2 ++ 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/beets/ui/commands/modify.py b/beets/ui/commands/modify.py index 60d9f5880d..90929bed54 100644 --- a/beets/ui/commands/modify.py +++ b/beets/ui/commands/modify.py @@ -111,9 +111,7 @@ def modify_objects( extra = "" selected_changes = ui.input_select_objects( - f"Really modify{extra}", - changed, - lambda o: print_and_modify(o, mods, dels), + f"Really modify{extra}", changed, ui.show_model_changes ) else: selected_changes = changed @@ -133,9 +131,7 @@ def modify_albums(lib: Library, query: Sequence[str], *args, **kwargs) -> None: def print_and_modify( - obj: LibModel, - mods: dict[str, list[str]] | dict[str, ModifyOperation], - dels: Sequence[str], + obj: LibModel, mods: dict[str, list[str]], dels: Sequence[str] ) -> bool: """Print the modifications to an item and return a bool indicating whether any changes were made. diff --git a/docs/changelog.rst b/docs/changelog.rst index 5fa420a1d8..86c7684236 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -69,6 +69,8 @@ Bug fixes text and the filters are empty. :bug:`6862` - :doc:`plugins/ipfs`: Fix ``beet ipfs --play`` option to invoke the Play plugin through its command interface. +- :ref:`modify-cmd`: Fix applying changes when choosing objects in interactive + select mode. :bug:`4880` .. For plugin developers