diff --git a/src/Gui/Dialogs/DlgThemeEditor.cpp b/src/Gui/Dialogs/DlgThemeEditor.cpp index e12180a8ed..efe66aa1c2 100644 --- a/src/Gui/Dialogs/DlgThemeEditor.cpp +++ b/src/Gui/Dialogs/DlgThemeEditor.cpp @@ -514,6 +514,10 @@ QVariant StyleParametersModel::data(const QModelIndex& index, int role) const const auto& [name, token, _] = *parameterItem; const auto& value = manager->resolve(name.toStdString()); + if (!value) { + return {}; + } + if (role == Qt::DisplayRole) { if (index.column() == ParameterName) { return name; @@ -522,16 +526,16 @@ QVariant StyleParametersModel::data(const QModelIndex& index, int role) const return QString::fromStdString(token.value); } if (index.column() == ParameterType) { - return typeOfTokenValue(value); + return typeOfTokenValue(*value); } if (index.column() == ParameterPreview) { - return QString::fromStdString(value.toString()); + return QString::fromStdString(value->toString()); } } if (role == Qt::DecorationRole) { - if (index.column() == ParameterPreview && std::holds_alternative(value)) { - return colorPreview(std::get(value).asValue()); + if (index.column() == ParameterPreview && std::holds_alternative(*value)) { + return colorPreview(std::get(*value).asValue()); } } } diff --git a/src/Gui/StyleParameters/ParameterManager.cpp b/src/Gui/StyleParameters/ParameterManager.cpp index 54efe995a0..3e2e24ec8f 100644 --- a/src/Gui/StyleParameters/ParameterManager.cpp +++ b/src/Gui/StyleParameters/ParameterManager.cpp @@ -260,12 +260,16 @@ std::string ParameterManager::replacePlaceholders(const std::string& expression, QString::fromStdString(expression), [&](const QRegularExpressionMatch& match) { auto tokenName = match.captured(1).toStdString(); - auto tokenValue = resolve(tokenName, context); - context.visited.erase(tokenName); - return QString::fromStdString(tokenValue.toString()); - } + if (!tokenValue) { + Base::Console().warning("Requested non-existent style parameter token '%s'.\n", tokenName); + return QStringLiteral(""); + } + + context.visited.erase(tokenName); + return QString::fromStdString(tokenValue->toString()); + } ).toStdString(); // clang-format on } @@ -293,18 +297,18 @@ std::optional ParameterManager::expression(const std::string& name) return {}; } -Value ParameterManager::resolve(const std::string& name, ResolveContext context) const +std::optional ParameterManager::resolve(const std::string& name, + ResolveContext context) const { std::optional maybeParameter = this->parameter(name); if (!maybeParameter) { - Base::Console().warning("Requested non-existent design token '%s'.\n", name); - return std::string {}; + return std::nullopt; } if (context.visited.contains(name)) { - Base::Console().warning("The design token '%s' contains circular-reference.\n", name); - return expression(name).value_or(std::string {}); + Base::Console().warning("The style parameter '%s' contains circular-reference.\n", name); + return expression(name); } const Parameter& token = *maybeParameter; diff --git a/src/Gui/StyleParameters/Parser.cpp b/src/Gui/StyleParameters/Parser.cpp index 6e68e57a31..26f664d95b 100644 --- a/src/Gui/StyleParameters/Parser.cpp +++ b/src/Gui/StyleParameters/Parser.cpp @@ -42,7 +42,7 @@ namespace Gui::StyleParameters Value ParameterReference::evaluate(const EvaluationContext& context) const { - return context.manager->resolve(name, context.context); + return context.manager->resolve(name, context.context).value_or("@" + name); } Value Number::evaluate([[maybe_unused]] const EvaluationContext& context) const diff --git a/tests/src/Gui/StyleParameters/ParameterManagerTest.cpp b/tests/src/Gui/StyleParameters/ParameterManagerTest.cpp index e3c53326f2..ceefab2298 100644 --- a/tests/src/Gui/StyleParameters/ParameterManagerTest.cpp +++ b/tests/src/Gui/StyleParameters/ParameterManagerTest.cpp @@ -66,16 +66,18 @@ TEST_F(ParameterManagerTest, BasicParameterResolution) { { auto result = manager.resolve("BaseSize"); - EXPECT_TRUE(std::holds_alternative(result)); - auto length = std::get(result); + EXPECT_TRUE(result.has_value()); + EXPECT_TRUE(std::holds_alternative(*result)); + auto length = std::get(*result); EXPECT_DOUBLE_EQ(length.value, 16.0); // Should get value from source2 (later source) EXPECT_EQ(length.unit, "px"); } { auto result = manager.resolve("PrimaryColor"); - EXPECT_TRUE(std::holds_alternative(result)); - auto color = std::get(result); + EXPECT_TRUE(result.has_value()); + EXPECT_TRUE(std::holds_alternative(*result)); + auto color = std::get(*result); EXPECT_EQ(color.r, 1); EXPECT_EQ(color.g, 0); EXPECT_EQ(color.b, 0); @@ -83,8 +85,9 @@ TEST_F(ParameterManagerTest, BasicParameterResolution) { auto result = manager.resolve("SecondaryColor"); - EXPECT_TRUE(std::holds_alternative(result)); - auto color = std::get(result); + EXPECT_TRUE(result.has_value()); + EXPECT_TRUE(std::holds_alternative(*result)); + auto color = std::get(*result); EXPECT_EQ(color.r, 0); EXPECT_EQ(color.g, 1); EXPECT_EQ(color.b, 0); @@ -96,16 +99,16 @@ TEST_F(ParameterManagerTest, ParameterReferences) { { auto result = manager.resolve("Margin"); - EXPECT_TRUE(std::holds_alternative(result)); - auto length = std::get(result); + EXPECT_TRUE(std::holds_alternative(*result)); + auto length = std::get(*result); EXPECT_DOUBLE_EQ(length.value, 32.0); // @BaseSize * 2 = 16 * 2 = 32 EXPECT_EQ(length.unit, "px"); } { auto result = manager.resolve("Padding"); - EXPECT_TRUE(std::holds_alternative(result)); - auto length = std::get(result); + EXPECT_TRUE(std::holds_alternative(*result)); + auto length = std::get(*result); EXPECT_DOUBLE_EQ(length.value, 8.0); // @BaseSize / 2 = 16 / 2 = 8 EXPECT_EQ(length.unit, "px"); } @@ -116,15 +119,15 @@ TEST_F(ParameterManagerTest, Caching) { // First resolution should cache the result auto result1 = manager.resolve("BaseSize"); - EXPECT_TRUE(std::holds_alternative(result1)); + EXPECT_TRUE(std::holds_alternative(*result1)); // Second resolution should use cached value auto result2 = manager.resolve("BaseSize"); - EXPECT_TRUE(std::holds_alternative(result2)); + EXPECT_TRUE(std::holds_alternative(*result2)); // Results should be identical - auto length1 = std::get(result1); - auto length2 = std::get(result2); + auto length1 = std::get(*result1); + auto length2 = std::get(*result2); EXPECT_DOUBLE_EQ(length1.value, length2.value); EXPECT_EQ(length1.unit, length2.unit); } @@ -134,8 +137,8 @@ TEST_F(ParameterManagerTest, CacheInvalidation) { // Initial resolution auto result1 = manager.resolve("BaseSize"); - EXPECT_TRUE(std::holds_alternative(result1)); - auto length1 = std::get(result1); + EXPECT_TRUE(std::holds_alternative(*result1)); + auto length1 = std::get(*result1); EXPECT_DOUBLE_EQ(length1.value, 16.0); // Reload should clear cache @@ -143,8 +146,8 @@ TEST_F(ParameterManagerTest, CacheInvalidation) // Resolution after reload should work the same auto result2 = manager.resolve("BaseSize"); - EXPECT_TRUE(std::holds_alternative(result2)); - auto length2 = std::get(result2); + EXPECT_TRUE(std::holds_alternative(*result2)); + auto length2 = std::get(*result2); EXPECT_DOUBLE_EQ(length2.value, 16.0); EXPECT_EQ(length1.unit, length2.unit); } @@ -164,8 +167,8 @@ TEST_F(ParameterManagerTest, SourcePriority) // Should get value from the latest source (highest priority) auto result = manager.resolve("BaseSize"); - EXPECT_TRUE(std::holds_alternative(result)); - auto length = std::get(result); + EXPECT_TRUE(std::holds_alternative(*result)); + auto length = std::get(*result); EXPECT_DOUBLE_EQ(length.value, 24.0); EXPECT_EQ(length.unit, "px"); } @@ -259,7 +262,7 @@ TEST_F(ParameterManagerTest, CircularReferenceDetection) // Should handle circular reference gracefully auto result = manager.resolve("A"); // Should return the expression string as fallback - EXPECT_TRUE(std::holds_alternative(result)); + EXPECT_TRUE(std::holds_alternative(*result)); } // Test complex expressions @@ -279,24 +282,24 @@ TEST_F(ParameterManagerTest, ComplexExpressions) { auto result = manager.resolve("ComplexMargin"); - EXPECT_TRUE(std::holds_alternative(result)); - auto length = std::get(result); + EXPECT_TRUE(std::holds_alternative(*result)); + auto length = std::get(*result); EXPECT_DOUBLE_EQ(length.value, 40.0); // (16 + 4) * 2 = 20 * 2 = 40 EXPECT_EQ(length.unit, "px"); } { auto result = manager.resolve("ComplexPadding"); - EXPECT_TRUE(std::holds_alternative(result)); - auto length = std::get(result); + EXPECT_TRUE(std::holds_alternative(*result)); + auto length = std::get(*result); EXPECT_DOUBLE_EQ(length.value, 7.0); // (16 - 2) / 2 = 14 / 2 = 7 EXPECT_EQ(length.unit, "px"); } { auto result = manager.resolve("ColorWithFunction"); - EXPECT_TRUE(std::holds_alternative(result)); - auto color = std::get(result).asValue(); + EXPECT_TRUE(std::holds_alternative(*result)); + auto color = std::get(*result).asValue(); // Should be lighter than the original red EXPECT_GT(color.lightness(), QColor(0xff0000).lightness()); } @@ -307,8 +310,7 @@ TEST_F(ParameterManagerTest, ErrorHandling) { // Test non-existent parameter auto result = manager.resolve("NonExistent"); - EXPECT_TRUE(std::holds_alternative(result)); - EXPECT_EQ(std::get(result), ""); + EXPECT_FALSE(result.has_value()); // Test invalid expression auto invalidSource = std::make_unique( @@ -323,5 +325,6 @@ TEST_F(ParameterManagerTest, ErrorHandling) // Should handle invalid expression gracefully auto invalidResult = manager.resolve("Invalid"); // Should return the expression string as fallback - EXPECT_TRUE(std::holds_alternative(invalidResult)); + EXPECT_TRUE(invalidResult.has_value()); + EXPECT_TRUE(std::holds_alternative(*invalidResult)); }