Merge remote-tracking branch 'upstream/master' into smartplaylist

README.rst

@@ -85,7 +85,7 @@ simple if you know a little Python.
 
 .. _transcode audio: https://beets.readthedocs.org/page/plugins/convert.html
 
-.. _writing your own plugin: https://beets.readthedocs.org/page/dev/plugins.html
+.. _writing your own plugin: https://beets.readthedocs.org/page/dev/plugins/index.html
 
 Install
 -------

README_kr.rst

@@ -79,7 +79,7 @@ Beets는 라이브러리로 디자인 되었기 때문에, 당신이 음악들
 
 .. _transcode audio: https://beets.readthedocs.org/page/plugins/convert.html
 
-.. _writing your own plugin: https://beets.readthedocs.org/page/dev/plugins.html
+.. _writing your own plugin: https://beets.readthedocs.org/page/dev/plugins/index.html
 
 설치
 -------

beets/__init__.py

@@ -17,7 +17,7 @@ from sys import stderr
 
 import confuse
 
-from .util import deprecate_imports
+from .util.deprecation import deprecate_imports
 
 __version__ = "2.5.1"
 __author__ = "Adrian Sampson <adrian@radbox.org>"
@@ -26,13 +26,9 @@ __author__ = "Adrian Sampson <adrian@radbox.org>"
 def __getattr__(name: str):
     """Handle deprecated imports."""
     return deprecate_imports(
-        old_module=__name__,
+        __name__,
-        new_module_by_name={
+        {"art": "beetsplug._utils", "vfs": "beetsplug._utils"},
-            "art": "beetsplug._utils",
+        name,
-            "vfs": "beetsplug._utils",
-        },
-        name=name,
-        version="3.0.0",
     )
 
 

beets/autotag/__init__.py

@@ -16,7 +16,6 @@
 
 from __future__ import annotations
 
-import warnings
 from importlib import import_module
 from typing import TYPE_CHECKING
 
@@ -24,8 +23,8 @@ from beets import config, logging
 
 # Parts of external interface.
 from beets.util import unique_list
+from beets.util.deprecation import deprecate_for_maintainers, deprecate_imports
 
-from ..util import deprecate_imports
 from .hooks import AlbumInfo, AlbumMatch, TrackInfo, TrackMatch
 from .match import Proposal, Recommendation, tag_album, tag_item
 
@@ -37,18 +36,13 @@ if TYPE_CHECKING:
 
 def __getattr__(name: str):
     if name == "current_metadata":
