Merge branch 'master' into atomic-move-fix

beetsplug/convert.py

@@ -514,17 +514,23 @@ class ConvertPlugin(BeetsPlugin):
             except subprocess.CalledProcessError:
                 return
 
-            # Change the newly-imported database entry to point to the
-            # converted file.
-            source_path = item.path
-            item.path = dest
-            item.write()
-            item.read()  # Load new audio information data.
-            item.store()
+            pretend = self.config['pretend'].get(bool)
+            quiet = self.config['quiet'].get(bool)
 
-            if self.config['delete_originals']:
-                self._log.info('Removing original file {0}', source_path)
-                util.remove(source_path, False)
+            if not pretend:
+                # Change the newly-imported database entry to point to the
+                # converted file.
+                source_path = item.path
+                item.path = dest
+                item.write()
+                item.read()  # Load new audio information data.
+                item.store()
+
+                if self.config['delete_originals']:
+                    if not quiet:
+                        self._log.info('Removing original file {0}',
+                                       source_path)
+                    util.remove(source_path, False)
 
     def _cleanup(self, task, session):
         for path in task.old_paths:

beetsplug/discogs.py

@@ -157,6 +157,11 @@ class DiscogsPlugin(BeetsPlugin):
         if not self.discogs_client:
             return
 
+        if not album and not artist:
+            self._log.debug('Skipping Discogs query. Files missing album and '
+                            'artist tags.')
+            return []
+
         if va_likely:
             query = album
         else:

beetsplug/limit.py

@@ -0,0 +1,101 @@
+# 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.
+
+"""Adds head/tail functionality to list/ls.
+
+1. Implemented as `lslimit` command with `--head` and `--tail` options. This is
+   the idiomatic way to use this plugin.
+2. Implemented as query prefix `<` for head functionality only. This is the
+   composable way to use the plugin (plays nicely with anything that uses the
+   query language).
+"""
+
+from beets.dbcore import FieldQuery
+from beets.plugins import BeetsPlugin
+from beets.ui import Subcommand, decargs, print_
+from collections import deque
+from itertools import islice
+
+
+def lslimit(lib, opts, args):
+    """Query command with head/tail."""
+
+    if (opts.head is not None) and (opts.tail is not None):
+        raise ValueError("Only use one of --head and --tail")
+    if (opts.head or opts.tail or 0) < 0:
+        raise ValueError("Limit value must be non-negative")
+
+    query = decargs(args)
+    if opts.album:
+        objs = lib.albums(query)
+    else:
+        objs = lib.items(query)
+
+    if opts.head is not None:
+        objs = islice(objs, opts.head)
+    elif opts.tail is not None:
+        objs = deque(objs, opts.tail)
+
+    for obj in objs:
+        print_(format(obj))
+
+
+lslimit_cmd = Subcommand(
+    "lslimit",
+    help="query with optional head or tail"
+)
+
+lslimit_cmd.parser.add_option(
+    "--head",
+    action="store",
+    type="int",
+    default=None
+)
+
+lslimit_cmd.parser.add_option(
+    "--tail",
+    action="store",
+    type="int",
+    default=None
+)
+
+lslimit_cmd.parser.add_all_common_options()
+lslimit_cmd.func = lslimit
+
+
+class LimitPlugin(BeetsPlugin):
+    """Query limit functionality via command and query prefix."""
+
+    def commands(self):
+        """Expose `lslimit` subcommand."""
+        return [lslimit_cmd]
+
+    def queries(self):
+
+        class HeadQuery(FieldQuery):
+            """This inner class pattern allows the query to track state."""
+            n = 0
+            N = None
+
+            @classmethod
+            def value_match(cls, pattern, value):
+                if cls.N is None:
+                    cls.N = int(pattern)
+                    if cls.N < 0:
+                        raise ValueError("Limit value must be non-negative")
+                cls.n += 1
+                return cls.n <= cls.N
+
+        return {
+            "<": HeadQuery
+        }

beetsplug/lyrics.py

@@ -419,11 +419,17 @@ class Genius(Backend):
             lyrics_div = verse_div.parent
             for br in lyrics_div.find_all("br"):
                 br.replace_with("\n")
