From 9467df526cefec5dcc5df5c00ddc7c257c306538 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 15 Nov 2022 21:29:53 +0100 Subject: [PATCH] song/StringFilter: add enum Position Replaces two conflicting bools. --- src/song/Filter.cxx | 69 ++++++++++++++++++++++++++++---------- src/song/StringFilter.cxx | 34 +++++++++++++------ src/song/StringFilter.hxx | 38 ++++++++++++++------- test/TestStringFilter.cxx | 26 +++++++------- test/TestTagSongFilter.cxx | 60 ++++++++++++++++++++++----------- 5 files changed, 153 insertions(+), 74 deletions(-) diff --git a/src/song/Filter.cxx b/src/song/Filter.cxx index d3266d681..37edad653 100644 --- a/src/song/Filter.cxx +++ b/src/song/Filter.cxx @@ -90,8 +90,12 @@ SongFilter::SongFilter(TagType tag, const char *value, bool fold_case) { /* for compatibility with MPD 0.20 and older, "fold_case" also switches on "substring" */ + const auto position = fold_case + ? StringFilter::Position::ANYWHERE + : StringFilter::Position::FULL; + and_filter.AddItem(std::make_unique(tag, - StringFilter(value, fold_case, fold_case, false, false))); + StringFilter(value, fold_case, position, false))); } /* this destructor exists here just so it won't get inlined */ @@ -209,25 +213,41 @@ ParseStringFilter(const char *&s, bool fold_case) if (auto after_contains = StringAfterPrefixIgnoreCase(s, "contains ")) { s = StripLeft(after_contains); auto value = ExpectQuoted(s); - return {std::move(value), fold_case, true, false, false}; + return { + std::move(value), fold_case, + StringFilter::Position::ANYWHERE, + false, + }; } if (auto after_not_contains = StringAfterPrefixIgnoreCase(s, "!contains ")) { s = StripLeft(after_not_contains); auto value = ExpectQuoted(s); - return {std::move(value), fold_case, true, false, true}; + return { + std::move(value), fold_case, + StringFilter::Position::ANYWHERE, + true, + }; } if (auto after_starts_with = StringAfterPrefixIgnoreCase(s, "starts_with ")) { s = StripLeft(after_starts_with); auto value = ExpectQuoted(s); - return {std::move(value), fold_case, false, true, false}; + return { + std::move(value), fold_case, + StringFilter::Position::PREFIX, + false, + }; } if (auto after_not_starts_with = StringAfterPrefixIgnoreCase(s, "!starts_with ")) { s = StripLeft(after_not_starts_with); auto value = ExpectQuoted(s); - return {std::move(value), fold_case, false, true, true}; + return { + std::move(value), fold_case, + StringFilter::Position::PREFIX, + true, + }; } bool negated = false; @@ -237,7 +257,11 @@ ParseStringFilter(const char *&s, bool fold_case) negated = s[0] == '!'; s = StripLeft(s + 2); auto value = ExpectQuoted(s); - StringFilter f(std::move(value), fold_case, false, false, negated); + StringFilter f{ + std::move(value), fold_case, + StringFilter::Position::FULL, + negated, + }; f.SetRegex(std::make_shared(f.GetValue().c_str(), false, false, fold_case)); @@ -253,7 +277,11 @@ ParseStringFilter(const char *&s, bool fold_case) s = StripLeft(s + 2); auto value = ExpectQuoted(s); - return {std::move(value), fold_case, false, false, negated}; + return { + std::move(value), fold_case, + StringFilter::Position::FULL, + negated, + }; } ISongFilterPtr @@ -399,11 +427,14 @@ SongFilter::Parse(const char *tag_string, const char *value, bool fold_case) case LOCATE_TAG_FILE_TYPE: /* for compatibility with MPD 0.20 and older, "fold_case" also switches on "substring" */ - and_filter.AddItem(std::make_unique(StringFilter(value, - fold_case, - fold_case, - false, - false))); + and_filter.AddItem(std::make_unique(StringFilter{ + value, + fold_case, + fold_case + ? StringFilter::Position::ANYWHERE + : StringFilter::Position::FULL, + false, + })); break; default: @@ -412,12 +443,14 @@ SongFilter::Parse(const char *tag_string, const char *value, bool fold_case) /* for compatibility with MPD 0.20 and older, "fold_case" also switches on "substring" */ - and_filter.AddItem(std::make_unique(TagType(tag), - StringFilter(value, - fold_case, - fold_case, - false, - false))); + and_filter.AddItem(std::make_unique(TagType(tag), StringFilter{ + value, + fold_case, + fold_case + ? StringFilter::Position::ANYWHERE + : StringFilter::Position::FULL, + false, + })); break; } } diff --git a/src/song/StringFilter.cxx b/src/song/StringFilter.cxx index 301466b92..d1e99f44c 100644 --- a/src/song/StringFilter.cxx +++ b/src/song/StringFilter.cxx @@ -33,17 +33,31 @@ StringFilter::MatchWithoutNegation(const char *s) const noexcept #endif if (fold_case) { - return substring - ? fold_case.IsIn(s) - : (starts_with - ? fold_case.StartsWith(s) - : fold_case == s); + switch (position) { + case Position::FULL: + break; + + case Position::ANYWHERE: + return fold_case.IsIn(s); + + case Position::PREFIX: + return fold_case.StartsWith(s); + } + + return fold_case == s; } else { - return substring - ? StringFind(s, value.c_str()) != nullptr - : (starts_with - ? StringIsEqual(s, value.c_str(), value.length()) - : value == s); + switch (position) { + case Position::FULL: + break; + + case Position::ANYWHERE: + return StringFind(s, value.c_str()) != nullptr; + + case Position::PREFIX: + return StringIsEqual(s, value.c_str(), value.length()); + } + + return value == s; } } diff --git a/src/song/StringFilter.hxx b/src/song/StringFilter.hxx index 652176af8..76fb8314a 100644 --- a/src/song/StringFilter.hxx +++ b/src/song/StringFilter.hxx @@ -27,10 +27,24 @@ #include "lib/pcre/UniqueRegex.hxx" #endif +#include #include #include class StringFilter { +public: + enum class Position : uint_least8_t { + /** compare the whole haystack */ + FULL, + + /** find the phrase anywhere in the haystack */ + ANYWHERE, + + /** check if the haystack starts with the given prefix */ + PREFIX, + }; + +private: std::string value; /** @@ -42,26 +56,19 @@ class StringFilter { std::shared_ptr regex; #endif - /** - * Search for substrings instead of matching the whole string? - */ - bool substring; - - /** - * Search for substrings instead of matching the whole string? - */ - bool starts_with; + Position position; bool negated; public: template - StringFilter(V &&_value, bool _fold_case, bool _substring, bool _starts_with, bool _negated) + StringFilter(V &&_value, bool _fold_case, Position _position, bool _negated) :value(std::forward(_value)), fold_case(_fold_case ? IcuCompare(value) : IcuCompare()), - substring(_substring), starts_with(_starts_with), negated(_negated) {} + position(_position), + negated(_negated) {} bool empty() const noexcept { return value.empty(); @@ -102,11 +109,16 @@ public: if (IsRegex()) return negated ? "!~" : "=~"; - if (substring) + switch (position) { + case Position::FULL: + break; + + case Position::ANYWHERE: return negated ? "!contains" : "contains"; - if (starts_with) + case Position::PREFIX: return negated ? "!starts_with" : "starts_with"; + } return negated ? "!=" : "=="; } diff --git a/test/TestStringFilter.cxx b/test/TestStringFilter.cxx index a09bf1884..bd863fca9 100644 --- a/test/TestStringFilter.cxx +++ b/test/TestStringFilter.cxx @@ -36,7 +36,7 @@ protected: TEST_F(StringFilterTest, ASCII) { - const StringFilter f{"needle", false, false, false, false}; + const StringFilter f{"needle", false, StringFilter::Position::FULL, false}; EXPECT_TRUE(f.Match("needle")); EXPECT_FALSE(f.Match("nëedle")); @@ -53,7 +53,7 @@ TEST_F(StringFilterTest, ASCII) TEST_F(StringFilterTest, Negated) { - const StringFilter f{"needle", false, false, false, true}; + const StringFilter f{"needle", false, StringFilter::Position::FULL, true}; EXPECT_FALSE(f.Match("needle")); EXPECT_TRUE(f.Match("Needle")); @@ -66,7 +66,7 @@ TEST_F(StringFilterTest, Negated) TEST_F(StringFilterTest, StartsWith) { - const StringFilter f{"needle", false, false, true, false}; + const StringFilter f{"needle", false, StringFilter::Position::PREFIX, false}; EXPECT_TRUE(f.Match("needle")); EXPECT_FALSE(f.Match("Needle")); @@ -80,7 +80,7 @@ TEST_F(StringFilterTest, StartsWith) TEST_F(StringFilterTest, IsIn) { - const StringFilter f{"needle", false, true, false, false}; + const StringFilter f{"needle", false, StringFilter::Position::ANYWHERE, false}; EXPECT_TRUE(f.Match("needle")); EXPECT_FALSE(f.Match("Needle")); @@ -94,7 +94,7 @@ TEST_F(StringFilterTest, IsIn) TEST_F(StringFilterTest, Latin) { - const StringFilter f{"nëedlé", false, false, false, false}; + const StringFilter f{"nëedlé", false, StringFilter::Position::FULL, false}; EXPECT_TRUE(f.Match("nëedlé")); #if defined(HAVE_ICU) || defined(_WIN32) @@ -117,7 +117,7 @@ TEST_F(StringFilterTest, Latin) TEST_F(StringFilterTest, Normalize) { - const StringFilter f{"1①H", true, false, false, false}; + const StringFilter f{"1①H", true, StringFilter::Position::FULL, false}; EXPECT_TRUE(f.Match("1①H")); EXPECT_TRUE(f.Match("¹₁H")); @@ -127,17 +127,17 @@ TEST_F(StringFilterTest, Normalize) #ifndef _WIN32 // fails with Windows CompareStringEx() - EXPECT_TRUE(StringFilter("dž", true, false, false, false).Match("dž")); + EXPECT_TRUE(StringFilter("dž", true, StringFilter::Position::FULL, false).Match("dž")); #endif - EXPECT_TRUE(StringFilter("\u212b", true, false, false, false).Match("\u0041\u030a")); - EXPECT_TRUE(StringFilter("\u212b", true, false, false, false).Match("\u00c5")); + EXPECT_TRUE(StringFilter("\u212b", true, StringFilter::Position::FULL, false).Match("\u0041\u030a")); + EXPECT_TRUE(StringFilter("\u212b", true, StringFilter::Position::FULL, false).Match("\u00c5")); - EXPECT_TRUE(StringFilter("\u1e69", true, false, false, false).Match("\u0073\u0323\u0307")); + EXPECT_TRUE(StringFilter("\u1e69", true, StringFilter::Position::FULL, false).Match("\u0073\u0323\u0307")); #ifndef _WIN32 // fails with Windows CompareStringEx() - EXPECT_TRUE(StringFilter("\u1e69", true, false, false, false).Match("\u0073\u0307\u0323")); + EXPECT_TRUE(StringFilter("\u1e69", true, StringFilter::Position::FULL, false).Match("\u0073\u0307\u0323")); #endif } @@ -147,7 +147,7 @@ TEST_F(StringFilterTest, Normalize) TEST_F(StringFilterTest, Transliterate) { - const StringFilter f{"'", true, false, false, false}; + const StringFilter f{"'", true, StringFilter::Position::FULL, false}; EXPECT_TRUE(f.Match("’")); EXPECT_FALSE(f.Match("\"")); @@ -157,7 +157,7 @@ TEST_F(StringFilterTest, Transliterate) TEST_F(StringFilterTest, FoldCase) { - const StringFilter f{"nëedlé", true, false, false, false}; + const StringFilter f{"nëedlé", true, StringFilter::Position::FULL, false}; EXPECT_TRUE(f.Match("nëedlé")); #if defined(HAVE_ICU) || defined(_WIN32) diff --git a/test/TestTagSongFilter.cxx b/test/TestTagSongFilter.cxx index d48251121..be407044c 100644 --- a/test/TestTagSongFilter.cxx +++ b/test/TestTagSongFilter.cxx @@ -44,8 +44,10 @@ InvokeFilter(const TagSongFilter &f, const Tag &tag) noexcept TEST_F(TagSongFilterTest, Basic) { - const TagSongFilter f(TAG_TITLE, - StringFilter("needle", false, false, false, false)); + const TagSongFilter f{ + TAG_TITLE, + {"needle", false, StringFilter::Position::FULL, false}, + }; EXPECT_TRUE(InvokeFilter(f, MakeTag(TAG_TITLE, "needle"))); EXPECT_TRUE(InvokeFilter(f, MakeTag(TAG_TITLE, "foo", TAG_TITLE, "needle"))); @@ -65,8 +67,10 @@ TEST_F(TagSongFilterTest, Basic) */ TEST_F(TagSongFilterTest, Empty) { - const TagSongFilter f(TAG_TITLE, - StringFilter("", false, false, false, false)); + const TagSongFilter f{ + TAG_TITLE, + {"", false, StringFilter::Position::FULL, false}, + }; EXPECT_TRUE(InvokeFilter(f, MakeTag())); @@ -76,8 +80,10 @@ TEST_F(TagSongFilterTest, Empty) TEST_F(TagSongFilterTest, Substring) { - const TagSongFilter f(TAG_TITLE, - StringFilter("needle", false, true, false, false)); + const TagSongFilter f{ + TAG_TITLE, + {"needle", false, StringFilter::Position::ANYWHERE, false}, + }; EXPECT_TRUE(InvokeFilter(f, MakeTag(TAG_TITLE, "needle"))); EXPECT_TRUE(InvokeFilter(f, MakeTag(TAG_TITLE, "needleBAR"))); @@ -90,8 +96,10 @@ TEST_F(TagSongFilterTest, Substring) TEST_F(TagSongFilterTest, Startswith) { - const TagSongFilter f(TAG_TITLE, - StringFilter("needle", false, false, true, false)); + const TagSongFilter f{ + TAG_TITLE, + {"needle", false, StringFilter::Position::PREFIX, false}, + }; EXPECT_TRUE(InvokeFilter(f, MakeTag(TAG_TITLE, "needle"))); EXPECT_TRUE(InvokeFilter(f, MakeTag(TAG_TITLE, "needleBAR"))); @@ -104,8 +112,10 @@ TEST_F(TagSongFilterTest, Startswith) TEST_F(TagSongFilterTest, Negated) { - const TagSongFilter f(TAG_TITLE, - StringFilter("needle", false, false, false, true)); + const TagSongFilter f{ + TAG_TITLE, + {"needle", false, StringFilter::Position::FULL, true}, + }; EXPECT_TRUE(InvokeFilter(f, MakeTag())); EXPECT_FALSE(InvokeFilter(f, MakeTag(TAG_TITLE, "needle"))); @@ -117,8 +127,10 @@ TEST_F(TagSongFilterTest, Negated) */ TEST_F(TagSongFilterTest, EmptyNegated) { - const TagSongFilter f(TAG_TITLE, - StringFilter("", false, false, false, true)); + const TagSongFilter f{ + TAG_TITLE, + {"", false, StringFilter::Position::FULL, true}, + }; EXPECT_FALSE(InvokeFilter(f, MakeTag())); EXPECT_TRUE(InvokeFilter(f, MakeTag(TAG_TITLE, "foo"))); @@ -129,8 +141,10 @@ TEST_F(TagSongFilterTest, EmptyNegated) */ TEST_F(TagSongFilterTest, MultiNegated) { - const TagSongFilter f(TAG_TITLE, - StringFilter("needle", false, false, false, true)); + const TagSongFilter f{ + TAG_TITLE, + {"needle", false, StringFilter::Position::FULL, true}, + }; EXPECT_TRUE(InvokeFilter(f, MakeTag(TAG_TITLE, "foo", TAG_TITLE, "bar"))); EXPECT_FALSE(InvokeFilter(f, MakeTag(TAG_TITLE, "needle", TAG_TITLE, "bar"))); @@ -143,8 +157,10 @@ TEST_F(TagSongFilterTest, MultiNegated) */ TEST_F(TagSongFilterTest, Fallback) { - const TagSongFilter f(TAG_ALBUM_ARTIST, - StringFilter("needle", false, false, false, false)); + const TagSongFilter f{ + TAG_ALBUM_ARTIST, + {"needle", false, StringFilter::Position::FULL, false}, + }; EXPECT_TRUE(InvokeFilter(f, MakeTag(TAG_ALBUM_ARTIST, "needle"))); EXPECT_TRUE(InvokeFilter(f, MakeTag(TAG_ARTIST, "needle"))); @@ -163,8 +179,10 @@ TEST_F(TagSongFilterTest, Fallback) */ TEST_F(TagSongFilterTest, EmptyFallback) { - const TagSongFilter f(TAG_ALBUM_ARTIST, - StringFilter("", false, false, false, false)); + const TagSongFilter f{ + TAG_ALBUM_ARTIST, + {"", false, StringFilter::Position::FULL, false}, + }; EXPECT_TRUE(InvokeFilter(f, MakeTag())); @@ -177,8 +195,10 @@ TEST_F(TagSongFilterTest, EmptyFallback) */ TEST_F(TagSongFilterTest, NegatedFallback) { - const TagSongFilter f(TAG_ALBUM_ARTIST, - StringFilter("needle", false, false, false, true)); + const TagSongFilter f{ + TAG_ALBUM_ARTIST, + {"needle", false, StringFilter::Position::FULL, true}, + }; EXPECT_TRUE(InvokeFilter(f, MakeTag())); EXPECT_TRUE(InvokeFilter(f, MakeTag(TAG_ALBUM_ARTIST, "foo")));