-        warnings.warn(
+        deprecate_for_maintainers(
-            (
+            f"'beets.autotag.{name}'", "'beets.util.get_most_common_tags'"
-                f"'beets.autotag.{name}' is deprecated and will be removed in"
-                " 3.0.0. Use 'beets.util.get_most_common_tags' instead."
-            ),
-            DeprecationWarning,
-            stacklevel=2,
         )
         return import_module("beets.util").get_most_common_tags
 
     return deprecate_imports(
-        __name__, {"Distance": "beets.autotag.distance"}, name, "3.0.0"
+        __name__, {"Distance": "beets.autotag.distance"}, name
     )
 
 

beets/library/__init__.py

@@ -1,4 +1,4 @@
-from beets.util import deprecate_imports
+from beets.util.deprecation import deprecate_imports
 
 from .exceptions import FileOperationError, ReadError, WriteError
 from .library import Library
@@ -13,7 +13,7 @@ NEW_MODULE_BY_NAME = dict.fromkeys(
 
 
 def __getattr__(name: str):
-    return deprecate_imports(__name__, NEW_MODULE_BY_NAME, name, "3.0.0")
+    return deprecate_imports(__name__, NEW_MODULE_BY_NAME, name)
 
 
 __all__ = [

beets/logging.py

@@ -22,6 +22,7 @@ calls (`debug`, `info`, etc).
 
 from __future__ import annotations
 
+import re
 import threading
 from copy import copy
 from logging import (
@@ -68,6 +69,15 @@ if TYPE_CHECKING:
     _ArgsType = Union[tuple[object, ...], Mapping[str, object]]
 
 
+# Regular expression to match:
+# - C0 control characters (0x00-0x1F) except useful whitespace (\t, \n, \r)
+# - DEL control character (0x7f)
+# - C1 control characters (0x80-0x9F)
+# Used to sanitize log messages that could disrupt terminal output
+_CONTROL_CHAR_REGEX = re.compile(r"[\x00-\x08\x0b\x0c\x0e-\x1f\x7f\x80-\x9f]")
+_UNICODE_REPLACEMENT_CHARACTER = "\ufffd"
+
+
 def _logsafe(val: T) -> str | T:
     """Coerce `bytes` to `str` to avoid crashes solely due to logging.
 
@@ -82,6 +92,10 @@ def _logsafe(val: T) -> str | T:
         # type, and (b) warn the developer if they do this for other
         # bytestrings.
         return val.decode("utf-8", "replace")
+    if isinstance(val, str):
+        # Sanitize log messages by replacing control characters that can disrupt
+        # terminals.
+        return _CONTROL_CHAR_REGEX.sub(_UNICODE_REPLACEMENT_CHARACTER, val)
 
     # Other objects are used as-is so field access, etc., still works in
     # the format string. Relies on a working __str__ implementation.

beets/mediafile.py

@@ -13,17 +13,11 @@
 # included in all copies or substantial portions of the Software.
 
 
-import warnings
-
 import mediafile
 
-warnings.warn(
+from .util.deprecation import deprecate_for_maintainers
-    "beets.mediafile is deprecated; use mediafile instead",
+
-    # Show the location of the `import mediafile` statement as the warning's
+deprecate_for_maintainers("'beets.mediafile'", "'mediafile'", stacklevel=2)
-    # source, rather than this file, such that the offending module can be
-    # identified easily.
-    stacklevel=2,
-)
 
 # Import everything from the mediafile module into this module.
 for key, value in mediafile.__dict__.items():
@@ -31,4 +25,4 @@ for key, value in mediafile.__dict__.items():
         globals()[key] = value
 
 # Cleanup namespace.
-del key, value, warnings, mediafile
+del key, value, mediafile

beets/plugins.py

@@ -20,7 +20,6 @@ import abc
 import inspect
 import re
 import sys
-import warnings
 from collections import defaultdict
 from functools import cached_property, wraps
 from importlib import import_module
@@ -33,6 +32,7 @@ from typing_extensions import ParamSpec
 import beets
 from beets import logging
 from beets.util import unique_list
+from beets.util.deprecation import deprecate_for_maintainers, deprecate_for_user
 
 if TYPE_CHECKING:
     from collections.abc import Callable, Iterable, Sequence
@@ -184,11 +184,12 @@ class BeetsPlugin(metaclass=abc.ABCMeta):
         ):
             return
 
-        warnings.warn(
+        deprecate_for_maintainers(
-            f"{cls.__name__} is used as a legacy metadata source. "
+            (
-            "It should extend MetadataSourcePlugin instead of BeetsPlugin. "
+                f"'{cls.__name__}' is used as a legacy metadata source since it"
-            "Support for this will be removed in the v3.0.0 release!",
+                " inherits 'beets.plugins.BeetsPlugin'. Support for this"
-            DeprecationWarning,
+            ),
+            "'beets.metadata_plugins.MetadataSourcePlugin'",
             stacklevel=3,
         )
 
@@ -256,16 +257,19 @@ class BeetsPlugin(metaclass=abc.ABCMeta):
         ):
             return
 
-        message = (
-            "'source_weight' configuration option is deprecated and will be"
-            " removed in v3.0.0. Use 'data_source_mismatch_penalty' instead"
-        )
         for source in self.config.root().sources:
             if "source_weight" in (source.get(self.name) or {}):
                 if source.filename:  # user config
-                    self._log.warning(message)
+                    deprecate_for_user(
+                        self._log,
+                        f"'{self.name}.source_weight' configuration option",
+                        f"'{self.name}.data_source_mismatch_penalty'",
+                    )
                 else:  # 3rd-party plugin config
-                    warnings.warn(message, DeprecationWarning, stacklevel=0)
+                    deprecate_for_maintainers(
+                        "'source_weight' configuration option",
+                        "'data_source_mismatch_penalty'",
+                    )
 
     def commands(self) -> Sequence[Subcommand]:
         """Should return a list of beets.ui.Subcommand objects for
@@ -410,16 +414,22 @@ def get_plugin_names() -> list[str]:
     # *contain* a `beetsplug` package.
     sys.path += paths
     plugins = unique_list(beets.config["plugins"].as_str_seq())
-    # TODO: Remove in v3.0.0
-    if (
-        "musicbrainz" not in plugins
-        and "musicbrainz" in beets.config
-        and beets.config["musicbrainz"].get().get("enabled")
-    ):
-        plugins.append("musicbrainz")
-
     beets.config.add({"disabled_plugins": []})
     disabled_plugins = set(beets.config["disabled_plugins"].as_str_seq())
+    # TODO: Remove in v3.0.0
+    mb_enabled = beets.config["musicbrainz"].flatten().get("enabled")
+    if mb_enabled:
+        deprecate_for_user(
+            log,
+            "'musicbrainz.enabled' configuration option",
+            "'plugins' configuration to explicitly add 'musicbrainz'",
+        )
+        if "musicbrainz" not in plugins:
+            plugins.append("musicbrainz")
+    elif mb_enabled is False:
+        deprecate_for_user(log, "'musicbrainz.enabled' configuration option")
+        disabled_plugins.add("musicbrainz")
+
     return [p for p in plugins if p not in disabled_plugins]
 
 

beets/ui/__init__.py

@@ -28,7 +28,6 @@ import sqlite3
 import sys
 import textwrap
 import traceback
-import warnings
 from difflib import SequenceMatcher
 from functools import cache
 from itertools import chain
@@ -40,6 +39,7 @@ from beets import config, library, logging, plugins, util
 from beets.dbcore import db
 from beets.dbcore import query as db_query
 from beets.util import as_string
+from beets.util.deprecation import deprecate_for_maintainers
 from beets.util.functemplate import template
 
 if TYPE_CHECKING:
@@ -114,11 +114,7 @@ def decargs(arglist):
     .. deprecated:: 2.4.0
        This function will be removed in 3.0.0.
     """
-    warnings.warn(
+    deprecate_for_maintainers("'beets.ui.decargs'")
-        "decargs() is deprecated and will be removed in version 3.0.0.",
-        DeprecationWarning,
-        stacklevel=2,
-    )
     return arglist
 
 

beets/ui/commands/__init__.py

@@ -16,7 +16,7 @@
 interface.
 """
 
-from beets.util import deprecate_imports
+from beets.util.deprecation import deprecate_imports
 
 from .completion import completion_cmd
 from .config import config_cmd
@@ -36,14 +36,12 @@ from .write import write_cmd
 def __getattr__(name: str):
     """Handle deprecated imports."""
     return deprecate_imports(
-        old_module=__name__,
+        __name__,
-        new_module_by_name={
+        {
             "TerminalImportSession": "beets.ui.commands.import_.session",
-            "PromptChoice": "beets.ui.commands.import_.session",
+            "PromptChoice": "beets.util",
-            # TODO: We might want to add more deprecated imports here
         },
-        name=name,
+        name,
-        version="3.0.0",
     )
 
 

beets/ui/commands/import_/session.py

@@ -1,10 +1,9 @@
 from collections import Counter
 from itertools import chain
-from typing import Any, NamedTuple
 
 from beets import autotag, config, importer, logging, plugins, ui
 from beets.autotag import Recommendation
-from beets.util import displayable_path
+from beets.util import PromptChoice, displayable_path
 from beets.util.units import human_bytes, human_seconds_short
 
 from .display import (
@@ -368,12 +367,6 @@ def _summary_judgment(rec):
     return action
 
 
-class PromptChoice(NamedTuple):
-    short: str
-    long: str
-    callback: Any
-
-
 def choose_candidate(
     candidates,
     singleton,

beets/util/__init__.py

@@ -27,7 +27,6 @@ import subprocess
 import sys
 import tempfile
 import traceback
-import warnings
 from collections import Counter
 from collections.abc import Callable, Sequence
 from contextlib import suppress
@@ -168,6 +167,12 @@ class MoveOperation(Enum):
     REFLINK_AUTO = 5
 
 
+class PromptChoice(NamedTuple):
+    short: str
+    long: str
+    callback: Any
+
+
 def normpath(path: PathLike) -> bytes:
     """Provide the canonical form of the path suitable for storing in
     the database.
@@ -1195,26 +1200,3 @@ def get_temp_filename(
 def unique_list(elements: Iterable[T]) -> list[T]:
     """Return a list with unique elements in the original order."""
     return list(dict.fromkeys(elements))
-
-
-def deprecate_imports(
-    old_module: str, new_module_by_name: dict[str, str], name: str, version: str
-) -> Any:
-    """Handle deprecated module imports by redirecting to new locations.
-
-    Facilitates gradual migration of module structure by intercepting import
-    attempts for relocated functionality. Issues deprecation warnings while
-    transparently providing access to the moved implementation, allowing
-    existing code to continue working during transition periods.
-    """
-    if new_module := new_module_by_name.get(name):
-        warnings.warn(
-            (
-                f"'{old_module}.{name}' is deprecated and will be removed"
-                f" in {version}. Use '{new_module}.{name}' instead."
-            ),
-            DeprecationWarning,
-            stacklevel=2,
-        )
-        return getattr(import_module(new_module), name)
-    raise AttributeError(f"module '{old_module}' has no attribute '{name}'")

beets/util/deprecation.py

@@ -0,0 +1,60 @@
+from __future__ import annotations
+
+import warnings
+from importlib import import_module
+from typing import TYPE_CHECKING, Any
+
+from packaging.version import Version
+
+import beets
+
+if TYPE_CHECKING:
+    from logging import Logger
+
+
+def _format_message(old: str, new: str | None = None) -> str:
+    next_major = f"{Version(beets.__version__).major + 1}.0.0"
+    msg = f"{old} is deprecated and will be removed in version {next_major}."
+    if new:
+        msg += f" Use {new} instead."
+
+    return msg
+
+
+def deprecate_for_user(
+    logger: Logger, old: str, new: str | None = None
+) -> None:
+    logger.warning(_format_message(old, new))
+
+
+def deprecate_for_maintainers(
+    old: str, new: str | None = None, stacklevel: int = 1
+) -> None:
+    """Issue a deprecation warning visible to maintainers during development.
+
+    Emits a DeprecationWarning that alerts developers about deprecated code
+    patterns. Unlike user-facing warnings, these are primarily for internal
+    code maintenance and appear during test runs or with warnings enabled.
+    """
+    warnings.warn(
+        _format_message(old, new), DeprecationWarning, stacklevel=stacklevel + 1
+    )
+
+
+def deprecate_imports(
+    old_module: str, new_module_by_name: dict[str, str], name: str
+) -> Any:
+    """Handle deprecated module imports by redirecting to new locations.
+
+    Facilitates gradual migration of module structure by intercepting import
+    attempts for relocated functionality. Issues deprecation warnings while
+    transparently providing access to the moved implementation, allowing
+    existing code to continue working during transition periods.
+    """
+    if new_module := new_module_by_name.get(name):
+        deprecate_for_maintainers(
+            f"'{old_module}.{name}'", f"'{new_module}.{name}'", stacklevel=2
+        )
+
+        return getattr(import_module(new_module), name)
+    raise AttributeError(f"module '{old_module}' has no attribute '{name}'")

beetsplug/edit.py

@@ -25,8 +25,8 @@ import yaml
 from beets import plugins, ui, util
 from beets.dbcore import types
 from beets.importer import Action
-from beets.ui.commands.import_.session import PromptChoice
 from beets.ui.commands.utils import do_query
+from beets.util import PromptChoice
 
 # These "safe" types can avoid the format/parse cycle that most fields go
 # through: they are safe to edit with native YAML types.

beetsplug/mbsubmit.py

@@ -26,8 +26,7 @@ import subprocess
 from beets import ui
 from beets.autotag import Recommendation
 from beets.plugins import BeetsPlugin
-from beets.ui.commands import PromptChoice
+from beets.util import PromptChoice, displayable_path
-from beets.util import displayable_path
 from beetsplug.info import print_data
 
 

beetsplug/musicbrainz.py

@@ -31,6 +31,7 @@ import beets
 import beets.autotag.hooks
 from beets import config, plugins, util
 from beets.metadata_plugins import MetadataSourcePlugin
+from beets.util.deprecation import deprecate_for_user
 from beets.util.id_extractors import extract_release_id
 
 if TYPE_CHECKING:
@@ -403,9 +404,10 @@ class MusicBrainzPlugin(MetadataSourcePlugin):
             self.config["search_limit"] = self.config["match"][
                 "searchlimit"
             ].get()
-            self._log.warning(
+            deprecate_for_user(
-                "'musicbrainz.searchlimit' option is deprecated and will be "
+                self._log,
-                "removed in 3.0.0. Use 'musicbrainz.search_limit' instead."
+                "'musicbrainz.searchlimit' configuration option",
+                "'musicbrainz.search_limit'",
             )
         hostname = self.config["host"].as_str()
         https = self.config["https"].get(bool)

beetsplug/play.py

@@ -21,8 +21,7 @@ from os.path import relpath
 from beets import config, ui, util
 from beets.plugins import BeetsPlugin
 from beets.ui import Subcommand
-from beets.ui.commands import PromptChoice
+from beets.util import PromptChoice, get_temp_filename
-from beets.util import get_temp_filename
 
 # Indicate where arguments should be inserted into the command string.
 # If this is missing, they're placed at the end.

docs/changelog.rst

@@ -58,6 +58,8 @@ Bug fixes:
   endpoints. Previously, due to single-quotes (ie. string literal) in the SQL
   query, the query eg. `GET /item/values/albumartist` would return the literal
   "albumartist" instead of a list of unique album artists.
+- Sanitize log messages by removing control characters preventing terminal
+  rendering issues.
 
 For plugin developers:
 

docs/dev/plugins/other/prompts.rst

@@ -13,7 +13,7 @@ shall expose to the user:
 .. code-block:: python
 
     from beets.plugins import BeetsPlugin
-    from beets.ui.commands import PromptChoice
+    from beets.util import PromptChoice
 
 
     class ExamplePlugin(BeetsPlugin):

test/test_files.py

@@ -579,7 +579,7 @@ class SafeMoveCopyTest(FilePathTestCase):
 
     @NEEDS_REFLINK
     def test_successful_reflink(self):
-        util.reflink(self.path, self.dest)
+        util.reflink(str(self.path), str(self.dest))
         assert self.dest.exists()
         assert self.path.exists()
 

test/test_logging.py

@@ -67,6 +67,58 @@ class TestStrFormatLogger:
         assert str(caplog.records[0].msg) == expected
 
 
+class TestLogSanitization:
+    """Log messages should have control characters removed from:
+    - String arguments
+    - Keyword argument values
+    - Bytes arguments (which get decoded first)
+    """
+
+    @pytest.mark.parametrize(
+        "msg, args, kwargs, expected",
+        [
+            # Valid UTF-8 bytes are decoded and preserved
+            (
+                "foo {} bar {bar}",
+                (b"oof \xc3\xa9",),
+                {"bar": b"baz \xc3\xa9"},
+                "foo oof é bar baz é",
+            ),
+            # Invalid UTF-8 bytes are decoded with replacement characters
+            (
+                "foo {} bar {bar}",
+                (b"oof \xff",),
+                {"bar": b"baz \xff"},
+                "foo oof <20> bar baz <20>",
+            ),
+            # Control characters should be removed
+            (
+                "foo {} bar {bar}",
+                ("oof \x9e",),
+                {"bar": "baz \x9e"},
+                "foo oof <20> bar baz <20>",
+            ),
+            # Whitespace control characters should be preserved
+            (
+                "foo {} bar {bar}",
+                ("foo\t\n",),
+                {"bar": "bar\r"},
+                "foo foo\t\n bar bar\r",
+            ),
+        ],
+    )
+    def test_sanitization(self, msg, args, kwargs, expected, caplog):
+        level = log.INFO
+        logger = blog.getLogger("test_logger")
+        logger.setLevel(level)
+
+        with caplog.at_level(level, logger="test_logger"):
+            logger.log(level, msg, *args, **kwargs)
+
+        assert caplog.records, "No log records were captured"
+        assert str(caplog.records[0].msg) == expected
+
+
 class DummyModule(ModuleType):
     class DummyPlugin(plugins.BeetsPlugin):
         def __init__(self):

test/test_plugins.py

@@ -41,7 +41,7 @@ from beets.test.helper import (
     PluginTestCase,
     TerminalImportMixin,
 )
-from beets.util import displayable_path, syspath
+from beets.util import PromptChoice, displayable_path, syspath
 
 
 class TestPluginRegistration(PluginTestCase):
@@ -292,8 +292,8 @@ class PromptChoicesTest(TerminalImportMixin, PluginImportTestCase):
 
             def return_choices(self, session, task):
                 return [
-                    ui.commands.PromptChoice("f", "Foo", None),
+                    PromptChoice("f", "Foo", None),
-                    ui.commands.PromptChoice("r", "baR", None),
+                    PromptChoice("r", "baR", None),
                 ]
 
         self.register_plugin(DummyPlugin)
@@ -328,8 +328,8 @@ class PromptChoicesTest(TerminalImportMixin, PluginImportTestCase):
 
             def return_choices(self, session, task):
                 return [
-                    ui.commands.PromptChoice("f", "Foo", None),
+                    PromptChoice("f", "Foo", None),
-                    ui.commands.PromptChoice("r", "baR", None),
+                    PromptChoice("r", "baR", None),
                 ]
 
         self.register_plugin(DummyPlugin)
@@ -363,10 +363,10 @@ class PromptChoicesTest(TerminalImportMixin, PluginImportTestCase):
 
             def return_choices(self, session, task):
                 return [
-                    ui.commands.PromptChoice("a", "A foo", None),  # dupe
+                    PromptChoice("a", "A foo", None),  # dupe
-                    ui.commands.PromptChoice("z", "baZ", None),  # ok
+                    PromptChoice("z", "baZ", None),  # ok
-                    ui.commands.PromptChoice("z", "Zupe", None),  # dupe
+                    PromptChoice("z", "Zupe", None),  # dupe
-                    ui.commands.PromptChoice("z", "Zoo", None),
+                    PromptChoice("z", "Zoo", None),
                 ]  # dupe
 
         self.register_plugin(DummyPlugin)
@@ -399,7 +399,7 @@ class PromptChoicesTest(TerminalImportMixin, PluginImportTestCase):
                 )
 
             def return_choices(self, session, task):
-                return [ui.commands.PromptChoice("f", "Foo", self.foo)]
+                return [PromptChoice("f", "Foo", self.foo)]
 
             def foo(self, session, task):
                 pass
@@ -441,7 +441,7 @@ class PromptChoicesTest(TerminalImportMixin, PluginImportTestCase):
                 )
 
             def return_choices(self, session, task):
-                return [ui.commands.PromptChoice("f", "Foo", self.foo)]
+                return [PromptChoice("f", "Foo", self.foo)]
 
             def foo(self, session, task):
                 return Action.SKIP
@@ -543,3 +543,39 @@ class TestDeprecationCopy:
         assert hasattr(LegacyMetadataPlugin, "data_source_mismatch_penalty")
         assert hasattr(LegacyMetadataPlugin, "_extract_id")
         assert hasattr(LegacyMetadataPlugin, "get_artist")
+
+
+class TestMusicBrainzPluginLoading:
+    @pytest.fixture(autouse=True)
+    def config(self):
+        _config = config
+        _config.sources = []
+        _config.read(user=False, defaults=True)
+        return _config
+
+    def test_default(self):
+        assert "musicbrainz" in plugins.get_plugin_names()
+
+    def test_other_plugin_enabled(self, config):
+        config["plugins"] = ["anything"]
+
+        assert "musicbrainz" not in plugins.get_plugin_names()
+
+    def test_deprecated_enabled(self, config, caplog):
+        config["plugins"] = ["anything"]
+        config["musicbrainz"]["enabled"] = True
+
+        assert "musicbrainz" in plugins.get_plugin_names()
+        assert (
+            "musicbrainz.enabled' configuration option is deprecated"
+            in caplog.text
+        )
+
+    def test_deprecated_disabled(self, config, caplog):
+        config["musicbrainz"]["enabled"] = False
+
+        assert "musicbrainz" not in plugins.get_plugin_names()
+        assert (
+            "musicbrainz.enabled' configuration option is deprecated"
+            in caplog.text
+        )