From d68ed1adca6e43ef158a528e24d89090bea7734a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= <16212750+snejus@users.noreply.github.com> Date: Tue, 31 May 2022 21:51:47 +0100 Subject: [PATCH 1/5] Make implicit path queries explicit and simplify their handling --- beets/library.py | 22 ++++------------------ docs/changelog.rst | 3 +++ test/test_query.py | 1 - 3 files changed, 7 insertions(+), 19 deletions(-) diff --git a/beets/library.py b/beets/library.py index 69fcd34cf..e15c3e287 100644 --- a/beets/library.py +++ b/beets/library.py @@ -1382,7 +1382,7 @@ def parse_query_parts(parts, model_cls): `Query` and `Sort` they represent. Like `dbcore.parse_sorted_query`, with beets query prefixes and - special path query detection. + ensuring that implicit path queries are made explicit with 'path::' """ # Get query types and their prefix characters. prefixes = { @@ -1394,28 +1394,14 @@ def parse_query_parts(parts, model_cls): # Special-case path-like queries, which are non-field queries # containing path separators (/). - path_parts = [] - non_path_parts = [] - for s in parts: - if PathQuery.is_path_query(s): - path_parts.append(s) - else: - non_path_parts.append(s) + parts = [f"path::{s}" if PathQuery.is_path_query(s) else s for s in parts] case_insensitive = beets.config['sort_case_insensitive'].get(bool) - query, sort = dbcore.parse_sorted_query( - model_cls, non_path_parts, prefixes, case_insensitive + return dbcore.parse_sorted_query( + model_cls, parts, prefixes, case_insensitive ) - # Add path queries to aggregate query. - # Match field / flexattr depending on whether the model has the path field - fast_path_query = 'path' in model_cls._fields - query.subqueries += [PathQuery('path', s, fast_path_query) - for s in path_parts] - - return query, sort - def parse_query_string(s, model_cls): """Given a beets query string, return the `Query` and `Sort` they diff --git a/docs/changelog.rst b/docs/changelog.rst index 72b1cf1fe..d6c74e451 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -28,6 +28,9 @@ New features: Bug fixes: +* Fix implicit paths OR queries (e.g. ``beet list /path/ , /other-path/``) + which have previously been returning the entire library. + :bug:`1865` * The Discogs release ID is now populated correctly to the discogs_albumid field again (it was no longer working after Discogs changed their release URL format). diff --git a/test/test_query.py b/test/test_query.py index 0be4b7d7f..42ac59822 100644 --- a/test/test_query.py +++ b/test/test_query.py @@ -529,7 +529,6 @@ class PathQueryTest(_common.LibTestCase, TestHelper, AssertsMixin): results = self.lib.albums(q) self.assert_albums_matched(results, ['path album']) - @unittest.skip('unfixed (#1865)') def test_path_query_in_or_query(self): q = '/a/b , /a/b' results = self.lib.items(q) From ba777dda5011ed3e02aeb7d9e2b5ecf8877b0bcb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= <16212750+snejus@users.noreply.github.com> Date: Tue, 31 May 2022 22:34:40 +0100 Subject: [PATCH 2/5] Skip implicit paths tests for win32 --- test/test_query.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/test/test_query.py b/test/test_query.py index 42ac59822..4035b2b7b 100644 --- a/test/test_query.py +++ b/test/test_query.py @@ -33,6 +33,9 @@ from beets.library import Library, Item from beets import util import platform +# Because the absolute path begins with something like C:, we +# can't disambiguate it from an ordinary query. +WIN32_NO_IMPLICIT_PATHS = 'Implicit paths are not supported on Windows' class TestHelper(helper.TestHelper): @@ -521,6 +524,7 @@ class PathQueryTest(_common.LibTestCase, TestHelper, AssertsMixin): results = self.lib.albums(q) self.assert_albums_matched(results, ['path album']) + @unittest.skipIf(sys.platform == 'win32', WIN32_NO_IMPLICIT_PATHS) def test_slashed_query_matches_path(self): q = '/a/b' results = self.lib.items(q) @@ -529,6 +533,7 @@ class PathQueryTest(_common.LibTestCase, TestHelper, AssertsMixin): results = self.lib.albums(q) self.assert_albums_matched(results, ['path album']) + @unittest.skipIf(sys.platform == 'win32', WIN32_NO_IMPLICIT_PATHS) def test_path_query_in_or_query(self): q = '/a/b , /a/b' results = self.lib.items(q) @@ -648,12 +653,8 @@ class PathQueryTest(_common.LibTestCase, TestHelper, AssertsMixin): self.assertFalse(is_path('foo:bar/')) self.assertFalse(is_path('foo:/bar')) + @unittest.skipIf(sys.platform == 'win32', WIN32_NO_IMPLICIT_PATHS) def test_detect_absolute_path(self): - if platform.system() == 'Windows': - # Because the absolute path begins with something like C:, we - # can't disambiguate it from an ordinary query. - self.skipTest('Windows absolute paths do not work as queries') - # Don't patch `os.path.exists`; we'll actually create a file when # it exists. self.patcher_exists.stop() From 72c530200419d788280f83fd3728073810971929 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Tue, 31 May 2022 22:45:05 +0100 Subject: [PATCH 3/5] Fix lints --- test/test_query.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_query.py b/test/test_query.py index 4035b2b7b..8a9043fa3 100644 --- a/test/test_query.py +++ b/test/test_query.py @@ -31,12 +31,12 @@ from beets.dbcore.query import (NoneQuery, ParsingError, InvalidQueryArgumentValueError) from beets.library import Library, Item from beets import util -import platform # Because the absolute path begins with something like C:, we # can't disambiguate it from an ordinary query. WIN32_NO_IMPLICIT_PATHS = 'Implicit paths are not supported on Windows' + class TestHelper(helper.TestHelper): def assertInResult(self, item, results): # noqa From d65fcfbc8ea407bbd1e37d52b31ff4f9ba99441e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Wed, 1 Jun 2022 01:40:16 +0100 Subject: [PATCH 4/5] Use : instead of :: --- beets/library.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beets/library.py b/beets/library.py index e15c3e287..7be7bc79f 100644 --- a/beets/library.py +++ b/beets/library.py @@ -1394,7 +1394,7 @@ def parse_query_parts(parts, model_cls): # Special-case path-like queries, which are non-field queries # containing path separators (/). - parts = [f"path::{s}" if PathQuery.is_path_query(s) else s for s in parts] + parts = [f"path={s}" if PathQuery.is_path_query(s) else s for s in parts] case_insensitive = beets.config['sort_case_insensitive'].get(bool) From 5f4b46e3888d4b7e3c48921149a98563012699ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Wed, 1 Jun 2022 01:47:55 +0100 Subject: [PATCH 5/5] Actually use : --- beets/library.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beets/library.py b/beets/library.py index 7be7bc79f..c8fa2b5fc 100644 --- a/beets/library.py +++ b/beets/library.py @@ -1394,7 +1394,7 @@ def parse_query_parts(parts, model_cls): # Special-case path-like queries, which are non-field queries # containing path separators (/). - parts = [f"path={s}" if PathQuery.is_path_query(s) else s for s in parts] + parts = [f"path:{s}" if PathQuery.is_path_query(s) else s for s in parts] case_insensitive = beets.config['sort_case_insensitive'].get(bool)