song/StringFilter: add enum Position

Replaces two conflicting bools.

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<TagSongFilter>(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<UniqueRegex>(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<UriSongFilter>(StringFilter(value,
+		and_filter.AddItem(std::make_unique<UriSongFilter>(StringFilter{
-										fold_case,
+					value,
-										fold_case,
+					fold_case,
-										false,
+					fold_case
-										false)));
+					? 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<TagSongFilter>(TagType(tag),
+		and_filter.AddItem(std::make_unique<TagSongFilter>(TagType(tag), StringFilter{
-								   StringFilter(value,
+					value,
-										fold_case,
+					fold_case,
-										fold_case,
+					fold_case
-										false,
+					? StringFilter::Position::ANYWHERE
-										false)));
+					: StringFilter::Position::FULL,
+					false,
+				}));
 		break;
 	}
 }

src/song/StringFilter.cxx

@@ -33,17 +33,31 @@ StringFilter::MatchWithoutNegation(const char *s) const noexcept
 #endif
 
 	if (fold_case) {
-		return substring
+		switch (position) {
-			? fold_case.IsIn(s)
+		case Position::FULL:
-			: (starts_with
+			break;
-				? fold_case.StartsWith(s)
+
-				: fold_case == s);
+		case Position::ANYWHERE:
+			return fold_case.IsIn(s);
+
+		case Position::PREFIX:
+			return fold_case.StartsWith(s);
+		}
+
+		return fold_case == s;
 	} else {
-		return substring
+		switch (position) {
-			? StringFind(s, value.c_str()) != nullptr
+		case Position::FULL:
-			: (starts_with
+			break;
-				? StringIsEqual(s, value.c_str(), value.length())
+
-				: value == s);
+		case Position::ANYWHERE:
+			return StringFind(s, value.c_str()) != nullptr;
+
+		case Position::PREFIX:
+			return StringIsEqual(s, value.c_str(), value.length());
+		}
+
+		return value == s;
 	}
 }
 

src/song/StringFilter.hxx

@@ -27,10 +27,24 @@
 #include "lib/pcre/UniqueRegex.hxx"
 #endif
 
+#include <cstdint>
 #include <string>
 #include <memory>
 
 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<UniqueRegex> regex;
 #endif
 
-	/**
+	Position position;
-	 * Search for substrings instead of matching the whole string?
-	 */
-	bool substring;
-
-	/**
-	 * Search for substrings instead of matching the whole string?
-	 */
-	bool starts_with;
 
 	bool negated;
 
 public:
 	template<typename V>
-	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<V>(_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 ? "!=" : "==";
 	}

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, StringFilter::Position::FULL, false).Match("\u0041\u030a"));
-	EXPECT_TRUE(StringFilter("\u212b", true, false, false, false).Match("\u00c5"));
+	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)

test/TestTagSongFilter.cxx

@@ -44,8 +44,10 @@ InvokeFilter(const TagSongFilter &f, const Tag &tag) noexcept
 
 TEST_F(TagSongFilterTest, Basic)
 {
-	const TagSongFilter f(TAG_TITLE,
+	const TagSongFilter f{
-			      StringFilter("needle", false, false, false, false));
+		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,
+	const TagSongFilter f{
-			      StringFilter("", false, false, false, false));
+		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,
+	const TagSongFilter f{
-			      StringFilter("needle", false, true, false, false));
+		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,
+	const TagSongFilter f{
-			      StringFilter("needle", false, false, true, false));
+		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,
+	const TagSongFilter f{
-			      StringFilter("needle", false, false, false, true));
+		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,
+	const TagSongFilter f{
-			      StringFilter("", false, false, false, true));
+		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,
+	const TagSongFilter f{
-			      StringFilter("needle", false, false, false, true));
+		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,
+	const TagSongFilter f{
-			      StringFilter("needle", false, false, false, false));
+		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,
+	const TagSongFilter f{
-			      StringFilter("", false, false, false, false));
+		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,
+	const TagSongFilter f{
-			      StringFilter("needle", false, false, false, true));
+		TAG_ALBUM_ARTIST,
+		{"needle", false, StringFilter::Position::FULL, true},
+	};
 
 	EXPECT_TRUE(InvokeFilter(f, MakeTag()));
 	EXPECT_TRUE(InvokeFilter(f, MakeTag(TAG_ALBUM_ARTIST, "foo")));