diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index 83f0543d2..110cd70d0 100755 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -105,6 +105,8 @@ class FormattedMapping(Mapping[str, str]): are replaced. """ + model: Model + ALL_KEYS = "*" def __init__( @@ -714,7 +716,7 @@ class Model(ABC, Generic[D]): self, included_keys: str = _formatter.ALL_KEYS, for_path: bool = False, - ): + ) -> FormattedMapping: """Get a mapping containing all values on this object formatted as human-readable unicode strings. """ diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index cbe0fb109..5eeef815d 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -43,7 +43,10 @@ from beets.util.deprecation import deprecate_for_maintainers from beets.util.functemplate import template if TYPE_CHECKING: - from collections.abc import Callable + from collections.abc import Callable, Iterable + + from beets.dbcore.db import FormattedMapping + # On Windows platforms, use colorama to support "ANSI" terminal colors. if sys.platform == "win32": @@ -1028,42 +1031,47 @@ def print_newline_layout( FLOAT_EPSILON = 0.01 -def _field_diff(field, old, old_fmt, new, new_fmt): +def _field_diff( + field: str, old: FormattedMapping, new: FormattedMapping +) -> str | None: """Given two Model objects and their formatted views, format their values for `field` and highlight changes among them. Return a human-readable string. If the value has not changed, return None instead. """ - oldval = old.get(field) - newval = new.get(field) - # If no change, abort. - if ( + if (oldval := old.model.get(field)) == (newval := new.model.get(field)) or ( isinstance(oldval, float) and isinstance(newval, float) and abs(oldval - newval) < FLOAT_EPSILON ): return None - elif oldval == newval: - return None # Get formatted values for output. - oldstr = old_fmt.get(field, "") - newstr = new_fmt.get(field, "") + oldstr, newstr = old.get(field, ""), new.get(field, "") + if field not in new: + return colorize("text_diff_removed", f"{field}: {oldstr}") + + if field not in old: + return colorize("text_diff_added", f"{field}: {newstr}") # For strings, highlight changes. For others, colorize the whole # thing. if isinstance(oldval, str): - oldstr, newstr = colordiff(oldval, newstr) + oldstr, newstr = colordiff(oldstr, newstr) else: oldstr = colorize("text_diff_removed", oldstr) newstr = colorize("text_diff_added", newstr) - return f"{oldstr} -> {newstr}" + return f"{field}: {oldstr} -> {newstr}" def show_model_changes( - new, old=None, fields=None, always=False, print_obj: bool = True -): + new: library.LibModel, + old: library.LibModel | None = None, + fields: Iterable[str] | None = None, + always: bool = False, + print_obj: bool = True, +) -> bool: """Given a Model object, print a list of changes from its pristine version stored in the database. Return a boolean indicating whether any changes were found. @@ -1081,31 +1089,21 @@ def show_model_changes( new_fmt = new.formatted() # Build up lines showing changed fields. - changes = [] - for field in old: - # Subset of the fields. Never show mtime. - if field == "mtime" or (fields and field not in fields): - continue + diff_fields = (set(old) | set(new)) - {"mtime"} + if allowed_fields := set(fields or {}): + diff_fields &= allowed_fields - # Detect and show difference for this field. - line = _field_diff(field, old, old_fmt, new, new_fmt) - if line: - changes.append(f" {field}: {line}") - - # New fields. - for field in set(new) - set(old): - if fields and field not in fields: - continue - - changes.append( - f" {field}: {colorize('text_highlight', new_fmt[field])}" - ) + changes = [ + d + for f in sorted(diff_fields) + if (d := _field_diff(f, old_fmt, new_fmt)) + ] # Print changes. if print_obj and (changes or always): print_(format(old)) if changes: - print_("\n".join(changes)) + print_(textwrap.indent("\n".join(changes), " ")) return bool(changes) diff --git a/docs/changelog.rst b/docs/changelog.rst index a471b4c56..b292863ee 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -70,6 +70,8 @@ Bug fixes: - When using :doc:`plugins/fromfilename` together with :doc:`plugins/edit`, temporary tags extracted from filenames are no longer lost when discarding or cancelling an edit session during import. :bug:`6104` +- :ref:`update-cmd` :doc:`plugins/edit` fix display formatting of field changes + to clearly show added and removed flexible fields. For plugin developers: diff --git a/pyproject.toml b/pyproject.toml index 24cf21b33..bc694de90 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -322,6 +322,7 @@ ignore = [ [tool.ruff.lint.per-file-ignores] "beets/**" = ["PT"] "test/test_util.py" = ["E501"] +"test/ui/test_field_diff.py" = ["E501"] [tool.ruff.lint.isort] split-on-trailing-comma = false diff --git a/test/ui/test_field_diff.py b/test/ui/test_field_diff.py new file mode 100644 index 000000000..dce7ba161 --- /dev/null +++ b/test/ui/test_field_diff.py @@ -0,0 +1,64 @@ +import pytest + +from beets.library import Item +from beets.test.helper import ConfigMixin +from beets.ui import _field_diff + +p = pytest.param + + +class TestFieldDiff: + @pytest.fixture(scope="class", autouse=True) + def config(self): + return ConfigMixin().config + + @pytest.fixture(autouse=True) + def configure_color(self, config, color): + config["ui"]["color"] = color + + @pytest.fixture(autouse=True) + def patch_colorize(self, monkeypatch): + """Patch to return a deterministic string format instead of ANSI codes.""" + monkeypatch.setattr( + "beets.ui.colorize", + lambda color_name, text: f"[{color_name}]{text}[/]", + ) + + @staticmethod + def diff_fmt(old, new): + return f"[text_diff_removed]{old}[/] -> [text_diff_added]{new}[/]" + + @pytest.mark.parametrize( + "old_data, new_data, field, expected_diff", + [ + p({"title": "foo"}, {"title": "foo"}, "title", None, id="no_change"), + p({"bpm": 120.0}, {"bpm": 120.005}, "bpm", None, id="float_close_enough"), + p({"bpm": 120.0}, {"bpm": 121.0}, "bpm", f"bpm: {diff_fmt('120', '121')}", id="float_changed"), + p({"title": "foo"}, {"title": "bar"}, "title", f"title: {diff_fmt('foo', 'bar')}", id="string_full_replace"), + p({"title": "prefix foo"}, {"title": "prefix bar"}, "title", "title: prefix [text_diff_removed]foo[/] -> prefix [text_diff_added]bar[/]", id="string_partial_change"), + p({"year": 2000}, {"year": 2001}, "year", f"year: {diff_fmt('2000', '2001')}", id="int_changed"), + p({}, {"genre": "Rock"}, "genre", "genre: -> [text_diff_added]Rock[/]", id="field_added"), + p({"genre": "Rock"}, {}, "genre", "genre: [text_diff_removed]Rock[/] -> ", id="field_removed"), + p({"track": 1}, {"track": 2}, "track", f"track: {diff_fmt('01', '02')}", id="formatted_value_changed"), + p({"mb_trackid": None}, {"mb_trackid": "1234"}, "mb_trackid", "mb_trackid: -> [text_diff_added]1234[/]", id="none_to_value"), + p({}, {"new_flex": "foo"}, "new_flex", "[text_diff_added]new_flex: foo[/]", id="flex_field_added"), + p({"old_flex": "foo"}, {}, "old_flex", "[text_diff_removed]old_flex: foo[/]", id="flex_field_removed"), + ], + ) # fmt: skip + @pytest.mark.parametrize("color", [True], ids=["color_enabled"]) + def test_field_diff_colors(self, old_data, new_data, field, expected_diff): + old_item = Item(**old_data) + new_item = Item(**new_data) + + diff = _field_diff(field, old_item.formatted(), new_item.formatted()) + + assert diff == expected_diff + + @pytest.mark.parametrize("color", [False], ids=["color_disabled"]) + def test_field_diff_no_color(self): + old_item = Item(title="foo") + new_item = Item(title="bar") + + diff = _field_diff("title", old_item.formatted(), new_item.formatted()) + + assert diff == "title: foo -> bar"