From cdb6b21f1af0e72082e6ad8dee559a2c9a87b73f Mon Sep 17 00:00:00 2001 From: Patrick Nicholson Date: Tue, 7 Dec 2021 18:47:55 -0500 Subject: [PATCH 01/21] Adding limit and its documentation --- beetsplug/limit.py | 95 ++++++++++++++++++++++++++++++++++++++++++ docs/plugins/limit.rst | 52 +++++++++++++++++++++++ 2 files changed, 147 insertions(+) create mode 100644 beetsplug/limit.py create mode 100644 docs/plugins/limit.rst diff --git a/beetsplug/limit.py b/beetsplug/limit.py new file mode 100644 index 000000000..20b1ff263 --- /dev/null +++ b/beetsplug/limit.py @@ -0,0 +1,95 @@ +# 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. + +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""" + + head = opts.head + tail = opts.tail + + if head and tail: + raise RuntimeError("Only use one of --head and --tail") + + query = decargs(args) + if opts.album: + objs = lib.albums(query) + else: + objs = lib.items(query) + + if head: + objs = islice(objs, head) + elif tail: + objs = deque(objs, 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 LsLimitPlugin(BeetsPlugin): + def commands(self): + return [lslimit_cmd] + + +class HeadPlugin(BeetsPlugin): + """Head of an arbitrary query. + + This allows a user to limit the results of any query to the first + `pattern` rows. Example usage: return first 10 tracks `beet ls '<10'`. + """ + + def queries(self): + + class HeadQuery(FieldQuery): + """Singleton query implementation that tracks result count.""" + n = 0 + include = True + @classmethod + def value_match(cls, pattern, value): + cls.n += 1 + if cls.include: + cls.include = cls.n <= int(pattern) + return cls.include + + return { + "<": HeadQuery + } \ No newline at end of file diff --git a/docs/plugins/limit.rst b/docs/plugins/limit.rst new file mode 100644 index 000000000..7265ac34d --- /dev/null +++ b/docs/plugins/limit.rst @@ -0,0 +1,52 @@ +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 ``' Date: Tue, 7 Dec 2021 21:27:57 -0500 Subject: [PATCH 02/21] Tests for limit plugin --- test/test_limit.py | 80 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 test/test_limit.py diff --git a/test/test_limit.py b/test/test_limit.py new file mode 100644 index 000000000..1dbe415f4 --- /dev/null +++ b/test/test_limit.py @@ -0,0 +1,80 @@ +# 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 import _common +from beetsplug import limit +from beets import config + +from test.helper import TestHelper + + +class LsLimitPluginTest(unittest.TestCase, TestHelper): + + def setUp(self): + self.setup_beets() + self.load_plugins("limit") + self.num_test_items = 10 + assert self.num_test_items % 2 == 0 + self.num_limit = self.num_test_items / 2 + self.num_limit_prefix = "'<" + str(self.num_limit) + "'" + self.track_head_range = "track:.." + str(self.num_limit) + self.track_tail_range = "track:" + str(self.num_limit + 1) + ".." + for item_no, item in enumerate(self.add_item_fixtures(count=self.num_test_items)): + item.track = item_no + 1 + item.store() + + def tearDown(self): + self.teardown_beets() + + def test_no_limit(self): + result = self.run_with_output("lslimit") + self.assertEqual(result.count("\n"), self.num_test_items) + + def test_lslimit_head(self): + result = self.run_with_output("lslimit", "--head", str(self.num_limit)) + self.assertEqual(result.count("\n"), self.num_limit) + + def test_lslimit_tail(self): + 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): + 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): + 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): + result = self.run_with_output("ls", self.num_limit_prefix) + self.assertEqual(result.count("\n"), self.num_limit) + + def test_prefix_when_correctly_ordered(self): + result = self.run_with_output("ls", self.track_tail_range, self.num_limit_prefix) + self.assertEqual(result.count("\n"), self.num_limit) + + def test_prefix_when_incorrectly_ordred(self): + result = self.run_with_output("ls", self.num_limit_prefix, self.track_tail_range) + self.assertEqual(result.count("\n"), 0) + + +def suite(): + return unittest.TestLoader().loadTestsFromName(__name__) + +if __name__ == '__main__': + unittest.main(defaultTest='suite') From 9838369f02a5623eed593b75d517f3657322748f Mon Sep 17 00:00:00 2001 From: Patrick Nicholson Date: Tue, 7 Dec 2021 21:32:39 -0500 Subject: [PATCH 03/21] limit added to changelog --- docs/changelog.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 8d95b7fb0..c76291571 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -17,6 +17,10 @@ For packagers: * We fixed a version for the dependency on the `Confuse`_ library. :bug:`4167` +Other new things: + +* :doc:`/plugins/limit`: Limit query results to head or tail (``lslimit`` + command only) 1.6.0 (November 27, 2021) ------------------------- From 24bc4e77e2a41be7243e8703b301e24fccb520e0 Mon Sep 17 00:00:00 2001 From: patrick-nicholson Date: Sun, 26 Dec 2021 21:56:57 -0500 Subject: [PATCH 04/21] Being more careful about truthiness and catching negative values (they could be supported, but it's probably not intuitive). Moving command into single plugin as suggested. Fixing linter objections. --- beetsplug/limit.py | 62 +++++++++++++++++++++++----------------------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/beetsplug/limit.py b/beetsplug/limit.py index 20b1ff263..6dfb2c40d 100644 --- a/beetsplug/limit.py +++ b/beetsplug/limit.py @@ -20,43 +20,42 @@ from itertools import islice def lslimit(lib, opts, args): """Query command with head/tail""" - - head = opts.head - tail = opts.tail - if head and tail: - raise RuntimeError("Only use one of --head and --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 head: - objs = islice(objs, head) - elif tail: - objs = deque(objs, tail) + + 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", + "lslimit", help="query with optional head or tail" ) lslimit_cmd.parser.add_option( - '--head', - action='store', + '--head', + action='store', type="int", default=None ) lslimit_cmd.parser.add_option( '--tail', - action='store', + action='store', type="int", default=None ) @@ -65,31 +64,32 @@ lslimit_cmd.parser.add_all_common_options() lslimit_cmd.func = lslimit -class LsLimitPlugin(BeetsPlugin): +class LimitPlugin(BeetsPlugin): + """Query limit functionality via command and query prefix + """ + def commands(self): return [lslimit_cmd] - -class HeadPlugin(BeetsPlugin): - """Head of an arbitrary query. - - This allows a user to limit the results of any query to the first - `pattern` rows. Example usage: return first 10 tracks `beet ls '<10'`. - """ - def queries(self): class HeadQuery(FieldQuery): - """Singleton query implementation that tracks result count.""" + """This inner class pattern allows the query to track state + """ n = 0 - include = True + 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 - if cls.include: - cls.include = cls.n <= int(pattern) - return cls.include - + return cls.n <= cls.N + return { "<": HeadQuery - } \ No newline at end of file + } From 126b4e94cfdd4e72b4db54664dcf8e4b8f6e0c26 Mon Sep 17 00:00:00 2001 From: patrick-nicholson Date: Sun, 26 Dec 2021 21:57:29 -0500 Subject: [PATCH 05/21] Fixing doc typo and adding limit to toctree. --- docs/plugins/index.rst | 1 + docs/plugins/limit.rst | 12 +++++++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/docs/plugins/index.rst b/docs/plugins/index.rst index 5ca8794fd..3d8b97606 100644 --- a/docs/plugins/index.rst +++ b/docs/plugins/index.rst @@ -98,6 +98,7 @@ following to your configuration:: kodiupdate lastgenre lastimport + limit loadext lyrics mbcollection diff --git a/docs/plugins/limit.rst b/docs/plugins/limit.rst index 7265ac34d..74bd47cb6 100644 --- a/docs/plugins/limit.rst +++ b/docs/plugins/limit.rst @@ -3,7 +3,7 @@ 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 ``' Date: Sun, 26 Dec 2021 21:58:45 -0500 Subject: [PATCH 06/21] Fixed truediv typo (switched to intdiv). Fixed linter objections. --- test/test_limit.py | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/test/test_limit.py b/test/test_limit.py index 1dbe415f4..d5a3d60af 100644 --- a/test/test_limit.py +++ b/test/test_limit.py @@ -15,10 +15,6 @@ import unittest -from test import _common -from beetsplug import limit -from beets import config - from test.helper import TestHelper @@ -29,11 +25,12 @@ class LsLimitPluginTest(unittest.TestCase, TestHelper): self.load_plugins("limit") self.num_test_items = 10 assert self.num_test_items % 2 == 0 - self.num_limit = self.num_test_items / 2 + self.num_limit = self.num_test_items // 2 self.num_limit_prefix = "'<" + str(self.num_limit) + "'" self.track_head_range = "track:.." + str(self.num_limit) self.track_tail_range = "track:" + str(self.num_limit + 1) + ".." - for item_no, item in enumerate(self.add_item_fixtures(count=self.num_test_items)): + for item_no, item in \ + enumerate(self.add_item_fixtures(count=self.num_test_items)): item.track = item_no + 1 item.store() @@ -43,21 +40,23 @@ class LsLimitPluginTest(unittest.TestCase, TestHelper): def test_no_limit(self): result = self.run_with_output("lslimit") self.assertEqual(result.count("\n"), self.num_test_items) - + def test_lslimit_head(self): result = self.run_with_output("lslimit", "--head", str(self.num_limit)) self.assertEqual(result.count("\n"), self.num_limit) - + def test_lslimit_tail(self): 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): - result = self.run_with_output("lslimit", "--head", str(self.num_limit), self.track_tail_range) + 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): - result = self.run_with_output("lslimit", "--tail", str(self.num_limit), self.track_head_range) + 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): @@ -65,16 +64,19 @@ class LsLimitPluginTest(unittest.TestCase, TestHelper): self.assertEqual(result.count("\n"), self.num_limit) def test_prefix_when_correctly_ordered(self): - result = self.run_with_output("ls", self.track_tail_range, self.num_limit_prefix) + result = self.run_with_output( + "ls", self.track_tail_range, self.num_limit_prefix) self.assertEqual(result.count("\n"), self.num_limit) def test_prefix_when_incorrectly_ordred(self): - result = self.run_with_output("ls", self.num_limit_prefix, self.track_tail_range) + result = self.run_with_output( + "ls", self.num_limit_prefix, self.track_tail_range) self.assertEqual(result.count("\n"), 0) def suite(): return unittest.TestLoader().loadTestsFromName(__name__) + if __name__ == '__main__': unittest.main(defaultTest='suite') From 6c64ab6df157a6f335d0a072d30b167eb4bc5c34 Mon Sep 17 00:00:00 2001 From: patrick-nicholson Date: Mon, 27 Dec 2021 12:11:18 -0500 Subject: [PATCH 07/21] Noticed GitHub linter wanted a docstring. --- beetsplug/limit.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/beetsplug/limit.py b/beetsplug/limit.py index 6dfb2c40d..5e5ee16be 100644 --- a/beetsplug/limit.py +++ b/beetsplug/limit.py @@ -11,6 +11,15 @@ # 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 a3754f7592794f4f7afa8915d7f12f99cafbb5d3 Mon Sep 17 00:00:00 2001 From: patrick-nicholson Date: Mon, 27 Dec 2021 13:26:38 -0500 Subject: [PATCH 08/21] Test failing due to unidentified argument processing issue; replacing with API calls gives expected results. Fixed some linting issues. --- beetsplug/limit.py | 19 ++++++++---------- setup.cfg | 1 + test/test_limit.py | 49 ++++++++++++++++++++++++++++++++++------------ 3 files changed, 45 insertions(+), 24 deletions(-) diff --git a/beetsplug/limit.py b/beetsplug/limit.py index 5e5ee16be..3942ced0f 100644 --- a/beetsplug/limit.py +++ b/beetsplug/limit.py @@ -28,7 +28,7 @@ from itertools import islice def lslimit(lib, opts, args): - """Query command with head/tail""" + """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") @@ -56,15 +56,15 @@ lslimit_cmd = Subcommand( ) lslimit_cmd.parser.add_option( - '--head', - action='store', + "--head", + action="store", type="int", default=None ) lslimit_cmd.parser.add_option( - '--tail', - action='store', + "--tail", + action="store", type="int", default=None ) @@ -74,28 +74,25 @@ lslimit_cmd.func = lslimit class LimitPlugin(BeetsPlugin): - """Query limit functionality via command and query prefix - """ + """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 - """ + """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 diff --git a/setup.cfg b/setup.cfg index 6aab6b7e6..31deba4b1 100644 --- a/setup.cfg +++ b/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 diff --git a/test/test_limit.py b/test/test_limit.py index d5a3d60af..35c01c41a 100644 --- a/test/test_limit.py +++ b/test/test_limit.py @@ -18,60 +18,83 @@ import unittest from test.helper import TestHelper -class LsLimitPluginTest(unittest.TestCase, 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 - self.num_limit = self.num_test_items // 2 - self.num_limit_prefix = "'<" + str(self.num_limit) + "'" - self.track_head_range = "track:.." + str(self.num_limit) - self.track_tail_range = "track:" + str(self.num_limit + 1) + ".." 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): - result = self.run_with_output("ls", self.num_limit_prefix) - self.assertEqual(result.count("\n"), self.num_limit) + """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): - result = self.run_with_output( - "ls", self.track_tail_range, self.num_limit_prefix) - self.assertEqual(result.count("\n"), self.num_limit) + """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): - result = self.run_with_output( - "ls", self.num_limit_prefix, self.track_tail_range) - self.assertEqual(result.count("\n"), 0) + """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(): From c3829c52c8f462fb43e861b1294a5e2657774375 Mon Sep 17 00:00:00 2001 From: patrick-nicholson Date: Mon, 27 Dec 2021 13:38:20 -0500 Subject: [PATCH 09/21] Lint ignore. --- setup.cfg | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.cfg b/setup.cfg index 31deba4b1..a3d4a866a 100644 --- a/setup.cfg +++ b/setup.cfg @@ -162,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 From 27c2b79f019ec5d2cb41976b68b57dd5bac02003 Mon Sep 17 00:00:00 2001 From: patrick-nicholson Date: Mon, 27 Dec 2021 13:39:35 -0500 Subject: [PATCH 10/21] Finally learning to read GitHub. --- docs/plugins/limit.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/plugins/limit.rst b/docs/plugins/limit.rst index 74bd47cb6..35d2b0428 100644 --- a/docs/plugins/limit.rst +++ b/docs/plugins/limit.rst @@ -27,9 +27,9 @@ 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 ``smartplaylists`` (see :doc:`/plugins/ -smartplaylists`) to limit the number of tracks in a smart playlist for -applications like most played and recently added. +you can use the query prefix in ``smartplaylists`` +(see :doc:`/plugins/smartplaylists`) to limit the number of tracks in a smart +playlist for applications like most played and recently added. Configuration ============= From 5b3479705629a50079870e2201d4f4f06e7c5108 Mon Sep 17 00:00:00 2001 From: patrick-nicholson Date: Mon, 27 Dec 2021 14:51:04 -0500 Subject: [PATCH 11/21] sigh --- docs/plugins/limit.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/plugins/limit.rst b/docs/plugins/limit.rst index 35d2b0428..ac8cc72c0 100644 --- a/docs/plugins/limit.rst +++ b/docs/plugins/limit.rst @@ -27,8 +27,8 @@ 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 ``smartplaylists`` -(see :doc:`/plugins/smartplaylists`) to limit the number of tracks in a smart +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 From a09c80447a85c8718e2cfd81139e1bf4b07a263b Mon Sep 17 00:00:00 2001 From: Lars Kruse Date: Fri, 3 Dec 2021 18:20:45 +0100 Subject: [PATCH 12/21] beetsplug/web: fix translation of query path The routing map translator `QueryConverter` was misconfigured: * decoding (parsing a path): splitting with "/" as tokenizer * encoding (translating back to a path): joining items with "," as separator This caused queries containing more than one condition (separated by a slash) to return an empty result. Queries with only a single condition were not affected. Instead the encoding should have used the same delimiter (the slash) for the backward conversion. How to reproduce: * query: `/album/query/albumartist::%5Efoo%24/original_year%2B/year%2B/album%2B` * resulting content in parsed argument `queries` in the `album_query` function: * previous (wrong): `['albumartist::^foo$,original_year+,year+,album+']` * new (correct): `['albumartist::^foo$', 'original_year+', 'year+', 'album+']` --- beetsplug/web/__init__.py | 2 +- docs/changelog.rst | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/beetsplug/web/__init__.py b/beetsplug/web/__init__.py index 240126e95..63f7f92ad 100644 --- a/beetsplug/web/__init__.py +++ b/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): diff --git a/docs/changelog.rst b/docs/changelog.rst index 190ed2558..5370ab0f7 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -28,6 +28,9 @@ Bug fixes: ``r128_album_gain`` fields was changed from integer to float to fix loss of precision due to truncation. :bug:`4169` +* :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. For packagers: From ec066940976555335770431449b9d022b390004d Mon Sep 17 00:00:00 2001 From: mousecloak Date: Fri, 7 Jan 2022 19:12:05 -0800 Subject: [PATCH 13/21] Makes the import converter respect the quiet and pretend flags. When the delete_originals was set, beets would print the following, regardless of the presence of the quiet parameter: convert: Removing original file /path/to/file.ext This commit ensures that the log is only printed when quiet is not present. --- beetsplug/convert.py | 25 +++++++++++++++---------- docs/changelog.rst | 4 ++++ test/test_convert.py | 27 +++++++++++++++++++++++---- 3 files changed, 42 insertions(+), 14 deletions(-) diff --git a/beetsplug/convert.py b/beetsplug/convert.py index 6bc07c287..16b6e7075 100644 --- a/beetsplug/convert.py +++ b/beetsplug/convert.py @@ -514,17 +514,22 @@ 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: diff --git a/docs/changelog.rst b/docs/changelog.rst index 190ed2558..d0b59245f 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -28,6 +28,10 @@ Bug fixes: ``r128_album_gain`` fields was changed from integer to float to fix loss of precision due to truncation. :bug:`4169` +* :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. For packagers: diff --git a/test/test_convert.py b/test/test_convert.py index ce0750119..53c1563b8 100644 --- a/test/test_convert.py +++ b/test/test_convert.py @@ -122,9 +122,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 +283,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) From 0132067a29eb970a4cbd54df1fab6cf439e29b44 Mon Sep 17 00:00:00 2001 From: mousecloak Date: Fri, 7 Jan 2022 19:35:40 -0800 Subject: [PATCH 14/21] Fix @unittest.skipIf annotations to ignore only win32 --- test/test_convert.py | 3 ++- test/test_hook.py | 15 ++++++++++----- test/test_play.py | 3 ++- test/test_query.py | 6 ++++-- test/test_ui.py | 3 ++- 5 files changed, 20 insertions(+), 10 deletions(-) diff --git a/test/test_convert.py b/test/test_convert.py index 53c1563b8..493d4ecca 100644 --- a/test/test_convert.py +++ b/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' diff --git a/test/test_hook.py b/test/test_hook.py index 6ade06349..5049b5d24 100644 --- a/test/test_hook.py +++ b/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') diff --git a/test/test_play.py b/test/test_play.py index 2007686c7..8577aee70 100644 --- a/test/test_play.py +++ b/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' diff --git a/test/test_query.py b/test/test_query.py index 709f42bd5..14f3f082a 100644 --- a/test/test_query.py +++ b/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) diff --git a/test/test_ui.py b/test/test_ui.py index 9804b0a12..dd24fce1a 100644 --- a/test/test_ui.py +++ b/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: From 438262844a1697a38a33db060489e272a639090a Mon Sep 17 00:00:00 2001 From: mousecloak Date: Fri, 7 Jan 2022 21:39:19 -0800 Subject: [PATCH 15/21] Fixed style violation --- beetsplug/convert.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/beetsplug/convert.py b/beetsplug/convert.py index 16b6e7075..82e62af62 100644 --- a/beetsplug/convert.py +++ b/beetsplug/convert.py @@ -528,7 +528,8 @@ class ConvertPlugin(BeetsPlugin): if self.config['delete_originals']: if not quiet: - self._log.info('Removing original file {0}', source_path) + self._log.info('Removing original file {0}', + source_path) util.remove(source_path, False) def _cleanup(self, task, session): From e35c767e2c935b392e7ca70419fe2c1861a7fa76 Mon Sep 17 00:00:00 2001 From: J0J0 T Date: Sun, 9 Jan 2022 14:29:47 +0100 Subject: [PATCH 16/21] Skip Discogs query on insufficiently tagged files - When files are missing both, album and artist tags, the Discogs metadata plugin sends empty information to the Discogs API which returns arbitrary query results. - This patch catches this case and states it in beets import verbose output. --- beetsplug/discogs.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index d015e4201..8c950c521 100644 --- a/beetsplug/discogs.py +++ b/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: From 4401de94f7b70d55867b1cfed24ebbfa330e6fa9 Mon Sep 17 00:00:00 2001 From: J0J0 T Date: Mon, 10 Jan 2022 08:20:46 +0100 Subject: [PATCH 17/21] Add changelog entry for PR #4227 (discogs: Skip Discogs query on insufficiently tagged files). --- docs/changelog.rst | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index d13dcdd4a..88d465d7d 100644 --- a/docs/changelog.rst +++ b/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 @@ -35,6 +35,9 @@ Bug fixes: * :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` For packagers: From 2a53b890be70b6cf537e045c3a6ccbbb96b67f31 Mon Sep 17 00:00:00 2001 From: J0J0 T Date: Mon, 10 Jan 2022 09:10:19 +0100 Subject: [PATCH 18/21] Add to discogs plugin docs regarding PR #4227 - Clarify basic search behaviour in intro chapter of discogs plugin, - and state change introduced in PR#4227 (discogs: Discogs query on insufficiently tagged files) --- docs/plugins/discogs.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/plugins/discogs.rst b/docs/plugins/discogs.rst index 40875b022..5aea1ae6b 100644 --- a/docs/plugins/discogs.rst +++ b/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. From 3f896ab28117cd9032a0d9fcc8fc5c871f954324 Mon Sep 17 00:00:00 2001 From: ybnd Date: Mon, 10 Jan 2022 19:03:36 +0100 Subject: [PATCH 19/21] Make Tekstowo scraper more specific --- beetsplug/lyrics.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/beetsplug/lyrics.py b/beetsplug/lyrics.py index 7d026def1..0856ebb34 100644 --- a/beetsplug/lyrics.py +++ b/beetsplug/lyrics.py @@ -488,11 +488,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): From 3a8520e30ab9ec6c435cacfa1d1508421ca2ecbe Mon Sep 17 00:00:00 2001 From: ybnd Date: Mon, 10 Jan 2022 19:07:59 +0100 Subject: [PATCH 20/21] Add changelog entry --- docs/changelog.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index d13dcdd4a..e4a10dc9c 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -35,6 +35,8 @@ Bug fixes: * :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/lyrics`: Fixed an issue with the Tekstowo.pl scraper where some + non-lyrics content got included in the lyrics For packagers: From 414760282b9ec374a26488a89da230f545f35b52 Mon Sep 17 00:00:00 2001 From: ybnd Date: Mon, 10 Jan 2022 22:07:58 +0100 Subject: [PATCH 21/21] Remove footer text from Genius lyrics --- beetsplug/lyrics.py | 6 ++++++ docs/changelog.rst | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/beetsplug/lyrics.py b/beetsplug/lyrics.py index 0856ebb34..1f215df45 100644 --- a/beetsplug/lyrics.py +++ b/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() diff --git a/docs/changelog.rst b/docs/changelog.rst index e4a10dc9c..715853b66 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -35,8 +35,8 @@ Bug fixes: * :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/lyrics`: Fixed an issue with the Tekstowo.pl scraper where some - non-lyrics content got included in the lyrics +* :doc:`plugins/lyrics`: Fixed issues with the Tekstowo.pl and Genius + backends where some non-lyrics content got included in the lyrics For packagers: