Browse Source

Data expressions: Allow transform functions to also be called using C-style function call syntax (breaking change)

The signature of transform functions has been changed from
    Variant& first_argument_and_result, const VariantList& other_arguments -> bool success
to
    const VariantList& arguments -> Variant result
Michael Ragazzon 3 years ago
parent
commit
c53744efb3

+ 1 - 1
Include/RmlUi/Core/DataTypeRegister.h

@@ -48,7 +48,7 @@ struct is_builtin_data_scalar {
 class RMLUICORE_API TransformFuncRegister {
 public:
 	void Register(const String& name, DataTransformFunc transform_func);
-	bool Call(const String& name, Variant& inout_result, const VariantList& arguments) const;
+	bool Call(const String& name, const VariantList& arguments, Variant& out_result) const;
 
 private:
 	UnorderedMap<String, DataTransformFunc> transform_functions;

+ 1 - 1
Include/RmlUi/Core/DataTypes.h

@@ -43,7 +43,7 @@ class DataVariable;
 
 using DataGetFunc = Function<void(Variant&)>;
 using DataSetFunc = Function<void(const Variant&)>;
-using DataTransformFunc = Function<bool(Variant&, const VariantList&)>;
+using DataTransformFunc = Function<Variant(const VariantList&)>;
 using DataEventFunc = Function<void(DataModelHandle, Event&, const VariantList&)>;
 
 template<typename T> using MemberGetFunc = void(T::*)(Variant&);

+ 5 - 4
Samples/basic/databinding/src/main.cpp

@@ -242,12 +242,13 @@ namespace InvadersExample {
 			}
 		);
 		// Register a transform function for formatting time
-		constructor.RegisterTransformFunc("format_time", [](Rml::Variant& variant, const Rml::VariantList& /*arguments*/) -> bool {
-			const double t = variant.Get<double>();
+		constructor.RegisterTransformFunc("format_time", [](const Rml::VariantList& arguments) -> Rml::Variant {
+			if (arguments.empty())
+				return {};
+			const double t = arguments[0].Get<double>();
 			const int minutes = int(t) / 60;
 			const double seconds = t - 60.0 * double(minutes);
-			variant = Rml::CreateString(10, "%02d:%05.2f", minutes, seconds);
-			return true;
+			return Rml::Variant(Rml::CreateString(10, "%02d:%05.2f", minutes, seconds));
 		});
 
 		// Structs are registered by adding all their members through the returned handle.

+ 116 - 114
Source/Core/DataExpression.cpp

@@ -52,9 +52,8 @@ class DataParser;
 		L  Typically left-hand side arguments.
 		C  Typically center arguments (eg. in ternary operator).
 
-	And two stacks:
-		S  The main program stack.
-		A  The arguments stack, only used to pass arguments to an external transform function.
+	And a stack:
+		S  The program stack.
 
 	In addition, each instruction has an optional payload:
 		D  Instruction data (payload).
@@ -83,9 +82,9 @@ enum class Instruction {
 	Equal        = '=',     //       R = L == R
 	NotEqual     = 'N',     //       R = L != R
 	Ternary      = '?',     //       R = L ? C : R
-	Arguments    = 'a',     //      A+ = S-  (Repeated D times, where D gives the num. arguments)
-	TransformFnc = 'T',     //       R = DataModel.Execute( D, R, A ); A.Clear();  (D determines function name, R the input value, A the arguments)
-	EventFnc     = 'E',     //       DataModel.EventCallback(D, A); A.Clear();
+	NumArguments = '#',     //       R = D  (Contains the num. arguments currently on the stack, immediately followed by a 'T' or 'E' instruction)
+	TransformFnc = 'T',     //       R = DataModel.Execute(D, A) where A = S[TOP - R, TOP]; S -= R;  (D determines function name, input R the num. arguments, A the arguments)
+	EventFnc     = 'E',     //       DataModel.EventCallback(D, A); S -= R;
 	Assign       = 'A',     //       DataModel.SetVariable(D, R)
 };
 
@@ -201,37 +200,43 @@ public:
 
 	void Emit(Instruction instruction, Variant data = Variant())
 	{
-		RMLUI_ASSERTMSG(instruction != Instruction::Push && instruction != Instruction::Pop &&
-			instruction != Instruction::Arguments && instruction != Instruction::Variable && instruction != Instruction::Assign,
-			"Use the Push(), Pop(), Arguments(), Variable(), and Assign() procedures for stack manipulation and variable instructions.");
-		program.push_back(InstructionData{ instruction, std::move(data) });
+		RMLUI_ASSERTMSG(instruction != Instruction::Push && instruction != Instruction::Pop && instruction != Instruction::NumArguments &&
+				instruction != Instruction::TransformFnc && instruction != Instruction::EventFnc && instruction != Instruction::Variable &&
+				instruction != Instruction::Assign,
+			"Use Push(), Pop(), Function(), Variable(), and Assign() procedures for stack manipulation and variable instructions.");
+		program.push_back(InstructionData{instruction, std::move(data)});
 	}
-	void Push() {
+	void Push()
+	{
 		program_stack_size += 1;
-		program.push_back(InstructionData{ Instruction::Push, Variant() });
+		program.push_back(InstructionData{Instruction::Push, Variant()});
 	}
-	void Pop(Register destination) {
-		if (program_stack_size <= 0) {
+	void Pop(Register destination)
+	{
+		if (program_stack_size <= 0)
+		{
 			Error("Internal parser error: Tried to pop an empty stack.");
 			return;
 		}
 		program_stack_size -= 1;
-		program.push_back(InstructionData{ Instruction::Pop, Variant(int(destination)) });
+		program.push_back(InstructionData{Instruction::Pop, Variant(int(destination))});
 	}
-	void Arguments(int num_arguments) {
-		if (program_stack_size < num_arguments) {
-			Error(CreateString(128, "Internal parser error: Popping %d arguments, but the stack contains only %d elements.", num_arguments, program_stack_size));
+	void Function(Instruction instruction, int num_arguments, String&& name)
+	{
+		RMLUI_ASSERT(instruction == Instruction::TransformFnc || instruction == Instruction::EventFnc);
+		RMLUI_ASSERT(num_arguments >= 0);
+		if (program_stack_size < num_arguments)
+		{
+			Error(CreateString(128, "Internal parser error: Popping %d arguments, but the stack contains only %d elements.", num_arguments,
+				program_stack_size));
 			return;
 		}
 		program_stack_size -= num_arguments;
-		program.push_back(InstructionData{ Instruction::Arguments, Variant(int(num_arguments)) });
-	}
-	void Variable(const String& name) {
-		VariableGetSet(name, false);
-	}
-	void Assign(const String& name) {
-		VariableGetSet(name, true);
+		program.push_back(InstructionData{Instruction::NumArguments, Variant(int(num_arguments))});
+		program.push_back(InstructionData{instruction, Variant(std::move(name))});
 	}
+	void Variable(const String& name) { VariableGetSet(name, false); }
+	void Assign(const String& name) { VariableGetSet(name, true); }
 
 private:
 	void VariableGetSet(const String& name, bool is_assignment)
@@ -274,7 +279,7 @@ namespace Parse {
 
 	static void NumberLiteral(DataParser& parser);
 	static void StringLiteral(DataParser& parser);
-	static void Variable(DataParser& parser);
+	static void VariableOrFunction(DataParser& parser);
 
 	static void Add(DataParser& parser);
 	static void Subtract(DataParser& parser);
@@ -290,7 +295,7 @@ namespace Parse {
 	static void NotEqual(DataParser& parser);
 
 	static void Ternary(DataParser& parser);
-	static void Function(DataParser& parser, Instruction function_type, const String& name);
+	static void Function(DataParser& parser, Instruction function_type, String&& name, bool first_argument_piped);
 
 	// Helper functions
 	static bool IsVariableCharacter(char c, bool is_first_character)
@@ -311,10 +316,9 @@ namespace Parse {
 
 		return false;
 	}
-	static String VariableName(DataParser& parser)
+	static String VariableOrFunctionName(DataParser& parser, bool* out_valid_function_name)
 	{
 		String name;
-
 		bool is_first_character = true;
 		char c = parser.Look();
 
@@ -337,6 +341,9 @@ namespace Parse {
 		if (new_size != String::npos)
 			name.resize(new_size);
 
+		if (out_valid_function_name)
+			*out_valid_function_name = (name.find_first_of(".[] ") == String::npos);
+
 		return name;
 	}
 
@@ -348,7 +355,7 @@ namespace Parse {
 		{
 			if (parser.Look() != '\0')
 			{
-				const String variable_name = VariableName(parser);
+				String variable_name = VariableOrFunctionName(parser, nullptr);
 				if (variable_name.empty()) {
 					parser.Error("Expected a variable for assignment but got an empty name.");
 					return;
@@ -363,7 +370,7 @@ namespace Parse {
 				}
 				else if (c == '(' || c == ';' || c == '\0')
 				{
-					Function(parser, Instruction::EventFnc, variable_name);
+					Function(parser, Instruction::EventFnc, std::move(variable_name), false);
 				}
 				else
 				{
@@ -401,14 +408,22 @@ namespace Parse {
 					Or(parser);
 				else
 				{
+					parser.Push();
 					parser.SkipWhitespace();
-					const String fnc_name = VariableName(parser);
-					if (fnc_name.empty()) {
+					bool valid_function_name = true;
+					String name = VariableOrFunctionName(parser, &valid_function_name);
+					if (name.empty())
+					{
 						parser.Error("Expected a transform function name but got an empty name.");
 						return;
 					}
+					if (!valid_function_name)
+					{
+						parser.Error("Expected a transform function name but got an invalid name '" + name + "'.");
+						return;
+					}
 
-					Function(parser, Instruction::TransformFnc, fnc_name);
+					Function(parser, Instruction::TransformFnc, std::move(name), true);
 				}
 			}
 			break;
@@ -500,11 +515,11 @@ namespace Parse {
 		}
 		else if ((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z'))
 		{
-			Variable(parser);
+			VariableOrFunction(parser);
 			parser.SkipWhitespace();
 		}
 		else
-			parser.Expected("literal, variable name, parenthesis, or '!'");
+			parser.Expected("literal, variable name, function name, parenthesis, or '!'");
 	}
 
 	static void NumberLiteral(DataParser& parser)
@@ -562,20 +577,29 @@ namespace Parse {
 
 		parser.Emit(Instruction::Literal, Variant(str));
 	}
-	static void Variable(DataParser& parser)
+	static void VariableOrFunction(DataParser& parser)
 	{
-		String name = VariableName(parser);
+		bool valid_function_name = true;
+		String name = VariableOrFunctionName(parser, &valid_function_name);
 		if (name.empty()) {
-			parser.Error("Expected a variable but got an empty name.");
+			parser.Error("Expected a variable or function name but got an empty name.");
 			return;
 		}
 
-		// Keywords are parsed like variables, but are really literals.
-		// Check for them here.
+		// Keywords are parsed like variables, but are really literals. Check for them here.
 		if (name == "true")
 			parser.Emit(Instruction::Literal, Variant(true));
 		else if (name == "false")
 			parser.Emit(Instruction::Literal, Variant(false));
+		else if (parser.Look() == '(')
+		{
+			if (!valid_function_name)
+			{
+				parser.Error("Invalid function name '" + name + "'.");
+				return;
+			}
+			Function(parser, Instruction::TransformFnc, std::move(name), false);
+		}
 		else
 			parser.Variable(name);
 	}
@@ -700,14 +724,14 @@ namespace Parse {
 		parser.Pop(Register::L);
 		parser.Emit(Instruction::Ternary);
 	}
-	static void Function(DataParser& parser, Instruction function_type, const String& func_name)
+	static void Function(DataParser& parser, Instruction function_type, String&& func_name, bool first_argument_piped)
 	{
 		RMLUI_ASSERT(function_type == Instruction::TransformFnc || function_type == Instruction::EventFnc);
 
-		// We already matched the variable name (and '|' for transform functions)
+		// We already matched the variable name, and also pushed the first argument to the stack if it was piped using '|'.
+		int num_arguments = first_argument_piped ? 1 : 0;
 		if (parser.Look() == '(')
 		{
-			int num_arguments = 0;
 			bool looping = true;
 
 			parser.Match('(');
@@ -715,8 +739,6 @@ namespace Parse {
 				parser.Match(')');
 				looping = false;
 			}
-			else
-				parser.Push();
 
 			while (looping)
 			{
@@ -732,23 +754,26 @@ namespace Parse {
 					looping = false;
 				}
 			}
-
-			if (num_arguments > 0) {
-				parser.Arguments(num_arguments);
-				parser.Pop(Register::R);
-			}
 		}
 		else {
 			parser.SkipWhitespace();
 		}
 
-		parser.Emit(function_type, Variant(func_name));
+		parser.Function(function_type, num_arguments, std::move(func_name));
 	}
 
-
 } // </namespace Parse>
 
-
+static String DumpProgram(const Program& program)
+{
+	String str;
+	for (size_t i = 0; i < program.size(); i++)
+	{
+		String instruction_str = program[i].data.Get<String>();
+		str += CreateString(50 + instruction_str.size(), "  %4zu  '%c'  %s\n", i, char(program[i].instruction), instruction_str.c_str());
+	}
+	return str;
+}
 
 class DataInterpreter {
 public:
@@ -778,9 +803,9 @@ public:
 		if(success && !stack.empty())
 			Log::Message(Log::LT_WARNING, "Possible data interpreter stack corruption. Stack size is %zu at end of execution (should be zero).", stack.size());
 
-		if(!success)
+		if (!success)
 		{
-			String program_str = DumpProgram();
+			String program_str = DumpProgram(program);
 			Log::Message(Log::LT_WARNING, "Failed to execute program with %zu instructions:", program.size());
 			Log::Message(Log::LT_WARNING, "%s", program_str.c_str());
 		}
@@ -788,17 +813,6 @@ public:
 		return success;
 	}
 
-	String DumpProgram() const
-	{
-		String str;
-		for (size_t i = 0; i < program.size(); i++)
-		{
-			String instruction_str = program[i].data.Get<String>();
-			str += CreateString(50 + instruction_str.size(), "  %4zu  '%c'  %s\n", i, char(program[i].instruction), instruction_str.c_str());
-		}
-		return str;
-	}
-
 	Variant Result() const {
 		return R;
 	}
@@ -806,8 +820,7 @@ public:
 
 private:
 	Variant R, L, C;
-	Stack<Variant> stack;
-	Vector<Variant> arguments;
+	Vector<Variant> stack;
 
 	const Program& program;
 	const AddressList& addresses;
@@ -823,7 +836,7 @@ private:
 		{
 		case Instruction::Push:
 		{
-			stack.push(std::move(R));
+			stack.push_back(std::move(R));
 			R.Clear();
 		}
 		break;
@@ -834,9 +847,9 @@ private:
 
 			Register reg = Register(data.Get<int>(-1));
 			switch (reg) {
-			case Register::R:  R = stack.top(); stack.pop(); break;
-			case Register::L:  L = stack.top(); stack.pop(); break;
-			case Register::C:  C = stack.top(); stack.pop(); break;
+			case Register::R:  R = stack.back(); stack.pop_back(); break;
+			case Register::L:  L = stack.back(); stack.pop_back(); break;
+			case Register::C:  C = stack.back(); stack.pop_back(); break;
 			default:
 				return Error(CreateString(50, "Invalid register %d.", int(reg)));
 			}
@@ -896,49 +909,23 @@ private:
 				R = C;
 		}
 		break;
-		case Instruction::Arguments:
+		case Instruction::NumArguments:
 		{
-			if (!arguments.empty())
-				return Error("Argument stack is not empty.");
-
-			int num_arguments = data.Get<int>(-1);
-			if (num_arguments < 0)
-				return Error("Invalid number of arguments.");
-			if (stack.size() < size_t(num_arguments))
-				return Error(CreateString(100, "Cannot pop %d arguments, stack contains only %zu elements.", num_arguments, stack.size()));
-
-			arguments.resize(num_arguments);
-			for (int i = num_arguments - 1; i >= 0; i--)
-			{
-				arguments[i] = std::move(stack.top());
-				stack.pop();
-			}
+			const int num_arguments = data.Get<int>(-1);
+			R = num_arguments;
 		}
 		break;
 		case Instruction::TransformFnc:
-		{
-			const String function_name = data.Get<String>();
-			
-			if (!expression_interface.CallTransform(function_name, R, arguments))
-			{
-				String arguments_str;
-				for (size_t i = 0; i < arguments.size(); i++)
-				{
-					arguments_str += arguments[i].Get<String>();
-					if (i < arguments.size() - 1)
-						arguments_str += ", ";
-				}
-				Error(CreateString(50 + function_name.size() + arguments_str.size(), "Failed to execute data function: %s(%s)", function_name.c_str(), arguments_str.c_str()));
-			}
-
-			arguments.clear();
-		}
-		break;
 		case Instruction::EventFnc:
 		{
-			const String function_name = data.Get<String>();
+			Vector<Variant> arguments;
+			if (!ExtractArgumentsFromStack(arguments))
+				return false;
 
-			if (!expression_interface.EventCallback(function_name, arguments))
+			const String function_name = data.Get<String>();
+			const bool result = (instruction == Instruction::TransformFnc ? expression_interface.CallTransform(function_name, arguments, R)
+																		  : expression_interface.EventCallback(function_name, arguments));
+			if (!result)
 			{
 				String arguments_str;
 				for (size_t i = 0; i < arguments.size(); i++)
@@ -947,10 +934,10 @@ private:
 					if (i < arguments.size() - 1)
 						arguments_str += ", ";
 				}
-				Error(CreateString(50 + function_name.size() + arguments_str.size(), "Failed to execute event callback: %s(%s)", function_name.c_str(), arguments_str.c_str()));
+				Error(CreateString(60 + function_name.size() + arguments_str.size(), "Failed to execute %s: %s(%s)",
+					instruction == Instruction::TransformFnc ? "transform function" : "event callback", function_name.c_str(),
+					arguments_str.c_str()));
 			}
-
-			arguments.clear();
 		}
 		break;
 		case Instruction::Assign:
@@ -970,6 +957,21 @@ private:
 		}
 		return true;
 	}
+
+	bool ExtractArgumentsFromStack(Vector<Variant>& out_arguments)
+	{
+		int num_arguments = R.Get<int>(-1);
+		if (num_arguments < 0)
+			return Error("Invalid number of arguments.");
+		if (stack.size() < size_t(num_arguments))
+			return Error(CreateString(100, "Cannot pop %d arguments, stack contains only %zu elements.", num_arguments, stack.size()));
+
+		const auto it_stack_begin_arguments = stack.end() - num_arguments;
+		out_arguments.insert(out_arguments.end(), std::make_move_iterator(it_stack_begin_arguments), std::make_move_iterator(stack.end()));
+
+		stack.erase(it_stack_begin_arguments, stack.end());
+		return true;
+	}
 };
 
 
@@ -1055,9 +1057,9 @@ bool DataExpressionInterface::SetValue(const DataAddress& address, const Variant
 	return result;
 }
 
-bool DataExpressionInterface::CallTransform(const String& name, Variant& inout_variant, const VariantList& arguments)
+bool DataExpressionInterface::CallTransform(const String& name, const VariantList& arguments, Variant& out_result)
 {
-	return data_model ? data_model->CallTransform(name, inout_variant, arguments) : false;
+	return data_model ? data_model->CallTransform(name, arguments, out_result) : false;
 }
 
 bool DataExpressionInterface::EventCallback(const String& name, const VariantList& arguments)

+ 1 - 1
Source/Core/DataExpression.h

@@ -49,7 +49,7 @@ public:
     DataAddress ParseAddress(const String& address_str) const;
     Variant GetValue(const DataAddress& address) const;
     bool SetValue(const DataAddress& address, const Variant& value) const;
-    bool CallTransform(const String& name, Variant& inout_result, const VariantList& arguments);
+    bool CallTransform(const String& name, const VariantList& arguments, Variant& out_result);
     bool EventCallback(const String& name, const VariantList& arguments);
 
 private:

+ 2 - 2
Source/Core/DataModel.cpp

@@ -348,10 +348,10 @@ void DataModel::DirtyAllVariables() {
 	}
 }
 
-bool DataModel::CallTransform(const String& name, Variant& inout_result, const VariantList& arguments) const
+bool DataModel::CallTransform(const String& name, const VariantList& arguments, Variant& out_result) const
 {
 	if (transform_register)
-		return transform_register->Call(name, inout_result, arguments);
+		return transform_register->Call(name, arguments, out_result);
 	return false;
 }
 

+ 1 - 1
Source/Core/DataModel.h

@@ -70,7 +70,7 @@ public:
 	bool IsVariableDirty(const String& variable_name) const;
 	void DirtyAllVariables();
 
-	bool CallTransform(const String& name, Variant& inout_result, const VariantList& arguments) const;
+	bool CallTransform(const String& name, const VariantList& arguments, Variant& out_result) const;
 
 	// Elements declaring 'data-model' need to be attached.
 	void AttachModelRootElement(Element* element);

+ 68 - 62
Source/Core/DataTypeRegister.cpp

@@ -15,7 +15,7 @@
  *
  * The above copyright notice and this permission notice shall be included in
  * all copies or substantial portions of the Software.
- * 
+ *
  * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
  * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
  * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
@@ -34,92 +34,98 @@ namespace Rml {
 
 DataTypeRegister::DataTypeRegister()
 {
-    // Add default transform functions.
+	// Add default transform functions.
 
-	transform_register.Register("to_lower", [](Variant& variant, const VariantList& /*arguments*/) -> bool {
+	transform_register.Register("to_lower", [](const VariantList& arguments) -> Variant {
+		if (arguments.size() != 1)
+			return {};
 		String value;
-		if (!variant.GetInto(value))
-			return false;
-		variant = StringUtilities::ToLower(value);
-		return true;
+		if (!arguments[0].GetInto(value))
+			return {};
+		return Variant(StringUtilities::ToLower(value));
 	});
 
-	transform_register.Register("to_upper", [](Variant& variant, const VariantList& /*arguments*/) -> bool {
+	transform_register.Register("to_upper", [](const VariantList& arguments) -> Variant {
+		if (arguments.size() != 1)
+			return {};
 		String value;
-		if (!variant.GetInto(value))
-			return false;
-		variant = StringUtilities::ToUpper(value);
-		return true;
+		if (!arguments[0].GetInto(value))
+			return {};
+		return Variant(StringUtilities::ToUpper(value));
 	});
 
-	transform_register.Register("format", [](Variant& variant, const VariantList& arguments) -> bool {
-        // Arguments in:
-        //   0 : int[0,32]  Precision. Number of digits after the decimal point.
-        //  [1]: bool       True to remove trailing zeros (default = false).
-        if (arguments.size() < 1 || arguments.size() > 2) {
-            Log::Message(Log::LT_WARNING, "Transform function 'format' requires at least one argument, at most two arguments.");
-            return false;
-        }
-        int precision = 0;
-        if (!arguments[0].GetInto(precision) || precision < 0 || precision > 32) {
-            Log::Message(Log::LT_WARNING, "Transform function 'format': First argument must be an integer in [0, 32].");
-            return false;
-        }
-        bool remove_trailing_zeros = false;
-        if (arguments.size() >= 2) {
-            if (!arguments[1].GetInto(remove_trailing_zeros))
-                return false;
-        }
+	transform_register.Register("format", [](const VariantList& arguments) -> Variant {
+		// Arguments in:
+		//   0 : number     Number to format.
+		//   1 : int[0,32]  Precision. Number of digits after the decimal point.
+		//  [2]: bool       True to remove trailing zeros (default = false).
+		if (arguments.empty() || arguments.size() > 3)
+		{
+			Log::Message(Log::LT_WARNING, "Transform function 'format' requires at least two arguments, at most three arguments.");
+			return {};
+		}
+		int precision = 0;
+		if (!arguments[1].GetInto(precision) || precision < 0 || precision > 32)
+		{
+			Log::Message(Log::LT_WARNING, "Transform function 'format': Second argument must be an integer in [0, 32].");
+			return {};
+		}
+		bool remove_trailing_zeros = false;
+		if (arguments.size() >= 3)
+		{
+			if (!arguments[2].GetInto(remove_trailing_zeros))
+				return {};
+		}
 
 		double value = 0;
-		if (!variant.GetInto(value))
-			return false;
+		if (!arguments[0].GetInto(value))
+			return {};
 
-        String format_specifier = String(remove_trailing_zeros ? "%#." : "%.") + ToString(precision) + 'f';
-        String result;
-        if (FormatString(result, 64, format_specifier.c_str(), value) == 0)
-            return false;
+		String format_specifier = String(remove_trailing_zeros ? "%#." : "%.") + ToString(precision) + 'f';
+		String result;
+		if (FormatString(result, 64, format_specifier.c_str(), value) == 0)
+			return {};
 
-        if (remove_trailing_zeros)
-            StringUtilities::TrimTrailingDotZeros(result);
+		if (remove_trailing_zeros)
+			StringUtilities::TrimTrailingDotZeros(result);
 
-        variant = result;
-		return true;
+		return Variant(std::move(result));
 	});
 
-	transform_register.Register("round", [](Variant& variant, const VariantList& /*arguments*/) -> bool {
-		double value = 0;
-		if (!variant.GetInto(value))
-			return false;
-        variant = Math::RoundFloat(value);
-		return true;
+	transform_register.Register("round", [](const VariantList& arguments) -> Variant {
+		if (arguments.size() != 1)
+			return {};
+		double value;
+		if (!arguments[0].GetInto(value))
+			return {};
+		return Variant(Math::RoundFloat(value));
 	});
 }
 
-DataTypeRegister::~DataTypeRegister()
-{}
+DataTypeRegister::~DataTypeRegister() {}
 
 void TransformFuncRegister::Register(const String& name, DataTransformFunc transform_func)
 {
-    RMLUI_ASSERT(transform_func);
-    bool inserted = transform_functions.emplace(name, std::move(transform_func)).second;
-    if (!inserted)
-    {
-        Log::Message(Log::LT_ERROR, "Transform function '%s' already exists.", name.c_str());
-        RMLUI_ERROR;
-    }
+	RMLUI_ASSERT(transform_func);
+	bool inserted = transform_functions.emplace(name, std::move(transform_func)).second;
+	if (!inserted)
+	{
+		Log::Message(Log::LT_ERROR, "Transform function '%s' already exists.", name.c_str());
+		RMLUI_ERROR;
+	}
 }
 
-bool TransformFuncRegister::Call(const String& name, Variant& inout_result, const VariantList& arguments) const
+bool TransformFuncRegister::Call(const String& name, const VariantList& arguments, Variant& out_result) const
 {
-    auto it = transform_functions.find(name);
-    if (it == transform_functions.end())
-        return false;
+	auto it = transform_functions.find(name);
+	if (it == transform_functions.end())
+		return false;
 
-    const DataTransformFunc& transform_func = it->second;
-    RMLUI_ASSERT(transform_func);
+	const DataTransformFunc& transform_func = it->second;
+	RMLUI_ASSERT(transform_func);
 
-    return transform_func(inout_result, arguments);
+	out_result = transform_func(arguments);
+	return true;
 }
 
 } // namespace Rml

+ 43 - 18
Tests/Source/UnitTests/DataExpression.cpp

@@ -26,12 +26,9 @@
  *
  */
 
-
 #include "../../../Source/Core/DataExpression.cpp"
-
 #include <RmlUi/Core/DataModelHandle.h>
 #include <RmlUi/Core/Types.h>
-
 #include <doctest.h>
 
 using namespace Rml;
@@ -40,7 +37,6 @@ static DataTypeRegister type_register;
 static DataModel model(type_register.GetTransformFuncRegister());
 static DataExpressionInterface interface(&model, nullptr);
 
-
 static String TestExpression(const String& expression)
 {
 	String result;
@@ -57,11 +53,12 @@ static String TestExpression(const String& expression)
 		if (interpreter.Run())
 			result = interpreter.Result().Get<String>();
 		else
-			FAIL_CHECK("Could not execute expression: " << expression << "\n\n  Parsed program: \n" << interpreter.DumpProgram());
+			FAIL_CHECK("Could not execute expression: " << expression << "\n\n  Parsed program: \n" << DumpProgram(program));
 	}
 	else
 	{
-		FAIL_CHECK("Could not parse expression: " << expression);
+		Program program = parser.ReleaseProgram();
+		FAIL_CHECK("Could not parse expression: " << expression << "\n\n  Parsed result: \n" << DumpProgram(program));
 	}
 
 	return result;
@@ -80,33 +77,43 @@ static bool TestAssignment(const String& expression)
 		if (interpreter.Run())
 			result = true;
 		else
-			FAIL_CHECK("Could not execute assignment expression: " << expression << "\n\n  Parsed program: \n" << interpreter.DumpProgram());
+			FAIL_CHECK("Could not execute assignment expression: " << expression << "\n\n  Parsed program: \n" << DumpProgram(program));
 	}
 	else
 	{
-		FAIL_CHECK("Could not parse assignment expression: " << expression);
+		Program program = parser.ReleaseProgram();
+		FAIL_CHECK("Could not parse assignment expression: " << expression << "\n\n  Parsed result: \n" << DumpProgram(program));
 	}
 	return result;
 }
 
-
-
 TEST_CASE("Data expressions")
 {
 	float radius = 8.7f;
 	String color_name = "color";
 	Colourb color_value = Colourb(180, 100, 255);
 
-	DataModelConstructor handle(&model, &type_register);
-	handle.Bind("radius", &radius);
-	handle.Bind("color_name", &color_name);
-	handle.BindFunc("color_value", [&](Variant& variant) {
-		variant = ToString(color_value);
+	DataModelConstructor constructor(&model, &type_register);
+	constructor.Bind("radius", &radius);
+	constructor.Bind("color_name", &color_name);
+	constructor.BindFunc("color_value", [&](Variant& variant) { variant = ToString(color_value); });
+
+	constructor.RegisterTransformFunc("concatenate", [](const VariantList& arguments) -> Variant {
+		StringList list;
+		list.reserve(arguments.size());
+		for (const Variant& argument : arguments)
+			list.push_back(argument.Get<String>());
+		String result;
+		StringUtilities::JoinString(result, list);
+		return Variant(std::move(result));
 	});
 
+	CHECK(TestExpression("3.62345 | round | format(2)") == "4.00");
+
+	CHECK(TestExpression("'a' | to_upper") == "A");
 	CHECK(TestExpression("!!10 - 1 ? 'hello' : 'world' | to_upper") == "WORLD");
 	CHECK(TestExpression("(color_name) + (': rgba(' + color_value + ')')") == "color: rgba(180, 100, 255, 255)");
-	CHECK(TestExpression("'hello world' | to_upper(5 + 12 == 17 ? 'yes' : 'no', 9*2)") == "HELLO WORLD");
+	CHECK(TestExpression("'hello world' | to_upper | concatenate(5 + 12 == 17 ? 'yes' : 'no', 9*2)") == "HELLO WORLD,yes,18");
 	CHECK(TestExpression("true == false") == "0");
 	CHECK(TestExpression("true != false") == "1");
 	CHECK(TestExpression("true") == "1");
@@ -130,6 +137,11 @@ TEST_CASE("Data expressions")
 	CHECK(color_name == "image-color");
 	CHECK(TestExpression("radius == 4 && color_name == 'image-color'") == "1");
 
+	CHECK(TestAssignment("color_name = 'a' | concatenate('b')"));
+	CHECK(color_name == "a,b");
+	CHECK(TestAssignment("color_name = concatenate('c','d')"));
+	CHECK(color_name == "c,d");
+
 	CHECK(TestExpression("5 == 1 + 2*2 || 8 == 1 + 4  ? 'yes' : 'no'") == "yes");
 	CHECK(TestExpression("!!('fa' + 'lse')") == "0");
 	CHECK(TestExpression("!!('tr' + 'ue')") == "1");
@@ -143,10 +155,23 @@ TEST_CASE("Data expressions")
 	CHECK(TestExpression("3.62345 | round | format(2)") == "4.00");
 	CHECK(TestExpression("3.0001 | format(2, false)") == "3.00");
 	CHECK(TestExpression("3.0001 | format(2, true)") == "3");
+	CHECK(TestExpression("format(3.0001, 2, true)") == "3");
 
 	CHECK(TestExpression("0.2 + 3.42345 | round") == "4");
 	CHECK(TestExpression("(3.42345 | round) + 0.2") == "3.2");
 	CHECK(TestExpression("(3.42345 | format(0)) + 0.2") == "30.2"); // Here, format(0) returns a string, so the + means string concatenation.
-}
-
 
+	CHECK(TestExpression("'Hi' | concatenate") == "Hi");
+	CHECK(TestExpression("'Hi' | concatenate('there')") == "Hi,there");
+	CHECK(TestExpression("'A' | concatenate('b','c', 'd','e')") == "A,b,c,d,e");
+	CHECK(TestExpression("3.6 | concatenate") == "3.6");
+
+	CHECK(TestExpression("concatenate()") == "");
+	CHECK(TestExpression("concatenate('Hi')") == "Hi");
+	CHECK(TestExpression("concatenate( 'Hi',  'there' )") == "Hi,there");
+	CHECK(TestExpression("concatenate('A', 'b'+'c', 'd','e')") == "A,bc,d,e");
+	CHECK(TestExpression("concatenate('r', 2*radius)") == "r,8");
+	CHECK(TestExpression("concatenate(3.6)") == "3.6");
+	CHECK(TestExpression("concatenate(3.6+1)") == "4.6");
+	CHECK(TestExpression("concatenate(3.6+1) | round | format(3, false)") == "5.000");
+}