+
             ads = lyrics_div.find_all("div",
                                       class_=re.compile("InreadAd__Container"))
             for ad in ads:
                 ad.replace_with("\n")
 
+            footers = lyrics_div.find_all("div",
+                                          class_=re.compile("Lyrics__Footer"))
+            for footer in footers:
+                footer.replace_with("")
+
         return lyrics_div.get_text()
 
 
@@ -488,11 +494,11 @@ class Tekstowo(Backend):
         if not soup:
             return None
 
-        lyrics_div = soup.find("div", class_="song-text")
+        lyrics_div = soup.select("div.song-text > div.inner-text")
         if not lyrics_div:
             return None
 
-        return lyrics_div.get_text()
+        return lyrics_div[0].get_text()
 
 
 def remove_credits(text):

beetsplug/web/__init__.py

@@ -261,7 +261,7 @@ class QueryConverter(PathConverter):
                 for query in queries]
 
     def to_url(self, value):
-        return ','.join([v.replace(os.sep, '\\') for v in value])
+        return '/'.join([v.replace(os.sep, '\\') for v in value])
 
 
 class EverythingConverter(PathConverter):

docs/changelog.rst

@@ -22,7 +22,7 @@ Bug fixes:
 * :doc:`/plugins/unimported`: The new ``ignore_subdirectories`` configuration
   option added in 1.6.0 now has a default value if it hasn't been set.
 * :doc:`/plugins/deezer`: Tolerate missing fields when searching for singleton
-  tracks
+  tracks.
   :bug:`4116`
 * :doc:`/plugins/replaygain`: The type of the internal ``r128_track_gain`` and
   ``r128_album_gain`` fields was changed from integer to float to fix loss of
@@ -31,6 +31,18 @@ Bug fixes:
 * Fix a regression in the previous release that caused a `TypeError` when
   moving files across filesystems.
   :bug:`4168`
+* :doc:`/plugins/convert`: Files are no longer converted when running import in
+  ``--pretend`` mode.
+* :doc:`/plugins/convert`: Deleting the original files during conversion no
+  longer logs output when the ``quiet`` flag is enabled.
+* :doc:`plugins/web`: Fix handling of "query" requests. Previously queries
+  consisting of more than one token (separated by a slash) always returned an
+  empty result.
+* :doc:`/plugins/discogs`: Skip Discogs query on insufficiently tagged files
+  (artist and album tags missing) to prevent arbitrary candidate results.
+  :bug:`4227`
+* :doc:`plugins/lyrics`: Fixed issues with the Tekstowo.pl and Genius
+  backends where some non-lyrics content got included in the lyrics
 
 For packagers:
 
@@ -38,6 +50,10 @@ For packagers:
   :bug:`4167`
 * The minimum required version of :pypi:`mediafile` is now 0.9.0.
 
+Other new things:
+
+* :doc:`/plugins/limit`: Limit query results to head or tail (``lslimit`` 
+  command only)
 
 1.6.0 (November 27, 2021)
 -------------------------

docs/plugins/discogs.rst

@@ -19,7 +19,8 @@ authentication credentials via a personal access token or an OAuth2
 authorization.
 
 Matches from Discogs will now show up during import alongside matches from
-MusicBrainz.
+MusicBrainz. The search terms sent to the Discogs API are based on the artist
+and album tags of your tracks. If those are empty no query will be issued.
 
 If you have a Discogs ID for an album you want to tag, you can also enter it
 at the "enter Id" prompt in the importer.

docs/plugins/index.rst

@@ -98,6 +98,7 @@ following to your configuration::
    kodiupdate
    lastgenre
    lastimport
+   limit
    loadext
    lyrics
    mbcollection

docs/plugins/limit.rst

