From 1e4378d636e035c293d76cc436083dd9c9fa087e Mon Sep 17 00:00:00 2001 From: Antonio Larrosa Date: Sat, 21 Jan 2017 11:22:49 +0100 Subject: [PATCH 01/36] Run python2 or python3 depending on what's used On a system with dependencies installed for python3 but not for python2, we have to make sure python3 is used everywhere since 'python' might be running the python2 interpreter. This helps with some problems in #2400, but doesn't fix the issue completely. --- test/test_convert.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/test_convert.py b/test/test_convert.py index 2a32e51e7..fe74b0b73 100644 --- a/test/test_convert.py +++ b/test/test_convert.py @@ -15,6 +15,7 @@ from __future__ import division, absolute_import, print_function +import sys import re import os.path import unittest @@ -39,7 +40,7 @@ class TestHelper(helper.TestHelper): # A Python script that copies the file and appends a tag. stub = os.path.join(_common.RSRC, b'convert_stub.py').decode('utf-8') - return u"python '{}' $source $dest {}".format(stub, tag) + return u"python{} '{}' $source $dest {}".format(sys.version_info.major, stub, tag) def assertFileTag(self, path, tag): # noqa """Assert that the path is a file and the files content ends with `tag`. From 42b4e54391e362f4dd144d0ed5bd48f1fcca5d97 Mon Sep 17 00:00:00 2001 From: Antonio Larrosa Date: Sun, 22 Jan 2017 11:45:45 +0100 Subject: [PATCH 02/36] Use sys.executable instead of composing the executable name Better use sys.executable than using sys.version_info.major and compose the name of the python executable. --- test/test_convert.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_convert.py b/test/test_convert.py index fe74b0b73..208bcdb7d 100644 --- a/test/test_convert.py +++ b/test/test_convert.py @@ -40,7 +40,7 @@ class TestHelper(helper.TestHelper): # A Python script that copies the file and appends a tag. stub = os.path.join(_common.RSRC, b'convert_stub.py').decode('utf-8') - return u"python{} '{}' $source $dest {}".format(sys.version_info.major, stub, tag) + return u"{} '{}' $source $dest {}".format(sys.executable, stub, tag) def assertFileTag(self, path, tag): # noqa """Assert that the path is a file and the files content ends with `tag`. From 44ddd2e8f5d2af920865fe3f79856e267e8d071a Mon Sep 17 00:00:00 2001 From: Antonio Larrosa Date: Fri, 27 Jan 2017 11:33:37 +0100 Subject: [PATCH 03/36] Shell-escape sys.executable sys.executable needs to be shell-escaped on windows. --- test/test_convert.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_convert.py b/test/test_convert.py index 208bcdb7d..237cafdeb 100644 --- a/test/test_convert.py +++ b/test/test_convert.py @@ -40,7 +40,7 @@ class TestHelper(helper.TestHelper): # A Python script that copies the file and appends a tag. stub = os.path.join(_common.RSRC, b'convert_stub.py').decode('utf-8') - return u"{} '{}' $source $dest {}".format(sys.executable, stub, tag) + return u"'{}' '{}' $source $dest {}".format(sys.executable, stub, tag) def assertFileTag(self, path, tag): # noqa """Assert that the path is a file and the files content ends with `tag`. From fa468ce9d1fa1f57b50627db14387f7ad98de28f Mon Sep 17 00:00:00 2001 From: Antonio Larrosa Date: Wed, 22 Mar 2017 19:44:42 +0100 Subject: [PATCH 04/36] Properly quote executable and command line parameter Use shlex.quote (on python3) or pipes.quote (on python2) to properly quote the python executable and parameter instead of using single quotes --- test/test_convert.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/test/test_convert.py b/test/test_convert.py index 237cafdeb..69a23afd7 100644 --- a/test/test_convert.py +++ b/test/test_convert.py @@ -28,6 +28,15 @@ from beets.mediafile import MediaFile from beets import util +def shell_quote(text): + if sys.version_info[0] < 3: + import pipes + return pipes.quote(text) + else: + import shlex + return shlex.quote(text) + + class TestHelper(helper.TestHelper): def tagged_copy_cmd(self, tag): @@ -40,7 +49,7 @@ class TestHelper(helper.TestHelper): # A Python script that copies the file and appends a tag. stub = os.path.join(_common.RSRC, b'convert_stub.py').decode('utf-8') - return u"'{}' '{}' $source $dest {}".format(sys.executable, stub, tag) + return u"{} {} $source $dest {}".format(shell_quote(sys.executable), shell_quote(stub), tag) def assertFileTag(self, path, tag): # noqa """Assert that the path is a file and the files content ends with `tag`. From 85e0c0dcee70d4983973485e0fe29f417aea3069 Mon Sep 17 00:00:00 2001 From: Antonio Larrosa Date: Thu, 23 Mar 2017 20:19:50 +0100 Subject: [PATCH 05/36] Fixed E501 and E305 PEP8 errors --- test/test_convert.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/test_convert.py b/test/test_convert.py index 69a23afd7..aa0cd0a34 100644 --- a/test/test_convert.py +++ b/test/test_convert.py @@ -49,7 +49,8 @@ class TestHelper(helper.TestHelper): # A Python script that copies the file and appends a tag. stub = os.path.join(_common.RSRC, b'convert_stub.py').decode('utf-8') - return u"{} {} $source $dest {}".format(shell_quote(sys.executable), shell_quote(stub), tag) + return u"{} {} $source $dest {}".format(shell_quote(sys.executable), + shell_quote(stub), tag) def assertFileTag(self, path, tag): # noqa """Assert that the path is a file and the files content ends with `tag`. @@ -283,5 +284,6 @@ class NeverConvertLossyFilesTest(unittest.TestCase, TestHelper, def suite(): return unittest.TestLoader().loadTestsFromName(__name__) + if __name__ == '__main__': unittest.main(defaultTest='suite') From 11e3a5a923dfa878af6147cdd758aa38ac3328d5 Mon Sep 17 00:00:00 2001 From: Mary011196 Date: Wed, 12 Apr 2017 00:38:58 +0300 Subject: [PATCH 06/36] OperationalError from SQlite that indicates a permissions problem on database file. --- beets/ui/__init__.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index 29e228497..d208aa692 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -1159,10 +1159,13 @@ def _open_library(config): ) lib.get_item(0) # Test database connection. except (sqlite3.OperationalError, sqlite3.DatabaseError): + message = "" log.debug(u'{}', traceback.format_exc()) - raise UserError(u"database file {0} could not be opened".format( + if e.args[0] == "unable to open database file": + message = "It might be a permissions problem." + raise UserError(u"database file {0} could not be opened.%s".format( util.displayable_path(dbpath) - )) + )%message) log.debug(u'library database: {0}\n' u'library directory: {1}', util.displayable_path(lib.path), From e224621e83d129d69e08029139b9750a624f49df Mon Sep 17 00:00:00 2001 From: Matt Murch Date: Wed, 12 Apr 2017 13:38:03 -0400 Subject: [PATCH 07/36] Add default replace for - to _ Also updated changelog and docs. Resolves: #549 --- beets/config_default.yaml | 1 + docs/changelog.rst | 1 + docs/reference/config.rst | 1 + 3 files changed, 3 insertions(+) diff --git a/beets/config_default.yaml b/beets/config_default.yaml index 45f13efef..b7e6b1e2b 100644 --- a/beets/config_default.yaml +++ b/beets/config_default.yaml @@ -39,6 +39,7 @@ replace: '\.$': _ '\s+$': '' '^\s+': '' + '^-': _ path_sep_replace: _ asciify_paths: false art_filename: cover diff --git a/docs/changelog.rst b/docs/changelog.rst index ddb6ec603..d6d85452c 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -52,6 +52,7 @@ New features: Fixes: +* :doc:`/beets/config_default`: Added default replace for - to _. :bug:`549` * :doc:`/plugins/absubmit`: Do not filter for supported formats. :bug:`2471` * :doc:`/plugins/mpdupdate`: Fix Python 3 compatibility. :bug:`2381` * :doc:`/plugins/replaygain`: Fix Python 3 compatibility in the ``bs1770gain`` diff --git a/docs/reference/config.rst b/docs/reference/config.rst index 7fb7c96c4..82c11238a 100644 --- a/docs/reference/config.rst +++ b/docs/reference/config.rst @@ -131,6 +131,7 @@ unexpected behavior on all popular platforms:: '\.$': _ '\s+$': '' '^\s+': '' + '^-': _ These substitutions remove forward and back slashes, leading dots, and control characters—all of which is a good idea on any OS. The fourth line From 6e29514d4c2b9e75d4030088c1ef5db699ae709f Mon Sep 17 00:00:00 2001 From: Matt Murch Date: Wed, 12 Apr 2017 19:04:02 -0400 Subject: [PATCH 08/36] Fix Changelog Bug --- docs/changelog.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index d6d85452c..f153ab410 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -52,7 +52,7 @@ New features: Fixes: -* :doc:`/beets/config_default`: Added default replace for - to _. :bug:`549` +* :ref:`replace`: Added default replace for - to _. :bug:`549` * :doc:`/plugins/absubmit`: Do not filter for supported formats. :bug:`2471` * :doc:`/plugins/mpdupdate`: Fix Python 3 compatibility. :bug:`2381` * :doc:`/plugins/replaygain`: Fix Python 3 compatibility in the ``bs1770gain`` From ad87d2af9231c4bd735ba477a621fb51b0b263b3 Mon Sep 17 00:00:00 2001 From: MolarAmbiguity Date: Fri, 14 Apr 2017 12:40:32 +1000 Subject: [PATCH 09/36] Add info about overlaying configs --- beets/ui/__init__.py | 9 ++++++++- docs/reference/cli.rst | 5 +++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index 29e228497..762a21174 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -1122,9 +1122,12 @@ def _configure(options): # special handling lets specified plugins get loaded before we # finish parsing the command line. if getattr(options, 'config', None) is not None: + overlay_path = True config_path = options.config del options.config config.set_file(config_path) + else: + overlay_path = False config.set_args(options) # Configure the logger. @@ -1133,9 +1136,13 @@ def _configure(options): else: log.set_global_level(logging.INFO) + if overlay_path: + log.debug(u'overlaying configuration: {0}', + util.displayable_path(config_path)) + config_path = config.user_config_path() if os.path.isfile(config_path): - log.debug(u'user configuration: {0}', + log.debug(u'base user configuration: {0}', util.displayable_path(config_path)) else: log.debug(u'no user configuration found at {0}', diff --git a/docs/reference/cli.rst b/docs/reference/cli.rst index 403c1e174..7bdb891c3 100644 --- a/docs/reference/cli.rst +++ b/docs/reference/cli.rst @@ -72,7 +72,7 @@ box. To extract `rar` files, install the `rarfile`_ package and the Optional command flags: * By default, the command copies files your the library directory and - updates the ID3 tags on your music. In order to move the files, instead of + updates the ID3 tags on your music. In order to move the files, instead of copying, use the ``-m`` (move) option. If you'd like to leave your music files untouched, try the ``-C`` (don't copy) and ``-W`` (don't write tags) options. You can also disable this behavior by default in the @@ -409,7 +409,8 @@ import ...``. * ``-v``: verbose mode; prints out a deluge of debugging information. Please use this flag when reporting bugs. You can use it twice, as in ``-vv``, to make beets even more verbose. -* ``-c FILE``: read a specified YAML :doc:`configuration file `. +* ``-c FILE``: read a specified YAML :doc:`configuration file `. any +options set in the specified config will override your normal config. Beets also uses the ``BEETSDIR`` environment variable to look for configuration and data. From 94b75e8fb4ab9f96156ae00877eef8fb4c83169e Mon Sep 17 00:00:00 2001 From: MolarAmbiguity Date: Fri, 14 Apr 2017 12:57:42 +1000 Subject: [PATCH 10/36] Fix bulleted list --- docs/reference/cli.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/reference/cli.rst b/docs/reference/cli.rst index 7bdb891c3..58ce8c324 100644 --- a/docs/reference/cli.rst +++ b/docs/reference/cli.rst @@ -410,7 +410,7 @@ import ...``. this flag when reporting bugs. You can use it twice, as in ``-vv``, to make beets even more verbose. * ``-c FILE``: read a specified YAML :doc:`configuration file `. any -options set in the specified config will override your normal config. + options set in the specified config will override your normal config. Beets also uses the ``BEETSDIR`` environment variable to look for configuration and data. From 3fd04ad642f42439a24c1693c0b90f9c05e86d37 Mon Sep 17 00:00:00 2001 From: Mary011196 Date: Fri, 14 Apr 2017 09:28:13 +0300 Subject: [PATCH 11/36] adding AccessFileError as new class error excpetion --- beets/dbcore/db.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index 6b0ed8b43..89b7bd17b 100755 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -32,6 +32,10 @@ from beets.dbcore import types from .query import MatchQuery, NullSort, TrueQuery import six +class AccessFileError(Exception): + """UI exception. Commands should throw this in order to display + nonrecoverable errors to the user. + """ class FormattedMapping(collections.Mapping): """A `dict`-like formatted view of a model. @@ -680,8 +684,11 @@ class Transaction(object): """Execute an SQL statement with substitution values and return the row ID of the last affected row. """ - cursor = self.db._connection().execute(statement, subvals) - return cursor.lastrowid + + cursor = self.db._connection().execute(statement, subvals) + raise AccessFileError("unable to open database file. It might be a permissions problem") + return cursor.lastrowid + def script(self, statements): """Execute a string containing multiple SQL statements.""" From bccfcb6b4f8707741ecef510a091305d915d5a8b Mon Sep 17 00:00:00 2001 From: Mary011196 Date: Fri, 14 Apr 2017 09:33:23 +0300 Subject: [PATCH 12/36] handlinig AccessFileError in main --- beets/ui/__init__.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index d208aa692..7720c9358 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -41,6 +41,7 @@ from beets import config from beets.util import confit, as_string from beets.autotag import mb from beets.dbcore import query as db_query +from beets.dbcore import db import six # On Windows platforms, use colorama to support "ANSI" terminal colors. @@ -1159,13 +1160,10 @@ def _open_library(config): ) lib.get_item(0) # Test database connection. except (sqlite3.OperationalError, sqlite3.DatabaseError): - message = "" log.debug(u'{}', traceback.format_exc()) - if e.args[0] == "unable to open database file": - message = "It might be a permissions problem." - raise UserError(u"database file {0} could not be opened.%s".format( + raise UserError(u"database file {0} could not be opened".format( util.displayable_path(dbpath) - )%message) + )) log.debug(u'library database: {0}\n' u'library directory: {1}', util.displayable_path(lib.path), @@ -1250,3 +1248,6 @@ def main(args=None): except KeyboardInterrupt: # Silently ignore ^C except in verbose mode. log.debug(u'{}', traceback.format_exc()) + except db.AccessFileError as exc: + log.error(u'{0}',exc) + sys.exit(1) From cb2f47d8d918ed7845cec899515cde48627e4411 Mon Sep 17 00:00:00 2001 From: Mary Koliopoulou Date: Fri, 14 Apr 2017 09:41:24 +0300 Subject: [PATCH 13/36] Update db.py --- beets/dbcore/db.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index 89b7bd17b..24ee4a1d7 100755 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -685,9 +685,9 @@ class Transaction(object): the row ID of the last affected row. """ - cursor = self.db._connection().execute(statement, subvals) - raise AccessFileError("unable to open database file. It might be a permissions problem") - return cursor.lastrowid + cursor = self.db._connection().execute(statement, subvals) + raise AccessFileError("unable to open database file. It might be a permissions problem") + return cursor.lastrowid def script(self, statements): From c7801d4cc0e9729b0d741384373c47671203ee2a Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Fri, 14 Apr 2017 09:31:30 -0400 Subject: [PATCH 14/36] Attempted fix for #2515 (convert on Windows) On Python 3, this tries to pass through the Unicode filename representation for paths to the Windows API. --- beetsplug/convert.py | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/beetsplug/convert.py b/beetsplug/convert.py index 8af9a62a1..ec4d7c62e 100644 --- a/beetsplug/convert.py +++ b/beetsplug/convert.py @@ -24,6 +24,7 @@ import tempfile import shlex import six from string import Template +import platform from beets import ui, util, plugins, config from beets.plugins import BeetsPlugin @@ -183,12 +184,22 @@ class ConvertPlugin(BeetsPlugin): if not quiet and not pretend: self._log.info(u'Encoding {0}', util.displayable_path(source)) - # Substitute $source and $dest in the argument list. + # On Python 3, we need to construct the command to invoke as a + # Unicode string. On Unix, this is a little unfortunate---the OS is + # expecting bytes---so we use surrogate escaping and decode with the + # argument encoding, which is the same encoding that will then be + # *reversed* to recover the same bytes before invoking the OS. On + # Windows, we want to preserve the Unicode filename "as is." if not six.PY2: command = command.decode(util.arg_encoding(), 'surrogateescape') - source = source.decode(util.arg_encoding(), 'surrogateescape') - dest = dest.decode(util.arg_encoding(), 'surrogateescape') + if platform.system() == 'Windows': + source = source.decode(util._fsencoding()) + dest = dest.decode(util._fsencoding()) + else: + source = source.decode(util.arg_encoding(), 'surrogateescape') + dest = dest.decode(util.arg_encoding(), 'surrogateescape') + # Substitute $source and $dest in the argument list. args = shlex.split(command) encode_cmd = [] for i, arg in enumerate(args): From cf744eb1f7519ba75f3b85d6abc45e188f2a5f91 Mon Sep 17 00:00:00 2001 From: Mary Koliopoulou Date: Fri, 14 Apr 2017 19:42:23 +0300 Subject: [PATCH 15/36] Update db.py --- beets/dbcore/db.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index 24ee4a1d7..df42ab05f 100755 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -684,17 +684,17 @@ class Transaction(object): """Execute an SQL statement with substitution values and return the row ID of the last affected row. """ - - cursor = self.db._connection().execute(statement, subvals) - raise AccessFileError("unable to open database file. It might be a permissions problem") - return cursor.lastrowid + try: + cursor = self.db._connection().execute(statement, subvals) + return cursor.lastrowid + except sqlite3.OperationalError as e: + raise AccessFileError("unable to open database file. It might be a permissions problem") def script(self, statements): """Execute a string containing multiple SQL statements.""" self.db._connection().executescript(statements) - class Database(object): """A container for Model objects that wraps an SQLite database as the backend. From a526eb7222826fbdb15b0d3288de7b73ec8a0853 Mon Sep 17 00:00:00 2001 From: MolarAmbiguity Date: Sat, 15 Apr 2017 09:23:35 +1000 Subject: [PATCH 16/36] Implement feedback from sampsyo --- beets/ui/__init__.py | 11 +++++------ docs/reference/cli.rst | 7 +++++-- testconfig.yaml | 10 ++++++++++ 3 files changed, 20 insertions(+), 8 deletions(-) create mode 100644 testconfig.yaml diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index 762a21174..75bce9e7a 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -1122,12 +1122,11 @@ def _configure(options): # special handling lets specified plugins get loaded before we # finish parsing the command line. if getattr(options, 'config', None) is not None: - overlay_path = True - config_path = options.config + overlay_path = options.config del options.config - config.set_file(config_path) + config.set_file(overlay_path) else: - overlay_path = False + overlay_path = None config.set_args(options) # Configure the logger. @@ -1138,11 +1137,11 @@ def _configure(options): if overlay_path: log.debug(u'overlaying configuration: {0}', - util.displayable_path(config_path)) + util.displayable_path(overlay_path)) config_path = config.user_config_path() if os.path.isfile(config_path): - log.debug(u'base user configuration: {0}', + log.debug(u'user configuration: {0}', util.displayable_path(config_path)) else: log.debug(u'no user configuration found at {0}', diff --git a/docs/reference/cli.rst b/docs/reference/cli.rst index 58ce8c324..37fc31767 100644 --- a/docs/reference/cli.rst +++ b/docs/reference/cli.rst @@ -409,8 +409,11 @@ import ...``. * ``-v``: verbose mode; prints out a deluge of debugging information. Please use this flag when reporting bugs. You can use it twice, as in ``-vv``, to make beets even more verbose. -* ``-c FILE``: read a specified YAML :doc:`configuration file `. any - options set in the specified config will override your normal config. +* ``-c FILE``: read a specified YAML :doc:`configuration file `. This + configuration works as an overlay: rather than replacing your normal + configuration options entirely, the two are merged. Any individual options set + in this config file will override your base configuration. + Beets also uses the ``BEETSDIR`` environment variable to look for configuration and data. diff --git a/testconfig.yaml b/testconfig.yaml new file mode 100644 index 000000000..bd864107f --- /dev/null +++ b/testconfig.yaml @@ -0,0 +1,10 @@ +directory: /home/jack/Music/ +library: /home/jack/Music/library.blb +import: + move: yes +paths: + default: $albumartist/$album%aunique{}/$disc-$track $title + singleton: $albumartist/$title + +ui: + color: no From d3664ad5db742a64d19b21d00e05f8e3f1ab598e Mon Sep 17 00:00:00 2001 From: Jakub Wilk Date: Sat, 15 Apr 2017 14:44:19 +0200 Subject: [PATCH 17/36] Fix misuse of flags in re.sub() calls The 4th argument of re.sub() is maximum number of substitutions, not flags. --- beetsplug/beatport.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/beetsplug/beatport.py b/beetsplug/beatport.py index 8a73efa16..fc412d998 100644 --- a/beetsplug/beatport.py +++ b/beetsplug/beatport.py @@ -394,10 +394,10 @@ class BeatportPlugin(BeetsPlugin): # cause a query to return no results, even if they match the artist or # album title. Use `re.UNICODE` flag to avoid stripping non-english # word characters. - query = re.sub(r'\W+', ' ', query, re.UNICODE) + query = re.sub(r'\W+', ' ', query, flags=re.UNICODE) # Strip medium information from query, Things like "CD1" and "disk 1" # can also negate an otherwise positive result. - query = re.sub(r'\b(CD|disc)\s*\d+', '', query, re.I) + query = re.sub(r'\b(CD|disc)\s*\d+', '', query, flags=re.I) albums = [self._get_album_info(x) for x in self.client.search(query)] return albums From e709ae6dd58e57c498075cfb5b8752dcb8e5dbf9 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Sat, 15 Apr 2017 11:13:47 -0400 Subject: [PATCH 18/36] Changelog for #2516 --- docs/changelog.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index ddb6ec603..7dbe02314 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -770,6 +770,8 @@ Fixes: does not exist, beets creates an empty file before editing it. This fixes an error on OS X, where the ``open`` command does not work with non-existent files. :bug:`1480` +* :doc:`/plugins/convert`: Fix a problem with filename encoding on Windows + under Python 3. :bug:`2515` :bug:`2516` .. _Python bug: http://bugs.python.org/issue16512 .. _ipfs: http://ipfs.io From fb7296711cd0d7fb6365b1ba5df3089b2eb02024 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Sat, 15 Apr 2017 11:27:19 -0400 Subject: [PATCH 19/36] Fix Windows encoding in convert stub (#2516) --- test/rsrc/convert_stub.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/test/rsrc/convert_stub.py b/test/rsrc/convert_stub.py index f32bce09a..cb42692d7 100755 --- a/test/rsrc/convert_stub.py +++ b/test/rsrc/convert_stub.py @@ -28,12 +28,9 @@ def convert(in_file, out_file, tag): if not isinstance(tag, bytes): tag = tag.encode('utf-8') - # On Windows, use Unicode paths. (The test harness gives them to us - # as UTF-8 bytes.) - if platform.system() == 'Windows': - if not PY2: - in_file = in_file.encode(arg_encoding()) - out_file = out_file.encode(arg_encoding()) + # On Windows, use Unicode paths. On Python 3, we get the actual, + # Unicode filenames. On Python 2, we get them as UTF-8 byes. + if platform.system() == 'Windows' and PY2: in_file = in_file.decode('utf-8') out_file = out_file.decode('utf-8') From 419d9a066776952d850464b8ead6b49d740098d4 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Sat, 15 Apr 2017 11:52:09 -0400 Subject: [PATCH 20/36] Remove an accidentally-committed config --- testconfig.yaml | 10 ---------- 1 file changed, 10 deletions(-) delete mode 100644 testconfig.yaml diff --git a/testconfig.yaml b/testconfig.yaml deleted file mode 100644 index bd864107f..000000000 --- a/testconfig.yaml +++ /dev/null @@ -1,10 +0,0 @@ -directory: /home/jack/Music/ -library: /home/jack/Music/library.blb -import: - move: yes -paths: - default: $albumartist/$album%aunique{}/$disc-$track $title - singleton: $albumartist/$title - -ui: - color: no From 8e58a61eb4a5a1fa26371263552d8a3c6bc8ed38 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Sat, 15 Apr 2017 15:34:09 -0400 Subject: [PATCH 21/36] Remove one blank line --- docs/reference/cli.rst | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/reference/cli.rst b/docs/reference/cli.rst index 37fc31767..43c217942 100644 --- a/docs/reference/cli.rst +++ b/docs/reference/cli.rst @@ -414,7 +414,6 @@ import ...``. configuration options entirely, the two are merged. Any individual options set in this config file will override your base configuration. - Beets also uses the ``BEETSDIR`` environment variable to look for configuration and data. From 346ecbc6d45e14dbe203556ef0d7a7a42a3983e6 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Sat, 15 Apr 2017 15:43:26 -0400 Subject: [PATCH 22/36] Slightly more verbose config overlay description --- docs/reference/cli.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/reference/cli.rst b/docs/reference/cli.rst index 43c217942..b4c7b9e1b 100644 --- a/docs/reference/cli.rst +++ b/docs/reference/cli.rst @@ -412,7 +412,8 @@ import ...``. * ``-c FILE``: read a specified YAML :doc:`configuration file `. This configuration works as an overlay: rather than replacing your normal configuration options entirely, the two are merged. Any individual options set - in this config file will override your base configuration. + in this config file will override the corresponding settings in your base + configuration. Beets also uses the ``BEETSDIR`` environment variable to look for configuration and data. From f7a58447f0230e83b76f68c5423e20708830ba93 Mon Sep 17 00:00:00 2001 From: Mary011196 Date: Sun, 16 Apr 2017 15:15:45 +0300 Subject: [PATCH 23/36] Change the error name to DBAccessError --- beets/ui/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index 7720c9358..ecd8f01f1 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -1248,6 +1248,6 @@ def main(args=None): except KeyboardInterrupt: # Silently ignore ^C except in verbose mode. log.debug(u'{}', traceback.format_exc()) - except db.AccessFileError as exc: - log.error(u'{0}',exc) + except db.DBAccessError as exc: + log.error(u'{0}', exc) sys.exit(1) From 0492741bf7013219cc4eca9813b4c46859b839f5 Mon Sep 17 00:00:00 2001 From: Mary011196 Date: Sun, 16 Apr 2017 15:18:40 +0300 Subject: [PATCH 24/36] Error check with if statemetnt --- beets/dbcore/db.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index df42ab05f..bfc870b38 100755 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -32,11 +32,15 @@ from beets.dbcore import types from .query import MatchQuery, NullSort, TrueQuery import six -class AccessFileError(Exception): - """UI exception. Commands should throw this in order to display - nonrecoverable errors to the user. + +class DBAccessError(Exception): + """The SQLite database became inaccessible. + This can happen when trying to read or write the + database when, for example, the database file is deleted or otherwise disappears. + There is probably no way to recover from this error. """ + class FormattedMapping(collections.Mapping): """A `dict`-like formatted view of a model. @@ -688,13 +692,16 @@ class Transaction(object): cursor = self.db._connection().execute(statement, subvals) return cursor.lastrowid except sqlite3.OperationalError as e: - raise AccessFileError("unable to open database file. It might be a permissions problem") - + if e.args[0] in ('unable to open database file', + 'attempt to write a readonly database file'): + raise DBAccessError("Unable to open database file." \ + "It might be a permissions problem.") def script(self, statements): """Execute a string containing multiple SQL statements.""" self.db._connection().executescript(statements) + class Database(object): """A container for Model objects that wraps an SQLite database as the backend. From 512031a0998025e96c83fc4e841354d5e6cbf0c4 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Sun, 16 Apr 2017 08:57:52 -0400 Subject: [PATCH 25/36] Revise changelog for #2509 --- docs/changelog.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 5fc981f9e..274f36665 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -52,7 +52,8 @@ New features: Fixes: -* :ref:`replace`: Added default replace for - to _. :bug:`549` +* In the :ref:`replace` configuration option, we now replace a leading hyphen + (-) with an underscore. :bug:`549` :bug:`2509` * :doc:`/plugins/absubmit`: Do not filter for supported formats. :bug:`2471` * :doc:`/plugins/mpdupdate`: Fix Python 3 compatibility. :bug:`2381` * :doc:`/plugins/replaygain`: Fix Python 3 compatibility in the ``bs1770gain`` From e756f988454a8c3c2a3fbdad76a8592a71aea24a Mon Sep 17 00:00:00 2001 From: Mary011196 Date: Sun, 16 Apr 2017 17:24:48 +0300 Subject: [PATCH 26/36] Correcting the mistakes that Travis CI showed me --- beets/dbcore/db.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index bfc870b38..c9f1f72a9 100755 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -36,7 +36,8 @@ import six class DBAccessError(Exception): """The SQLite database became inaccessible. This can happen when trying to read or write the - database when, for example, the database file is deleted or otherwise disappears. + database when, for example, the database file + is deleted or otherwise disappears. There is probably no way to recover from this error. """ @@ -692,10 +693,10 @@ class Transaction(object): cursor = self.db._connection().execute(statement, subvals) return cursor.lastrowid except sqlite3.OperationalError as e: - if e.args[0] in ('unable to open database file', - 'attempt to write a readonly database file'): - raise DBAccessError("Unable to open database file." \ - "It might be a permissions problem.") + if e.args[0] in ("attempt to write a readonly database", + "unable to open database file"): + raise DBAccessError('Unable to open database file.' + 'It might be a permissions problem') def script(self, statements): """Execute a string containing multiple SQL statements.""" From 621427fa636050bfd96b3e388ab24aff8a061c79 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Tue, 18 Apr 2017 10:31:09 -0400 Subject: [PATCH 27/36] Reformat a docstring --- beets/dbcore/db.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index c9f1f72a9..058b52ff5 100755 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -35,10 +35,10 @@ import six class DBAccessError(Exception): """The SQLite database became inaccessible. - This can happen when trying to read or write the - database when, for example, the database file - is deleted or otherwise disappears. - There is probably no way to recover from this error. + + This can happen when trying to read or write the database when, for + example, the database file is deleted or otherwise disappears. There + is probably no way to recover from this error. """ From 19e09585d8fc43cf59433af5ed8229acccaf8f12 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Tue, 18 Apr 2017 10:32:44 -0400 Subject: [PATCH 28/36] Re-raise other errors And re-use the SQLite error string instead of a hand-written one for now. --- beets/dbcore/db.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index 058b52ff5..ef7231a76 100755 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -693,10 +693,14 @@ class Transaction(object): cursor = self.db._connection().execute(statement, subvals) return cursor.lastrowid except sqlite3.OperationalError as e: + # In two specific cases, SQLite reports an error while accessing + # the underlying database file. We surface these exceptions as + # DBAccessError so the application can abort. if e.args[0] in ("attempt to write a readonly database", "unable to open database file"): - raise DBAccessError('Unable to open database file.' - 'It might be a permissions problem') + raise DBAccessError(e.args[0]) + else: + raise def script(self, statements): """Execute a string containing multiple SQL statements.""" From 906bd97d46f2535c271923ea879f9d1a45f46176 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Tue, 18 Apr 2017 17:57:13 -0400 Subject: [PATCH 29/36] Hint about database access errors --- beets/ui/__init__.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index ecd8f01f1..39d3d52ac 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -1249,5 +1249,9 @@ def main(args=None): # Silently ignore ^C except in verbose mode. log.debug(u'{}', traceback.format_exc()) except db.DBAccessError as exc: - log.error(u'{0}', exc) + log.error( + u'database access error: {0}\n' + u'the library file might have a permissions problem', + exc + ) sys.exit(1) From 7eaaa995668064aad334b75ba16456df9e532556 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Tue, 18 Apr 2017 17:59:12 -0400 Subject: [PATCH 30/36] Changelog entry for #2508 --- docs/changelog.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 37c1ef9fb..14e0f98fc 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -84,6 +84,8 @@ Fixes: AAC codec instead of faac. Thanks to :user:`jansol`. :bug:`2484` * Fix import of multidisc releases with subdirectories, which previously made each disc be imported separately in different releases. :bug:`2493` +* When the SQLite database stops being accessible, we now print a friendly + error message. Thanks to :user:`Mary011196`. :bug:`1676` :bug:`2508` 1.4.3 (January 9, 2017) From ffcaf3384548ab95275c5489bb8dbf1631dd0687 Mon Sep 17 00:00:00 2001 From: xarph Date: Tue, 18 Apr 2017 15:30:24 -0700 Subject: [PATCH 31/36] add -f argument to play command --- beetsplug/play.py | 7 ++++++- docs/changelog.rst | 3 +++ docs/plugins/play.rst | 3 +++ test/test_play.py | 14 ++++++++++++++ 4 files changed, 26 insertions(+), 1 deletion(-) diff --git a/beetsplug/play.py b/beetsplug/play.py index 9e912dbce..409e0c4ab 100644 --- a/beetsplug/play.py +++ b/beetsplug/play.py @@ -81,6 +81,11 @@ class PlayPlugin(BeetsPlugin): action='store', help=u'add additional arguments to the command', ) + play_command.parser.add_option( + u'-f', u'--force', + action="store_true", + help=u'disable the warning threshold', + ) play_command.func = self._play_command return [play_command] @@ -125,7 +130,7 @@ class PlayPlugin(BeetsPlugin): # Check if the selection exceeds configured threshold. If True, # cancel, otherwise proceed with play command. - if not self._exceeds_threshold(selection, command_str, open_args, + if opts.force or not self._exceeds_threshold(selection, command_str, open_args, item_type): play(command_str, selection, paths, open_args, self._log, item_type) diff --git a/docs/changelog.rst b/docs/changelog.rst index ddb6ec603..cd539d0cf 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -49,6 +49,9 @@ New features: :bug:`2366` :bug:`2495` * Importing a release with multiple release events now selects the event based on your :ref:`preferred` countries. :bug:`2501` +* :doc:`/plugins/play`: A new ``-f`` or ``--force`` parameter lets you skip + the warning message if you enqueue more items than the warning threshold + usually allows. Fixes: diff --git a/docs/plugins/play.rst b/docs/plugins/play.rst index 9b9110bde..71baa0388 100644 --- a/docs/plugins/play.rst +++ b/docs/plugins/play.rst @@ -95,6 +95,9 @@ example:: indicates that you need to insert extra arguments before specifying the playlist. +The ``--force`` (or ``-f``) flag to the ``play`` command will skip the warning +message if you choose to play more items than the **warning_threshold** value. + Note on the Leakage of the Generated Playlists ---------------------------------------------- diff --git a/test/test_play.py b/test/test_play.py index 86fef99a9..685632b40 100644 --- a/test/test_play.py +++ b/test/test_play.py @@ -115,6 +115,20 @@ class PlayPluginTest(unittest.TestCase, TestHelper): open_mock.assert_not_called() + def test_force_warning_threshold_bypass(self, open_mock): + self.config['play']['warning_threshold'] = 1 + self.other_item = self.add_item(title='another NiceTitle') + + expected_playlist = u'{0}\n{1}'.format( + self.item.path.decode('utf-8'), + self.other_item.path.decode('utf-8')) + + with control_stdin("a"): + self.run_and_assert( + open_mock, + [u'-f', u'NiceTitle'], + expected_playlist=expected_playlist) + def test_command_failed(self, open_mock): open_mock.side_effect = OSError(u"some reason") From 18c512893eeb2cb9d734ca1c5b3e4df5fba1a1ba Mon Sep 17 00:00:00 2001 From: discopatrick Date: Wed, 19 Apr 2017 14:10:04 +0100 Subject: [PATCH 32/36] more flake8 updates --- beets/dbcore/query.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py index cf0c13792..aa8aa4af8 100644 --- a/beets/dbcore/query.py +++ b/beets/dbcore/query.py @@ -556,13 +556,15 @@ class Period(object): ordinal = string.count('-') if ordinal >= len(cls.date_formats): # Too many components. - raise InvalidQueryArgumentTypeError(string, 'a valid datetime string') + raise InvalidQueryArgumentTypeError(string, + 'a valid datetime string') date_format = cls.date_formats[ordinal] try: date = datetime.strptime(string, date_format) except ValueError: # Parsing failed. - raise InvalidQueryArgumentTypeError(string, 'a valid datetime string') + raise InvalidQueryArgumentTypeError(string, + 'a valid datetime string') precision = cls.precisions[ordinal] return cls(date, precision) From 29b57fb5a7c2f5d960f9345475bd1a1cbf8730cb Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Wed, 19 Apr 2017 11:36:58 -0400 Subject: [PATCH 33/36] Changelog for #2517 --- docs/changelog.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 274f36665..5330869ee 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -88,6 +88,8 @@ Fixes: AAC codec instead of faac. Thanks to :user:`jansol`. :bug:`2484` * Fix import of multidisc releases with subdirectories, which previously made each disc be imported separately in different releases. :bug:`2493` +* Invalid date queries now print an error message instead of being silently + ignored. Thanks to :user:`discopatrick`. :bug:`2513` :bug:`2517` 1.4.3 (January 9, 2017) From 02aa6191c1af4fd39a1d5f31de6ef27ade3aa9f6 Mon Sep 17 00:00:00 2001 From: xarph Date: Wed, 19 Apr 2017 10:51:44 -0700 Subject: [PATCH 34/36] rename --force to --yes in play plugin fix some pep8 goo --- beetsplug/play.py | 8 ++++---- docs/changelog.rst | 2 +- docs/plugins/play.rst | 5 +++-- test/test_play.py | 4 ++-- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/beetsplug/play.py b/beetsplug/play.py index 409e0c4ab..8477acbfc 100644 --- a/beetsplug/play.py +++ b/beetsplug/play.py @@ -82,9 +82,9 @@ class PlayPlugin(BeetsPlugin): help=u'add additional arguments to the command', ) play_command.parser.add_option( - u'-f', u'--force', + u'-y', u'--yes', action="store_true", - help=u'disable the warning threshold', + help=u'skip the warning threshold', ) play_command.func = self._play_command return [play_command] @@ -130,8 +130,8 @@ class PlayPlugin(BeetsPlugin): # Check if the selection exceeds configured threshold. If True, # cancel, otherwise proceed with play command. - if opts.force or not self._exceeds_threshold(selection, command_str, open_args, - item_type): + if opts.yes or not self._exceeds_threshold( + selection, command_str, open_args, item_type): play(command_str, selection, paths, open_args, self._log, item_type) diff --git a/docs/changelog.rst b/docs/changelog.rst index cd539d0cf..7fa0dbcbb 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -49,7 +49,7 @@ New features: :bug:`2366` :bug:`2495` * Importing a release with multiple release events now selects the event based on your :ref:`preferred` countries. :bug:`2501` -* :doc:`/plugins/play`: A new ``-f`` or ``--force`` parameter lets you skip +* :doc:`/plugins/play`: A new ``-y`` or ``--yes`` parameter lets you skip the warning message if you enqueue more items than the warning threshold usually allows. diff --git a/docs/plugins/play.rst b/docs/plugins/play.rst index 71baa0388..3a08a4239 100644 --- a/docs/plugins/play.rst +++ b/docs/plugins/play.rst @@ -95,8 +95,9 @@ example:: indicates that you need to insert extra arguments before specifying the playlist. -The ``--force`` (or ``-f``) flag to the ``play`` command will skip the warning -message if you choose to play more items than the **warning_threshold** value. +The ``--yes`` (or ``-y``) flag to the ``play`` command will skip the warning +message if you choose to play more items than the **warning_threshold** +value usually allows. Note on the Leakage of the Generated Playlists ---------------------------------------------- diff --git a/test/test_play.py b/test/test_play.py index 685632b40..9721143cc 100644 --- a/test/test_play.py +++ b/test/test_play.py @@ -115,7 +115,7 @@ class PlayPluginTest(unittest.TestCase, TestHelper): open_mock.assert_not_called() - def test_force_warning_threshold_bypass(self, open_mock): + def test_skip_warning_threshold_bypass(self, open_mock): self.config['play']['warning_threshold'] = 1 self.other_item = self.add_item(title='another NiceTitle') @@ -126,7 +126,7 @@ class PlayPluginTest(unittest.TestCase, TestHelper): with control_stdin("a"): self.run_and_assert( open_mock, - [u'-f', u'NiceTitle'], + [u'-y', u'NiceTitle'], expected_playlist=expected_playlist) def test_command_failed(self, open_mock): From 9d42728f7fdb05a00c59df561d3eb67a2dc2cf06 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Wed, 19 Apr 2017 19:07:29 -0400 Subject: [PATCH 35/36] ftintitle: Clarify control flow Assigning to this variable made it hard to tell what the function was actually returning. --- beetsplug/ftintitle.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/beetsplug/ftintitle.py b/beetsplug/ftintitle.py index db450d9c8..3b1caffe6 100644 --- a/beetsplug/ftintitle.py +++ b/beetsplug/ftintitle.py @@ -49,13 +49,11 @@ def find_feat_part(artist, albumartist): """Attempt to find featured artists in the item's artist fields and return the results. Returns None if no featured artist found. """ - feat_part = None - # Look for the album artist in the artist field. If it's not # present, give up. albumartist_split = artist.split(albumartist, 1) if len(albumartist_split) <= 1: - return feat_part + return None # If the last element of the split (the right-hand side of the # album artist) is nonempty, then it probably contains the @@ -63,15 +61,16 @@ def find_feat_part(artist, albumartist): elif albumartist_split[-1] != '': # Extract the featured artist from the right-hand side. _, feat_part = split_on_feat(albumartist_split[-1]) + return feat_part # Otherwise, if there's nothing on the right-hand side, look for a # featuring artist on the left-hand side. else: lhs, rhs = split_on_feat(albumartist_split[0]) if lhs: - feat_part = lhs + return lhs - return feat_part + return None class FtInTitlePlugin(plugins.BeetsPlugin): From fae8fcc932ffbb22f30d334cbe35d3b0850679b5 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Wed, 19 Apr 2017 19:08:15 -0400 Subject: [PATCH 36/36] ftintitle: Clarify indexing This can only be a two-element array, so just use the index 1. This matches better with the comments, that say "right-hand side" instead of "the last value in the list." --- beetsplug/ftintitle.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/beetsplug/ftintitle.py b/beetsplug/ftintitle.py index 3b1caffe6..1060a2dd8 100644 --- a/beetsplug/ftintitle.py +++ b/beetsplug/ftintitle.py @@ -58,9 +58,9 @@ def find_feat_part(artist, albumartist): # If the last element of the split (the right-hand side of the # album artist) is nonempty, then it probably contains the # featured artist. - elif albumartist_split[-1] != '': + elif albumartist_split[1] != '': # Extract the featured artist from the right-hand side. - _, feat_part = split_on_feat(albumartist_split[-1]) + _, feat_part = split_on_feat(albumartist_split[1]) return feat_part # Otherwise, if there's nothing on the right-hand side, look for a