Update `StringUtilities::ExpandString` behavior

- The value is now passed as a StringView instead of a String.
- Both flavors now use the same underlying implementation.
- Whitespace is always stripped, including when using a whitespace delimiter.
- With that change, `ignore_repeated_delimiters` is no longer needed and removed.
- Double and single quote characters now act just like any other character.
  - Previously they were considered as quotes, but this behavior was not needed in any library usage.

Include/RmlUi/Core/StringUtilities.h

@@ -48,23 +48,19 @@ RMLUICORE_API String CreateString(const char* format, ...) RMLUI_ATTRIBUTE_FORMA
 RMLUICORE_API int FormatString(String& string, const char* format, ...) RMLUI_ATTRIBUTE_FORMAT_PRINTF(2, 3);
 
 namespace StringUtilities {
-	/// Expands character-delimited list of values in a single string to a whitespace-trimmed list
+	/// Expands a character-delimited string to a whitespace-trimmed list of values.
 	/// of values.
 	/// @param[out] string_list Resulting list of values.
 	/// @param[in] string String to expand.
 	/// @param[in] delimiter Delimiter found between entries in the string list.
-	/// @param[in] ignore_repeated_delimiters If true, repeated values of the delimiter will not add additional entries to the list.
-	RMLUICORE_API void ExpandString(StringList& string_list, const String& string, const char delimiter = ',',
-		bool ignore_repeated_delimiters = false);
-	/// Expands character-delimited list of values with custom quote characters.
-	/// @param[out] string_list Resulting list of values.
+	RMLUICORE_API void ExpandString(StringList& string_list, StringView string, char delimiter = ',');
+	/// Expands a character-delimited string to a whitespace-trimmed list of values, with quote characters.
+	/// @param[out] string_list Resulting list of values, each entry stripped of whitespace.
 	/// @param[in] string String to expand.
 	/// @param[in] delimiter Delimiter found between entries in the string list.
-	/// @param[in] quote_character Begin quote
-	/// @param[in] unquote_character End quote
-	/// @param[in] ignore_repeated_delimiters If true, repeated values of the delimiter will not add additional entries to the list.
-	RMLUICORE_API void ExpandString(StringList& string_list, const String& string, const char delimiter, char quote_character, char unquote_character,
-		bool ignore_repeated_delimiters = false);
+	/// @param[in] quote_character Begin quote.
+	/// @param[in] unquote_character End quote.
+	RMLUICORE_API void ExpandString(StringList& string_list, StringView string, char delimiter, char quote_character, char unquote_character);
 	/// Joins a list of string values into a single string separated by a character delimiter.
 	/// @param[out] string Resulting concatenated string.
 	/// @param[in] string_list Input list of string values.

Source/Core/PropertyParserColorStopList.cpp

@@ -62,7 +62,7 @@ bool PropertyParserColorStopList::ParseValue(Property& property, const String& v
 	for (const String& color_stop_str : color_stop_str_list)
 	{
 		StringList values;
-		StringUtilities::ExpandString(values, color_stop_str, ' ', '(', ')', true);
+		StringUtilities::ExpandString(values, color_stop_str, ' ', '(', ')');
 
 		if (values.empty() || values.size() > 3)
 			return false;

Source/Core/PropertyParserColour.cpp

@@ -386,7 +386,8 @@ bool PropertyParserColour::ParseCIELABColour(Colourb& colour, const String& valu
 	// Parse lightness and alpha (same for both lab and lch).
 	for (int i : {0, 3})
 	{
-		// Value can either be 'none' (representing 0.0), a percentage between 0% and 100%, or a number (between 0.0 and 100.0 for lightness and between 0.0 and 1.0 for alpha).
+		// Value can either be 'none' (representing 0.0), a percentage between 0% and 100%, or a number (between 0.0 and 100.0 for lightness and
+		// between 0.0 and 1.0 for alpha).
 		if (values[i] == "none")
 			lab_values[i] = 0.0f;
 		else if (values[i][values[i].size() - 1] == '%')
@@ -567,8 +568,7 @@ bool PropertyParserColour::GetColourFunctionValues(StringList& values, const Str
 
 	size_t begin_values = find + 1;
 
-	StringUtilities::ExpandString(values, value.substr(begin_values, value.rfind(')') - begin_values), is_comma_separated ? ',' : ' ',
-		!is_comma_separated);
+	StringUtilities::ExpandString(values, value.substr(begin_values, value.rfind(')') - begin_values), is_comma_separated ? ',' : ' ');
 
 	return true;
 }

Source/Core/PropertyParserFilter.cpp

@@ -57,7 +57,7 @@ bool PropertyParserFilter::ParseValue(Property& property, const String& filter_s
 
 	// Make sure we don't split inside the parenthesis since they may appear in filter shorthands.
 	StringList filter_string_list;
-	StringUtilities::ExpandString(filter_string_list, filter_string_value, ' ', '(', ')', true);
+	StringUtilities::ExpandString(filter_string_list, filter_string_value, ' ', '(', ')');
 
 	FilterDeclarationList filters;
 	filters.value = filter_string_value;

Source/Core/StringUtilities.cpp

@@ -255,106 +255,55 @@ String StringUtilities::Replace(String subject, char search, char replace)
 	return subject;
 }
 
-void StringUtilities::ExpandString(StringList& string_list, const String& string, const char delimiter, bool ignore_repeated_delimiters)
+void StringUtilities::ExpandString(StringList& string_list, StringView string, char delimiter)
 {
-	char quote = 0;
-	bool last_char_delimiter = true;
-	const char* ptr = string.c_str();
-	const char* start_ptr = nullptr;
-	const char* end_ptr = ptr;
-
-	size_t num_delimiter_values = std::count(string.begin(), string.end(), delimiter);
-	if (num_delimiter_values == 0)
-	{
-		string_list.push_back(StripWhitespace(string));
-		return;
-	}
-	string_list.reserve(string_list.size() + num_delimiter_values + 1);
-
-	while (*ptr)
-	{
-		// Switch into quote mode if the last char was a delimeter ( excluding whitespace )
-		// and we're not already in quote mode
-		if (last_char_delimiter && !quote && (*ptr == '"' || *ptr == '\''))
-		{
-			quote = *ptr;
-		}
-		// Switch out of quote mode if we encounter a quote that hasn't been escaped
-		else if (*ptr == quote && *(ptr - 1) != '\\')
-		{
-			quote = 0;
-		}
-		// If we encounter a delimiter while not in quote mode, add the item to the list
-		else if (*ptr == delimiter && !quote)
-		{
-			if (start_ptr)
-				string_list.emplace_back(start_ptr, end_ptr + 1);
-			else if (!ignore_repeated_delimiters)
-				string_list.emplace_back();
-			last_char_delimiter = true;
-			start_ptr = nullptr;
-		}
-		// Otherwise if its not white space or we're in quote mode, advance the pointers
-		else if (!IsWhitespace(*ptr) || quote)
-		{
-			if (!start_ptr)
-				start_ptr = ptr;
-			end_ptr = ptr;
-			last_char_delimiter = false;
-		}
-
-		ptr++;
-	}
-
-	// If there's data pending, add it.
-	if (start_ptr)
-		string_list.emplace_back(start_ptr, end_ptr + 1);
+	ExpandString(string_list, string, delimiter, '\0', '\0');
 }
 
-void StringUtilities::ExpandString(StringList& string_list, const String& string, const char delimiter, char quote_character, char unquote_character,
-	bool ignore_repeated_delimiters)
+void StringUtilities::ExpandString(StringList& string_list, StringView string, char delimiter, char quote_character, char unquote_character)
 {
+	// Quote depth is incremented for each encountered quote character, and decremented on unquote character. If quote
+	// and unquote are the same characters, then quote mode is instead toggled.
 	int quote_mode_depth = 0;
-	const char* ptr = string.c_str();
 	const char* start_ptr = nullptr;
-	const char* end_ptr = ptr;
+	const char* last_ptr = nullptr;
+
+	string_list.reserve(string_list.size() + std::count(string.begin(), string.end(), delimiter) + 1);
 
-	while (*ptr)
+	for (const char& c : string)
 	{
-		// Increment the quote depth for each quote character encountered
-		if (*ptr == quote_character)
+		if (c == quote_character)
 		{
-			++quote_mode_depth;
+			if (quote_character == unquote_character)
+				quote_mode_depth = (quote_mode_depth == 0 ? 1 : 0);
+			else
+				++quote_mode_depth;
 		}
-		// And decrement it for every unquote character
-		else if (*ptr == unquote_character)
+		else if (c == unquote_character)
 		{
 			--quote_mode_depth;
 		}
 
-		// If we encounter a delimiter while not in quote mode, add the item to the list
-		if (*ptr == delimiter && quote_mode_depth == 0)
+		if (c == delimiter && quote_mode_depth == 0)
 		{
 			if (start_ptr)
-				string_list.emplace_back(start_ptr, end_ptr + 1);
-			else if (!ignore_repeated_delimiters)
+				string_list.emplace_back(start_ptr, last_ptr + 1);
+			else if (!IsWhitespace(delimiter))
 				string_list.emplace_back();
 			start_ptr = nullptr;
 		}
-		// Otherwise if its not white space or we're in quote mode, advance the pointers
-		else if (!IsWhitespace(*ptr) || quote_mode_depth > 0)
+		else if (!IsWhitespace(c) || quote_mode_depth > 0)
 		{
 			if (!start_ptr)
-				start_ptr = ptr;
-			end_ptr = ptr;
+				start_ptr = &c;
+			last_ptr = &c;
 		}
-
-		ptr++;
 	}
 
-	// If there's data pending, add it.
 	if (start_ptr)
-		string_list.emplace_back(start_ptr, end_ptr + 1);
+		string_list.emplace_back(start_ptr, last_ptr + 1);
+	else if (!IsWhitespace(delimiter) || std::all_of(string.begin(), string.end(), IsWhitespace))
+		string_list.emplace_back();
 }
 
 void StringUtilities::JoinString(String& string, const StringList& string_list, const char delimiter)

Source/Core/URL.cpp

@@ -184,7 +184,7 @@ bool URL::SetURL(const String& _url)
 
 		// Loop through all parameters, loading them
 		StringList parameter_list;
-		StringUtilities::ExpandString(parameter_list, parameters + 1, '&');
+		StringUtilities::ExpandString(parameter_list, StringView(parameters + 1, _url.c_str() + _url.size()), '&');
 		for (size_t i = 0; i < parameter_list.size(); i++)
 		{
 			// Split into key and value

Tests/Source/UnitTests/StringUtilities.cpp

@@ -64,25 +64,27 @@ TEST_CASE("StringUtilities::TrimTrailingDotZeros")
 
 TEST_CASE("StringUtilities::ExpandString")
 {
-	auto ExpandStringShort = [](const String string, char delimiter, bool ignore_repeated_delimiters = false) {
+	auto ExpandStringShort = [](const String string, char delimiter) {
 		StringList result;
-		StringUtilities::ExpandString(result, string, delimiter, ignore_repeated_delimiters);
+		StringUtilities::ExpandString(result, string, delimiter);
 		return StringListWrapper{result};
 	};
-	auto ExpandStringLong = [](const String string, char delimiter, char quote, char unquote, bool ignore_repeated_delimiters) {
+	auto ExpandStringLong = [](const String string, char delimiter, char quote, char unquote) {
 		StringList result;
-		StringUtilities::ExpandString(result, string, delimiter, quote, unquote, ignore_repeated_delimiters);
+		StringUtilities::ExpandString(result, string, delimiter, quote, unquote);
 		return StringListWrapper{result};
 	};
-	auto ExpandString = [&](const String string, char delimiter, bool ignore_repeated_delimiters = false) {
-		const StringListWrapper short_result = ExpandStringShort(string, delimiter, ignore_repeated_delimiters);
-		const StringListWrapper long_result = ExpandStringLong(string, delimiter, '(', ')', ignore_repeated_delimiters);
+	auto ExpandString = [&](const String string, char delimiter) {
+		const StringListWrapper short_result = ExpandStringShort(string, delimiter);
+		const StringListWrapper long_result = ExpandStringLong(string, delimiter, '(', ')');
 		CAPTURE(string);
 		CAPTURE(String{delimiter});
 		CHECK(short_result == long_result);
 		return short_result;
 	};
 
+	CHECK(ExpandString("a", ',') == StringListWrapper{{"a"}});
+	CHECK(ExpandString("a", ' ') == StringListWrapper{{"a"}});
 	CHECK(ExpandString("a,b,c", ',') == StringListWrapper{{"a", "b", "c"}});
 	CHECK(ExpandString("a,b,c", ' ') == StringListWrapper{{"a,b,c"}});
 	CHECK(ExpandString("a b c", ' ') == StringListWrapper{{"a", "b", "c"}});
@@ -90,29 +92,34 @@ TEST_CASE("StringUtilities::ExpandString")
 	CHECK(ExpandString("a,b,,c", ',') == StringListWrapper{{"a", "b", "", "c"}});
 	CHECK(ExpandString(" a ,  b  , c ", ',') == StringListWrapper{{"a", "b", "c"}});
 
-	CHECK(ExpandString("a", ',') == StringListWrapper{{"a"}});
-	CHECK(ExpandStringShort("", ',') == StringListWrapper{{""}});
-	CHECK(ExpandStringShort("", ' ') == StringListWrapper{{""}});
-	CHECK(ExpandStringShort(" ", ' ') == StringListWrapper{{""}});
+	CHECK(ExpandString(" a ", ' ') == StringListWrapper{{"a"}});
+	CHECK(ExpandString(" a ", ',') == StringListWrapper{{"a"}});
+	CHECK(ExpandString(" a", ' ') == StringListWrapper{{"a"}});
+	CHECK(ExpandString(" a", ',') == StringListWrapper{{"a"}});
+	CHECK(ExpandString("a ", ' ') == StringListWrapper{{"a"}});
+	CHECK(ExpandString("a ", ',') == StringListWrapper{{"a"}});
+
+	CHECK(ExpandString("", ',') == StringListWrapper{{""}});
+	CHECK(ExpandString("", ' ') == StringListWrapper{{""}});
+	CHECK(ExpandString(" ", ' ') == StringListWrapper{{""}});
+
 	CHECK(ExpandString(",a,b", ',') == StringListWrapper{{"", "a", "b"}});
+	CHECK(ExpandString("a,b,", ',') == StringListWrapper{{"a", "b", ""}});
+	CHECK(ExpandString("  ,  ", ',') == StringListWrapper{{"", ""}});
+	CHECK(ExpandString(" a  b  c ", ' ') == StringListWrapper{{"a", "b", "c"}});
 	CHECK(ExpandString("a,,b", ',') == StringListWrapper{{"a", "", "b"}});
 	CHECK(ExpandString("a;b;c", ';') == StringListWrapper{{"a", "b", "c"}});
 
-	CHECK(ExpandStringLong("(a,b),c", ',', '(', ')', false) == StringListWrapper{{"(a,b)", "c"}});
-	CHECK(ExpandStringLong("((a,b),c),d", ',', '(', ')', false) == StringListWrapper{{"((a,b),c)", "d"}});
-	CHECK(ExpandStringLong("a,,b", ',', '(', ')', false) == StringListWrapper{{"a", "", "b"}});
-	CHECK(ExpandStringLong("a,,b", ',', '(', ')', true) == StringListWrapper{{"a", "b"}});
-	CHECK(ExpandStringLong("a  b", ' ', '(', ')', false) == StringListWrapper{{"a", "", "b"}});
-	CHECK(ExpandStringLong("a  b", ' ', '(', ')', true) == StringListWrapper{{"a", "b"}});
-
-	// Questionable behavior, consider changing implementation to match the suggestion.
-	CHECK(ExpandString("a,b,", ',') == StringListWrapper{{"a", "b"}});                       // { "a", "b", ""}
-	CHECK(ExpandString("  ,  ", ',') == StringListWrapper{{""}});                            // {"", ""}}
-	CHECK(ExpandString(" a  b  c ", ' ') == StringListWrapper{{"", "a", "", "b", "", "c"}}); // {"a", "b", "c"}}
-	CHECK(ExpandStringShort("'a,b',c", ',') == StringListWrapper{{"a,b", "c"}});             // {"'a,b'", "c"}
-	CHECK(ExpandStringShort(R"("a,b",c)", ',') == StringListWrapper{{R"(a,b)", "c"}});       // {R"("a,b")", "c"}
-	CHECK(ExpandStringShort("\"a\\\",b\",c", ',') == StringListWrapper{{R"(a\",b)", "c"}});  // {R"("a\",b")", "c"} (MSVC fails using raw string here)
-	CHECK(ExpandStringLong("\"a,b,c\",d", ',', '"', '"', false) == StringListWrapper{{"\"a,b,c\",d"}}); // {"\"a,b,c\"", "d"}}
+	CHECK(ExpandStringLong("(a,b),c", ',', '(', ')') == StringListWrapper{{"(a,b)", "c"}});
+	CHECK(ExpandStringLong("((a,b),c),d", ',', '(', ')') == StringListWrapper{{"((a,b),c)", "d"}});
+	CHECK(ExpandStringLong("a(b,c),d(e,f)", ',', '(', ')') == StringListWrapper{{"a(b,c)", "d(e,f)"}});
+	CHECK(ExpandStringLong("a,,b", ',', '(', ')') == StringListWrapper{{"a", "", "b"}});
+	CHECK(ExpandStringLong("a  b", ' ', '(', ')') == StringListWrapper{{"a", "b"}});
+	CHECK(ExpandStringLong("'a,b',c", ',', '\'', '\'') == StringListWrapper{{"'a,b'", "c"}});
+	CHECK(ExpandStringLong(R"("a,b",c)", ',', '"', '"') == StringListWrapper{{R"("a,b")", "c"}});
+	CHECK(ExpandStringLong("a,'b, c'", ',', '\'', '\'') == StringListWrapper{{"a", "'b, c'"}});
+	CHECK(ExpandStringLong(" a,'b, c' ", ',', '\'', '\'') == StringListWrapper{{"a", "'b, c'"}});
+	CHECK(ExpandStringLong(" a 'b, c' ", ' ', '\'', '\'') == StringListWrapper{{"a", "'b, c'"}});
 }
 
 TEST_CASE("StringUtilities::StartsWith")

Tests/Source/VisualTests/TestConfig.cpp

@@ -32,6 +32,7 @@
 #include <PlatformExtensions.h>
 #include <Shell.h>
 #include <cstdlib>
+#include <cstring>
 
 Rml::String GetCompareInputDirectory()
 {
@@ -64,7 +65,7 @@ Rml::StringList GetTestInputDirectories()
 	Rml::StringList directories = {samples_root + "../Tests/Data/VisualTests"};
 
 	if (const char* env_variable = std::getenv("RMLUI_VISUAL_TESTS_RML_DIRECTORIES"))
-		Rml::StringUtilities::ExpandString(directories, env_variable, ',');
+		Rml::StringUtilities::ExpandString(directories, Rml::StringView(env_variable, env_variable + std::strlen(env_variable)), ',');
 
 	return directories;
 }