@@ -0,0 +1,58 @@
+Limit Query Plugin
+==================
+
+``limit`` is a plugin to limit a query to the first or last set of 
+results. We also provide a query prefix ``'<n'`` to inline the same 
+behavior in the ``list`` command. They are analagous to piping results:
+
+    $ beet [list|ls] [QUERY] | [head|tail] -n n
+
+There are two provided interfaces:
+
+1. ``beet lslimit [--head n | --tail n] [QUERY]`` returns the head or 
+tail of a query
+
+2. ``beet [list|ls] [QUERY] '<n'`` returns the head of a query
+
+There are two differences in behavior: 
+
+1. The query prefix does not support tail.
+
+2. The query prefix could appear anywhere in the query but will only 
+have the same behavior as the ``lslimit`` command and piping to ``head`` 
+when it appears last.
+
+Performance for the query previx is much worse due to the current  
+singleton-based implementation. 
+
+So why does the query prefix exist? Because it composes with any other 
+query-based API or plugin (see :doc:`/reference/query`). For example, 
+you can use the query prefix in ``smartplaylist``
+(see :doc:`/plugins/smartplaylist`) to limit the number of tracks in a smart
+playlist for applications like most played and recently added.
+
+Configuration
+=============
+
+Enable the ``limit`` plugin in your configuration (see
+:ref:`using-plugins`).
+
+Examples
+========
+
+First 10 tracks
+
+    $ beet ls | head -n 10
+    $ beet lslimit --head 10
+    $ beet ls '<10'
+
+Last 10 tracks
+
+    $ beet ls | tail -n 10
+    $ beet lslimit --tail 10
+
+100 mostly recently released tracks
+
+    $ beet lslimit --head 100 year- month- day-
+    $ beet ls year- month- day- '<100'
+    $ beet lslimit --tail 100 year+ month+ day+

setup.cfg

@@ -69,6 +69,7 @@ per-file-ignores =
     ./beetsplug/permissions.py:D
     ./beetsplug/spotify.py:D
     ./beetsplug/lastgenre/__init__.py:D
+    ./beetsplug/limit.py:D
     ./beetsplug/mbcollection.py:D
     ./beetsplug/metasync/amarok.py:D
     ./beetsplug/metasync/itunes.py:D
@@ -161,6 +162,7 @@ per-file-ignores =
     ./test/test_library.py:D
     ./test/test_ui_commands.py:D
     ./test/test_lyrics.py:D
+    ./test/test_limit.py:D
     ./test/test_beatport.py:D
     ./test/test_random.py:D
     ./test/test_embyupdate.py:D

test/test_convert.py

@@ -107,7 +107,8 @@ class ImportConvertTest(unittest.TestCase, TestHelper):
         item = self.lib.items().get()
         self.assertFileTag(item.path, 'convert')
 
-    @unittest.skipIf(sys.platform, 'win32')  # FIXME: fails on windows
+    # FIXME: fails on windows
+    @unittest.skipIf(sys.platform == 'win32', 'win32')
     def test_import_original_on_convert_error(self):
         # `false` exits with non-zero code
         self.config['convert']['command'] = 'false'
@@ -122,9 +123,28 @@ class ImportConvertTest(unittest.TestCase, TestHelper):
         self.importer.run()
         for path in self.importer.paths:
             for root, dirnames, filenames in os.walk(path):
-                self.assertTrue(len(fnmatch.filter(filenames, '*.mp3')) == 0,
-                                'Non-empty import directory {}'
-                                .format(util.displayable_path(path)))
+                self.assertEqual(len(fnmatch.filter(filenames, '*.mp3')), 0,
+                                 'Non-empty import directory {}'
+                                 .format(util.displayable_path(path)))
+
+    def test_delete_originals_keeps_originals_when_pretend_enabled(self):
+        import_file_count = self.get_count_of_import_files()
+
+        self.config['convert']['delete_originals'] = True
+        self.config['convert']['pretend'] = True
+        self.importer.run()
+
+        self.assertEqual(self.get_count_of_import_files(), import_file_count,
+                         'Count of files differs after running import')
+
+    def get_count_of_import_files(self):
+        import_file_count = 0
+
+        for path in self.importer.paths:
+            for root, _, filenames in os.walk(path):
+                import_file_count += len(filenames)
+
+        return import_file_count
 
 
 class ConvertCommand:
