Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions beets/ui/commands/modify.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,18 @@ def modify_items(lib, mods, dels, query, write, move, album, confirm, inherit):
# objects.
ui.print_(f"Modifying {len(objs)} {'album' if album else 'item'}s.")
changed = []
templates = {
key: functemplate.template(value) for key, value in mods.items()
}
for obj in objs:
obj_mods = {
templates = {key: functemplate.template(value) for key, value in mods.items()}

def parse_mods(obj):
# Parse the raw assignment strings into properly typed values for the
# given object (e.g. dates into timestamps).
return {
key: model_cls._parse(key, obj.evaluate_template(templates[key]))
for key in mods.keys()
}
if print_and_modify(obj, obj_mods, dels) and obj not in changed:

for obj in objs:
if print_and_modify(obj, parse_mods(obj), dels) and obj not in changed:
changed.append(obj)

# Still something to do?
Expand All @@ -56,7 +59,7 @@ def modify_items(lib, mods, dels, query, write, move, album, confirm, inherit):
changed = ui.input_select_objects(
f"Really modify{extra}",
changed,
lambda o: print_and_modify(o, mods, dels),
lambda o: print_and_modify(o, parse_mods(o), dels),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reuse parsed modifications for select prompts

When the user chooses select after a templated modification such as beet modify 'title=${title} - append', changed contains the same objects that the preview loop already mutated. Calling parse_mods(o) here evaluates the template against that mutated value, so confirming an individual object applies the template a second time and stores Tracktitle - append - append instead of the single append shown by the initial preview/all path. The select callback should reuse the per-object values computed before the preview or evaluate against a fresh/original object.

Useful? React with 👍 / 👎.

)

# Apply changes to database and files
Expand Down Expand Up @@ -121,9 +124,7 @@ def modify_func(lib, opts, args):
)


modify_cmd = ui.Subcommand(
"modify", help="change metadata fields", aliases=("mod",)
)
modify_cmd = ui.Subcommand("modify", help="change metadata fields", aliases=("mod",))
modify_cmd.parser.add_option(
"-m",
"--move",
Expand Down
3 changes: 3 additions & 0 deletions docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ Bug fixes
example a multi-disc album whose cover lives in the album root rather than a
per-disc directory); the missing art is skipped instead. :bug:`4692`
- :doc:`plugins/tidal`: Normalize Tidal album types to lowercase.
- ``modify``: Fix a crash when choosing ``select`` at the confirmation prompt
while modifying a non-string field such as ``added``; the per-object value is
now parsed before being applied. :bug:`4880`

..
For plugin developers
Expand Down
25 changes: 14 additions & 11 deletions test/ui/commands/test_modify.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,11 +108,17 @@
assert len(list(original_items)) == 3
assert len(list(new_items)) == 7

def test_selective_modify_typed_field(self):
# Regression test for a crash when selecting individual objects to
# modify a non-string (here date) field: the confirmation callback
# used to re-apply the raw string instead of the parsed value.
self.modify_inp(["s", "y"], "added=2002-06-03 00:00:00")
item = self.lib.items().get()
assert item.added == pytest.approx(1023062400, abs=24 * 60 * 60)

def test_modify_formatted(self):
for i in range(3):
self.add_item_fixture(
title=f"title{i}", artist="artist", album="album"
)
self.add_item_fixture(title=f"title{i}", artist="artist", album="album")
items = list(self.lib.items())
self.modify("title=${title} - append")
for item in items:
Expand Down Expand Up @@ -162,16 +168,15 @@
self.modify("--album", "artists=Charli XCX")
for item in self.lib.items():
assert item.artists == ["Charli XCX"], (
f"artists should be a list with one element, "
f"got {item.artists!r}"
f"artists should be a list with one element, " f"got {item.artists!r}"

Check failure on line 171 in test/ui/commands/test_modify.py

View workflow job for this annotation

GitHub Actions / Check linting

ruff (ISC001)

test/ui/commands/test_modify.py:171:17: ISC001 Implicitly concatenated string literals on one line help: Combine string literals
)

def test_album_modify_genres_not_split(self):
self.modify("--album", "genres=Rock")
for item in self.lib.items():
assert item.genres == ["Rock"], (
f"genres should be a list with one element, got {item.genres!r}"
)
assert item.genres == [
"Rock"
], f"genres should be a list with one element, got {item.genres!r}"

# Misc

Expand Down Expand Up @@ -217,9 +222,7 @@
assert mods == {"title": "newTitle"}

def test_arg_parsing_delete(self):
query, _, dels = modify_parse_args(
["title:oldTitle", "title!"], is_album=False
)
query, _, dels = modify_parse_args(["title:oldTitle", "title!"], is_album=False)
assert query == ["title:oldTitle"]
assert dels == ["title"]

Expand Down
Loading