mirror of
https://github.com/beetbox/beets.git
synced 2026-02-27 17:53:12 +01:00
Fix handling multi valued fields (#6387)
While working on #6367 I noticed that users are currently required to use our internal separator `\␀` in order to edit multi-valued fields, for example `beet modify artists='a\␀b'`. Similarly, this separator is used in output, for example, reporting of field changes: ``` $ beet modify path::aaa artists='a\␀b' Modifying 8 items. 54898 | 2022 / RAVE SLUTZ: Fallen Shrine & dj Christian NXC - DEEEJAAAY artists: Fallen Shrine\␀dj Christian NXC -> a\␀b ``` This PR replaces `\␀` separator with `; ` for input and formats changes in multi-valued fields clearly: ``` $ beet modify path::aaa artists='a; b' 54898 | 2022 / RAVE SLUTZ: Fallen Shrine & dj Christian NXC - DEEEJAAAY artists: - Fallen Shrine - dj Christian NXC + a + b ``` <img width="539" height="142" alt="image" src="https://github.com/user-attachments/assets/72299db1-d0f8-4f8f-9f30-65caaac85d9e" /> ### Architecture-level changes - `DelimitedString` now separates concerns between: - database serialization via `db_delimiter` (`to_sql`) - user-facing/template formatting via a fixed `'; '` delimiter (`format`) - parsing that accepts both DB and user-facing separators (`parse`) - Field diff rendering now has a dedicated path for list fields: - `_field_diff` detects list values - `_multi_value_diff` computes set-based added/removed entries and renders per-item diff lines - Coloring responsibilities were streamlined: - raw ANSI application moved to `_colorize` - `colorize` is now only the feature-flag/environment gate - `colordiff` is reduced to string diff highlighting logic, with redundant wrapper logic removed ### High-level impact - Multi-valued fields behave consistently between DB storage and CLI/template usage (`'; '` for user input/output, DB delimiter internally). - Diff output for list fields is much more readable, showing explicit `+`/`-` item-level changes instead of generic string diffs. - Docs and tests were updated to reflect the new multi-value behavior, including `%first` usage and `beet modify` examples.
This commit is contained in:
Šarūnas Nejus
GitHub
commit
2ea7886c0c
12 changed files with 129 additions and 74 deletions
|
|
@ -66,6 +66,8 @@ class Type(ABC, Generic[T, N]):
|
|||
"""The `Query` subclass to be used when querying the field.
|
||||
"""
|
||||
|
||||
# For sequence-like types, keep ``model_type`` unsubscripted as it's used
|
||||
# for ``isinstance`` checks. Use ``list`` instead of ``list[str]``
|
||||
model_type: type[T]
|
||||
"""The Python type that is used to represent the value in the model.
|
||||
|
||||
|
|
@ -287,26 +289,38 @@ class String(BaseString[str, Any]):
|
|||
model_type = str
|
||||
|
||||
|
||||
class DelimitedString(BaseString[list[str], list[str]]):
|
||||
"""A list of Unicode strings, represented in-database by a single string
|
||||
class DelimitedString(BaseString[list, list]): # type: ignore[type-arg]
|
||||
r"""A list of Unicode strings, represented in-database by a single string
|
||||
containing delimiter-separated values.
|
||||
|
||||
In template evaluation the list is formatted by joining the values with
|
||||
a fixed '; ' delimiter regardless of the database delimiter. That is because
|
||||
the '\␀' character used for multi-value fields is mishandled on Windows
|
||||
as it contains a backslash character.
|
||||
"""
|
||||
|
||||
model_type = list[str]
|
||||
model_type = list
|
||||
fmt_delimiter = "; "
|
||||
|
||||
def __init__(self, delimiter: str):
|
||||
self.delimiter = delimiter
|
||||
def __init__(self, db_delimiter: str):
|
||||
self.db_delimiter = db_delimiter
|
||||
|
||||
def format(self, value: list[str]):
|
||||
return self.delimiter.join(value)
|
||||
return self.fmt_delimiter.join(value)
|
||||
|
||||
def parse(self, string: str):
|
||||
if not string:
|
||||
return []
|
||||
return string.split(self.delimiter)
|
||||
|
||||
delimiter = (
|
||||
self.db_delimiter
|
||||
if self.db_delimiter in string
|
||||
else self.fmt_delimiter
|
||||
)
|
||||
return string.split(delimiter)
|
||||
|
||||
def to_sql(self, model_value: list[str]):
|
||||
return self.delimiter.join(model_value)
|
||||
return self.db_delimiter.join(model_value)
|
||||
|
||||
|
||||
class Boolean(Type):
|
||||
|
|
@ -464,7 +478,7 @@ NULL_FLOAT = NullFloat()
|
|||
STRING = String()
|
||||
BOOLEAN = Boolean()
|
||||
DATE = DateType()
|
||||
SEMICOLON_SPACE_DSV = DelimitedString(delimiter="; ")
|
||||
SEMICOLON_SPACE_DSV = DelimitedString("; ")
|
||||
|
||||
# Will set the proper null char in mediafile
|
||||
MULTI_VALUE_DSV = DelimitedString(delimiter="\\␀")
|
||||
MULTI_VALUE_DSV = DelimitedString("\\␀")
|
||||
|
|
|
|||
|
|
@ -1546,8 +1546,8 @@ class DefaultTemplateFunctions:
|
|||
s: the string
|
||||
count: The number of items included
|
||||
skip: The number of items skipped
|
||||
sep: the separator. Usually is '; ' (default) or '/ '
|
||||
join_str: the string which will join the items, default '; '.
|
||||
sep: the separator
|
||||
join_str: the string which will join the items
|
||||
"""
|
||||
skip = int(skip)
|
||||
count = skip + int(count)
|
||||
|
|
|
|||
|
|
@ -571,15 +571,16 @@ def get_color_config() -> dict[ColorName, str]:
|
|||
}
|
||||
|
||||
|
||||
def colorize(color_name: ColorName, text: str) -> str:
|
||||
"""Apply ANSI color formatting to text based on configuration settings.
|
||||
def _colorize(color_name: ColorName, text: str) -> str:
|
||||
"""Apply ANSI color formatting to text based on configuration settings."""
|
||||
color_code = get_color_config()[color_name]
|
||||
return f"{COLOR_ESCAPE}[{color_code}m{text}{RESET_COLOR}"
|
||||
|
||||
Returns colored text when color output is enabled and NO_COLOR environment
|
||||
variable is not set, otherwise returns plain text unchanged.
|
||||
"""
|
||||
|
||||
def colorize(color_name: ColorName, text: str) -> str:
|
||||
"""Colorize text when color output is enabled."""
|
||||
if config["ui"]["color"] and "NO_COLOR" not in os.environ:
|
||||
color_code = get_color_config()[color_name]
|
||||
return f"{COLOR_ESCAPE}[{color_code}m{text}{RESET_COLOR}"
|
||||
return _colorize(color_name, text)
|
||||
|
||||
return text
|
||||
|
||||
|
|
@ -643,32 +644,12 @@ def color_len(colored_text):
|
|||
return len(uncolorize(colored_text))
|
||||
|
||||
|
||||
def _colordiff(a: Any, b: Any) -> tuple[str, str]:
|
||||
"""Given two values, return the same pair of strings except with
|
||||
their differences highlighted in the specified color. Strings are
|
||||
highlighted intelligently to show differences; other values are
|
||||
stringified and highlighted in their entirety.
|
||||
"""
|
||||
# First, convert paths to readable format
|
||||
if isinstance(a, bytes) or isinstance(b, bytes):
|
||||
# A path field.
|
||||
a = util.displayable_path(a)
|
||||
b = util.displayable_path(b)
|
||||
|
||||
if not isinstance(a, str) or not isinstance(b, str):
|
||||
# Non-strings: use ordinary equality.
|
||||
if a == b:
|
||||
return str(a), str(b)
|
||||
else:
|
||||
return (
|
||||
colorize("text_diff_removed", str(a)),
|
||||
colorize("text_diff_added", str(b)),
|
||||
)
|
||||
|
||||
def colordiff(a: str, b: str) -> tuple[str, str]:
|
||||
"""Intelligently highlight the differences between two strings."""
|
||||
before = ""
|
||||
after = ""
|
||||
|
||||
matcher = SequenceMatcher(lambda x: False, a, b)
|
||||
matcher = SequenceMatcher(lambda _: False, a, b)
|
||||
for op, a_start, a_end, b_start, b_end in matcher.get_opcodes():
|
||||
before_part, after_part = a[a_start:a_end], b[b_start:b_end]
|
||||
if op in {"delete", "replace"}:
|
||||
|
|
@ -682,16 +663,6 @@ def _colordiff(a: Any, b: Any) -> tuple[str, str]:
|
|||
return before, after
|
||||
|
||||
|
||||
def colordiff(a, b):
|
||||
"""Colorize differences between two values if color is enabled.
|
||||
(Like _colordiff but conditional.)
|
||||
"""
|
||||
if config["ui"]["color"]:
|
||||
return _colordiff(a, b)
|
||||
else:
|
||||
return str(a), str(b)
|
||||
|
||||
|
||||
def get_path_formats(subview=None):
|
||||
"""Get the configuration's path formats as a list of query/template
|
||||
pairs.
|
||||
|
|
@ -1048,6 +1019,18 @@ def print_newline_layout(
|
|||
FLOAT_EPSILON = 0.01
|
||||
|
||||
|
||||
def _multi_value_diff(field: str, oldset: set[str], newset: set[str]) -> str:
|
||||
added = newset - oldset
|
||||
removed = oldset - newset
|
||||
|
||||
parts = [
|
||||
f"{field}:",
|
||||
*(colorize("text_diff_removed", f" - {i}") for i in sorted(removed)),
|
||||
*(colorize("text_diff_added", f" + {i}") for i in sorted(added)),
|
||||
]
|
||||
return "\n".join(parts)
|
||||
|
||||
|
||||
def _field_diff(
|
||||
field: str, old: FormattedMapping, new: FormattedMapping
|
||||
) -> str | None:
|
||||
|
|
@ -1063,6 +1046,11 @@ def _field_diff(
|
|||
):
|
||||
return None
|
||||
|
||||
if isinstance(oldval, list):
|
||||
if (oldset := set(oldval)) != (newset := set(newval)):
|
||||
return _multi_value_diff(field, oldset, newset)
|
||||
return None
|
||||
|
||||
# Get formatted values for output.
|
||||
oldstr, newstr = old.get(field, ""), new.get(field, "")
|
||||
if field not in new:
|
||||
|
|
@ -1071,8 +1059,7 @@ def _field_diff(
|
|||
if field not in old:
|
||||
return colorize("text_diff_added", f"{field}: {newstr}")
|
||||
|
||||
# For strings, highlight changes. For others, colorize the whole
|
||||
# thing.
|
||||
# For strings, highlight changes. For others, colorize the whole thing.
|
||||
if isinstance(oldval, str):
|
||||
oldstr, newstr = colordiff(oldstr, newstr)
|
||||
else:
|
||||
|
|
|
|||
|
|
@ -127,7 +127,7 @@ class ChangeRepresentation:
|
|||
and artist name.
|
||||
"""
|
||||
# Artist.
|
||||
artist_l, artist_r = self.cur_artist or "", self.match.info.artist
|
||||
artist_l, artist_r = self.cur_artist or "", self.match.info.artist or ""
|
||||
if artist_r == VARIOUS_ARTISTS:
|
||||
# Hide artists for VA releases.
|
||||
artist_l, artist_r = "", ""
|
||||
|
|
|
|||
|
|
@ -30,7 +30,12 @@ from beets.util import PromptChoice
|
|||
|
||||
# These "safe" types can avoid the format/parse cycle that most fields go
|
||||
# through: they are safe to edit with native YAML types.
|
||||
SAFE_TYPES = (types.BaseFloat, types.BaseInteger, types.Boolean)
|
||||
SAFE_TYPES = (
|
||||
types.BaseFloat,
|
||||
types.BaseInteger,
|
||||
types.Boolean,
|
||||
types.DelimitedString,
|
||||
)
|
||||
|
||||
|
||||
class ParseError(Exception):
|
||||
|
|
|
|||
|
|
@ -21,9 +21,17 @@ Unreleased
|
|||
For plugin developers
|
||||
~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
..
|
||||
Other changes
|
||||
~~~~~~~~~~~~~
|
||||
Other changes
|
||||
~~~~~~~~~~~~~
|
||||
|
||||
- :ref:`modify-cmd`: Use the following separator to delimite multiple field
|
||||
values: |semicolon_space|. For example ``beet modify albumtypes="album; ep"``.
|
||||
Previously, ``\␀`` was used as a separator. This applies to fields such as
|
||||
``artists``, ``albumtypes`` etc.
|
||||
- Improve highlighting of multi-valued fields changes.
|
||||
- :doc:`plugins/edit`: Editing multi-valued fields now behaves more naturally,
|
||||
with list values handled directly to make metadata edits smoother and more
|
||||
predictable.
|
||||
|
||||
2.6.2 (February 22, 2026)
|
||||
-------------------------
|
||||
|
|
|
|||
|
|
@ -98,7 +98,7 @@ man_pages = [
|
|||
]
|
||||
|
||||
# Global substitutions that can be used anywhere in the documentation.
|
||||
rst_epilog = """
|
||||
rst_epilog = r"""
|
||||
.. |Album| replace:: :class:`~beets.library.models.Album`
|
||||
.. |AlbumInfo| replace:: :class:`beets.autotag.hooks.AlbumInfo`
|
||||
.. |BeetsPlugin| replace:: :class:`beets.plugins.BeetsPlugin`
|
||||
|
|
@ -108,6 +108,7 @@ rst_epilog = """
|
|||
.. |Library| replace:: :class:`~beets.library.library.Library`
|
||||
.. |Model| replace:: :class:`~beets.dbcore.db.Model`
|
||||
.. |TrackInfo| replace:: :class:`beets.autotag.hooks.TrackInfo`
|
||||
.. |semicolon_space| replace:: :literal:`; \ `
|
||||
"""
|
||||
|
||||
# -- Options for HTML output -------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -267,6 +267,9 @@ Values can also be *templates*, using the same syntax as :doc:`path formats
|
|||
artist sort name into the artist field for all your tracks, and ``beet modify
|
||||
title='$track $title'`` will add track numbers to their title metadata.
|
||||
|
||||
To adjust a multi-valued field, such as ``genres``, separate the values with
|
||||
|semicolon_space|. For example, ``beet modify genres="rock; pop"``.
|
||||
|
||||
The ``-a`` option changes to querying album fields instead of track fields and
|
||||
also enables to operate on albums in addition to the individual tracks. Without
|
||||
this flag, the command will only change *track-level* data, even if all the
|
||||
|
|
|
|||
|
|
@ -75,11 +75,34 @@ These functions are built in to beets:
|
|||
- ``%time{date_time,format}``: Return the date and time in any format accepted
|
||||
by strftime_. For example, to get the year some music was added to your
|
||||
library, use ``%time{$added,%Y}``.
|
||||
- ``%first{text}``: Returns the first item, separated by ``;`` (a semicolon
|
||||
followed by a space). You can use ``%first{text,count,skip}``, where ``count``
|
||||
is the number of items (default 1) and ``skip`` is number to skip (default 0).
|
||||
You can also use ``%first{text,count,skip,sep,join}`` where ``sep`` is the
|
||||
separator, like ``;`` or ``/`` and join is the text to concatenate the items.
|
||||
- ``%first{text,count,skip,sep,join}``: Extract a subset of items from a
|
||||
delimited string. Splits ``text`` by ``sep``, skips the first ``skip`` items,
|
||||
then returns the next ``count`` items joined by ``join``.
|
||||
|
||||
This is especially useful for multi-valued fields like ``artists`` or
|
||||
``genres`` where you may only want the first artist or a limited number of
|
||||
genres in a path.
|
||||
|
||||
Defaults:
|
||||
|
||||
..
|
||||
Comically, you need to follow |semicolon_space| with some punctuation to
|
||||
make sure it gets rendered correctly as '; ' in the docs.
|
||||
|
||||
- **count**: 1,
|
||||
- **skip**: 0,
|
||||
- **sep**: |semicolon_space|,
|
||||
- **join**: |semicolon_space|.
|
||||
|
||||
Examples:
|
||||
|
||||
::
|
||||
|
||||
%first{$genres} → returns the first genre
|
||||
%first{$genres,2} → returns the first two genres, joined by "; "
|
||||
%first{$genres,2,1} → skips the first genre, returns the next two
|
||||
%first{$genres,2,0, , -> } → splits by space, joins with " -> "
|
||||
|
||||
- ``%ifdef{field}``, ``%ifdef{field,truetext}`` or
|
||||
``%ifdef{field,truetext,falsetext}``: Checks if an flexible attribute
|
||||
``field`` is defined. If it exists, then return ``truetext`` or ``field``
|
||||
|
|
|
|||
|
|
@ -688,14 +688,14 @@ class DestinationFunctionTest(BeetsTestCase, PathFormattingMixin):
|
|||
self._assert_dest(b"/base/not_played")
|
||||
|
||||
def test_first(self):
|
||||
self.i.genres = "Pop; Rock; Classical Crossover"
|
||||
self._setf("%first{$genres}")
|
||||
self._assert_dest(b"/base/Pop")
|
||||
self.i.albumtypes = ["album", "compilation"]
|
||||
self._setf("%first{$albumtypes}")
|
||||
self._assert_dest(b"/base/album")
|
||||
|
||||
def test_first_skip(self):
|
||||
self.i.genres = "Pop; Rock; Classical Crossover"
|
||||
self._setf("%first{$genres,1,2}")
|
||||
self._assert_dest(b"/base/Classical Crossover")
|
||||
self.i.albumtype = "album; ep; compilation"
|
||||
self._setf("%first{$albumtype,1,2}")
|
||||
self._assert_dest(b"/base/compilation")
|
||||
|
||||
def test_first_different_sep(self):
|
||||
self._setf("%first{Alice / Bob / Eve,2,0, / , & }")
|
||||
|
|
|
|||
|
|
@ -96,8 +96,7 @@ class TestPluginRegistration(IOMixin, PluginTestCase):
|
|||
item.add(self.lib)
|
||||
|
||||
out = self.run_with_output("ls", "-f", "$multi_value")
|
||||
delimiter = types.MULTI_VALUE_DSV.delimiter
|
||||
assert out == f"one{delimiter}two{delimiter}three\n"
|
||||
assert out == "one; two; three\n"
|
||||
|
||||
|
||||
class PluginImportTestCase(ImportHelper, PluginTestCase):
|
||||
|
|
|
|||
|
|
@ -1,3 +1,5 @@
|
|||
from textwrap import dedent
|
||||
|
||||
import pytest
|
||||
|
||||
from beets.library import Item
|
||||
|
|
@ -15,7 +17,7 @@ class TestFieldDiff:
|
|||
def patch_colorize(self, monkeypatch):
|
||||
"""Patch to return a deterministic string format instead of ANSI codes."""
|
||||
monkeypatch.setattr(
|
||||
"beets.ui.colorize",
|
||||
"beets.ui._colorize",
|
||||
lambda color_name, text: f"[{color_name}]{text}[/]",
|
||||
)
|
||||
|
||||
|
|
@ -38,6 +40,19 @@ class TestFieldDiff:
|
|||
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"),
|
||||
p({"albumtypes": ["album", "ep"]}, {"albumtypes": ["ep", "album"]}, "albumtypes", None, id="multi_value_unchanged"),
|
||||
p(
|
||||
{"albumtypes": ["ep"]},
|
||||
{"albumtypes": ["album", "compilation"]},
|
||||
"albumtypes",
|
||||
dedent("""
|
||||
albumtypes:
|
||||
[text_diff_removed] - ep[/]
|
||||
[text_diff_added] + album[/]
|
||||
[text_diff_added] + compilation[/]
|
||||
""").strip(),
|
||||
id="multi_value_changed"
|
||||
),
|
||||
],
|
||||
) # fmt: skip
|
||||
@pytest.mark.parametrize("color", [True], ids=["color_enabled"])
|
||||
|
|
|
|||
Loading…
Reference in a new issue