From b5216a06f489b5da8c1e66b86603eedfc9d15856 Mon Sep 17 00:00:00 2001 From: Vrihub Date: Sat, 15 Jun 2024 20:52:55 +0200 Subject: [PATCH 1/4] Proposed fix for issue #5218 Check for existence of "title" matching group before using it --- beetsplug/fromfilename.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/beetsplug/fromfilename.py b/beetsplug/fromfilename.py index 103e829016..70b9d41fbb 100644 --- a/beetsplug/fromfilename.py +++ b/beetsplug/fromfilename.py @@ -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,14 +114,15 @@ def apply_matches(d, log): if not item.artist: item.artist = artist log.info("Artist replaced with: {}".format(item.artist)) - - # 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): + if title_field and bad_title(item.title): item.title = str(d[item][title_field]) log.info("Title replaced with: {}".format(item.title)) From 09660426a887c69fc6269c20a4548cd65a308df3 Mon Sep 17 00:00:00 2001 From: Vrihub Date: Sat, 15 Jun 2024 20:56:40 +0200 Subject: [PATCH 2/4] Logging: add message about the pattern being tried --- beetsplug/fromfilename.py | 1 + 1 file changed, 1 insertion(+) diff --git a/beetsplug/fromfilename.py b/beetsplug/fromfilename.py index 70b9d41fbb..598cbeda9a 100644 --- a/beetsplug/fromfilename.py +++ b/beetsplug/fromfilename.py @@ -162,6 +162,7 @@ def filename_task(self, task, session): # Look for useful information in the filenames. for pattern in PATTERNS: + self._log.debug("Trying pattern: {}".format(pattern)) d = all_matches(names, pattern) if d: apply_matches(d, self._log) From e6b773561b1cb588e6e3204030092af62859bd1a Mon Sep 17 00:00:00 2001 From: Vrihub Date: Sat, 15 Jun 2024 20:58:41 +0200 Subject: [PATCH 3/4] Refactor regexps in PATTERNS --- beetsplug/fromfilename.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/beetsplug/fromfilename.py b/beetsplug/fromfilename.py index 598cbeda9a..0f34fd09cb 100644 --- a/beetsplug/fromfilename.py +++ b/beetsplug/fromfilename.py @@ -25,12 +25,10 @@ # 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>.+)$", From 2f2680fa8d9431e3f2bebef82c2feb7aad9b1248 Mon Sep 17 00:00:00 2001 From: Vrihub <Vrihub@users.noreply.github.com> Date: Wed, 26 Jun 2024 16:02:33 +0200 Subject: [PATCH 4/4] Added tests for the fromfilename plugin --- test/plugins/test_fromfilename.py | 143 ++++++++++++++++++++++++++++++ 1 file changed, 143 insertions(+) create mode 100644 test/plugins/test_fromfilename.py diff --git a/test/plugins/test_fromfilename.py b/test/plugins/test_fromfilename.py new file mode 100644 index 0000000000..3dc600cedd --- /dev/null +++ b/test/plugins/test_fromfilename.py @@ -0,0 +1,143 @@ +# This file is part of beets. +# Copyright 2016, Jan-Erik Dahlin. +# +# 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 unittest +from unittest.mock import Mock +from beetsplug import fromfilename + + +class FromfilenamePluginTest(unittest.TestCase): + + def setUp(self): + """Create mock objects for import session and task.""" + self.session = Mock() + + item1config = {'path': '', 'track': 0, 'artist': '', 'title': ''} + self.item1 = Mock(**item1config) + + item2config = {'path': '', 'track': 0, 'artist': '', 'title': ''} + self.item2 = Mock(**item2config) + + taskconfig = {'is_album': True, 'items': [self.item1, self.item2]} + self.task = Mock(**taskconfig) + + def tearDown(self): + del self.session, self.task, self.item1, self.item2 + + def test_sep_sds(self): + """Test filenames that use " - " as separator.""" + + self.item1.path = "/music/files/01 - Artist Name - Song One.m4a" + self.item2.path = "/music/files/02. - Artist Name - Song Two.m4a" + + f = fromfilename.FromFilenamePlugin() + f.filename_task(self.task, self.session) + + self.assertEqual(self.task.items[0].track, 1) + self.assertEqual(self.task.items[1].track, 2) + self.assertEqual(self.task.items[0].artist, "Artist Name") + self.assertEqual(self.task.items[1].artist, "Artist Name") + self.assertEqual(self.task.items[0].title, "Song One") + self.assertEqual(self.task.items[1].title, "Song Two") + + def test_sep_dash(self): + """Test filenames that use "-" as separator.""" + + self.item1.path = "/music/files/01-Artist_Name-Song_One.m4a" + self.item2.path = "/music/files/02.-Artist_Name-Song_Two.m4a" + + f = fromfilename.FromFilenamePlugin() + f.filename_task(self.task, self.session) + + self.assertEqual(self.task.items[0].track, 1) + self.assertEqual(self.task.items[1].track, 2) + self.assertEqual(self.task.items[0].artist, "Artist_Name") + self.assertEqual(self.task.items[1].artist, "Artist_Name") + self.assertEqual(self.task.items[0].title, "Song_One") + self.assertEqual(self.task.items[1].title, "Song_Two") + + def test_track_title(self): + """Test filenames including track and title.""" + + self.item1.path = "/music/files/01 - Song_One.m4a" + self.item2.path = "/music/files/02. Song_Two.m4a" + + f = fromfilename.FromFilenamePlugin() + f.filename_task(self.task, self.session) + + self.assertEqual(self.task.items[0].track, 1) + self.assertEqual(self.task.items[1].track, 2) + self.assertEqual(self.task.items[0].artist, "") + self.assertEqual(self.task.items[1].artist, "") + self.assertEqual(self.task.items[0].title, "Song_One") + self.assertEqual(self.task.items[1].title, "Song_Two") + + def test_title_by_artist(self): + """Test filenames including title by artist.""" + + self.item1.path = "/music/files/Song One by The Artist.m4a" + self.item2.path = "/music/files/Song Two by The Artist.m4a" + + f = fromfilename.FromFilenamePlugin() + f.filename_task(self.task, self.session) + + self.assertEqual(self.task.items[0].track, 0) + self.assertEqual(self.task.items[1].track, 0) + self.assertEqual(self.task.items[0].artist, "The Artist") + self.assertEqual(self.task.items[1].artist, "The Artist") + self.assertEqual(self.task.items[0].title, "Song One") + self.assertEqual(self.task.items[1].title, "Song Two") + + def test_track_only(self): + """Test filenames including only track.""" + + self.item1.path = "/music/files/01.m4a" + self.item2.path = "/music/files/02.m4a" + + f = fromfilename.FromFilenamePlugin() + f.filename_task(self.task, self.session) + + self.assertEqual(self.task.items[0].track, 1) + self.assertEqual(self.task.items[1].track, 2) + self.assertEqual(self.task.items[0].artist, "") + self.assertEqual(self.task.items[1].artist, "") + self.assertEqual(self.task.items[0].title, "01") + self.assertEqual(self.task.items[1].title, "02") + + def test_title_only(self): + """Test filenames including only title.""" + + self.item1.path = "/music/files/Song One.m4a" + self.item2.path = "/music/files/Song Two.m4a" + + f = fromfilename.FromFilenamePlugin() + f.filename_task(self.task, self.session) + + self.assertEqual(self.task.items[0].track, 0) + self.assertEqual(self.task.items[1].track, 0) + self.assertEqual(self.task.items[0].artist, "") + self.assertEqual(self.task.items[1].artist, "") + self.assertEqual(self.task.items[0].title, "Song One") + self.assertEqual(self.task.items[1].title, "Song Two") + + +def suite(): + return unittest.TestLoader().loadTestsFromName(__name__) + + +if __name__ == "__main__": + unittest.main(defaultTest="suite")