kindred/create
1
1
Fork 0
Code Issues 53 Pull Requests Actions Packages Projects 9 Releases 2 Wiki Activity

Gui: Make StyleParameterManager::resolve return optional result

Browse Source 
This changes the resolve method of style parameter manager to return
optional intead of definitive result. In reality tokens may not be
defioned correctly and optionals provide a good way to nicely handle
defaults.
This commit is contained in:
Kacper Donat
2025-08-10 23:11:34 +02:00
parent 322ed2c7bc
commit 0cce9c1261
4 changed files with 55 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
src/Gui/Dialogs/DlgThemeEditor.cpp
View File

@@ -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<Base::Color>(value)) {
return colorPreview(std::get<Base::Color>(value).asValue<QColor>());
if (index.column() == ParameterPreview && std::holds_alternative<Base::Color>(*value)) {
return colorPreview(std::get<Base::Color>(*value).asValue<QColor>());
}
}
}

22
src/Gui/StyleParameters/ParameterManager.cpp
View File

@@ -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<std::string> ParameterManager::expression(const std::string& name)
return {};
}
Value ParameterManager::resolve(const std::string& name, ResolveContext context) const
std::optional<Value> ParameterManager::resolve(const std::string& name,
ResolveContext context) const
{
std::optional<Parameter> 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;

2
src/Gui/StyleParameters/Parser.cpp
View File

@@ -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

63
tests/src/Gui/StyleParameters/ParameterManagerTest.cpp
View File

@@ -66,16 +66,18 @@ TEST_F(ParameterManagerTest, BasicParameterResolution)
{
{
auto result = manager.resolve("BaseSize");
EXPECT_TRUE(std::holds_alternative<Numeric>(result));
auto length = std::get<Numeric>(result);
EXPECT_TRUE(result.has_value());
EXPECT_TRUE(std::holds_alternative<Numeric>(*result));
auto length = std::get<Numeric>(*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<Base::Color>(result));
auto color = std::get<Base::Color>(result);
EXPECT_TRUE(result.has_value());
EXPECT_TRUE(std::holds_alternative<Base::Color>(*result));
auto color = std::get<Base::Color>(*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<Base::Color>(result));
auto color = std::get<Base::Color>(result);
EXPECT_TRUE(result.has_value());
EXPECT_TRUE(std::holds_alternative<Base::Color>(*result));
auto color = std::get<Base::Color>(*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<Numeric>(result));
auto length = std::get<Numeric>(result);
EXPECT_TRUE(std::holds_alternative<Numeric>(*result));
auto length = std::get<Numeric>(*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<Numeric>(result));
auto length = std::get<Numeric>(result);
EXPECT_TRUE(std::holds_alternative<Numeric>(*result));
auto length = std::get<Numeric>(*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<Numeric>(result1));
EXPECT_TRUE(std::holds_alternative<Numeric>(*result1));
// Second resolution should use cached value
auto result2 = manager.resolve("BaseSize");
EXPECT_TRUE(std::holds_alternative<Numeric>(result2));
EXPECT_TRUE(std::holds_alternative<Numeric>(*result2));
// Results should be identical
auto length1 = std::get<Numeric>(result1);
auto length2 = std::get<Numeric>(result2);
auto length1 = std::get<Numeric>(*result1);
auto length2 = std::get<Numeric>(*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<Numeric>(result1));
auto length1 = std::get<Numeric>(result1);
EXPECT_TRUE(std::holds_alternative<Numeric>(*result1));
auto length1 = std::get<Numeric>(*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<Numeric>(result2));
auto length2 = std::get<Numeric>(result2);
EXPECT_TRUE(std::holds_alternative<Numeric>(*result2));
auto length2 = std::get<Numeric>(*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<Numeric>(result));
auto length = std::get<Numeric>(result);
EXPECT_TRUE(std::holds_alternative<Numeric>(*result));
auto length = std::get<Numeric>(*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<std::string>(result));
EXPECT_TRUE(std::holds_alternative<std::string>(*result));
}
// Test complex expressions
@@ -279,24 +282,24 @@ TEST_F(ParameterManagerTest, ComplexExpressions)
{
auto result = manager.resolve("ComplexMargin");
EXPECT_TRUE(std::holds_alternative<Numeric>(result));
auto length = std::get<Numeric>(result);
EXPECT_TRUE(std::holds_alternative<Numeric>(*result));
auto length = std::get<Numeric>(*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<Numeric>(result));
auto length = std::get<Numeric>(result);
EXPECT_TRUE(std::holds_alternative<Numeric>(*result));
auto length = std::get<Numeric>(*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<Base::Color>(result));
auto color = std::get<Base::Color>(result).asValue<QColor>();
EXPECT_TRUE(std::holds_alternative<Base::Color>(*result));
auto color = std::get<Base::Color>(*result).asValue<QColor>();
// 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<std::string>(result));
EXPECT_EQ(std::get<std::string>(result), "");
EXPECT_FALSE(result.has_value());
// Test invalid expression
auto invalidSource = std::make_unique<InMemoryParameterSource>(
@@ -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<std::string>(invalidResult));
EXPECT_TRUE(invalidResult.has_value());
EXPECT_TRUE(std::holds_alternative<std::string>(*invalidResult));
}