diff --git a/beetsplug/fromfilename.py b/beetsplug/fromfilename.py index 7f9e9b92e..c3fb4bc6b 100644 --- a/beetsplug/fromfilename.py +++ b/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.+)[\-_](?P.+)[\-_](?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) diff --git a/docs/changelog.rst b/docs/changelog.rst index ed424668b..b56413ee9 100644 --- a/docs/changelog.rst +++ b/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: diff --git a/test/plugins/test_fromfilename.py b/test/plugins/test_fromfilename.py new file mode 100644 index 000000000..f13e88aea --- /dev/null +++ b/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]