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

beetsplug/fromfilename.py

@@ -12,8 +12,8 @@
 # The above copyright notice and this permission notice shall be
 # included in all copies or substantial portions of the Software.
 
-"""If the title is empty, try to extract track and title from the
-filename.
+"""If the title is empty, try to extract it from the filename
+(possibly also extract track and artist)
 """
 
 import os
@@ -25,12 +25,12 @@ from beets.util import displayable_path
 # Filename field extraction patterns.
 PATTERNS = [
     # Useful patterns.
-    r"^(?P<artist>.+)[\-_](?P<title>.+)[\-_](?P<tag>.*)$",
-    r"^(?P<track>\d+)[\s.\-_]+(?P<artist>.+)[\-_](?P<title>.+)[\-_](?P<tag>.*)$",
-    r"^(?P<artist>.+)[\-_](?P<title>.+)$",
-    r"^(?P<track>\d+)[\s.\-_]+(?P<artist>.+)[\-_](?P<title>.+)$",
-    r"^(?P<track>\d+)[\s.\-_]+(?P<title>.+)$",
-    r"^(?P<track>\d+)\s+(?P<title>.+)$",
+    (
+        r"^(?P<track>\d+)\.?\s*-\s*(?P<artist>.+?)\s*-\s*(?P<title>.+?)"
+        r"(\s*-\s*(?P<tag>.*))?$"
+    ),
+    r"^(?P<artist>.+?)\s*-\s*(?P<title>.+?)(\s*-\s*(?P<tag>.*))?$",
+    r"^(?P<track>\d+)\.?[\s_-]+(?P<title>.+)$",
     r"^(?P<title>.+) by (?P<artist>.+)$",
     r"^(?P<track>\d+).*$",
     r"^(?P<title>.+)$",
@@ -98,6 +98,7 @@ def apply_matches(d, log):
     # Given both an "artist" and "title" field, assume that one is
     # *actually* the artist, which must be uniform, and use the other
     # for the title. This, of course, won't work for VA albums.
+    # Only check for "artist": patterns containing it, also contain "title"
     if "artist" in keys:
         if equal_fields(d, "artist"):
             artist = some_map["artist"]
@@ -113,15 +114,16 @@ def apply_matches(d, log):
             if not item.artist:
                 item.artist = artist
                 log.info("Artist replaced with: {.artist}", item)
-
-    # No artist field: remaining field is the title.
-    else:
+    # otherwise, if the pattern contains "title", use that for title_field
+    elif "title" in keys:
         title_field = "title"
+    else:
+        title_field = None
 
-    # Apply the title and track.
+    # Apply the title and track, if any.
     for item in d:
-        if bad_title(item.title):
-            item.title = str(d[item].get(title_field, ""))
+        if title_field and bad_title(item.title):
+            item.title = str(d[item][title_field])
             log.info("Title replaced with: {.title}", item)
 
         if "track" in d[item] and item.track == 0:
@@ -160,6 +162,7 @@ class FromFilenamePlugin(plugins.BeetsPlugin):
 
             # Look for useful information in the filenames.
             for pattern in PATTERNS:
+                self._log.debug(f"Trying pattern: {pattern}")
                 d = all_matches(names, pattern)
                 if d:
                     apply_matches(d, self._log)

docs/changelog.rst

@@ -43,6 +43,8 @@ Bug fixes:
   artists but not labels. :bug:`5366`
 - :doc:`plugins/chroma` :doc:`plugins/bpsync` Fix plugin loading issue caused by
   an import of another :class:`beets.plugins.BeetsPlugin` class. :bug:`6033`
+- :doc:`/plugins/fromfilename`: Fix :bug:`5218`, improve the code (refactor
+  regexps, allow for more cases, add some logging), add tests.
 
 For packagers:
 

test/plugins/test_fromfilename.py

@@ -0,0 +1,99 @@
+# This file is part of beets.
+#
+# Permission is hereby granted, free of charge, to any person obtaining
+# a copy of this software and associated documentation files (the
+# "Software"), to deal in the Software without restriction, including
+# without limitation the rights to use, copy, modify, merge, publish,
+# distribute, sublicense, and/or sell copies of the Software, and to
+# permit persons to whom the Software is furnished to do so, subject to
+# the following conditions:
+#
+# The above copyright notice and this permission notice shall be
+# included in all copies or substantial portions of the Software.
+
+"""Tests for the fromfilename plugin."""
+
+import pytest
+
+from beetsplug import fromfilename
+
+
+class Session:
+    pass
+
+
+class Item:
+    def __init__(self, path):
+        self.path = path
+        self.track = 0
+        self.artist = ""
+        self.title = ""
+
+
+class Task:
+    def __init__(self, items):
+        self.items = items
+        self.is_album = True
+
+
+@pytest.mark.parametrize(
+    "song1, song2",
+    [
+        (
+            (
+                "/tmp/01 - The Artist - Song One.m4a",
+                1,
+                "The Artist",
+                "Song One",
+            ),
+            (
+                "/tmp/02. - The Artist - Song Two.m4a",
+                2,
+                "The Artist",
+                "Song Two",
+            ),
+        ),
+        (
+            ("/tmp/01-The_Artist-Song_One.m4a", 1, "The_Artist", "Song_One"),
+            ("/tmp/02.-The_Artist-Song_Two.m4a", 2, "The_Artist", "Song_Two"),
+        ),
+        (
+            ("/tmp/01 - Song_One.m4a", 1, "", "Song_One"),
+            ("/tmp/02. - Song_Two.m4a", 2, "", "Song_Two"),
+        ),
+        (
+            ("/tmp/Song One by The Artist.m4a", 0, "The Artist", "Song One"),
+            ("/tmp/Song Two by The Artist.m4a", 0, "The Artist", "Song Two"),
+        ),
+        (("/tmp/01.m4a", 1, "", "01"), ("/tmp/02.m4a", 2, "", "02")),
+        (
+            ("/tmp/Song One.m4a", 0, "", "Song One"),
+            ("/tmp/Song Two.m4a", 0, "", "Song Two"),
+        ),
+    ],
+)
+def test_fromfilename(song1, song2):
+    """
+    Each "song" is a tuple of path, expected track number, expected artist,
+    expected title.
+
+    We use two songs for each test for two reasons:
+    - The plugin needs more than one item to look for uniform strings in paths
+      in order to guess if the string describes an artist or a title.
+    - Sometimes we allow for an optional "." after the track number in paths.
+    """
+
+    session = Session()
+    item1 = Item(song1[0])
+    item2 = Item(song2[0])
+    task = Task([item1, item2])
+
+    f = fromfilename.FromFilenamePlugin()
+    f.filename_task(task, session)
+
+    assert task.items[0].track == song1[1]
+    assert task.items[0].artist == song1[2]
+    assert task.items[0].title == song1[3]
+    assert task.items[1].track == song2[1]
+    assert task.items[1].artist == song2[2]
+    assert task.items[1].title == song2[3]