@@ -264,7 +284,7 @@ class NeverConvertLossyFilesTest(unittest.TestCase, TestHelper,
         self.unload_plugins()
         self.teardown_beets()
 
-    def test_transcode_from_lossles(self):
+    def test_transcode_from_lossless(self):
         [item] = self.add_item_fixtures(ext='flac')
         with control_stdin('y'):
             self.run_convert_path(item.path)

test/test_hook.py

@@ -63,7 +63,8 @@ class HookTest(_common.TestCase, TestHelper):
 
         self.assertIn('hook: invalid command ""', logs)
 
-    @unittest.skipIf(sys.platform, 'win32')  # FIXME: fails on windows
+    # FIXME: fails on windows
+    @unittest.skipIf(sys.platform == 'win32', 'win32')
     def test_hook_non_zero_exit(self):
         self._add_hook('test_event', 'sh -c "exit 1"')
 
@@ -86,7 +87,8 @@ class HookTest(_common.TestCase, TestHelper):
             message.startswith("hook: hook for test_event failed: ")
             for message in logs))
 
-    @unittest.skipIf(sys.platform, 'win32')  # FIXME: fails on windows
+    # FIXME: fails on windows
+    @unittest.skipIf(sys.platform == 'win32', 'win32')
     def test_hook_no_arguments(self):
         temporary_paths = [
             get_temporary_path() for i in range(self.TEST_HOOK_COUNT)
@@ -105,7 +107,8 @@ class HookTest(_common.TestCase, TestHelper):
             self.assertTrue(os.path.isfile(path))
             os.remove(path)
 
-    @unittest.skipIf(sys.platform, 'win32')  # FIXME: fails on windows
+    # FIXME: fails on windows
+    @unittest.skipIf(sys.platform == 'win32', 'win32')
     def test_hook_event_substitution(self):
         temporary_directory = tempfile._get_default_tempdir()
         event_names = [f'test_event_event_{i}' for i in
@@ -126,7 +129,8 @@ class HookTest(_common.TestCase, TestHelper):
             self.assertTrue(os.path.isfile(path))
             os.remove(path)
 
-    @unittest.skipIf(sys.platform, 'win32')  # FIXME: fails on windows
+    # FIXME: fails on windows
+    @unittest.skipIf(sys.platform == 'win32', 'win32')
     def test_hook_argument_substitution(self):
         temporary_paths = [
             get_temporary_path() for i in range(self.TEST_HOOK_COUNT)
@@ -145,7 +149,8 @@ class HookTest(_common.TestCase, TestHelper):
             self.assertTrue(os.path.isfile(path))
             os.remove(path)
 
-    @unittest.skipIf(sys.platform, 'win32')  # FIXME: fails on windows
+    # FIXME: fails on windows
+    @unittest.skipIf(sys.platform == 'win32', 'win32')
     def test_hook_bytes_interpolation(self):
         temporary_paths = [
             get_temporary_path().encode('utf-8')

test/test_limit.py

@@ -0,0 +1,105 @@
+# 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 'limit' plugin."""
+
+import unittest
+
+from test.helper import TestHelper
+
+
+class LimitPluginTest(unittest.TestCase, TestHelper):
+    """Unit tests for LimitPlugin
+
+    Note: query prefix tests do not work correctly with `run_with_output`.
+    """
+
+    def setUp(self):
+
+        self.setup_beets()
+        self.load_plugins("limit")
+
+        # we'll create an even number of tracks in the library
+        self.num_test_items = 10
+        assert self.num_test_items % 2 == 0
+        for item_no, item in \
+                enumerate(self.add_item_fixtures(count=self.num_test_items)):
+            item.track = item_no + 1
+            item.store()
+
+        # our limit tests will use half of this number
+        self.num_limit = self.num_test_items // 2
+        self.num_limit_prefix = "".join(["'", "<", str(self.num_limit), "'"])
+
+        # a subset of tests has only `num_limit` results, identified by a
+        # range filter on the track number
+        self.track_head_range = "track:.." + str(self.num_limit)
+        self.track_tail_range = "track:" + str(self.num_limit + 1) + ".."
+
+    def tearDown(self):
+        self.unload_plugins()
+        self.teardown_beets()
+
+    def test_no_limit(self):
+        """Returns all when there is no limit or filter."""
+        result = self.run_with_output("lslimit")
+        self.assertEqual(result.count("\n"), self.num_test_items)
+
+    def test_lslimit_head(self):
+        """Returns the expected number with `lslimit --head`."""
+        result = self.run_with_output("lslimit", "--head", str(self.num_limit))
+        self.assertEqual(result.count("\n"), self.num_limit)
+
+    def test_lslimit_tail(self):
+        """Returns the expected number with `lslimit --tail`."""
+        result = self.run_with_output("lslimit", "--tail", str(self.num_limit))
+        self.assertEqual(result.count("\n"), self.num_limit)
+
+    def test_lslimit_head_invariant(self):
+        """Returns the expected number with `lslimit --head` and a filter."""
+        result = self.run_with_output(
+            "lslimit", "--head", str(self.num_limit), self.track_tail_range)
+        self.assertEqual(result.count("\n"), self.num_limit)
+
+    def test_lslimit_tail_invariant(self):
+        """Returns the expected number with `lslimit --tail` and a filter."""
+        result = self.run_with_output(
+            "lslimit", "--tail", str(self.num_limit), self.track_head_range)
+        self.assertEqual(result.count("\n"), self.num_limit)
+
+    def test_prefix(self):
+        """Returns the expected number with the query prefix."""
+        result = self.lib.items(self.num_limit_prefix)
+        self.assertEqual(len(result), self.num_limit)
+
+    def test_prefix_when_correctly_ordered(self):
+        """Returns the expected number with the query prefix and filter when
+        the prefix portion (correctly) appears last."""
+        correct_order = self.track_tail_range + " " + self.num_limit_prefix
+        result = self.lib.items(correct_order)
+        self.assertEqual(len(result), self.num_limit)
+
+    def test_prefix_when_incorrectly_ordred(self):
+        """Returns no results with the query prefix and filter when the prefix
+        portion (incorrectly) appears first."""
+        incorrect_order = self.num_limit_prefix + " " + self.track_tail_range
+        result = self.lib.items(incorrect_order)
+        self.assertEqual(len(result), 0)
+
+
+def suite():
+    return unittest.TestLoader().loadTestsFromName(__name__)
+
+
+if __name__ == '__main__':
+    unittest.main(defaultTest='suite')

test/test_play.py

@@ -72,7 +72,8 @@ class PlayPluginTest(unittest.TestCase, TestHelper):
         self.run_and_assert(
             open_mock, ['title:aNiceTitle'], 'echo other')
 
-    @unittest.skipIf(sys.platform, 'win32')  # FIXME: fails on windows
+    # FIXME: fails on windows
+    @unittest.skipIf(sys.platform == 'win32', 'win32')
     def test_relative_to(self, open_mock):
         self.config['play']['command'] = 'echo'
         self.config['play']['relative_to'] = '/something'

test/test_query.py

@@ -425,7 +425,8 @@ class PathQueryTest(_common.LibTestCase, TestHelper, AssertsMixin):
         results = self.lib.albums(q)
         self.assert_albums_matched(results, [])
 
-    @unittest.skipIf(sys.platform, 'win32')  # FIXME: fails on windows
+    # FIXME: fails on windows
+    @unittest.skipIf(sys.platform == 'win32', 'win32')
     def test_parent_directory_no_slash(self):
         q = 'path:/a'
         results = self.lib.items(q)
@@ -434,7 +435,8 @@ class PathQueryTest(_common.LibTestCase, TestHelper, AssertsMixin):
         results = self.lib.albums(q)
         self.assert_albums_matched(results, ['path album'])
 
-    @unittest.skipIf(sys.platform, 'win32')  # FIXME: fails on windows
+    # FIXME: fails on windows
+    @unittest.skipIf(sys.platform == 'win32', 'win32')
     def test_parent_directory_with_slash(self):
         q = 'path:/a/'
         results = self.lib.items(q)

test/test_ui.py

@@ -918,7 +918,8 @@ class ConfigTest(unittest.TestCase, TestHelper, _common.Assertions):
 #                      '--config', cli_overwrite_config_path, 'test')
 #        self.assertEqual(config['anoption'].get(), 'cli overwrite')
 
-    @unittest.skipIf(sys.platform, 'win32')  # FIXME: fails on windows
+    # FIXME: fails on windows
+    @unittest.skipIf(sys.platform == 'win32', 'win32')
     def test_cli_config_paths_resolve_relative_to_user_dir(self):
         cli_config_path = os.path.join(self.temp_dir, b'config.yaml')
         with open(cli_config_path, 'w') as file: