From 1d239d6e27c4f4ce4ab36566af4569cf294042dd Mon Sep 17 00:00:00 2001 From: Trey Turner Date: Wed, 12 Nov 2025 07:03:30 -0600 Subject: [PATCH 01/39] feat(ftintitle): Insert featured artist before track variant --- beetsplug/ftintitle.py | 106 ++++++++++++++++++++++++++++++++- docs/changelog.rst | 7 +++ test/plugins/test_ftintitle.py | 106 +++++++++++++++++++++++++++++++++ 3 files changed, 217 insertions(+), 2 deletions(-) diff --git a/beetsplug/ftintitle.py b/beetsplug/ftintitle.py index dd681a972..cec22af3f 100644 --- a/beetsplug/ftintitle.py +++ b/beetsplug/ftintitle.py @@ -59,6 +59,89 @@ def contains_feat(title: str, custom_words: list[str] | None = None) -> bool: ) +# Default keywords that indicate remix/edit/version content +DEFAULT_BRACKET_KEYWORDS = [ + "abridged", + "acapella", + "club", + "demo", + "edit", + "extended", + "instrumental", + "live", + "mix", + "radio", + "release", + "remaster", + "remastered", + "remix", + "rmx", + "unabridged", + "unreleased", + "version", + "vip", +] + + +def find_bracket_position( + title: str, keywords: list[str] | None = None +) -> int | None: + """Find the position of the first opening bracket that contains + remix/edit-related keywords and has a matching closing bracket. + + Args: + title: The title to search in. + keywords: List of keywords to match. If None, uses DEFAULT_BRACKET_KEYWORDS. + If an empty list, matches any bracket content (not just keywords). + + Returns: + The position of the opening bracket, or None if no match found. + """ + if keywords is None: + keywords = DEFAULT_BRACKET_KEYWORDS + + # If keywords is empty, match any bracket content + if not keywords: + pattern = None + else: + # Build regex pattern with word boundaries + keyword_pattern = "|".join(rf"\b{re.escape(kw)}\b" for kw in keywords) + pattern = re.compile(keyword_pattern, re.IGNORECASE) + + # Bracket pairs (opening, closing) + bracket_pairs = [("(", ")"), ("[", "]"), ("<", ">"), ("{", "}")] + + # Track the earliest valid bracket position + earliest_pos = None + + for open_char, close_char in bracket_pairs: + pos = 0 + while True: + # Find next opening bracket + open_pos = title.find(open_char, pos) + if open_pos == -1: + break + + # Find matching closing bracket + close_pos = title.find(close_char, open_pos + 1) + if close_pos == -1: + break + + # Extract content between brackets + content = title[open_pos + 1 : close_pos] + + # Check if content matches: if pattern is None (empty keywords), + # match any content; otherwise check for keywords + if pattern is None or pattern.search(content): + if earliest_pos is None or open_pos < earliest_pos: + earliest_pos = open_pos + + # Continue searching from after this closing bracket + pos = close_pos + 1 + + return earliest_pos + + def find_feat_part( artist: str, albumartist: str | None, @@ -110,6 +193,7 @@ class FtInTitlePlugin(plugins.BeetsPlugin): "keep_in_artist": False, "preserve_album_artist": True, "custom_words": [], + "bracket_keywords": DEFAULT_BRACKET_KEYWORDS, } ) @@ -138,6 +222,7 @@ class FtInTitlePlugin(plugins.BeetsPlugin): bool ) custom_words = self.config["custom_words"].get(list) + bracket_keywords = self.config["bracket_keywords"].get(list) write = ui.should_write() for item in lib.items(args): @@ -147,6 +232,7 @@ class FtInTitlePlugin(plugins.BeetsPlugin): keep_in_artist_field, preserve_album_artist, custom_words, + bracket_keywords, ): item.store() if write: @@ -161,6 +247,7 @@ class FtInTitlePlugin(plugins.BeetsPlugin): keep_in_artist_field = self.config["keep_in_artist"].get(bool) preserve_album_artist = self.config["preserve_album_artist"].get(bool) custom_words = self.config["custom_words"].get(list) + bracket_keywords = self.config["bracket_keywords"].get(list) for item in task.imported_items(): if self.ft_in_title( @@ -169,6 +256,7 @@ class FtInTitlePlugin(plugins.BeetsPlugin): keep_in_artist_field, preserve_album_artist, custom_words, + bracket_keywords, ): item.store() @@ -179,6 +267,7 @@ class FtInTitlePlugin(plugins.BeetsPlugin): drop_feat: bool, keep_in_artist_field: bool, custom_words: list[str], + bracket_keywords: list[str] | None = None, ) -> None: """Choose how to add new artists to the title and set the new metadata. Also, print out messages about any changes that are made. @@ -208,7 +297,14 @@ class FtInTitlePlugin(plugins.BeetsPlugin): if not drop_feat and not contains_feat(item.title, custom_words): feat_format = self.config["format"].as_str() new_format = feat_format.format(feat_part) - new_title = f"{item.title} {new_format}" + # Insert before the first bracket containing remix/edit keywords + bracket_pos = find_bracket_position(item.title, bracket_keywords) + if bracket_pos is not None: + title_before = item.title[:bracket_pos].rstrip() + title_after = item.title[bracket_pos:] + new_title = f"{title_before} {new_format} {title_after}" + else: + new_title = f"{item.title} {new_format}" self._log.info("title: {.title} -> {}", item, new_title) item.title = new_title @@ -219,6 +315,7 @@ class FtInTitlePlugin(plugins.BeetsPlugin): keep_in_artist_field: bool, preserve_album_artist: bool, custom_words: list[str], + bracket_keywords: list[str] | None = None, ) -> bool: """Look for featured artists in the item's artist fields and move them to the title. @@ -250,6 +347,11 @@ class FtInTitlePlugin(plugins.BeetsPlugin): # If we have a featuring artist, move it to the title. self.update_metadata( - item, feat_part, drop_feat, keep_in_artist_field, custom_words + item, + feat_part, + drop_feat, + keep_in_artist_field, + custom_words, + bracket_keywords, ) return True diff --git a/docs/changelog.rst b/docs/changelog.rst index c5a0dab53..c3591b80e 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -26,6 +26,13 @@ New features: - :doc:`plugins/mbpseudo`: Add a new `mbpseudo` plugin to proactively receive MusicBrainz pseudo-releases as recommendations during import. - Added support for Python 3.13. +- :doc:`plugins/ftintitle`: Featured artists are now inserted before brackets + containing remix/edit-related keywords (e.g., "Remix", "Live", "Edit") instead + of being appended at the end. This improves formatting for titles like "Song 1 + (Carol Remix) ft. Bob" which becomes "Song 1 ft. Bob (Carol Remix)". A variety + of brackets are supported and a new ``bracket_keywords`` configuration option + allows customizing the keywords. Setting ``bracket_keywords`` to an empty list + matches any bracket content regardless of keywords. Bug fixes: diff --git a/test/plugins/test_ftintitle.py b/test/plugins/test_ftintitle.py index b4259666d..b2e2bad9b 100644 --- a/test/plugins/test_ftintitle.py +++ b/test/plugins/test_ftintitle.py @@ -246,6 +246,49 @@ def add_item( ("Alice", "Song 1 feat. Bob"), id="skip-if-artist-and-album-artists-is-the-same-matching-match-b", ), + # ---- titles with brackets/parentheses ---- + pytest.param( + {"format": "ft. {}"}, + ("ftintitle",), + ("Alice ft. Bob", "Song 1 (Carol Remix)", "Alice"), + ("Alice", "Song 1 ft. Bob (Carol Remix)"), + id="title-with-brackets-insert-before", + ), + pytest.param( + {"format": "ft. {}", "keep_in_artist": True}, + ("ftintitle",), + ("Alice ft. Bob", "Song 1 (Carol Remix)", "Alice"), + ("Alice ft. Bob", "Song 1 ft. Bob (Carol Remix)"), + id="title-with-brackets-keep-in-artist", + ), + pytest.param( + {"format": "ft. {}"}, + ("ftintitle",), + ("Alice ft. Bob", "Song 1 (Remix) (Live)", "Alice"), + ("Alice", "Song 1 ft. Bob (Remix) (Live)"), + id="title-with-multiple-brackets-uses-first-with-keyword", + ), + pytest.param( + {"format": "ft. {}"}, + ("ftintitle",), + ("Alice ft. Bob", "Song 1 (Arbitrary)", "Alice"), + ("Alice", "Song 1 (Arbitrary) ft. Bob"), + id="title-with-brackets-no-keyword-appends", + ), + pytest.param( + {"format": "ft. {}"}, + ("ftintitle",), + ("Alice ft. Bob", "Song 1 [Edit]", "Alice"), + ("Alice", "Song 1 ft. Bob [Edit]"), + id="title-with-square-brackets-keyword", + ), + pytest.param( + {"format": "ft. {}"}, + ("ftintitle",), + ("Alice ft. Bob", "Song 1 ", "Alice"), + ("Alice", "Song 1 ft. Bob "), + id="title-with-angle-brackets-keyword", + ), ], ) def test_ftintitle_functional( @@ -312,6 +355,69 @@ def test_split_on_feat( assert ftintitle.split_on_feat(given) == expected +@pytest.mark.parametrize( + "given,expected", + [ + # different braces and keywords + ("Song (Remix)", 5), + ("Song [Version]", 5), + ("Song {Extended Mix}", 5), + ("Song ", 5), + # two keyword clauses + ("Song (Remix) (Live)", 5), + # brace insensitivity + ("Song (Live) [Remix]", 5), + ("Song [Edit] (Remastered)", 5), + # negative cases + ("Song", None), # no clause + ("Song (Arbitrary)", None), # no keyword + ("Song (", None), # no matching brace or keyword + ("Song (Live", None), # no matching brace with keyword + # one keyword clause, one non-keyword clause + ("Song (Live) (Arbitrary)", 5), + ("Song (Arbitrary) (Remix)", 17), + # nested brackets - same type + ("Song (Remix (Extended))", 5), + ("Song [Arbitrary [Description]]", None), + # nested brackets - different types + ("Song (Remix [Extended])", 5), + # nested - returns outer start position despite inner keyword + ("Song [Arbitrary {Extended}]", 5), + ("Song {Live }", 5), + ("Song ", 5), + ("Song [Live]", 5), + ("Song (Version) ", 5), + ("Song (Arbitrary [Description])", None), + ("Song [Description (Arbitrary)]", None), + ], +) +def test_find_bracket_position(given: str, expected: int | None) -> None: + assert ftintitle.find_bracket_position(given) == expected + + +@pytest.mark.parametrize( + "given,keywords,expected", + [ + ("Song (Live)", ["live"], 5), + ("Song (Live)", None, 5), + ("Song (Arbitrary)", None, None), + ("Song (Concert)", ["concert"], 5), + ("Song (Concert)", None, None), + ("Song (Remix)", ["custom"], None), + ("Song (Custom)", ["custom"], 5), + ("Song (Live)", [], 5), + ("Song (Anything)", [], 5), + ("Song (Remix)", [], 5), + ("Song", [], None), + ("Song (", [], None), + ], +) +def test_find_bracket_position_custom_keywords( + given: str, keywords: list[str] | None, expected: int | None +) -> None: + assert ftintitle.find_bracket_position(given, keywords) == expected + + @pytest.mark.parametrize( "given,expected", [ From 62e1a41ff2e6242cdae54ce63236af0e6c105514 Mon Sep 17 00:00:00 2001 From: Trey Turner Date: Sat, 15 Nov 2025 17:19:12 -0600 Subject: [PATCH 02/39] chore(ftintitle): add 'edition' to keyword defaults --- beetsplug/ftintitle.py | 1 + 1 file changed, 1 insertion(+) diff --git a/beetsplug/ftintitle.py b/beetsplug/ftintitle.py index cec22af3f..9702bf9a5 100644 --- a/beetsplug/ftintitle.py +++ b/beetsplug/ftintitle.py @@ -66,6 +66,7 @@ DEFAULT_BRACKET_KEYWORDS = [ "club", "demo", "edit", + "edition", "extended", "instrumental", "live", From 15daebb55f9b356cdd2ac5687d7af5c4ef53f67b Mon Sep 17 00:00:00 2001 From: Trey Turner Date: Sat, 15 Nov 2025 17:31:40 -0600 Subject: [PATCH 03/39] test(ftintitle): mock import task to exercise import hooks --- test/plugins/test_ftintitle.py | 37 ++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/test/plugins/test_ftintitle.py b/test/plugins/test_ftintitle.py index b2e2bad9b..a6be02b3b 100644 --- a/test/plugins/test_ftintitle.py +++ b/test/plugins/test_ftintitle.py @@ -15,9 +15,11 @@ """Tests for the 'ftintitle' plugin.""" from collections.abc import Generator +from typing import cast import pytest +from beets.importer import ImportSession, ImportTask from beets.library.models import Item from beets.test.helper import PluginTestCase from beetsplug import ftintitle @@ -68,6 +70,16 @@ def add_item( ) +class DummyImportTask: + """Minimal stand-in for ImportTask used to exercise import hooks.""" + + def __init__(self, items: list[Item]) -> None: + self._items = items + + def imported_items(self) -> list[Item]: + return self._items + + @pytest.mark.parametrize( "cfg, cmd_args, given, expected", [ @@ -312,6 +324,31 @@ def test_ftintitle_functional( assert item["title"] == expected_title +def test_imported_stage_moves_featured_artist( + env: FtInTitlePluginFunctional, +) -> None: + """The import-stage hook should fetch config settings and process items.""" + set_config(env, None) + plugin = ftintitle.FtInTitlePlugin() + item = add_item( + env, + "/imported-hook", + "Alice feat. Bob", + "Song 1 (Carol Remix)", + "Various Artists", + ) + task = DummyImportTask([item]) + + plugin.imported( + cast(ImportSession, None), + cast(ImportTask, task), + ) + item.load() + + assert item["artist"] == "Alice" + assert item["title"] == "Song 1 feat. Bob (Carol Remix)" + + @pytest.mark.parametrize( "artist,albumartist,expected", [ From a9ed637c407c6788e0510e75fa98261c9afb31ad Mon Sep 17 00:00:00 2001 From: Trey Turner Date: Sat, 15 Nov 2025 17:31:49 -0600 Subject: [PATCH 04/39] docs(ftintitle): add bracket_keywords --- docs/plugins/ftintitle.rst | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/docs/plugins/ftintitle.rst b/docs/plugins/ftintitle.rst index 1d2ec5c20..fef1bc9bf 100644 --- a/docs/plugins/ftintitle.rst +++ b/docs/plugins/ftintitle.rst @@ -32,6 +32,18 @@ file. The available options are: skip the ftintitle processing. Default: ``yes``. - **custom_words**: List of additional words that will be treated as a marker for artist features. Default: ``[]``. +- **bracket_keywords**: Controls where the featuring text is inserted when the + title includes bracketed qualifiers such as ``(Remix)`` or ``[Live]``. + FtInTitle inserts the new text before the first bracket whose contents match + any of these keywords. Supply a list of words to fine-tune the behavior or set + the list to ``[]`` to match *any* bracket regardless of its contents. Default: + + :: + + ["abridged", "acapella", "club", "demo", "edit", "edition", "extended", + "instrumental", "live", "mix", "radio", "release", "remastered" + "remastered", "remix", "rmx", "unabridged", "unreleased", + "version", "vip"] Running Manually ---------------- From 3dd3bf5640eebb0da282edae4169e8a636b1aeac Mon Sep 17 00:00:00 2001 From: Trey Turner Date: Sat, 15 Nov 2025 22:34:43 -0600 Subject: [PATCH 05/39] fix: address sourcery comments --- beetsplug/ftintitle.py | 13 ++++++++----- docs/plugins/ftintitle.rst | 2 +- test/plugins/test_ftintitle.py | 18 ++++++++++++++++++ 3 files changed, 27 insertions(+), 6 deletions(-) diff --git a/beetsplug/ftintitle.py b/beetsplug/ftintitle.py index 9702bf9a5..d3d600958 100644 --- a/beetsplug/ftintitle.py +++ b/beetsplug/ftintitle.py @@ -105,7 +105,9 @@ def find_bracket_position( if not keywords: pattern = None else: - # Build regex pattern with word boundaries + # Build regex pattern to support multi-word keywords/phrases. + # Each keyword/phrase is escaped and surrounded by word boundaries at + # start and end, matching phrases like "club mix" as a whole. keyword_pattern = "|".join(rf"\b{re.escape(kw)}\b" for kw in keywords) pattern = re.compile(keyword_pattern, re.IGNORECASE) @@ -133,9 +135,10 @@ def find_bracket_position( # Check if content matches: if pattern is None (empty keywords), # match any content; otherwise check for keywords - if pattern is None or pattern.search(content): - if earliest_pos is None or open_pos < earliest_pos: - earliest_pos = open_pos + if (pattern is None or pattern.search(content)) and ( + earliest_pos is None or open_pos < earliest_pos + ): + earliest_pos = open_pos # Continue searching from after this closing bracket pos = close_pos + 1 @@ -194,7 +197,7 @@ class FtInTitlePlugin(plugins.BeetsPlugin): "keep_in_artist": False, "preserve_album_artist": True, "custom_words": [], - "bracket_keywords": DEFAULT_BRACKET_KEYWORDS, + "bracket_keywords": DEFAULT_BRACKET_KEYWORDS.copy(), } ) diff --git a/docs/plugins/ftintitle.rst b/docs/plugins/ftintitle.rst index fef1bc9bf..347e2792d 100644 --- a/docs/plugins/ftintitle.rst +++ b/docs/plugins/ftintitle.rst @@ -41,7 +41,7 @@ file. The available options are: :: ["abridged", "acapella", "club", "demo", "edit", "edition", "extended", - "instrumental", "live", "mix", "radio", "release", "remastered" + "instrumental", "live", "mix", "radio", "release", "remaster", "remastered", "remix", "rmx", "unabridged", "unreleased", "version", "vip"] diff --git a/test/plugins/test_ftintitle.py b/test/plugins/test_ftintitle.py index a6be02b3b..065f23ade 100644 --- a/test/plugins/test_ftintitle.py +++ b/test/plugins/test_ftintitle.py @@ -301,6 +301,21 @@ class DummyImportTask: ("Alice", "Song 1 ft. Bob "), id="title-with-angle-brackets-keyword", ), + # multi-word keyword + pytest.param( + {"format": "ft. {}", "bracket_keywords": ["club mix"]}, + ("ftintitle",), + ("Alice ft. Bob", "Song 1 (Club Mix)", "Alice"), + ("Alice", "Song 1 ft. Bob (Club Mix)"), + id="multi-word-keyword-positive-match", + ), + pytest.param( + {"format": "ft. {}", "bracket_keywords": ["club mix"]}, + ("ftintitle",), + ("Alice ft. Bob", "Song 1 (Club Remix)", "Alice"), + ("Alice", "Song 1 (Club Remix) ft. Bob"), + id="multi-word-keyword-negative-no-match", + ), ], ) def test_ftintitle_functional( @@ -447,6 +462,9 @@ def test_find_bracket_position(given: str, expected: int | None) -> None: ("Song (Remix)", [], 5), ("Song", [], None), ("Song (", [], None), + # Multi-word keyword tests + ("Song (Club Mix)", ["club mix"], 5), # Positive: matches multi-word + ("Song (Club Remix)", ["club mix"], None), # Negative: no match ], ) def test_find_bracket_position_custom_keywords( From 2aa949e5a0f9da3e079296b376f0b8e72d3da70b Mon Sep 17 00:00:00 2001 From: Trey Turner Date: Sun, 16 Nov 2025 19:48:46 -0600 Subject: [PATCH 06/39] fix(fitintitle): simplify keyword_pattern using map() instead of list comprehension --- beetsplug/ftintitle.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/beetsplug/ftintitle.py b/beetsplug/ftintitle.py index d3d600958..4d0821593 100644 --- a/beetsplug/ftintitle.py +++ b/beetsplug/ftintitle.py @@ -106,9 +106,7 @@ def find_bracket_position( pattern = None else: # Build regex pattern to support multi-word keywords/phrases. - # Each keyword/phrase is escaped and surrounded by word boundaries at - # start and end, matching phrases like "club mix" as a whole. - keyword_pattern = "|".join(rf"\b{re.escape(kw)}\b" for kw in keywords) + keyword_pattern = rf"\b{'|'.join(map(re.escape, keywords))}\b" pattern = re.compile(keyword_pattern, re.IGNORECASE) # Bracket pairs (opening, closing) From 50e55f85f4fd231d0023353355faed32cb007611 Mon Sep 17 00:00:00 2001 From: Trey Turner Date: Sun, 16 Nov 2025 19:54:27 -0600 Subject: [PATCH 07/39] fix(ftintitle): prune find_bracket_position docstring --- beetsplug/ftintitle.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/beetsplug/ftintitle.py b/beetsplug/ftintitle.py index 4d0821593..629e58f17 100644 --- a/beetsplug/ftintitle.py +++ b/beetsplug/ftintitle.py @@ -89,14 +89,6 @@ def find_bracket_position( ) -> int | None: """Find the position of the first opening bracket that contains remix/edit-related keywords and has a matching closing bracket. - - Args: - title: The title to search in. - keywords: List of keywords to match. If None, uses DEFAULT_BRACKET_KEYWORDS. - If an empty list, matches any bracket content (not just keywords). - - Returns: - The position of the opening bracket, or None if no match found. """ if keywords is None: keywords = DEFAULT_BRACKET_KEYWORDS From 3051af9eb64c60f7334c29fdf58013055cb245b3 Mon Sep 17 00:00:00 2001 From: Trey Turner Date: Mon, 17 Nov 2025 12:56:19 -0600 Subject: [PATCH 08/39] fix: abstract insert_ft_into_title, move bracket_keywords and find_bracket_position inside plugin --- beetsplug/ftintitle.py | 117 ++++++++++++++------------------- test/plugins/test_ftintitle.py | 37 +++++++++-- 2 files changed, 79 insertions(+), 75 deletions(-) diff --git a/beetsplug/ftintitle.py b/beetsplug/ftintitle.py index 629e58f17..06c5e69be 100644 --- a/beetsplug/ftintitle.py +++ b/beetsplug/ftintitle.py @@ -17,6 +17,7 @@ from __future__ import annotations import re +from functools import cached_property from typing import TYPE_CHECKING from beets import plugins, ui @@ -84,58 +85,6 @@ DEFAULT_BRACKET_KEYWORDS = [ ] -def find_bracket_position( - title: str, keywords: list[str] | None = None -) -> int | None: - """Find the position of the first opening bracket that contains - remix/edit-related keywords and has a matching closing bracket. - """ - if keywords is None: - keywords = DEFAULT_BRACKET_KEYWORDS - - # If keywords is empty, match any bracket content - if not keywords: - pattern = None - else: - # Build regex pattern to support multi-word keywords/phrases. - keyword_pattern = rf"\b{'|'.join(map(re.escape, keywords))}\b" - pattern = re.compile(keyword_pattern, re.IGNORECASE) - - # Bracket pairs (opening, closing) - bracket_pairs = [("(", ")"), ("[", "]"), ("<", ">"), ("{", "}")] - - # Track the earliest valid bracket position - earliest_pos = None - - for open_char, close_char in bracket_pairs: - pos = 0 - while True: - # Find next opening bracket - open_pos = title.find(open_char, pos) - if open_pos == -1: - break - - # Find matching closing bracket - close_pos = title.find(close_char, open_pos + 1) - if close_pos == -1: - break - - # Extract content between brackets - content = title[open_pos + 1 : close_pos] - - # Check if content matches: if pattern is None (empty keywords), - # match any content; otherwise check for keywords - if (pattern is None or pattern.search(content)) and ( - earliest_pos is None or open_pos < earliest_pos - ): - earliest_pos = open_pos - - # Continue searching from after this closing bracket - pos = close_pos + 1 - - return earliest_pos - - def find_feat_part( artist: str, albumartist: str | None, @@ -176,6 +125,10 @@ def find_feat_part( class FtInTitlePlugin(plugins.BeetsPlugin): + @cached_property + def bracket_keywords(self) -> list[str] | None: + return self.config["bracket_keywords"].as_str_seq() + def __init__(self) -> None: super().__init__() @@ -216,7 +169,6 @@ class FtInTitlePlugin(plugins.BeetsPlugin): bool ) custom_words = self.config["custom_words"].get(list) - bracket_keywords = self.config["bracket_keywords"].get(list) write = ui.should_write() for item in lib.items(args): @@ -226,7 +178,6 @@ class FtInTitlePlugin(plugins.BeetsPlugin): keep_in_artist_field, preserve_album_artist, custom_words, - bracket_keywords, ): item.store() if write: @@ -241,7 +192,6 @@ class FtInTitlePlugin(plugins.BeetsPlugin): keep_in_artist_field = self.config["keep_in_artist"].get(bool) preserve_album_artist = self.config["preserve_album_artist"].get(bool) custom_words = self.config["custom_words"].get(list) - bracket_keywords = self.config["bracket_keywords"].get(list) for item in task.imported_items(): if self.ft_in_title( @@ -250,7 +200,6 @@ class FtInTitlePlugin(plugins.BeetsPlugin): keep_in_artist_field, preserve_album_artist, custom_words, - bracket_keywords, ): item.store() @@ -261,7 +210,6 @@ class FtInTitlePlugin(plugins.BeetsPlugin): drop_feat: bool, keep_in_artist_field: bool, custom_words: list[str], - bracket_keywords: list[str] | None = None, ) -> None: """Choose how to add new artists to the title and set the new metadata. Also, print out messages about any changes that are made. @@ -290,15 +238,8 @@ class FtInTitlePlugin(plugins.BeetsPlugin): # artist and if we do not drop featuring information. if not drop_feat and not contains_feat(item.title, custom_words): feat_format = self.config["format"].as_str() - new_format = feat_format.format(feat_part) - # Insert before the first bracket containing remix/edit keywords - bracket_pos = find_bracket_position(item.title, bracket_keywords) - if bracket_pos is not None: - title_before = item.title[:bracket_pos].rstrip() - title_after = item.title[bracket_pos:] - new_title = f"{title_before} {new_format} {title_after}" - else: - new_title = f"{item.title} {new_format}" + formatted = feat_format.format(feat_part) + new_title = self.insert_ft_into_title(item.title, formatted) self._log.info("title: {.title} -> {}", item, new_title) item.title = new_title @@ -309,7 +250,6 @@ class FtInTitlePlugin(plugins.BeetsPlugin): keep_in_artist_field: bool, preserve_album_artist: bool, custom_words: list[str], - bracket_keywords: list[str] | None = None, ) -> bool: """Look for featured artists in the item's artist fields and move them to the title. @@ -346,6 +286,47 @@ class FtInTitlePlugin(plugins.BeetsPlugin): drop_feat, keep_in_artist_field, custom_words, - bracket_keywords, ) return True + + def find_bracket_position( + self, + title: str, + ) -> int | None: + """Find the position of the first opening bracket that contains + remix/edit-related keywords and has a matching closing bracket. + """ + keywords = self.bracket_keywords + + # If keywords is empty, match any bracket content + if not keywords: + keyword_ptn = ".*?" + else: + # Build regex supporting keywords/multi-word phrases. + keyword_ptn = rf"\b{'|'.join(map(re.escape, keywords))}\b" + + pattern = re.compile( + rf""" + \(.*?({keyword_ptn}).*?\) | + \[.*?({keyword_ptn}).*?\] | + <.*?({keyword_ptn}).*?> | + \{{.*?({keyword_ptn}).*?}} + """, + re.IGNORECASE | re.VERBOSE, + ) + + return m.start() if (m := pattern.search(title)) else None + + def insert_ft_into_title( + self, + title: str, + feat_part: str, + ) -> str: + """Insert featured artist before the first bracket containing + remix/edit keywords if present. + """ + if (bracket_pos := self.find_bracket_position(title)) is not None: + title_before = title[:bracket_pos].rstrip() + title_after = title[bracket_pos:] + return f"{title_before} {feat_part} {title_after}" + return f"{title} {feat_part}" diff --git a/test/plugins/test_ftintitle.py b/test/plugins/test_ftintitle.py index 065f23ade..73853e6c3 100644 --- a/test/plugins/test_ftintitle.py +++ b/test/plugins/test_ftintitle.py @@ -15,7 +15,7 @@ """Tests for the 'ftintitle' plugin.""" from collections.abc import Generator -from typing import cast +from typing import TypeAlias, cast import pytest @@ -24,6 +24,8 @@ from beets.library.models import Item from beets.test.helper import PluginTestCase from beetsplug import ftintitle +ConfigValue: TypeAlias = str | bool | list[str] + class FtInTitlePluginFunctional(PluginTestCase): plugin = "ftintitle" @@ -41,7 +43,7 @@ def env() -> Generator[FtInTitlePluginFunctional, None, None]: def set_config( env: FtInTitlePluginFunctional, - cfg: dict[str, str | bool | list[str]] | None, + cfg: dict[str, ConfigValue] | None, ) -> None: cfg = {} if cfg is None else cfg defaults = { @@ -49,11 +51,21 @@ def set_config( "auto": True, "keep_in_artist": False, "custom_words": [], + "bracket_keywords": ftintitle.DEFAULT_BRACKET_KEYWORDS.copy(), } env.config["ftintitle"].set(defaults) env.config["ftintitle"].set(cfg) +def build_plugin( + env: FtInTitlePluginFunctional, + cfg: dict[str, ConfigValue] | None = None, +) -> ftintitle.FtInTitlePlugin: + """Instantiate plugin with provided config applied first.""" + set_config(env, cfg) + return ftintitle.FtInTitlePlugin() + + def add_item( env: FtInTitlePluginFunctional, path: str, @@ -427,7 +439,7 @@ def test_split_on_feat( ("Song (Live", None), # no matching brace with keyword # one keyword clause, one non-keyword clause ("Song (Live) (Arbitrary)", 5), - ("Song (Arbitrary) (Remix)", 17), + ("Song (Arbitrary) (Remix)", 5), # nested brackets - same type ("Song (Remix (Extended))", 5), ("Song [Arbitrary [Description]]", None), @@ -443,8 +455,13 @@ def test_split_on_feat( ("Song [Description (Arbitrary)]", None), ], ) -def test_find_bracket_position(given: str, expected: int | None) -> None: - assert ftintitle.find_bracket_position(given) == expected +def test_find_bracket_position( + env: FtInTitlePluginFunctional, + given: str, + expected: int | None, +) -> None: + plugin = build_plugin(env) + assert plugin.find_bracket_position(given) == expected @pytest.mark.parametrize( @@ -468,9 +485,15 @@ def test_find_bracket_position(given: str, expected: int | None) -> None: ], ) def test_find_bracket_position_custom_keywords( - given: str, keywords: list[str] | None, expected: int | None + env: FtInTitlePluginFunctional, + given: str, + keywords: list[str] | None, + expected: int | None, ) -> None: - assert ftintitle.find_bracket_position(given, keywords) == expected + cfg: dict[str, ConfigValue] | None + cfg = None if keywords is None else {"bracket_keywords": keywords} + plugin = build_plugin(env, cfg) + assert plugin.find_bracket_position(given) == expected @pytest.mark.parametrize( From 03f84eb877c8a15086674d446bb1e839d3c5370e Mon Sep 17 00:00:00 2001 From: Gabriel Push Date: Tue, 2 Dec 2025 20:02:17 -0500 Subject: [PATCH 09/39] Fix edit plugin cancel flow restoring in-memory tags --- beetsplug/edit.py | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/beetsplug/edit.py b/beetsplug/edit.py index 7ed465cfe..168f72da1 100644 --- a/beetsplug/edit.py +++ b/beetsplug/edit.py @@ -282,16 +282,18 @@ class EditPlugin(plugins.BeetsPlugin): if choice == "a": # Apply. return True elif choice == "c": # Cancel. + # Revert all temporary changes made in this edit session + # so that objects return to their original in-memory + # state (including tags provided by other plugins such as + # `fromfilename`). + self.apply_data(objs, new_data, old_data) return False elif choice == "e": # Keep editing. - # Reset the temporary changes to the objects. I we have a - # copy from above, use that, else reload from the database. - objs = [ - (old_obj or obj) for old_obj, obj in zip(objs_old, objs) - ] - for obj in objs: - if not obj.id < 0: - obj.load() + # Revert changes on the objects, but keep the edited YAML + # file so the user can continue editing from their last + # version. On the next iteration, differences will again + # be computed against the original state (`old_data`). + self.apply_data(objs, new_data, old_data) continue # Remove the temporary file before returning. @@ -380,9 +382,11 @@ class EditPlugin(plugins.BeetsPlugin): # to the files if needed without re-applying metadata. return Action.RETAG else: - # Edit cancelled / no edits made. Revert changes. - for obj in task.items: - obj.read() + # Edit cancelled / no edits made. `edit_objects` has already + # restored each object to its original in-memory state, so there + # is nothing more to do here. Returning None lets the importer + # resume the candidate prompt. + return None def importer_edit_candidate(self, session, task): """Callback for invoking the functionality during an interactive From 556f3932ce6f82d73a69ecf2a564abcd5e47cd7c Mon Sep 17 00:00:00 2001 From: Gabriel Push Date: Tue, 2 Dec 2025 20:06:09 -0500 Subject: [PATCH 10/39] Added documentation --- docs/changelog.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index b9a5c1f3f..327ebae8a 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -34,6 +34,9 @@ New features: 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` - :doc:`plugins/inline`: Fix recursion error when an inline field definition shadows a built-in item field (e.g., redefining ``track_no``). Inline expressions now skip self-references during evaluation to avoid infinite From 8a089b5d77950af31e788abd61d3f4abcb98533e Mon Sep 17 00:00:00 2001 From: Gabriel Push Date: Tue, 2 Dec 2025 20:21:18 -0500 Subject: [PATCH 11/39] Fixed doc --- docs/changelog.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 327ebae8a..9d794e1a2 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -34,9 +34,9 @@ New features: 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` +- 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` - :doc:`plugins/inline`: Fix recursion error when an inline field definition shadows a built-in item field (e.g., redefining ``track_no``). Inline expressions now skip self-references during evaluation to avoid infinite From b242e3d052c1f8ce834e847a823aa03dfaa606c3 Mon Sep 17 00:00:00 2001 From: Gabriel Push Date: Tue, 2 Dec 2025 21:00:18 -0500 Subject: [PATCH 12/39] Added test for new case --- test/plugins/test_edit.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/plugins/test_edit.py b/test/plugins/test_edit.py index 4e6c97ab2..8be4c29b8 100644 --- a/test/plugins/test_edit.py +++ b/test/plugins/test_edit.py @@ -176,6 +176,25 @@ class EditCommandTest(EditMixin, BeetsTestCase): ) assert list(self.album.items())[-1].title == "modified t\u00eftle 9" + def test_title_edit_keep_editing_then_apply(self, mock_write): + """Edit titles, keep editing once, then apply changes.""" + # First, choose "keep editing" so changes are reverted in memory but + # kept in the YAML file; then choose "apply" to commit them. + self.run_mocked_command( + {"replacements": {"t\u00eftle": "modified t\u00eftle"}}, + # keep Editing, then Apply + ["e", "a"], + ) + + # Writes should only happen once per track, when we finally apply. + assert mock_write.call_count == self.TRACK_COUNT + # All item titles (and mtimes) should now reflect the modified values. + self.assertItemFieldsModified( + self.album.items(), + self.items_orig, + ["title", "mtime"], + ) + def test_noedit(self, mock_write): """Do not edit anything.""" # Do not edit anything. From cefb4bfe225459c4d36c72f82af6e26b5371097b Mon Sep 17 00:00:00 2001 From: Gabriel Push Date: Tue, 9 Dec 2025 12:12:53 -0500 Subject: [PATCH 13/39] Fix verbose comments and add e,c test --- beetsplug/edit.py | 15 ++------------- test/plugins/test_edit.py | 19 +++++++++++++++---- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/beetsplug/edit.py b/beetsplug/edit.py index 168f72da1..46e756122 100644 --- a/beetsplug/edit.py +++ b/beetsplug/edit.py @@ -275,24 +275,17 @@ class EditPlugin(plugins.BeetsPlugin): ui.print_("No changes to apply.") return False - # Confirm the changes. + # For cancel/keep-editing, restore objects to their original + # in-memory state so temp edits don't leak into the session choice = ui.input_options( ("continue Editing", "apply", "cancel") ) if choice == "a": # Apply. return True elif choice == "c": # Cancel. - # Revert all temporary changes made in this edit session - # so that objects return to their original in-memory - # state (including tags provided by other plugins such as - # `fromfilename`). self.apply_data(objs, new_data, old_data) return False elif choice == "e": # Keep editing. - # Revert changes on the objects, but keep the edited YAML - # file so the user can continue editing from their last - # version. On the next iteration, differences will again - # be computed against the original state (`old_data`). self.apply_data(objs, new_data, old_data) continue @@ -382,10 +375,6 @@ class EditPlugin(plugins.BeetsPlugin): # to the files if needed without re-applying metadata. return Action.RETAG else: - # Edit cancelled / no edits made. `edit_objects` has already - # restored each object to its original in-memory state, so there - # is nothing more to do here. Returning None lets the importer - # resume the candidate prompt. return None def importer_edit_candidate(self, session, task): diff --git a/test/plugins/test_edit.py b/test/plugins/test_edit.py index 8be4c29b8..d0e03d0e5 100644 --- a/test/plugins/test_edit.py +++ b/test/plugins/test_edit.py @@ -178,23 +178,34 @@ class EditCommandTest(EditMixin, BeetsTestCase): def test_title_edit_keep_editing_then_apply(self, mock_write): """Edit titles, keep editing once, then apply changes.""" - # First, choose "keep editing" so changes are reverted in memory but - # kept in the YAML file; then choose "apply" to commit them. self.run_mocked_command( {"replacements": {"t\u00eftle": "modified t\u00eftle"}}, # keep Editing, then Apply ["e", "a"], ) - # Writes should only happen once per track, when we finally apply. assert mock_write.call_count == self.TRACK_COUNT - # All item titles (and mtimes) should now reflect the modified values. self.assertItemFieldsModified( self.album.items(), self.items_orig, ["title", "mtime"], ) + def test_title_edit_keep_editing_then_cancel(self, mock_write): + """Edit titles, keep editing once, then cancel.""" + self.run_mocked_command( + {"replacements": {"t\u00eftle": "modified t\u00eftle"}}, + # keep Editing, then Cancel + ["e", "c"], + ) + + assert mock_write.call_count == 0 + self.assertItemFieldsModified( + self.album.items(), + self.items_orig, + [], + ) + def test_noedit(self, mock_write): """Do not edit anything.""" # Do not edit anything. From 84d37b820a9b80259185da9be172a0d6aa85fe8f Mon Sep 17 00:00:00 2001 From: Trey Turner Date: Sun, 14 Dec 2025 18:15:08 -0600 Subject: [PATCH 14/39] fix: inline default bracket_keywords instead of defining/cloning constant --- beetsplug/ftintitle.py | 48 +++++++++++++++++++----------------------- 1 file changed, 22 insertions(+), 26 deletions(-) diff --git a/beetsplug/ftintitle.py b/beetsplug/ftintitle.py index 06c5e69be..a81c58574 100644 --- a/beetsplug/ftintitle.py +++ b/beetsplug/ftintitle.py @@ -60,31 +60,6 @@ def contains_feat(title: str, custom_words: list[str] | None = None) -> bool: ) -# Default keywords that indicate remix/edit/version content -DEFAULT_BRACKET_KEYWORDS = [ - "abridged", - "acapella", - "club", - "demo", - "edit", - "edition", - "extended", - "instrumental", - "live", - "mix", - "radio", - "release", - "remaster", - "remastered", - "remix", - "rmx", - "unabridged", - "unreleased", - "version", - "vip", -] - - def find_feat_part( artist: str, albumartist: str | None, @@ -140,7 +115,28 @@ class FtInTitlePlugin(plugins.BeetsPlugin): "keep_in_artist": False, "preserve_album_artist": True, "custom_words": [], - "bracket_keywords": DEFAULT_BRACKET_KEYWORDS.copy(), + "bracket_keywords": [ + "abridged", + "acapella", + "club", + "demo", + "edit", + "edition", + "extended", + "instrumental", + "live", + "mix", + "radio", + "release", + "remaster", + "remastered", + "remix", + "rmx", + "unabridged", + "unreleased", + "version", + "vip", + ], } ) From ef40d1ac536bd979392acd4236f08203242c8616 Mon Sep 17 00:00:00 2001 From: Trey Turner Date: Sun, 14 Dec 2025 18:19:38 -0600 Subject: [PATCH 15/39] fix: revert needless whitespace change --- beetsplug/ftintitle.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/beetsplug/ftintitle.py b/beetsplug/ftintitle.py index a81c58574..e6c8c897a 100644 --- a/beetsplug/ftintitle.py +++ b/beetsplug/ftintitle.py @@ -277,11 +277,7 @@ class FtInTitlePlugin(plugins.BeetsPlugin): # If we have a featuring artist, move it to the title. self.update_metadata( - item, - feat_part, - drop_feat, - keep_in_artist_field, - custom_words, + item, feat_part, drop_feat, keep_in_artist_field, custom_words ) return True From 00792922b58b6a68d4c5e5fc32416ea926a98290 Mon Sep 17 00:00:00 2001 From: Trey Turner Date: Sat, 20 Dec 2025 02:19:54 -0600 Subject: [PATCH 16/39] fix: address remaining review comments --- beetsplug/ftintitle.py | 140 +++++++++++++++--------- test/plugins/test_ftintitle.py | 194 ++++++++------------------------- 2 files changed, 130 insertions(+), 204 deletions(-) diff --git a/beetsplug/ftintitle.py b/beetsplug/ftintitle.py index e6c8c897a..b8bd3e261 100644 --- a/beetsplug/ftintitle.py +++ b/beetsplug/ftintitle.py @@ -17,7 +17,7 @@ from __future__ import annotations import re -from functools import cached_property +from functools import cached_property, lru_cache from typing import TYPE_CHECKING from beets import plugins, ui @@ -99,11 +99,78 @@ def find_feat_part( return feat_part +DEFAULT_BRACKET_KEYWORDS: tuple[str, ...] = ( + "abridged", + "acapella", + "club", + "demo", + "edit", + "edition", + "extended", + "instrumental", + "live", + "mix", + "radio", + "release", + "remaster", + "remastered", + "remix", + "rmx", + "unabridged", + "unreleased", + "version", + "vip", +) + + class FtInTitlePlugin(plugins.BeetsPlugin): @cached_property - def bracket_keywords(self) -> list[str] | None: + def bracket_keywords(self) -> list[str]: return self.config["bracket_keywords"].as_str_seq() + @staticmethod + @lru_cache(maxsize=256) + def _bracket_position_pattern(keywords: tuple[str, ...]) -> re.Pattern[str]: + """ + Build a compiled regex to find the first bracketed segment that contains + any of the provided keywords. + + Cached by keyword tuple to avoid recompiling on every track/title. + """ + kw_inner = "|".join(map(re.escape, keywords)) + + # If we have keywords, require one of them to appear in the bracket text. + # If kw == "", the lookahead becomes trivially true and we match any bracket content. + kw = rf"\b(?:{kw_inner})\b" if kw_inner else "" + + return re.compile( + rf""" + (?: # Match ONE bracketed segment of any supported type + \( # "(" + (?=[^)]*{kw}) # Lookahead: keyword must appear before closing ")" + # - if kw == "", this is always true + [^)]* # Consume bracket content (no nested ")" handling) + \) # ")" + + | \[ # "[" + (?=[^\]]*{kw}) # Lookahead + [^\]]* # Consume content up to first "]" + \] # "]" + + | < # "<" + (?=[^>]*{kw}) # Lookahead + [^>]* # Consume content up to first ">" + > # ">" + + | \x7B # Literal open brace + (?=[^\x7D]*{kw}) # Lookahead + [^\x7D]* # Consume content up to first close brace + \x7D # Literal close brace + ) # End bracketed segment alternation + """, + re.IGNORECASE | re.VERBOSE, + ) + def __init__(self) -> None: super().__init__() @@ -115,28 +182,7 @@ class FtInTitlePlugin(plugins.BeetsPlugin): "keep_in_artist": False, "preserve_album_artist": True, "custom_words": [], - "bracket_keywords": [ - "abridged", - "acapella", - "club", - "demo", - "edit", - "edition", - "extended", - "instrumental", - "live", - "mix", - "radio", - "release", - "remaster", - "remastered", - "remix", - "rmx", - "unabridged", - "unreleased", - "version", - "vip", - ], + "bracket_keywords": list(DEFAULT_BRACKET_KEYWORDS), } ) @@ -235,7 +281,9 @@ class FtInTitlePlugin(plugins.BeetsPlugin): if not drop_feat and not contains_feat(item.title, custom_words): feat_format = self.config["format"].as_str() formatted = feat_format.format(feat_part) - new_title = self.insert_ft_into_title(item.title, formatted) + new_title = FtInTitlePlugin.insert_ft_into_title( + item.title, formatted, self.bracket_keywords + ) self._log.info("title: {.title} -> {}", item, new_title) item.title = new_title @@ -281,43 +329,29 @@ class FtInTitlePlugin(plugins.BeetsPlugin): ) return True + @staticmethod def find_bracket_position( - self, - title: str, + title: str, keywords: list[str] | None = None ) -> int | None: - """Find the position of the first opening bracket that contains - remix/edit-related keywords and has a matching closing bracket. - """ - keywords = self.bracket_keywords - - # If keywords is empty, match any bracket content - if not keywords: - keyword_ptn = ".*?" - else: - # Build regex supporting keywords/multi-word phrases. - keyword_ptn = rf"\b{'|'.join(map(re.escape, keywords))}\b" - - pattern = re.compile( - rf""" - \(.*?({keyword_ptn}).*?\) | - \[.*?({keyword_ptn}).*?\] | - <.*?({keyword_ptn}).*?> | - \{{.*?({keyword_ptn}).*?}} - """, - re.IGNORECASE | re.VERBOSE, + normalized = ( + DEFAULT_BRACKET_KEYWORDS if keywords is None else tuple(keywords) ) + pattern = FtInTitlePlugin._bracket_position_pattern(normalized) + m: re.Match[str] | None = pattern.search(title) + return m.start() if m else None - return m.start() if (m := pattern.search(title)) else None - + @staticmethod def insert_ft_into_title( - self, - title: str, - feat_part: str, + title: str, feat_part: str, keywords: list[str] | None = None ) -> str: """Insert featured artist before the first bracket containing remix/edit keywords if present. """ - if (bracket_pos := self.find_bracket_position(title)) is not None: + if ( + bracket_pos := FtInTitlePlugin.find_bracket_position( + title, keywords + ) + ) is not None: title_before = title[:bracket_pos].rstrip() title_after = title[bracket_pos:] return f"{title_before} {feat_part} {title_after}" diff --git a/test/plugins/test_ftintitle.py b/test/plugins/test_ftintitle.py index 73853e6c3..9d6b54a93 100644 --- a/test/plugins/test_ftintitle.py +++ b/test/plugins/test_ftintitle.py @@ -15,11 +15,10 @@ """Tests for the 'ftintitle' plugin.""" from collections.abc import Generator -from typing import TypeAlias, cast +from typing import TypeAlias import pytest -from beets.importer import ImportSession, ImportTask from beets.library.models import Item from beets.test.helper import PluginTestCase from beetsplug import ftintitle @@ -51,21 +50,11 @@ def set_config( "auto": True, "keep_in_artist": False, "custom_words": [], - "bracket_keywords": ftintitle.DEFAULT_BRACKET_KEYWORDS.copy(), } env.config["ftintitle"].set(defaults) env.config["ftintitle"].set(cfg) -def build_plugin( - env: FtInTitlePluginFunctional, - cfg: dict[str, ConfigValue] | None = None, -) -> ftintitle.FtInTitlePlugin: - """Instantiate plugin with provided config applied first.""" - set_config(env, cfg) - return ftintitle.FtInTitlePlugin() - - def add_item( env: FtInTitlePluginFunctional, path: str, @@ -82,16 +71,6 @@ def add_item( ) -class DummyImportTask: - """Minimal stand-in for ImportTask used to exercise import hooks.""" - - def __init__(self, items: list[Item]) -> None: - self._items = items - - def imported_items(self) -> list[Item]: - return self._items - - @pytest.mark.parametrize( "cfg, cmd_args, given, expected", [ @@ -272,61 +251,18 @@ class DummyImportTask: ), # ---- titles with brackets/parentheses ---- pytest.param( - {"format": "ft. {}"}, - ("ftintitle",), - ("Alice ft. Bob", "Song 1 (Carol Remix)", "Alice"), - ("Alice", "Song 1 ft. Bob (Carol Remix)"), - id="title-with-brackets-insert-before", - ), - pytest.param( - {"format": "ft. {}", "keep_in_artist": True}, - ("ftintitle",), - ("Alice ft. Bob", "Song 1 (Carol Remix)", "Alice"), - ("Alice ft. Bob", "Song 1 ft. Bob (Carol Remix)"), - id="title-with-brackets-keep-in-artist", - ), - pytest.param( - {"format": "ft. {}"}, - ("ftintitle",), - ("Alice ft. Bob", "Song 1 (Remix) (Live)", "Alice"), - ("Alice", "Song 1 ft. Bob (Remix) (Live)"), - id="title-with-multiple-brackets-uses-first-with-keyword", - ), - pytest.param( - {"format": "ft. {}"}, - ("ftintitle",), - ("Alice ft. Bob", "Song 1 (Arbitrary)", "Alice"), - ("Alice", "Song 1 (Arbitrary) ft. Bob"), - id="title-with-brackets-no-keyword-appends", - ), - pytest.param( - {"format": "ft. {}"}, - ("ftintitle",), - ("Alice ft. Bob", "Song 1 [Edit]", "Alice"), - ("Alice", "Song 1 ft. Bob [Edit]"), - id="title-with-square-brackets-keyword", - ), - pytest.param( - {"format": "ft. {}"}, - ("ftintitle",), - ("Alice ft. Bob", "Song 1 ", "Alice"), - ("Alice", "Song 1 ft. Bob "), - id="title-with-angle-brackets-keyword", - ), - # multi-word keyword - pytest.param( - {"format": "ft. {}", "bracket_keywords": ["club mix"]}, + {"format": "ft. {}", "bracket_keywords": ["mix"]}, ("ftintitle",), ("Alice ft. Bob", "Song 1 (Club Mix)", "Alice"), ("Alice", "Song 1 ft. Bob (Club Mix)"), - id="multi-word-keyword-positive-match", + id="ft-inserted-before-matching-bracket-keyword", ), pytest.param( - {"format": "ft. {}", "bracket_keywords": ["club mix"]}, + {"format": "ft. {}", "bracket_keywords": ["nomatch"]}, ("ftintitle",), ("Alice ft. Bob", "Song 1 (Club Remix)", "Alice"), ("Alice", "Song 1 (Club Remix) ft. Bob"), - id="multi-word-keyword-negative-no-match", + id="ft-inserted-at-end-no-bracket-keyword-match", ), ], ) @@ -351,31 +287,6 @@ def test_ftintitle_functional( assert item["title"] == expected_title -def test_imported_stage_moves_featured_artist( - env: FtInTitlePluginFunctional, -) -> None: - """The import-stage hook should fetch config settings and process items.""" - set_config(env, None) - plugin = ftintitle.FtInTitlePlugin() - item = add_item( - env, - "/imported-hook", - "Alice feat. Bob", - "Song 1 (Carol Remix)", - "Various Artists", - ) - task = DummyImportTask([item]) - - plugin.imported( - cast(ImportSession, None), - cast(ImportTask, task), - ) - item.load() - - assert item["artist"] == "Alice" - assert item["title"] == "Song 1 feat. Bob (Carol Remix)" - - @pytest.mark.parametrize( "artist,albumartist,expected", [ @@ -419,64 +330,46 @@ def test_split_on_feat( assert ftintitle.split_on_feat(given) == expected -@pytest.mark.parametrize( - "given,expected", - [ - # different braces and keywords - ("Song (Remix)", 5), - ("Song [Version]", 5), - ("Song {Extended Mix}", 5), - ("Song ", 5), - # two keyword clauses - ("Song (Remix) (Live)", 5), - # brace insensitivity - ("Song (Live) [Remix]", 5), - ("Song [Edit] (Remastered)", 5), - # negative cases - ("Song", None), # no clause - ("Song (Arbitrary)", None), # no keyword - ("Song (", None), # no matching brace or keyword - ("Song (Live", None), # no matching brace with keyword - # one keyword clause, one non-keyword clause - ("Song (Live) (Arbitrary)", 5), - ("Song (Arbitrary) (Remix)", 5), - # nested brackets - same type - ("Song (Remix (Extended))", 5), - ("Song [Arbitrary [Description]]", None), - # nested brackets - different types - ("Song (Remix [Extended])", 5), - # nested - returns outer start position despite inner keyword - ("Song [Arbitrary {Extended}]", 5), - ("Song {Live }", 5), - ("Song ", 5), - ("Song [Live]", 5), - ("Song (Version) ", 5), - ("Song (Arbitrary [Description])", None), - ("Song [Description (Arbitrary)]", None), - ], -) -def test_find_bracket_position( - env: FtInTitlePluginFunctional, - given: str, - expected: int | None, -) -> None: - plugin = build_plugin(env) - assert plugin.find_bracket_position(given) == expected - - @pytest.mark.parametrize( "given,keywords,expected", [ + ## default keywords + # different braces and keywords + ("Song (Remix)", None, 5), + ("Song [Version]", None, 5), + ("Song {Extended Mix}", None, 5), + ("Song ", None, 5), + # two keyword clauses + ("Song (Remix) (Live)", None, 5), + # brace insensitivity + ("Song (Live) [Remix]", None, 5), + ("Song [Edit] (Remastered)", None, 5), + # negative cases + ("Song", None, None), # no clause + ("Song (Arbitrary)", None, None), # no keyword + ("Song (", None, None), # no matching brace or keyword + ("Song (Live", None, None), # no matching brace with keyword + # one keyword clause, one non-keyword clause + ("Song (Live) (Arbitrary)", None, 5), + ("Song (Arbitrary) (Remix)", None, 17), + # nested brackets - same type + ("Song (Remix (Extended))", None, 5), + ("Song [Arbitrary [Description]]", None, None), + # nested brackets - different types + ("Song (Remix [Extended])", None, 5), + # nested - returns outer start position despite inner keyword + ("Song [Arbitrary {Extended}]", None, 5), + ("Song {Live }", None, 5), + ("Song ", None, 5), + ("Song [Live]", None, 5), + ("Song (Version) ", None, 5), + ("Song (Arbitrary [Description])", None, None), + ("Song [Description (Arbitrary)]", None, None), + ## custom keywords ("Song (Live)", ["live"], 5), - ("Song (Live)", None, 5), - ("Song (Arbitrary)", None, None), ("Song (Concert)", ["concert"], 5), - ("Song (Concert)", None, None), ("Song (Remix)", ["custom"], None), ("Song (Custom)", ["custom"], 5), - ("Song (Live)", [], 5), - ("Song (Anything)", [], 5), - ("Song (Remix)", [], 5), ("Song", [], None), ("Song (", [], None), # Multi-word keyword tests @@ -484,16 +377,15 @@ def test_find_bracket_position( ("Song (Club Remix)", ["club mix"], None), # Negative: no match ], ) -def test_find_bracket_position_custom_keywords( - env: FtInTitlePluginFunctional, +def test_find_bracket_position( given: str, keywords: list[str] | None, expected: int | None, ) -> None: - cfg: dict[str, ConfigValue] | None - cfg = None if keywords is None else {"bracket_keywords": keywords} - plugin = build_plugin(env, cfg) - assert plugin.find_bracket_position(given) == expected + assert ( + ftintitle.FtInTitlePlugin.find_bracket_position(given, keywords) + == expected + ) @pytest.mark.parametrize( From c0c7a9df8f487b7ca029a9f44800090c4a0e850f Mon Sep 17 00:00:00 2001 From: Trey Turner Date: Sat, 20 Dec 2025 02:34:15 -0600 Subject: [PATCH 17/39] fix: line length --- beetsplug/ftintitle.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beetsplug/ftintitle.py b/beetsplug/ftintitle.py index eafc9e191..44f17bc4e 100644 --- a/beetsplug/ftintitle.py +++ b/beetsplug/ftintitle.py @@ -145,7 +145,7 @@ class FtInTitlePlugin(plugins.BeetsPlugin): kw_inner = "|".join(map(re.escape, keywords)) # If we have keywords, require one of them to appear in the bracket text. - # If kw == "", the lookahead becomes trivially true and we match any bracket content. + # If kw == "", the lookahead becomes true and we match any bracket content. kw = rf"\b(?:{kw_inner})\b" if kw_inner else "" return re.compile( From ea157832fe3704efe105f004ee39c6423db65d69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Sun, 26 Oct 2025 01:57:15 +0000 Subject: [PATCH 18/39] hooks: make AlbumMatch.mapping a tuple --- beets/autotag/__init__.py | 12 ++++++------ beets/autotag/distance.py | 8 ++++---- beets/autotag/hooks.py | 8 ++++++++ beets/autotag/match.py | 12 +++++++----- beets/importer/tasks.py | 8 ++++---- beets/ui/commands/import_/display.py | 5 +++-- beetsplug/bpsync.py | 8 ++++---- beetsplug/mbpseudo.py | 7 ++----- beetsplug/mbsync.py | 12 +++++++----- test/autotag/test_distance.py | 2 +- test/test_autotag.py | 14 +++++++------- test/ui/commands/test_import.py | 8 +++++--- 12 files changed, 58 insertions(+), 46 deletions(-) diff --git a/beets/autotag/__init__.py b/beets/autotag/__init__.py index beaf4341c..feeefbf28 100644 --- a/beets/autotag/__init__.py +++ b/beets/autotag/__init__.py @@ -29,7 +29,7 @@ from .hooks import AlbumInfo, AlbumMatch, TrackInfo, TrackMatch from .match import Proposal, Recommendation, tag_album, tag_item if TYPE_CHECKING: - from collections.abc import Mapping, Sequence + from collections.abc import Sequence from beets.library import Album, Item, LibModel @@ -204,11 +204,11 @@ def apply_album_metadata(album_info: AlbumInfo, album: Album): correct_list_fields(album) -def apply_metadata(album_info: AlbumInfo, mapping: Mapping[Item, TrackInfo]): - """Set the items' metadata to match an AlbumInfo object using a - mapping from Items to TrackInfo objects. - """ - for item, track_info in mapping.items(): +def apply_metadata( + album_info: AlbumInfo, item_info_pairs: list[tuple[Item, TrackInfo]] +): + """Set items metadata to match corresponding tagged info.""" + for item, track_info in item_info_pairs: # Artist or artist credit. if config["artist_credit"]: item.artist = ( diff --git a/beets/autotag/distance.py b/beets/autotag/distance.py index 37c6f84f4..5e3f630e3 100644 --- a/beets/autotag/distance.py +++ b/beets/autotag/distance.py @@ -422,7 +422,7 @@ def track_distance( def distance( items: Sequence[Item], album_info: AlbumInfo, - mapping: dict[Item, TrackInfo], + item_info_pairs: list[tuple[Item, TrackInfo]], ) -> Distance: """Determines how "significant" an album metadata change would be. Returns a Distance object. `album_info` is an AlbumInfo object @@ -518,16 +518,16 @@ def distance( # Tracks. dist.tracks = {} - for item, track in mapping.items(): + for item, track in item_info_pairs: dist.tracks[track] = track_distance(item, track, album_info.va) dist.add("tracks", dist.tracks[track].distance) # Missing tracks. - for _ in range(len(album_info.tracks) - len(mapping)): + for _ in range(len(album_info.tracks) - len(item_info_pairs)): dist.add("missing_tracks", 1.0) # Unmatched tracks. - for _ in range(len(items) - len(mapping)): + for _ in range(len(items) - len(item_info_pairs)): dist.add("unmatched_tracks", 1.0) dist.add_data_source(likelies["data_source"], album_info.data_source) diff --git a/beets/autotag/hooks.py b/beets/autotag/hooks.py index b809609ea..b5d3e866c 100644 --- a/beets/autotag/hooks.py +++ b/beets/autotag/hooks.py @@ -223,6 +223,14 @@ class AlbumMatch(NamedTuple): extra_items: list[Item] extra_tracks: list[TrackInfo] + @property + def item_info_pairs(self) -> list[tuple[Item, TrackInfo]]: + return list(self.mapping.items()) + + @property + def items(self) -> list[Item]: + return [i for i, _ in self.item_info_pairs] + class TrackMatch(NamedTuple): distance: Distance diff --git a/beets/autotag/match.py b/beets/autotag/match.py index d0f3fd134..acbcca5ac 100644 --- a/beets/autotag/match.py +++ b/beets/autotag/match.py @@ -69,7 +69,7 @@ class Proposal(NamedTuple): def assign_items( items: Sequence[Item], tracks: Sequence[TrackInfo], -) -> tuple[dict[Item, TrackInfo], list[Item], list[TrackInfo]]: +) -> tuple[list[tuple[Item, TrackInfo]], list[Item], list[TrackInfo]]: """Given a list of Items and a list of TrackInfo objects, find the best mapping between them. Returns a mapping from Items to TrackInfo objects, a set of extra Items, and a set of extra TrackInfo @@ -95,7 +95,7 @@ def assign_items( extra_items.sort(key=lambda i: (i.disc, i.track, i.title)) extra_tracks = list(set(tracks) - set(mapping.values())) extra_tracks.sort(key=lambda t: (t.index, t.title)) - return mapping, extra_items, extra_tracks + return list(mapping.items()), extra_items, extra_tracks def match_by_id(items: Iterable[Item]) -> AlbumInfo | None: @@ -217,10 +217,12 @@ def _add_candidate( return # Find mapping between the items and the track info. - mapping, extra_items, extra_tracks = assign_items(items, info.tracks) + item_info_pairs, extra_items, extra_tracks = assign_items( + items, info.tracks + ) # Get the change distance. - dist = distance(items, info, mapping) + dist = distance(items, info, item_info_pairs) # Skip matches with ignored penalties. penalties = [key for key, _ in dist] @@ -232,7 +234,7 @@ def _add_candidate( log.debug("Success. Distance: {}", dist) results[info.album_id] = hooks.AlbumMatch( - dist, info, mapping, extra_items, extra_tracks + dist, info, dict(item_info_pairs), extra_items, extra_tracks ) diff --git a/beets/importer/tasks.py b/beets/importer/tasks.py index 9f60d7619..3a9c044b2 100644 --- a/beets/importer/tasks.py +++ b/beets/importer/tasks.py @@ -245,21 +245,21 @@ class ImportTask(BaseImportTask): matched items. """ if self.choice_flag in (Action.ASIS, Action.RETAG): - return list(self.items) + return self.items elif self.choice_flag == Action.APPLY and isinstance( self.match, autotag.AlbumMatch ): - return list(self.match.mapping.keys()) + return self.match.items else: assert False def apply_metadata(self): """Copy metadata from match info to the items.""" if config["import"]["from_scratch"]: - for item in self.match.mapping: + for item in self.match.items: item.clear() - autotag.apply_metadata(self.match.info, self.match.mapping) + autotag.apply_metadata(self.match.info, self.match.item_info_pairs) def duplicate_items(self, lib: library.Library): duplicate_items = [] diff --git a/beets/ui/commands/import_/display.py b/beets/ui/commands/import_/display.py index a12f1f8d3..fd6758b54 100644 --- a/beets/ui/commands/import_/display.py +++ b/beets/ui/commands/import_/display.py @@ -373,8 +373,9 @@ class AlbumChange(ChangeRepresentation): # Tracks. # match is an AlbumMatch NamedTuple, mapping is a dict # Sort the pairs by the track_info index (at index 1 of the NamedTuple) - pairs = list(self.match.mapping.items()) - pairs.sort(key=lambda item_and_track_info: item_and_track_info[1].index) + pairs = sorted( + self.match.item_info_pairs, key=lambda pair: pair[1].index + ) # Build up LHS and RHS for track difference display. The `lines` list # contains `(left, right)` tuples. lines = [] diff --git a/beetsplug/bpsync.py b/beetsplug/bpsync.py index 9ae6d47d5..fbdf8cc70 100644 --- a/beetsplug/bpsync.py +++ b/beetsplug/bpsync.py @@ -149,14 +149,14 @@ class BPSyncPlugin(BeetsPlugin): library_trackid_to_item = { int(item.mb_trackid): item for item in items } - item_to_trackinfo = { - item: beatport_trackid_to_trackinfo[track_id] + item_info_pairs = [ + (item, beatport_trackid_to_trackinfo[track_id]) for track_id, item in library_trackid_to_item.items() - } + ] self._log.info("applying changes to {}", album) with lib.transaction(): - autotag.apply_metadata(albuminfo, item_to_trackinfo) + autotag.apply_metadata(albuminfo, item_info_pairs) changed = False # Find any changed item to apply Beatport changes to album. any_changed_item = items[0] diff --git a/beetsplug/mbpseudo.py b/beetsplug/mbpseudo.py index 94b6f09a0..b61af2cc7 100644 --- a/beetsplug/mbpseudo.py +++ b/beetsplug/mbpseudo.py @@ -264,11 +264,8 @@ class MusicBrainzPseudoReleasePlugin(MusicBrainzPlugin): album_info.album_id, ) album_info.use_pseudo_as_ref() - mapping = match.mapping - new_mappings, _, _ = assign_items( - list(mapping.keys()), album_info.tracks - ) - mapping.update(new_mappings) + new_pairs, *_ = assign_items(match.items, album_info.tracks) + album_info.mapping = dict(new_pairs) if album_info.data_source == self.data_source: album_info.data_source = "MusicBrainz" diff --git a/beetsplug/mbsync.py b/beetsplug/mbsync.py index 3f7daec6c..5b74b67c9 100644 --- a/beetsplug/mbsync.py +++ b/beetsplug/mbsync.py @@ -121,18 +121,20 @@ class MBSyncPlugin(BeetsPlugin): # Construct a track mapping according to MBIDs (release track MBIDs # first, if available, and recording MBIDs otherwise). This should # work for albums that have missing or extra tracks. - mapping = {} + item_info_pairs = [] items = list(album.items()) for item in items: if ( item.mb_releasetrackid and item.mb_releasetrackid in releasetrack_index ): - mapping[item] = releasetrack_index[item.mb_releasetrackid] + item_info_pairs.append( + (item, releasetrack_index[item.mb_releasetrackid]) + ) else: candidates = track_index[item.mb_trackid] if len(candidates) == 1: - mapping[item] = candidates[0] + item_info_pairs.append((item, candidates[0])) else: # If there are multiple copies of a recording, they are # disambiguated using their disc and track number. @@ -141,13 +143,13 @@ class MBSyncPlugin(BeetsPlugin): c.medium_index == item.track and c.medium == item.disc ): - mapping[item] = c + item_info_pairs.append((item, c)) break # Apply. self._log.debug("applying changes to {}", album) with lib.transaction(): - autotag.apply_metadata(album_info, mapping) + autotag.apply_metadata(album_info, item_info_pairs) changed = False # Find any changed item to apply changes to album. any_changed_item = items[0] diff --git a/test/autotag/test_distance.py b/test/autotag/test_distance.py index 213d32956..9a658f5e1 100644 --- a/test/autotag/test_distance.py +++ b/test/autotag/test_distance.py @@ -182,7 +182,7 @@ class TestAlbumDistance: @pytest.fixture def get_dist(self, items): def inner(info: AlbumInfo): - return distance(items, info, dict(zip(items, info.tracks))) + return distance(items, info, list(zip(items, info.tracks))) return inner diff --git a/test/test_autotag.py b/test/test_autotag.py index 8d467e5ed..48ae09ccb 100644 --- a/test/test_autotag.py +++ b/test/test_autotag.py @@ -55,10 +55,12 @@ class TestAssignment(ConfigMixin): items = [Item(title=title) for title in item_titles] tracks = [TrackInfo(title=title) for title in track_titles] - mapping, extra_items, extra_tracks = match.assign_items(items, tracks) + item_info_pairs, extra_items, extra_tracks = match.assign_items( + items, tracks + ) assert ( - {i.title: t.title for i, t in mapping.items()}, + {i.title: t.title for i, t in item_info_pairs}, [i.title for i in extra_items], [t.title for t in extra_tracks], ) == (expected_mapping, expected_extra_items, expected_extra_tracks) @@ -105,7 +107,7 @@ class TestAssignment(ConfigMixin): trackinfo.append(info(11, "Beloved One", 243.733)) trackinfo.append(info(12, "In the Lord's Arms", 186.13300000000001)) - expected = dict(zip(items, trackinfo)), [], [] + expected = list(zip(items, trackinfo)), [], [] assert match.assign_items(items, trackinfo) == expected @@ -113,12 +115,10 @@ class TestAssignment(ConfigMixin): class ApplyTestUtil: def _apply(self, info=None, per_disc_numbering=False, artist_credit=False): info = info or self.info - mapping = {} - for i, t in zip(self.items, info.tracks): - mapping[i] = t + item_info_pairs = list(zip(self.items, info.tracks)) config["per_disc_numbering"] = per_disc_numbering config["artist_credit"] = artist_credit - autotag.apply_metadata(info, mapping) + autotag.apply_metadata(info, item_info_pairs) class ApplyTest(BeetsTestCase, ApplyTestUtil): diff --git a/test/ui/commands/test_import.py b/test/ui/commands/test_import.py index d74d2d816..6e96c3bf3 100644 --- a/test/ui/commands/test_import.py +++ b/test/ui/commands/test_import.py @@ -87,15 +87,17 @@ class ShowChangeTest(IOMixin, unittest.TestCase): """Return an unicode string representing the changes""" items = items or self.items info = info or self.info - mapping = dict(zip(items, info.tracks)) + item_info_pairs = list(zip(items, info.tracks)) config["ui"]["color"] = color config["import"]["detail"] = True - change_dist = distance(items, info, mapping) + change_dist = distance(items, info, item_info_pairs) change_dist._penalties = {"album": [dist], "artist": [dist]} show_change( cur_artist, cur_album, - autotag.AlbumMatch(change_dist, info, mapping, set(), set()), + autotag.AlbumMatch( + change_dist, info, dict(item_info_pairs), set(), set() + ), ) return self.io.getoutput().lower() From acc7c2aeacc37a87493b8a690ad6f597c334ad01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Sun, 26 Oct 2025 02:28:51 +0000 Subject: [PATCH 19/39] matching: replace search_title, search_album with search_name --- beets/autotag/match.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/beets/autotag/match.py b/beets/autotag/match.py index acbcca5ac..8adbaeda1 100644 --- a/beets/autotag/match.py +++ b/beets/autotag/match.py @@ -241,7 +241,7 @@ def _add_candidate( def tag_album( items, search_artist: str | None = None, - search_album: str | None = None, + search_name: str | None = None, search_ids: list[str] = [], ) -> tuple[str, str, Proposal]: """Return a tuple of the current artist name, the current album @@ -302,10 +302,10 @@ def tag_album( ) # Search terms. - if not (search_artist and search_album): + if not (search_artist and search_name): # No explicit search terms -- use current metadata. - search_artist, search_album = cur_artist, cur_album - log.debug("Search terms: {} - {}", search_artist, search_album) + search_artist, search_name = cur_artist, cur_album + log.debug("Search terms: {} - {}", search_artist, search_name) # Is this album likely to be a "various artist" release? va_likely = ( @@ -317,7 +317,7 @@ def tag_album( # Get the results from the data sources. for matched_candidate in metadata_plugins.candidates( - items, search_artist, search_album, va_likely + items, search_artist, search_name, va_likely ): _add_candidate(items, candidates, matched_candidate) if opt_candidate := candidates.get(matched_candidate.album_id): @@ -333,7 +333,7 @@ def tag_album( def tag_item( item, search_artist: str | None = None, - search_title: str | None = None, + search_name: str | None = None, search_ids: list[str] | None = None, ) -> Proposal: """Find metadata for a single track. Return a `Proposal` consisting @@ -375,12 +375,12 @@ def tag_item( # Search terms. search_artist = search_artist or item.artist - search_title = search_title or item.title - log.debug("Item search terms: {} - {}", search_artist, search_title) + search_name = search_name or item.title + log.debug("Item search terms: {} - {}", search_artist, search_name) # Get and evaluate candidate metadata. for track_info in metadata_plugins.item_candidates( - item, search_artist, search_title + item, search_artist, search_name ): dist = track_distance(item, track_info, incl_artist=True) candidates[track_info.track_id] = hooks.TrackMatch(dist, track_info) From 84f6ada739a94f96f75be9dfdcfc92a7a4d547b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Wed, 21 May 2025 01:34:32 +0100 Subject: [PATCH 20/39] hooks: Generalise AlbumMatch and TrackMatch into Match --- beets/autotag/hooks.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/beets/autotag/hooks.py b/beets/autotag/hooks.py index b5d3e866c..aae4846ca 100644 --- a/beets/autotag/hooks.py +++ b/beets/autotag/hooks.py @@ -17,7 +17,8 @@ from __future__ import annotations from copy import deepcopy -from typing import TYPE_CHECKING, Any, NamedTuple, TypeVar +from dataclasses import dataclass +from typing import TYPE_CHECKING, Any, TypeVar from typing_extensions import Self @@ -214,10 +215,14 @@ class TrackInfo(Info): # Structures that compose all the information for a candidate match. - - -class AlbumMatch(NamedTuple): +@dataclass +class Match: distance: Distance + info: Info + + +@dataclass +class AlbumMatch(Match): info: AlbumInfo mapping: dict[Item, TrackInfo] extra_items: list[Item] @@ -232,6 +237,6 @@ class AlbumMatch(NamedTuple): return [i for i, _ in self.item_info_pairs] -class TrackMatch(NamedTuple): - distance: Distance +@dataclass +class TrackMatch(Match): info: TrackInfo From 7873ae56f03df0db9494e2efb95f499c06cacea7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Fri, 19 Dec 2025 12:09:42 +0000 Subject: [PATCH 21/39] hooks: introduce Info.name property --- beets/autotag/hooks.py | 19 +++ beets/ui/commands/import_/display.py | 202 +++++++++++++-------------- beets/ui/commands/import_/session.py | 5 +- 3 files changed, 117 insertions(+), 109 deletions(-) diff --git a/beets/autotag/hooks.py b/beets/autotag/hooks.py index aae4846ca..82e685b7a 100644 --- a/beets/autotag/hooks.py +++ b/beets/autotag/hooks.py @@ -18,10 +18,13 @@ from __future__ import annotations from copy import deepcopy from dataclasses import dataclass +from functools import cached_property from typing import TYPE_CHECKING, Any, TypeVar from typing_extensions import Self +from beets.util import cached_classproperty + if TYPE_CHECKING: from beets.library import Item @@ -55,6 +58,10 @@ class AttrDict(dict[str, V]): class Info(AttrDict[Any]): """Container for metadata about a musical entity.""" + @cached_property + def name(self) -> str: + raise NotImplementedError + def __init__( self, album: str | None = None, @@ -96,6 +103,10 @@ class AlbumInfo(Info): user items, and later to drive tagging decisions once selected. """ + @cached_property + def name(self) -> str: + return self.album or "" + def __init__( self, tracks: list[TrackInfo], @@ -168,6 +179,10 @@ class TrackInfo(Info): stand alone for singleton matching. """ + @cached_property + def name(self) -> str: + return self.title or "" + def __init__( self, *, @@ -220,6 +235,10 @@ class Match: distance: Distance info: Info + @cached_classproperty + def type(cls) -> str: + return cls.__name__.removesuffix("Match") # type: ignore[attr-defined] + @dataclass class AlbumMatch(Match): diff --git a/beets/ui/commands/import_/display.py b/beets/ui/commands/import_/display.py index fd6758b54..057101ec5 100644 --- a/beets/ui/commands/import_/display.py +++ b/beets/ui/commands/import_/display.py @@ -1,15 +1,36 @@ +from __future__ import annotations + import os -from collections.abc import Sequence +from dataclasses import dataclass from functools import cached_property +from typing import TYPE_CHECKING, TypedDict + +from typing_extensions import NotRequired from beets import autotag, config, ui from beets.autotag import hooks from beets.util import displayable_path from beets.util.units import human_seconds_short +if TYPE_CHECKING: + from collections.abc import Sequence + + import confuse + + from beets.library.models import Item + from beets.ui import ColorName + VARIOUS_ARTISTS = "Various Artists" +class Side(TypedDict): + prefix: str + contents: str + suffix: str + width: NotRequired[int] + + +@dataclass class ChangeRepresentation: """Keeps track of all information needed to generate a (colored) text representation of the changes that will be made if an album or singleton's @@ -17,46 +38,46 @@ class ChangeRepresentation: TrackMatch object, accordingly. """ + cur_artist: str + cur_name: str + match: autotag.hooks.Match + @cached_property def changed_prefix(self) -> str: return ui.colorize("changed", "\u2260") - cur_artist = None - # cur_album set if album, cur_title set if singleton - cur_album = None - cur_title = None - match = None - indent_header = "" - indent_detail = "" + @cached_property + def _indentation_config(self) -> confuse.ConfigView: + return config["ui"]["import"]["indentation"] - def __init__(self): - # Read match header indentation width from config. - match_header_indent_width = config["ui"]["import"]["indentation"][ - "match_header" - ].as_number() - self.indent_header = ui.indent(match_header_indent_width) + @cached_property + def indent_header(self) -> str: + return ui.indent(self._indentation_config["match_header"].as_number()) - # Read match detail indentation width from config. - match_detail_indent_width = config["ui"]["import"]["indentation"][ - "match_details" - ].as_number() - self.indent_detail = ui.indent(match_detail_indent_width) + @cached_property + def indent_detail(self) -> str: + return ui.indent(self._indentation_config["match_details"].as_number()) - # Read match tracklist indentation width from config - match_tracklist_indent_width = config["ui"]["import"]["indentation"][ - "match_tracklist" - ].as_number() - self.indent_tracklist = ui.indent(match_tracklist_indent_width) - self.layout = config["ui"]["import"]["layout"].as_choice( - { - "column": 0, - "newline": 1, - } + @cached_property + def indent_tracklist(self) -> str: + return ui.indent( + self._indentation_config["match_tracklist"].as_number() + ) + + @cached_property + def layout(self) -> int: + return config["ui"]["import"]["layout"].as_choice( + {"column": 0, "newline": 1} ) def print_layout( - self, indent, left, right, separator=" -> ", max_width=None - ): + self, + indent: str, + left: Side, + right: Side, + separator: str = " -> ", + max_width: int | None = None, + ) -> None: if not max_width: # If no max_width provided, use terminal width max_width = ui.term_width() @@ -65,7 +86,7 @@ class ChangeRepresentation: else: ui.print_newline_layout(indent, left, right, separator, max_width) - def show_match_header(self): + def show_match_header(self) -> None: """Print out a 'header' identifying the suggested match (album name, artist name,...) and summarizing the changes that would be made should the user accept the match. @@ -78,19 +99,10 @@ class ChangeRepresentation: f"{self.indent_header}Match ({dist_string(self.match.distance)}):" ) - if isinstance(self.match.info, autotag.hooks.AlbumInfo): - # Matching an album - print that - artist_album_str = ( - f"{self.match.info.artist} - {self.match.info.album}" - ) - else: - # Matching a single track - artist_album_str = ( - f"{self.match.info.artist} - {self.match.info.title}" - ) + artist_name_str = f"{self.match.info.artist} - {self.match.info.name}" ui.print_( self.indent_header - + dist_colorize(artist_album_str, self.match.distance) + + dist_colorize(artist_name_str, self.match.distance) ) # Penalties. @@ -108,7 +120,7 @@ class ChangeRepresentation: url = ui.colorize("text_faint", f"{self.match.info.data_url}") ui.print_(f"{self.indent_header}{url}") - def show_match_details(self): + def show_match_details(self) -> None: """Print out the details of the match, including changes in album name and artist name. """ @@ -117,6 +129,8 @@ class ChangeRepresentation: if artist_r == VARIOUS_ARTISTS: # Hide artists for VA releases. artist_l, artist_r = "", "" + left: Side + right: Side if artist_l != artist_r: artist_l, artist_r = ui.colordiff(artist_l, artist_r) left = { @@ -130,39 +144,22 @@ class ChangeRepresentation: else: ui.print_(f"{self.indent_detail}*", "Artist:", artist_r) - if self.cur_album: - # Album - album_l, album_r = self.cur_album or "", self.match.info.album - if ( - self.cur_album != self.match.info.album - and self.match.info.album != VARIOUS_ARTISTS - ): - album_l, album_r = ui.colordiff(album_l, album_r) + if self.cur_name: + type_ = self.match.type + name_l, name_r = self.cur_name or "", self.match.info.name + if self.cur_name != self.match.info.name != VARIOUS_ARTISTS: + name_l, name_r = ui.colordiff(name_l, name_r) left = { - "prefix": f"{self.changed_prefix} Album: ", - "contents": album_l, + "prefix": f"{self.changed_prefix} {type_}: ", + "contents": name_l, "suffix": "", } - right = {"prefix": "", "contents": album_r, "suffix": ""} + right = {"prefix": "", "contents": name_r, "suffix": ""} self.print_layout(self.indent_detail, left, right) else: - ui.print_(f"{self.indent_detail}*", "Album:", album_r) - elif self.cur_title: - # Title - for singletons - title_l, title_r = self.cur_title or "", self.match.info.title - if self.cur_title != self.match.info.title: - title_l, title_r = ui.colordiff(title_l, title_r) - left = { - "prefix": f"{self.changed_prefix} Title: ", - "contents": title_l, - "suffix": "", - } - right = {"prefix": "", "contents": title_r, "suffix": ""} - self.print_layout(self.indent_detail, left, right) - else: - ui.print_(f"{self.indent_detail}*", "Title:", title_r) + ui.print_(f"{self.indent_detail}*", f"{type_}:", name_r) - def make_medium_info_line(self, track_info): + def make_medium_info_line(self, track_info: hooks.TrackInfo) -> str: """Construct a line with the current medium's info.""" track_media = track_info.get("media", "Media") # Build output string. @@ -177,7 +174,7 @@ class ChangeRepresentation: else: return "" - def format_index(self, track_info): + def format_index(self, track_info: hooks.TrackInfo | Item) -> str: """Return a string representing the track index of the given TrackInfo or Item object. """ @@ -198,12 +195,15 @@ class ChangeRepresentation: else: return str(index) - def make_track_numbers(self, item, track_info): + def make_track_numbers( + self, item, track_info: hooks.TrackInfo + ) -> tuple[str, str, bool]: """Format colored track indices.""" cur_track = self.format_index(item) new_track = self.format_index(track_info) changed = False # Choose color based on change. + highlight_color: ColorName if cur_track != new_track: changed = True if item.track in (track_info.index, track_info.medium_index): @@ -218,9 +218,11 @@ class ChangeRepresentation: return lhs_track, rhs_track, changed @staticmethod - def make_track_titles(item, track_info): + def make_track_titles( + item: Item, track_info: hooks.TrackInfo + ) -> tuple[str, str, bool]: """Format colored track titles.""" - new_title = track_info.title + new_title = track_info.name if not item.title.strip(): # If there's no title, we use the filename. Don't colordiff. cur_title = displayable_path(os.path.basename(item.path)) @@ -232,9 +234,12 @@ class ChangeRepresentation: return cur_col, new_col, cur_title != new_title @staticmethod - def make_track_lengths(item, track_info): + def make_track_lengths( + item: Item, track_info: hooks.TrackInfo + ) -> tuple[str, str, bool]: """Format colored track lengths.""" changed = False + highlight_color: ColorName if ( item.length and track_info.length @@ -258,7 +263,9 @@ class ChangeRepresentation: return lhs_length, rhs_length, changed - def make_line(self, item, track_info): + def make_line( + self, item: Item, track_info: hooks.TrackInfo + ) -> tuple[Side, Side]: """Extract changes from item -> new TrackInfo object, and colorize appropriately. Returns (lhs, rhs) for column printing. """ @@ -282,12 +289,12 @@ class ChangeRepresentation: # the case, thus the 'info' dictionary is unneeded. # penalties = penalty_string(self.match.distance.tracks[track_info]) - lhs = { + lhs: Side = { "prefix": f"{self.changed_prefix if changed else '*'} {lhs_track} ", "contents": lhs_title, "suffix": f" {lhs_length}", } - rhs = {"prefix": "", "contents": "", "suffix": ""} + rhs: Side = {"prefix": "", "contents": "", "suffix": ""} if not changed: # Only return the left side, as nothing changed. return (lhs, rhs) @@ -358,27 +365,18 @@ class ChangeRepresentation: class AlbumChange(ChangeRepresentation): - """Album change representation, setting cur_album""" + match: autotag.hooks.AlbumMatch - def __init__(self, cur_artist, cur_album, match): - super().__init__() - self.cur_artist = cur_artist - self.cur_album = cur_album - self.match = match - - def show_match_tracks(self): + def show_match_tracks(self) -> None: """Print out the tracks of the match, summarizing changes the match suggests for them. """ - # Tracks. - # match is an AlbumMatch NamedTuple, mapping is a dict - # Sort the pairs by the track_info index (at index 1 of the NamedTuple) pairs = sorted( - self.match.item_info_pairs, key=lambda pair: pair[1].index + self.match.item_info_pairs, key=lambda pair: pair[1].index or 0 ) # Build up LHS and RHS for track difference display. The `lines` list # contains `(left, right)` tuples. - lines = [] + lines: list[tuple[Side, Side]] = [] medium = disctitle = None for item, track_info in pairs: # If the track is the first on a new medium, show medium @@ -427,21 +425,17 @@ class AlbumChange(ChangeRepresentation): class TrackChange(ChangeRepresentation): """Track change representation, comparing item with match.""" - def __init__(self, cur_artist, cur_title, match): - super().__init__() - self.cur_artist = cur_artist - self.cur_title = cur_title - self.match = match + match: autotag.hooks.TrackMatch -def show_change(cur_artist, cur_album, match): +def show_change( + cur_artist: str, cur_album: str, match: hooks.AlbumMatch +) -> None: """Print out a representation of the changes that will be made if an album's tags are changed according to `match`, which must be an AlbumMatch object. """ - change = AlbumChange( - cur_artist=cur_artist, cur_album=cur_album, match=match - ) + change = AlbumChange(cur_artist, cur_album, match) # Print the match header. change.show_match_header() @@ -453,13 +447,11 @@ def show_change(cur_artist, cur_album, match): change.show_match_tracks() -def show_item_change(item, match): +def show_item_change(item: Item, match: hooks.TrackMatch) -> None: """Print out the change that would occur by tagging `item` with the metadata from `match`, a TrackMatch object. """ - change = TrackChange( - cur_artist=item.artist, cur_title=item.title, match=match - ) + change = TrackChange(item.artist, item.title, match) # Print the match header. change.show_match_header() # Print the match details. diff --git a/beets/ui/commands/import_/session.py b/beets/ui/commands/import_/session.py index dcc80b793..9c8c8dd62 100644 --- a/beets/ui/commands/import_/session.py +++ b/beets/ui/commands/import_/session.py @@ -444,10 +444,7 @@ def choose_candidate( index = dist_colorize(index0, match.distance) dist = f"({(1 - match.distance) * 100:.1f}%)" distance = dist_colorize(dist, match.distance) - metadata = ( - f"{match.info.artist} -" - f" {match.info.title if singleton else match.info.album}" - ) + metadata = f"{match.info.artist} - {match.info.name}" if i == 0: metadata = dist_colorize(metadata, match.distance) else: From 60b4a38c09a08ff2c8ce00742eac01f6610c0aaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Fri, 19 Dec 2025 12:10:21 +0000 Subject: [PATCH 22/39] Add missing type defs in import_/display.py --- beets/ui/commands/import_/display.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/beets/ui/commands/import_/display.py b/beets/ui/commands/import_/display.py index 057101ec5..113462d19 100644 --- a/beets/ui/commands/import_/display.py +++ b/beets/ui/commands/import_/display.py @@ -17,6 +17,7 @@ if TYPE_CHECKING: import confuse + from beets.autotag.distance import Distance from beets.library.models import Item from beets.ui import ColorName @@ -47,7 +48,7 @@ class ChangeRepresentation: return ui.colorize("changed", "\u2260") @cached_property - def _indentation_config(self) -> confuse.ConfigView: + def _indentation_config(self) -> confuse.Subview: return config["ui"]["import"]["indentation"] @cached_property @@ -196,7 +197,7 @@ class ChangeRepresentation: return str(index) def make_track_numbers( - self, item, track_info: hooks.TrackInfo + self, item: Item, track_info: hooks.TrackInfo ) -> tuple[str, str, bool]: """Format colored track indices.""" cur_track = self.format_index(item) @@ -307,7 +308,7 @@ class ChangeRepresentation: } return (lhs, rhs) - def print_tracklist(self, lines): + def print_tracklist(self, lines: list[tuple[Side, Side]]) -> None: """Calculates column widths for tracks stored as line tuples: (left, right). Then prints each line of tracklist. """ @@ -315,7 +316,7 @@ class ChangeRepresentation: # If no lines provided, e.g. details not required, do nothing. return - def get_width(side): + def get_width(side: Side) -> int: """Return the width of left or right in uncolorized characters.""" try: return len( @@ -458,7 +459,7 @@ def show_item_change(item: Item, match: hooks.TrackMatch) -> None: change.show_match_details() -def disambig_string(info): +def disambig_string(info: hooks.Info) -> str: """Generate a string for an AlbumInfo or TrackInfo object that provides context that helps disambiguate similar-looking albums and tracks. @@ -524,7 +525,7 @@ def get_album_disambig_fields(info: hooks.AlbumInfo) -> Sequence[str]: return out -def dist_colorize(string, dist): +def dist_colorize(string: str, dist: Distance) -> str: """Formats a string as a colorized similarity string according to a distance. """ @@ -537,7 +538,7 @@ def dist_colorize(string, dist): return string -def dist_string(dist): +def dist_string(dist: Distance) -> str: """Formats a distance (a float) as a colorized similarity percentage string. """ @@ -545,7 +546,7 @@ def dist_string(dist): return dist_colorize(string, dist) -def penalty_string(distance, limit=None): +def penalty_string(distance: Distance, limit: int | None = None) -> str: """Returns a colorized string that indicates all the penalties applied to a distance object. """ @@ -561,3 +562,5 @@ def penalty_string(distance, limit=None): # Prefix penalty string with U+2260: Not Equal To penalty_string = f"\u2260 {', '.join(penalties)}" return ui.colorize("changed", penalty_string) + + return "" From 8ccb33e4bc394513e15a29e7fcf12a3f41500ac7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Fri, 1 Aug 2025 17:08:08 +0100 Subject: [PATCH 23/39] dbcore: add Model.db cached attribute --- beets/dbcore/db.py | 28 +++++++++++++++++----------- beets/library/models.py | 8 +++----- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index cc172d0d8..c682f0c0c 100755 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -34,6 +34,7 @@ from collections.abc import ( Mapping, Sequence, ) +from functools import cached_property from sqlite3 import Connection, sqlite_version_info from typing import TYPE_CHECKING, Any, AnyStr, Generic @@ -360,6 +361,14 @@ class Model(ABC, Generic[D]): """Fields in the related table.""" return cls._relation._fields.keys() - cls.shared_db_fields + @cached_property + def db(self) -> D: + """Get the database associated with this object. + + This validates that the database is attached and the object has an id. + """ + return self._check_db() + @classmethod def _getters(cls: type[Model]): """Return a mapping from field names to getter functions.""" @@ -599,7 +608,6 @@ class Model(ABC, Generic[D]): """ if fields is None: fields = self._fields - db = self._check_db() # Build assignments for query. assignments = [] @@ -611,7 +619,7 @@ class Model(ABC, Generic[D]): value = self._type(key).to_sql(self[key]) subvars.append(value) - with db.transaction() as tx: + with self.db.transaction() as tx: # Main table update. if assignments: query = f"UPDATE {self._table} SET {','.join(assignments)} WHERE id=?" @@ -645,11 +653,10 @@ class Model(ABC, Generic[D]): If check_revision is true, the database is only queried loaded when a transaction has been committed since the item was last loaded. """ - db = self._check_db() - if not self._dirty and db.revision == self._revision: + if not self._dirty and self.db.revision == self._revision: # Exit early return - stored_obj = db._get(type(self), self.id) + stored_obj = self.db._get(type(self), self.id) assert stored_obj is not None, f"object {self.id} not in DB" self._values_fixed = LazyConvertDict(self) self._values_flex = LazyConvertDict(self) @@ -658,8 +665,7 @@ class Model(ABC, Generic[D]): def remove(self): """Remove the object's associated rows from the database.""" - db = self._check_db() - with db.transaction() as tx: + with self.db.transaction() as tx: tx.mutate(f"DELETE FROM {self._table} WHERE id=?", (self.id,)) tx.mutate( f"DELETE FROM {self._flex_table} WHERE entity_id=?", (self.id,) @@ -675,7 +681,7 @@ class Model(ABC, Generic[D]): """ if db: self._db = db - db = self._check_db(False) + db = self._check_db(need_id=False) with db.transaction() as tx: new_id = tx.mutate(f"INSERT INTO {self._table} DEFAULT VALUES") @@ -740,9 +746,9 @@ class Model(ABC, Generic[D]): Remove the database connection as sqlite connections are not picklable. """ - state = self.__dict__.copy() - state["_db"] = None - return state + return { + k: v for k, v in self.__dict__.items() if k not in {"_db", "db"} + } # Database controller and supporting interfaces. diff --git a/beets/library/models.py b/beets/library/models.py index cbee2a411..76618d929 100644 --- a/beets/library/models.py +++ b/beets/library/models.py @@ -1143,7 +1143,6 @@ class Item(LibModel): If `store` is `False` however, the item won't be stored and it will have to be manually stored after invoking this method. """ - self._check_db() dest = self.destination(basedir=basedir) # Create necessary ancestry for the move. @@ -1183,9 +1182,8 @@ class Item(LibModel): is true, returns just the fragment of the path underneath the library base directory. """ - db = self._check_db() - basedir = basedir or db.directory - path_formats = path_formats or db.path_formats + basedir = basedir or self.db.directory + path_formats = path_formats or self.db.path_formats # Use a path format based on a query, falling back on the # default. @@ -1224,7 +1222,7 @@ class Item(LibModel): ) lib_path_str, fallback = util.legalize_path( - subpath, db.replacements, self.filepath.suffix + subpath, self.db.replacements, self.filepath.suffix ) if fallback: # Print an error message if legalization fell back to From e1e0d945f84933cf17b96a7951258c698ccc350b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Fri, 26 Dec 2025 17:21:30 +0000 Subject: [PATCH 24/39] Add NotFoundError and Model.get_fresh_from_db; tidy DB getters Introduce NotFoundError and a Model.get_fresh_from_db helper that reloads an object from the database and raises when missing. Use it to simplify Model.load and UI change detection. --- beets/dbcore/db.py | 36 +++++++++++++++++++++--------------- beets/library/library.py | 18 +++++++----------- beets/library/models.py | 2 ++ beets/ui/__init__.py | 2 +- 4 files changed, 31 insertions(+), 27 deletions(-) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index c682f0c0c..83f0543d2 100755 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -38,7 +38,10 @@ from functools import cached_property from sqlite3 import Connection, sqlite_version_info from typing import TYPE_CHECKING, Any, AnyStr, Generic -from typing_extensions import TypeVar # default value support +from typing_extensions import ( + Self, + TypeVar, # default value support +) from unidecode import unidecode import beets @@ -84,6 +87,10 @@ class DBCustomFunctionError(Exception): ) +class NotFoundError(LookupError): + pass + + class FormattedMapping(Mapping[str, str]): """A `dict`-like formatted view of a model. @@ -369,6 +376,14 @@ class Model(ABC, Generic[D]): """ return self._check_db() + def get_fresh_from_db(self) -> Self: + """Load this object from the database.""" + model_cls = self.__class__ + if obj := self.db._get(model_cls, self.id): + return obj + + raise NotFoundError(f"No matching {model_cls.__name__} found") from None + @classmethod def _getters(cls: type[Model]): """Return a mapping from field names to getter functions.""" @@ -656,11 +671,8 @@ class Model(ABC, Generic[D]): if not self._dirty and self.db.revision == self._revision: # Exit early return - stored_obj = self.db._get(type(self), self.id) - assert stored_obj is not None, f"object {self.id} not in DB" - self._values_fixed = LazyConvertDict(self) - self._values_flex = LazyConvertDict(self) - self.update(dict(stored_obj)) + + self.__dict__.update(self.get_fresh_from_db().__dict__) self.clear_dirty() def remove(self): @@ -1309,12 +1321,6 @@ class Database: sort if sort.is_slow() else None, # Slow sort component. ) - def _get( - self, - model_cls: type[AnyModel], - id, - ) -> AnyModel | None: - """Get a Model object by its id or None if the id does not - exist. - """ - return self._fetch(model_cls, MatchQuery("id", id)).get() + def _get(self, model_cls: type[AnyModel], id_: int) -> AnyModel | None: + """Get a Model object by its id or None if the id does not exist.""" + return self._fetch(model_cls, MatchQuery("id", id_)).get() diff --git a/beets/library/library.py b/beets/library/library.py index 7370f7ecd..39d559901 100644 --- a/beets/library/library.py +++ b/beets/library/library.py @@ -125,24 +125,20 @@ class Library(dbcore.Database): return self._fetch(Item, query, sort or self.get_default_item_sort()) # Convenience accessors. - - def get_item(self, id): + def get_item(self, id_: int) -> Item | None: """Fetch a :class:`Item` by its ID. Return `None` if no match is found. """ - return self._get(Item, id) + return self._get(Item, id_) - def get_album(self, item_or_id): + def get_album(self, item_or_id: Item | int) -> Album | None: """Given an album ID or an item associated with an album, return a :class:`Album` object for the album. If no such album exists, return `None`. """ - if isinstance(item_or_id, int): - album_id = item_or_id - else: - album_id = item_or_id.album_id - if album_id is None: - return None - return self._get(Album, album_id) + album_id = ( + item_or_id if isinstance(item_or_id, int) else item_or_id.album_id + ) + return self._get(Album, album_id) if album_id else None diff --git a/beets/library/models.py b/beets/library/models.py index 76618d929..9609989bc 100644 --- a/beets/library/models.py +++ b/beets/library/models.py @@ -620,6 +620,8 @@ class Album(LibModel): class Item(LibModel): """Represent a song or track.""" + album_id: int | None + _table = "items" _flex_table = "item_attributes" _fields = { diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index cfd8b6bd7..cbe0fb109 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -1073,7 +1073,7 @@ def show_model_changes( restrict the detection to. `always` indicates whether the object is always identified, regardless of whether any changes are present. """ - old = old or new._db._get(type(new), new.id) + old = old or new.get_fresh_from_db() # Keep the formatted views around instead of re-creating them in each # iteration step From 75baec611acc66979cc28338bb857de5ad1b319b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Fri, 26 Dec 2025 16:40:29 +0000 Subject: [PATCH 25/39] Improve and simplify show_model_changes --- beets/dbcore/db.py | 4 ++- beets/ui/__init__.py | 64 ++++++++++++++++++-------------------- docs/changelog.rst | 2 ++ pyproject.toml | 1 + test/ui/test_field_diff.py | 64 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 101 insertions(+), 34 deletions(-) create mode 100644 test/ui/test_field_diff.py 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" From c807effeda334e1f7019dbb529e8292106a0e288 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Fri, 26 Dec 2025 16:41:57 +0000 Subject: [PATCH 26/39] Define a shared fixture for config --- test/autotag/test_distance.py | 4 +--- test/conftest.py | 7 +++++++ test/plugins/test_mbpseudo.py | 11 ++--------- test/test_autotag.py | 10 +++++----- test/ui/test_field_diff.py | 5 ----- 5 files changed, 15 insertions(+), 22 deletions(-) diff --git a/test/autotag/test_distance.py b/test/autotag/test_distance.py index 9a658f5e1..3686f82c9 100644 --- a/test/autotag/test_distance.py +++ b/test/autotag/test_distance.py @@ -12,15 +12,13 @@ from beets.autotag.distance import ( from beets.library import Item from beets.metadata_plugins import MetadataSourcePlugin, get_penalty from beets.plugins import BeetsPlugin -from beets.test.helper import ConfigMixin _p = pytest.param class TestDistance: @pytest.fixture(autouse=True, scope="class") - def setup_config(self): - config = ConfigMixin().config + def setup_config(self, config): config["match"]["distance_weights"]["data_source"] = 2.0 config["match"]["distance_weights"]["album"] = 4.0 config["match"]["distance_weights"]["medium"] = 2.0 diff --git a/test/conftest.py b/test/conftest.py index eb46b94b0..059526d2f 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -5,6 +5,7 @@ import pytest from beets.autotag.distance import Distance from beets.dbcore.query import Query +from beets.test.helper import ConfigMixin from beets.util import cached_classproperty @@ -53,3 +54,9 @@ def pytest_assertrepr_compare(op, left, right): @pytest.fixture(autouse=True) def clear_cached_classproperty(): cached_classproperty.cache.clear() + + +@pytest.fixture(scope="module") +def config(): + """Provide a fresh beets configuration for a module, when requested.""" + return ConfigMixin().config diff --git a/test/plugins/test_mbpseudo.py b/test/plugins/test_mbpseudo.py index b333800a3..a98a59248 100644 --- a/test/plugins/test_mbpseudo.py +++ b/test/plugins/test_mbpseudo.py @@ -8,7 +8,7 @@ from beets.autotag import AlbumMatch from beets.autotag.distance import Distance from beets.autotag.hooks import AlbumInfo, TrackInfo from beets.library import Item -from beets.test.helper import ConfigMixin, PluginMixin +from beets.test.helper import PluginMixin from beetsplug._typing import JSONDict from beetsplug.mbpseudo import ( _STATUS_PSEUDO, @@ -52,14 +52,7 @@ def pseudo_release_info() -> AlbumInfo: ) -@pytest.fixture(scope="module", autouse=True) -def config(): - config = ConfigMixin().config - with pytest.MonkeyPatch.context() as m: - m.setattr("beetsplug.mbpseudo.config", config) - yield config - - +@pytest.mark.usefixtures("config") class TestPseudoAlbumInfo: def test_album_id_always_from_pseudo( self, official_release_info: AlbumInfo, pseudo_release_info: AlbumInfo diff --git a/test/test_autotag.py b/test/test_autotag.py index 48ae09ccb..119ca15e8 100644 --- a/test/test_autotag.py +++ b/test/test_autotag.py @@ -19,18 +19,18 @@ import pytest from beets import autotag, config from beets.autotag import AlbumInfo, TrackInfo, correct_list_fields, match from beets.library import Item -from beets.test.helper import BeetsTestCase, ConfigMixin +from beets.test.helper import BeetsTestCase -class TestAssignment(ConfigMixin): +class TestAssignment: A = "one" B = "two" C = "three" @pytest.fixture(autouse=True) - def _setup_config(self): - self.config["match"]["track_length_grace"] = 10 - self.config["match"]["track_length_max"] = 30 + def config(self, config): + config["match"]["track_length_grace"] = 10 + config["match"]["track_length_max"] = 30 @pytest.mark.parametrize( # 'expected' is a tuple of expected (mapping, extra_items, extra_tracks) diff --git a/test/ui/test_field_diff.py b/test/ui/test_field_diff.py index dce7ba161..35f3c6ca7 100644 --- a/test/ui/test_field_diff.py +++ b/test/ui/test_field_diff.py @@ -1,17 +1,12 @@ 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 From f9c3aae4ed2212232e4ede683d5aa3fb8088eab6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Mon, 29 Dec 2025 17:05:32 +0000 Subject: [PATCH 27/39] Musicbrainz: fix original release id access for a pseudo releae --- beetsplug/musicbrainz.py | 2 +- test/plugins/test_musicbrainz.py | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/beetsplug/musicbrainz.py b/beetsplug/musicbrainz.py index 221afea71..8cab1786b 100644 --- a/beetsplug/musicbrainz.py +++ b/beetsplug/musicbrainz.py @@ -914,7 +914,7 @@ class MusicBrainzPlugin(MetadataSourcePlugin): rel["type"] == "transl-tracklisting" and rel["direction"] == "backward" ): - actual_res = self.api.get_release(rel["target"]) + actual_res = self.api.get_release(rel["release"]["id"]) # release is potentially a pseudo release release = self.album_info(res) diff --git a/test/plugins/test_musicbrainz.py b/test/plugins/test_musicbrainz.py index 0a3155430..30b9f7d1a 100644 --- a/test/plugins/test_musicbrainz.py +++ b/test/plugins/test_musicbrainz.py @@ -204,7 +204,6 @@ class MBAlbumInfoTest(MusicBrainzTestCase): { "type": "remixer", "type-id": "RELATION TYPE ID", - "target": "RECORDING REMIXER ARTIST ID", "direction": "RECORDING RELATION DIRECTION", "artist": { "id": "RECORDING REMIXER ARTIST ID", @@ -820,8 +819,10 @@ class MBLibraryTest(MusicBrainzTestCase): "release-relations": [ { "type": "transl-tracklisting", - "target": "d2a6f856-b553-40a0-ac54-a321e8e2da01", "direction": "backward", + "release": { + "id": "d2a6f856-b553-40a0-ac54-a321e8e2da01" + }, } ], }, @@ -993,8 +994,10 @@ class MBLibraryTest(MusicBrainzTestCase): "release-relations": [ { "type": "remaster", - "target": "d2a6f856-b553-40a0-ac54-a321e8e2da01", "direction": "backward", + "release": { + "id": "d2a6f856-b553-40a0-ac54-a321e8e2da01" + }, } ], } From 9ddddf4c3948d8405b7a9e4da761005d456e3aa9 Mon Sep 17 00:00:00 2001 From: Danny Trunk Date: Tue, 30 Dec 2025 00:19:27 +0100 Subject: [PATCH 28/39] fetchart: Add support for configurable fallback cover art --- beetsplug/fetchart.py | 14 ++++++++++++++ docs/changelog.rst | 1 + docs/plugins/fetchart.rst | 2 ++ test/plugins/test_art.py | 11 ++++++++++- 4 files changed, 27 insertions(+), 1 deletion(-) diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index e6bd05119..9f5ed69fb 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -1101,6 +1101,16 @@ class FileSystem(LocalArtSource): else: remaining.append(fn) + # Fall back to a configured image. + if plugin.fallback: + self._log.debug( + "using fallback art file {}", + util.displayable_path(plugin.fallback), + ) + yield self._candidate( + path=plugin.fallback, match=MetadataMatch.FALLBACK + ) + # Fall back to any image in the folder. if remaining and not plugin.cautious: self._log.debug( @@ -1332,6 +1342,7 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): "enforce_ratio": False, "cautious": False, "cover_names": ["cover", "front", "art", "album", "folder"], + "fallback": None, "sources": [ "filesystem", "coverart", @@ -1380,6 +1391,9 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): cover_names = self.config["cover_names"].as_str_seq() self.cover_names = list(map(util.bytestring_path, cover_names)) self.cautious = self.config["cautious"].get(bool) + self.fallback = self.config["fallback"].get( + confuse.Optional(confuse.Filename()) + ) self.store_source = self.config["store_source"].get(bool) self.cover_format = self.config["cover_format"].get( diff --git a/docs/changelog.rst b/docs/changelog.rst index b292863ee..0cfc1af24 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -12,6 +12,7 @@ been dropped. New features: +- :doc:`plugins/fetchart`: Added config setting for a fallback cover art image. - :doc:`plugins/ftintitle`: Added argument for custom feat. words in ftintitle. - :doc:`plugins/ftintitle`: Added album template value ``album_artist_no_feat``. - :doc:`plugins/musicbrainz`: Allow selecting tags or genres to populate the diff --git a/docs/plugins/fetchart.rst b/docs/plugins/fetchart.rst index 1d64f4b2e..fd578212a 100644 --- a/docs/plugins/fetchart.rst +++ b/docs/plugins/fetchart.rst @@ -33,6 +33,8 @@ file. The available options are: contain one of the keywords in ``cover_names``. Default: ``no``. - **cover_names**: Prioritize images containing words in this list. Default: ``cover front art album folder``. +- **fallback**: Path to a fallback album art file if no album art was found + otherwise. Default: ``None`` (disabled). - **minwidth**: Only images with a width bigger or equal to ``minwidth`` are considered as valid album art candidates. Default: 0. - **maxwidth**: A maximum image width to downscale fetched images if they are diff --git a/test/plugins/test_art.py b/test/plugins/test_art.py index 285bb70e5..02d23d59b 100644 --- a/test/plugins/test_art.py +++ b/test/plugins/test_art.py @@ -261,7 +261,9 @@ class FSArtTest(UseThePlugin): os.mkdir(syspath(self.dpath)) self.source = fetchart.FileSystem(logger, self.plugin.config) - self.settings = Settings(cautious=False, cover_names=("art",)) + self.settings = Settings( + cautious=False, cover_names=("art",), fallback=None + ) def test_finds_jpg_in_directory(self): _common.touch(os.path.join(self.dpath, b"a.jpg")) @@ -285,6 +287,13 @@ class FSArtTest(UseThePlugin): with pytest.raises(StopIteration): next(self.source.get(None, self.settings, [self.dpath])) + def test_configured_fallback_is_used(self): + fallback = os.path.join(self.temp_dir, b"a.jpg") + _common.touch(fallback) + self.settings.fallback = fallback + candidate = next(self.source.get(None, self.settings, [self.dpath])) + assert candidate.path == fallback + def test_empty_dir(self): with pytest.raises(StopIteration): next(self.source.get(None, self.settings, [self.dpath])) From 40a212a2c4d660d7f527d30a41bd419b0160e826 Mon Sep 17 00:00:00 2001 From: j0j0 Date: Sun, 16 Nov 2025 08:29:51 +0100 Subject: [PATCH 29/39] lastgenre: Simplify genre fetchers Reduce fetcher methods to 3: last.fm can be asked for for a genre for these combinations of metadata: - albumartist/album - artist/track - artist Passing them in the callers instead of hiding it in the methods also helps readability in _get_genre(). --- beetsplug/lastgenre/__init__.py | 36 ++++++++++++++++----------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/beetsplug/lastgenre/__init__.py b/beetsplug/lastgenre/__init__.py index ea0ab951a..698365078 100644 --- a/beetsplug/lastgenre/__init__.py +++ b/beetsplug/lastgenre/__init__.py @@ -300,24 +300,20 @@ class LastGenrePlugin(plugins.BeetsPlugin): self._tunelog("last.fm (unfiltered) {} tags: {}", entity, genre) return genre - def fetch_album_genre(self, obj): - """Return raw album genres from Last.fm for this Item or Album.""" + def fetch_album_genre(self, albumartist, albumtitle): + """Return genres from Last.fm for the album by albumartist.""" return self._last_lookup( - "album", LASTFM.get_album, obj.albumartist, obj.album + "album", LASTFM.get_album, albumartist, albumtitle ) - def fetch_album_artist_genre(self, obj): - """Return raw album artist genres from Last.fm for this Item or Album.""" - return self._last_lookup("artist", LASTFM.get_artist, obj.albumartist) + def fetch_artist_genre(self, artist): + """Return genres from Last.fm for the artist.""" + return self._last_lookup("artist", LASTFM.get_artist, artist) - def fetch_artist_genre(self, item): - """Returns raw track artist genres from Last.fm for this Item.""" - return self._last_lookup("artist", LASTFM.get_artist, item.artist) - - def fetch_track_genre(self, obj): - """Returns raw track genres from Last.fm for this Item.""" + def fetch_track_genre(self, trackartist, tracktitle): + """Return genres from Last.fm for the track by artist.""" return self._last_lookup( - "track", LASTFM.get_track, obj.artist, obj.title + "track", LASTFM.get_track, trackartist, tracktitle ) # Main processing: _get_genre() and helpers. @@ -405,14 +401,14 @@ class LastGenrePlugin(plugins.BeetsPlugin): # Run through stages: track, album, artist, # album artist, or most popular track genre. if isinstance(obj, library.Item) and "track" in self.sources: - if new_genres := self.fetch_track_genre(obj): + if new_genres := self.fetch_track_genre(obj.artist, obj.title): if result := _try_resolve_stage( "track", keep_genres, new_genres ): return result if "album" in self.sources: - if new_genres := self.fetch_album_genre(obj): + if new_genres := self.fetch_album_genre(obj.albumartist, obj.album): if result := _try_resolve_stage( "album", keep_genres, new_genres ): @@ -421,10 +417,10 @@ class LastGenrePlugin(plugins.BeetsPlugin): if "artist" in self.sources: new_genres = [] if isinstance(obj, library.Item): - new_genres = self.fetch_artist_genre(obj) + new_genres = self.fetch_artist_genre(obj.artist) stage_label = "artist" elif obj.albumartist != config["va_name"].as_str(): - new_genres = self.fetch_album_artist_genre(obj) + new_genres = self.fetch_artist_genre(obj.albumartist) stage_label = "album artist" else: # For "Various Artists", pick the most popular track genre. @@ -432,9 +428,11 @@ class LastGenrePlugin(plugins.BeetsPlugin): for item in obj.items(): item_genre = None if "track" in self.sources: - item_genre = self.fetch_track_genre(item) + item_genre = self.fetch_track_genre( + item.artist, item.title + ) if not item_genre: - item_genre = self.fetch_artist_genre(item) + item_genre = self.fetch_artist_genre(item.artist) if item_genre: item_genres += item_genre if item_genres: From 355c9cc1b608b1e5ed5956eac0b7e32a4b9bff62 Mon Sep 17 00:00:00 2001 From: j0j0 Date: Sun, 16 Nov 2025 08:40:15 +0100 Subject: [PATCH 30/39] lastgenre: Use multi-valued albumartists field In case the albumartist genre can't be found (often due to variations of artist-combination wording issues, eg "featuring", "+", "&" and so on) use the albumartists list field, fetch a genre for each artist separately and concatenate them. --- beetsplug/lastgenre/__init__.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/beetsplug/lastgenre/__init__.py b/beetsplug/lastgenre/__init__.py index 698365078..ba85c3871 100644 --- a/beetsplug/lastgenre/__init__.py +++ b/beetsplug/lastgenre/__init__.py @@ -422,6 +422,19 @@ class LastGenrePlugin(plugins.BeetsPlugin): elif obj.albumartist != config["va_name"].as_str(): new_genres = self.fetch_artist_genre(obj.albumartist) stage_label = "album artist" + if not new_genres: + self._tunelog( + 'No album artist genre found for "{}", ' + "trying multi-valued field...", + obj.albumartist, + ) + for albumartist in obj.albumartists: + self._tunelog( + 'Fetching artist genre for "{}"', albumartist + ) + new_genres += self.fetch_artist_genre(albumartist) + if new_genres: + stage_label = "multi-valued album artist" else: # For "Various Artists", pick the most popular track genre. item_genres = [] From a046f60c5173bd9c15c1402cab00383f01283cb0 Mon Sep 17 00:00:00 2001 From: J0J0 Todos Date: Wed, 19 Nov 2025 07:16:26 +0100 Subject: [PATCH 31/39] lastgenre: Hint mypy to Album.items() instead of obj.items() --- beetsplug/lastgenre/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/beetsplug/lastgenre/__init__.py b/beetsplug/lastgenre/__init__.py index ba85c3871..40019f548 100644 --- a/beetsplug/lastgenre/__init__.py +++ b/beetsplug/lastgenre/__init__.py @@ -438,6 +438,7 @@ class LastGenrePlugin(plugins.BeetsPlugin): else: # For "Various Artists", pick the most popular track genre. item_genres = [] + assert isinstance(obj, Album) # Type narrowing for mypy for item in obj.items(): item_genre = None if "track" in self.sources: From d72307a16ff3c08b2949fe99d433c257e7148641 Mon Sep 17 00:00:00 2001 From: J0J0 Todos Date: Thu, 20 Nov 2025 06:01:51 +0100 Subject: [PATCH 32/39] lastgenre: Adapt test_get_genre function signatures --- test/plugins/test_lastgenre.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/plugins/test_lastgenre.py b/test/plugins/test_lastgenre.py index 12ff30f8e..026001e38 100644 --- a/test/plugins/test_lastgenre.py +++ b/test/plugins/test_lastgenre.py @@ -546,13 +546,13 @@ class LastGenrePluginTest(PluginTestCase): def test_get_genre(config_values, item_genre, mock_genres, expected_result): """Test _get_genre with various configurations.""" - def mock_fetch_track_genre(self, obj=None): + def mock_fetch_track_genre(self, trackartist, tracktitle): return mock_genres["track"] - def mock_fetch_album_genre(self, obj): + def mock_fetch_album_genre(self, albumartist, albumtitle): return mock_genres["album"] - def mock_fetch_artist_genre(self, obj): + def mock_fetch_artist_genre(self, artist): return mock_genres["artist"] # Mock the last.fm fetchers. When whitelist enabled, we can assume only From f19d672016dbfae8b9296f4a07d2c98db7771ce3 Mon Sep 17 00:00:00 2001 From: J0J0 Todos Date: Thu, 25 Dec 2025 10:36:20 +0100 Subject: [PATCH 33/39] lastgenre: Type hints for genre fetch methods --- beetsplug/lastgenre/__init__.py | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/beetsplug/lastgenre/__init__.py b/beetsplug/lastgenre/__init__.py index 40019f548..3d4d5b6b0 100644 --- a/beetsplug/lastgenre/__init__.py +++ b/beetsplug/lastgenre/__init__.py @@ -28,7 +28,7 @@ import os import traceback from functools import singledispatchmethod from pathlib import Path -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any, Callable import pylast import yaml @@ -259,9 +259,11 @@ class LastGenrePlugin(plugins.BeetsPlugin): valid_tags = [t for t in tags if self._is_valid(t)] return valid_tags[:count] - def fetch_genre(self, lastfm_obj): - """Return the genre for a pylast entity or None if no suitable genre - can be found. Ex. 'Electronic, House, Dance' + def fetch_genre( + self, lastfm_obj: pylast.Album | pylast.Artist | pylast.Track + ) -> list[str]: + """Return genres for a pylast entity. Returns an empty list if + no suitable genres are found. """ min_weight = self.config["min_weight"].get(int) return self._tags_for(lastfm_obj, min_weight) @@ -278,8 +280,10 @@ class LastGenrePlugin(plugins.BeetsPlugin): # Cached last.fm entity lookups. - def _last_lookup(self, entity, method, *args): - """Get a genre based on the named entity using the callable `method` + def _last_lookup( + self, entity: str, method: Callable[..., Any], *args: str + ) -> list[str]: + """Get genres based on the named entity using the callable `method` whose arguments are given in the sequence `args`. The genre lookup is cached based on the entity name and the arguments. @@ -293,24 +297,24 @@ class LastGenrePlugin(plugins.BeetsPlugin): key = f"{entity}.{'-'.join(str(a) for a in args)}" if key not in self._genre_cache: - args = [a.replace("\u2010", "-") for a in args] - self._genre_cache[key] = self.fetch_genre(method(*args)) + args_replaced = [a.replace("\u2010", "-") for a in args] + self._genre_cache[key] = self.fetch_genre(method(*args_replaced)) genre = self._genre_cache[key] self._tunelog("last.fm (unfiltered) {} tags: {}", entity, genre) return genre - def fetch_album_genre(self, albumartist, albumtitle): + def fetch_album_genre(self, albumartist: str, albumtitle: str) -> list[str]: """Return genres from Last.fm for the album by albumartist.""" return self._last_lookup( "album", LASTFM.get_album, albumartist, albumtitle ) - def fetch_artist_genre(self, artist): + def fetch_artist_genre(self, artist: str) -> list[str]: """Return genres from Last.fm for the artist.""" return self._last_lookup("artist", LASTFM.get_artist, artist) - def fetch_track_genre(self, trackartist, tracktitle): + def fetch_track_genre(self, trackartist: str, tracktitle: str) -> list[str]: """Return genres from Last.fm for the track by artist.""" return self._last_lookup( "track", LASTFM.get_track, trackartist, tracktitle From 28dc78be95c3f862bc578a8e2a2dc689264f5bad Mon Sep 17 00:00:00 2001 From: j0j0 Date: Sun, 16 Nov 2025 08:54:12 +0100 Subject: [PATCH 34/39] lastgenre: Changelog for #5981 lastgenre --- docs/changelog.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 0cfc1af24..49402bad7 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -73,6 +73,11 @@ Bug fixes: 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. +- :doc:`plugins/lastgenre`: Fix the issue where last.fm doesn't return any + result in the artist genre stage because "concatenation" words in the artist + name (like "feat.", "+", or "&") prevent it. Using the albumartists list field + and fetching a genre for each artist separately improves the chance of + receiving valid results in that stage. For plugin developers: From b8c7c87b41f995dddd0aa3206f8196d946e81747 Mon Sep 17 00:00:00 2001 From: J0J0 Todos Date: Thu, 25 Dec 2025 10:50:02 +0100 Subject: [PATCH 35/39] lastgenre: Add typehints to remaining methods, to finally reach full type hint coverage in the plugin! --- beetsplug/lastgenre/__init__.py | 51 ++++++++++++++++++++++----------- 1 file changed, 34 insertions(+), 17 deletions(-) diff --git a/beetsplug/lastgenre/__init__.py b/beetsplug/lastgenre/__init__.py index 3d4d5b6b0..e622096cf 100644 --- a/beetsplug/lastgenre/__init__.py +++ b/beetsplug/lastgenre/__init__.py @@ -38,6 +38,8 @@ from beets.library import Album, Item from beets.util import plurality, unique_list if TYPE_CHECKING: + import optparse + from beets.library import LibModel LASTFM = pylast.LastFMNetwork(api_key=plugins.LASTFM_KEY) @@ -52,7 +54,11 @@ PYLAST_EXCEPTIONS = ( # Canonicalization tree processing. -def flatten_tree(elem, path, branches): +def flatten_tree( + elem: dict[Any, Any] | list[Any] | str, + path: list[str], + branches: list[list[str]], +) -> None: """Flatten nested lists/dictionaries into lists of strings (branches). """ @@ -69,7 +75,7 @@ def flatten_tree(elem, path, branches): branches.append(path + [str(elem)]) -def find_parents(candidate, branches): +def find_parents(candidate: str, branches: list[list[str]]) -> list[str]: """Find parents genre of a given genre, ordered from the closest to the further parent. """ @@ -89,7 +95,7 @@ C14N_TREE = os.path.join(os.path.dirname(__file__), "genres-tree.yaml") class LastGenrePlugin(plugins.BeetsPlugin): - def __init__(self): + def __init__(self) -> None: super().__init__() self.config.add( @@ -111,12 +117,12 @@ class LastGenrePlugin(plugins.BeetsPlugin): ) self.setup() - def setup(self): + def setup(self) -> None: """Setup plugin from config options""" if self.config["auto"]: self.import_stages = [self.imported] - self._genre_cache = {} + self._genre_cache: dict[str, list[str]] = {} self.whitelist = self._load_whitelist() self.c14n_branches, self.canonicalize = self._load_c14n_tree() @@ -161,7 +167,7 @@ class LastGenrePlugin(plugins.BeetsPlugin): flatten_tree(genres_tree, [], c14n_branches) return c14n_branches, canonicalize - def _tunelog(self, msg, *args, **kwargs): + def _tunelog(self, msg: str, *args: Any, **kwargs: Any) -> None: """Log tuning messages at DEBUG level when verbosity level is high enough.""" if config["verbose"].as_number() >= 3: self._log.debug(msg, *args, **kwargs) @@ -182,7 +188,7 @@ class LastGenrePlugin(plugins.BeetsPlugin): # More canonicalization and general helpers. - def _get_depth(self, tag): + def _get_depth(self, tag: str) -> int | None: """Find the depth of a tag in the genres tree.""" depth = None for key, value in enumerate(self.c14n_branches): @@ -191,7 +197,7 @@ class LastGenrePlugin(plugins.BeetsPlugin): break return depth - def _sort_by_depth(self, tags): + def _sort_by_depth(self, tags: list[str]) -> list[str]: """Given a list of tags, sort the tags by their depths in the genre tree. """ @@ -372,7 +378,9 @@ class LastGenrePlugin(plugins.BeetsPlugin): and the whitelist feature was disabled. """ - def _try_resolve_stage(stage_label: str, keep_genres, new_genres): + def _try_resolve_stage( + stage_label: str, keep_genres: list[str], new_genres: list[str] + ) -> tuple[str, str] | None: """Try to resolve genres for a given stage and log the result.""" resolved_genres = self._combine_resolve_and_log( keep_genres, new_genres @@ -516,7 +524,7 @@ class LastGenrePlugin(plugins.BeetsPlugin): write=write, move=False, inherit="track" not in self.sources ) - def commands(self): + def commands(self) -> list[ui.Subcommand]: lastgenre_cmd = ui.Subcommand("lastgenre", help="fetch genres") lastgenre_cmd.parser.add_option( "-p", @@ -575,7 +583,9 @@ class LastGenrePlugin(plugins.BeetsPlugin): ) lastgenre_cmd.parser.set_defaults(album=True) - def lastgenre_func(lib, opts, args): + def lastgenre_func( + lib: library.Library, opts: optparse.Values, args: list[str] + ) -> None: self.config.set_args(opts) method = lib.albums if opts.album else lib.items @@ -585,10 +595,16 @@ class LastGenrePlugin(plugins.BeetsPlugin): lastgenre_cmd.func = lastgenre_func return [lastgenre_cmd] - def imported(self, session, task): + def imported( + self, session: library.Session, task: library.ImportTask + ) -> None: self._process(task.album if task.is_album else task.item, write=False) - def _tags_for(self, obj, min_weight=None): + def _tags_for( + self, + obj: pylast.Album | pylast.Artist | pylast.Track, + min_weight: int | None = None, + ) -> list[str]: """Core genre identification routine. Given a pylast entity (album or track), return a list of @@ -600,11 +616,12 @@ class LastGenrePlugin(plugins.BeetsPlugin): # Work around an inconsistency in pylast where # Album.get_top_tags() does not return TopItem instances. # https://github.com/pylast/pylast/issues/86 + obj_to_query: Any = obj if isinstance(obj, pylast.Album): - obj = super(pylast.Album, obj) + obj_to_query = super(pylast.Album, obj) try: - res = obj.get_top_tags() + res: Any = obj_to_query.get_top_tags() except PYLAST_EXCEPTIONS as exc: self._log.debug("last.fm error: {}", exc) return [] @@ -619,6 +636,6 @@ class LastGenrePlugin(plugins.BeetsPlugin): res = [el for el in res if (int(el.weight or 0)) >= min_weight] # Get strings from tags. - res = [el.item.get_name().lower() for el in res] + tags: list[str] = [el.item.get_name().lower() for el in res] - return res + return tags From c1e36e52a865940ad9319d7716c35c07260bf496 Mon Sep 17 00:00:00 2001 From: Alexandre Detiste Date: Thu, 1 Jan 2026 01:49:17 +0100 Subject: [PATCH 36/39] drop extraneous dependency on old external "mock" --- pyproject.toml | 2 -- 1 file changed, 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index bc694de90..e7eebd3a1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -101,7 +101,6 @@ beautifulsoup4 = "*" codecov = ">=2.1.13" flask = "*" langdetect = "*" -mock = "*" pylast = "*" pytest = "*" pytest-cov = "*" @@ -125,7 +124,6 @@ sphinx-lint = ">=1.0.0" mypy = "*" types-beautifulsoup4 = "*" types-docutils = ">=0.22.2.20251006" -types-mock = "*" types-Flask-Cors = "*" types-Pillow = "*" types-PyYAML = "*" From d6da6cda7ebcfb8b999bf716c92bf0b5f41ad80c Mon Sep 17 00:00:00 2001 From: Jack Wilsdon Date: Thu, 1 Jan 2026 15:46:06 +0000 Subject: [PATCH 37/39] Update poetry.lock after removing mock --- poetry.lock | 29 +---------------------------- 1 file changed, 1 insertion(+), 28 deletions(-) diff --git a/poetry.lock b/poetry.lock index 8e489b4ed..dbd3ecf3d 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1731,22 +1731,6 @@ mutagen = ">=1.46" [package.extras] test = ["tox"] -[[package]] -name = "mock" -version = "5.2.0" -description = "Rolling backport of unittest.mock for all Pythons" -optional = false -python-versions = ">=3.6" -files = [ - {file = "mock-5.2.0-py3-none-any.whl", hash = "sha256:7ba87f72ca0e915175596069dbbcc7c75af7b5e9b9bc107ad6349ede0819982f"}, - {file = "mock-5.2.0.tar.gz", hash = "sha256:4e460e818629b4b173f32d08bf30d3af8123afbb8e04bb5707a1fd4799e503f0"}, -] - -[package.extras] -build = ["blurb", "twine", "wheel"] -docs = ["sphinx"] -test = ["pytest", "pytest-cov"] - [[package]] name = "msgpack" version = "1.1.2" @@ -4063,17 +4047,6 @@ files = [ {file = "types_html5lib-1.1.11.20251014.tar.gz", hash = "sha256:cc628d626e0111a2426a64f5f061ecfd113958b69ff6b3dc0eaaed2347ba9455"}, ] -[[package]] -name = "types-mock" -version = "5.2.0.20250924" -description = "Typing stubs for mock" -optional = false -python-versions = ">=3.9" -files = [ - {file = "types_mock-5.2.0.20250924-py3-none-any.whl", hash = "sha256:23617ffb4cf948c085db69ec90bd474afbce634ef74995045ae0a5748afbe57d"}, - {file = "types_mock-5.2.0.20250924.tar.gz", hash = "sha256:953197543b4183f00363e8e626f6c7abea1a3f7a4dd69d199addb70b01b6bb35"}, -] - [[package]] name = "types-pillow" version = "10.2.0.20240822" @@ -4226,4 +4199,4 @@ web = ["flask", "flask-cors"] [metadata] lock-version = "2.0" python-versions = ">=3.10,<4" -content-hash = "8cf2ad0e6a842511e1215720a63bfdf9d5f49345410644cbb0b5fd8fb74f50d2" +content-hash = "45c7dc4ec30f4460a09554d0ec0ebcafebff097386e005e29e12830d16d223dd" From afc26fa58f15a18f1d672eb1b2432027d7b6ad35 Mon Sep 17 00:00:00 2001 From: Jack Wilsdon Date: Thu, 1 Jan 2026 15:50:37 +0000 Subject: [PATCH 38/39] Add packaging note about mock dependency removal --- docs/changelog.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 49402bad7..d3d9f3f6a 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -88,6 +88,7 @@ For plugin developers: For packagers: - The minimum supported Python version is now 3.10. +- An unused dependency on ``mock`` has been removed. Other changes: From b14755df881b46a5fcce1ada4d5d895c5e54a331 Mon Sep 17 00:00:00 2001 From: Trey Turner Date: Thu, 1 Jan 2026 15:39:17 -0600 Subject: [PATCH 39/39] fix(ftintitle): remaining opportunities for improvement --- beetsplug/ftintitle.py | 56 ++++++++++---------------- pyproject.toml | 1 + test/plugins/test_ftintitle.py | 72 +++++++++++++++++----------------- 3 files changed, 58 insertions(+), 71 deletions(-) diff --git a/beetsplug/ftintitle.py b/beetsplug/ftintitle.py index 44f17bc4e..cf30e83f4 100644 --- a/beetsplug/ftintitle.py +++ b/beetsplug/ftintitle.py @@ -146,32 +146,19 @@ class FtInTitlePlugin(plugins.BeetsPlugin): # If we have keywords, require one of them to appear in the bracket text. # If kw == "", the lookahead becomes true and we match any bracket content. - kw = rf"\b(?:{kw_inner})\b" if kw_inner else "" - + kw = rf"\b(?={kw_inner})\b" if kw_inner else "" return re.compile( rf""" - (?: # Match ONE bracketed segment of any supported type - \( # "(" - (?=[^)]*{kw}) # Lookahead: keyword must appear before closing ")" - # - if kw == "", this is always true - [^)]* # Consume bracket content (no nested ")" handling) - \) # ")" - - | \[ # "[" - (?=[^\]]*{kw}) # Lookahead - [^\]]* # Consume content up to first "]" - \] # "]" - - | < # "<" - (?=[^>]*{kw}) # Lookahead - [^>]* # Consume content up to first ">" - > # ">" - - | \x7B # Literal open brace - (?=[^\x7D]*{kw}) # Lookahead - [^\x7D]* # Consume content up to first close brace - \x7D # Literal close brace - ) # End bracketed segment alternation + (?: # non-capturing group for the split + \s*? # optional whitespace before brackets + (?= # any bracket containing a keyword + \([^)]*{kw}.*?\) + | \[[^]]*{kw}.*?\] + | <[^>]*{kw}.*? > + | \{{[^}}]*{kw}.*?\}} + | $ # or the end of the string + ) + ) """, re.IGNORECASE | re.VERBOSE, ) @@ -290,7 +277,7 @@ class FtInTitlePlugin(plugins.BeetsPlugin): if not drop_feat and not contains_feat(item.title, custom_words): feat_format = self.config["format"].as_str() formatted = feat_format.format(feat_part) - new_title = FtInTitlePlugin.insert_ft_into_title( + new_title = self.insert_ft_into_title( item.title, formatted, self.bracket_keywords ) self._log.info("title: {.title} -> {}", item, new_title) @@ -349,19 +336,16 @@ class FtInTitlePlugin(plugins.BeetsPlugin): m: re.Match[str] | None = pattern.search(title) return m.start() if m else None - @staticmethod + @classmethod def insert_ft_into_title( - title: str, feat_part: str, keywords: list[str] | None = None + cls, title: str, feat_part: str, keywords: list[str] | None = None ) -> str: """Insert featured artist before the first bracket containing remix/edit keywords if present. """ - if ( - bracket_pos := FtInTitlePlugin.find_bracket_position( - title, keywords - ) - ) is not None: - title_before = title[:bracket_pos].rstrip() - title_after = title[bracket_pos:] - return f"{title_before} {feat_part} {title_after}" - return f"{title} {feat_part}" + normalized = ( + DEFAULT_BRACKET_KEYWORDS if keywords is None else tuple(keywords) + ) + pattern = cls._bracket_position_pattern(normalized) + parts = pattern.split(title, maxsplit=1) + return f" {feat_part} ".join(parts).strip() diff --git a/pyproject.toml b/pyproject.toml index 24cf21b33..06552f124 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/plugins/test_ftintitle.py" = ["E501"] [tool.ruff.lint.isort] split-on-trailing-comma = false diff --git a/test/plugins/test_ftintitle.py b/test/plugins/test_ftintitle.py index abba22d11..b21ac1c7f 100644 --- a/test/plugins/test_ftintitle.py +++ b/test/plugins/test_ftintitle.py @@ -335,55 +335,57 @@ def test_split_on_feat( [ ## default keywords # different braces and keywords - ("Song (Remix)", None, 5), - ("Song [Version]", None, 5), - ("Song {Extended Mix}", None, 5), - ("Song ", None, 5), + ("Song (Remix)", None, "Song ft. Bob (Remix)"), + ("Song [Version]", None, "Song ft. Bob [Version]"), + ("Song {Extended Mix}", None, "Song ft. Bob {Extended Mix}"), + ("Song ", None, "Song ft. Bob "), # two keyword clauses - ("Song (Remix) (Live)", None, 5), + ("Song (Remix) (Live)", None, "Song ft. Bob (Remix) (Live)"), # brace insensitivity - ("Song (Live) [Remix]", None, 5), - ("Song [Edit] (Remastered)", None, 5), + ("Song (Live) [Remix]", None, "Song ft. Bob (Live) [Remix]"), + ("Song [Edit] (Remastered)", None, "Song ft. Bob [Edit] (Remastered)"), # negative cases - ("Song", None, None), # no clause - ("Song (Arbitrary)", None, None), # no keyword - ("Song (", None, None), # no matching brace or keyword - ("Song (Live", None, None), # no matching brace with keyword + ("Song", None, "Song ft. Bob"), # no clause + ("Song (Arbitrary)", None, "Song (Arbitrary) ft. Bob"), # no keyword + ("Song (", None, "Song ( ft. Bob"), # no matching brace or keyword + ("Song (Live", None, "Song (Live ft. Bob"), # no matching brace with keyword # one keyword clause, one non-keyword clause - ("Song (Live) (Arbitrary)", None, 5), - ("Song (Arbitrary) (Remix)", None, 17), + ("Song (Live) (Arbitrary)", None, "Song ft. Bob (Live) (Arbitrary)"), + ("Song (Arbitrary) (Remix)", None, "Song (Arbitrary) ft. Bob (Remix)"), # nested brackets - same type - ("Song (Remix (Extended))", None, 5), - ("Song [Arbitrary [Description]]", None, None), + ("Song (Remix (Extended))", None, "Song ft. Bob (Remix (Extended))"), + ("Song [Arbitrary [Description]]", None, "Song [Arbitrary [Description]] ft. Bob"), # nested brackets - different types - ("Song (Remix [Extended])", None, 5), + ("Song (Remix [Extended])", None, "Song ft. Bob (Remix [Extended])"), # nested - returns outer start position despite inner keyword - ("Song [Arbitrary {Extended}]", None, 5), - ("Song {Live }", None, 5), - ("Song ", None, 5), - ("Song [Live]", None, 5), - ("Song (Version) ", None, 5), - ("Song (Arbitrary [Description])", None, None), - ("Song [Description (Arbitrary)]", None, None), + ("Song [Arbitrary {Extended}]", None, "Song ft. Bob [Arbitrary {Extended}]"), + ("Song {Live }", None, "Song ft. Bob {Live }"), + ("Song ", None, "Song ft. Bob "), + ("Song [Live]", None, "Song ft. Bob [Live]"), + ("Song (Version) ", None, "Song ft. Bob (Version) "), + ("Song (Arbitrary [Description])", None, "Song (Arbitrary [Description]) ft. Bob"), + ("Song [Description (Arbitrary)]", None, "Song [Description (Arbitrary)] ft. Bob"), ## custom keywords - ("Song (Live)", ["live"], 5), - ("Song (Concert)", ["concert"], 5), - ("Song (Remix)", ["custom"], None), - ("Song (Custom)", ["custom"], 5), - ("Song", [], None), - ("Song (", [], None), + ("Song (Live)", ["live"], "Song ft. Bob (Live)"), + ("Song (Concert)", ["concert"], "Song ft. Bob (Concert)"), + ("Song (Remix)", ["custom"], "Song (Remix) ft. Bob"), + ("Song (Custom)", ["custom"], "Song ft. Bob (Custom)"), + ("Song", [], "Song ft. Bob"), + ("Song (", [], "Song ( ft. Bob"), # Multi-word keyword tests - ("Song (Club Mix)", ["club mix"], 5), # Positive: matches multi-word - ("Song (Club Remix)", ["club mix"], None), # Negative: no match + ("Song (Club Mix)", ["club mix"], "Song ft. Bob (Club Mix)"), # Positive: matches multi-word + ("Song (Club Remix)", ["club mix"], "Song (Club Remix) ft. Bob"), # Negative: no match ], -) -def test_find_bracket_position( +) # fmt: skip +def test_insert_ft_into_title( given: str, keywords: list[str] | None, - expected: int | None, + expected: str, ) -> None: assert ( - ftintitle.FtInTitlePlugin.find_bracket_position(given, keywords) + ftintitle.FtInTitlePlugin.insert_ft_into_title( + given, "ft. Bob", keywords + ) == expected )