From acea58f85f9e0fc898a1eea69f0c384537f01191 Mon Sep 17 00:00:00 2001 From: Pieter Hijma Date: Sun, 20 Jul 2025 09:51:05 +0200 Subject: [PATCH 01/10] Gui: Improve layout expression dialog for varsets According to #17075 --- src/Gui/Dialogs/DlgExpressionInput.cpp | 40 +++--- src/Gui/Dialogs/DlgExpressionInput.h | 2 +- src/Gui/Dialogs/DlgExpressionInput.ui | 170 ++++++++++--------------- 3 files changed, 89 insertions(+), 123 deletions(-) diff --git a/src/Gui/Dialogs/DlgExpressionInput.cpp b/src/Gui/Dialogs/DlgExpressionInput.cpp index 70f384ebd7..d907362751 100644 --- a/src/Gui/Dialogs/DlgExpressionInput.cpp +++ b/src/Gui/Dialogs/DlgExpressionInput.cpp @@ -36,6 +36,7 @@ #include #include #include +#include #include #include "Dialogs/DlgExpressionInput.h" @@ -50,6 +51,9 @@ using namespace App; using namespace Gui::Dialog; +FC_LOG_LEVEL_INIT("DlgExpressionInput", true, true) + + bool DlgExpressionInput::varSetsVisible = false; DlgExpressionInput::DlgExpressionInput(const App::ObjectIdentifier & _path, @@ -204,9 +208,6 @@ bool DlgExpressionInput::typeOkForVarSet() void DlgExpressionInput::initializeVarSets() { - ui->labelInfoActive->setAlignment(Qt::AlignTop | Qt::AlignLeft); - ui->labelInfoActive->setWordWrap(true); - #if QT_VERSION >= QT_VERSION_CHECK(6,7,0) connect(ui->checkBoxVarSets, &QCheckBox::checkStateChanged, this, &DlgExpressionInput::onCheckVarSets); @@ -441,7 +442,6 @@ static bool isNamePropOk(const QString& nameProp, App::DocumentObject* obj, std::string name = nameProp.toStdString(); if (name.empty()) { - message << "Provide a name for the property."; return false; } @@ -688,21 +688,22 @@ static bool isNameGroupOk(const QString& nameGroup, return true; } -void DlgExpressionInput::reportVarSetInfo(const char* message) +void DlgExpressionInput::reportVarSetInfo(const std::string& message) { - ui->labelInfoActive->setText(QString::fromUtf8(message)); + if (!message.empty()) { + FC_ERR(message); + } } bool DlgExpressionInput::reportGroup(QString& nameGroup) { if (nameGroup.isEmpty()) { - reportVarSetInfo("Provide a group."); return true; } std::stringstream message; if (!isNameGroupOk(nameGroup, message)) { - reportVarSetInfo(message.str().c_str()); + reportVarSetInfo(message.str()); return true; } @@ -718,7 +719,7 @@ bool DlgExpressionInput::reportName(QTreeWidgetItem* item) App::DocumentObject* obj = doc->getObject(nameVarSet.toUtf8()); std::stringstream message; if (!isNamePropOk(nameProp, obj, message)) { - reportVarSetInfo(message.str().c_str()); + reportVarSetInfo(message.str()); return true; } @@ -743,17 +744,17 @@ void DlgExpressionInput::updateVarSetInfo(bool checkExpr) return; } - QString nameProp = ui->lineEditPropNew->text(); - QString labelVarSet = getValue(selected, ROLE_VARSET_LABEL); - QString nameDoc = getValue(selected, ROLE_DOC); - std::stringstream message; - message << "Adding property " << nameProp.toStdString() << std::endl - << "of type " << getType() << std::endl - << "to variable set " << labelVarSet.toStdString() << std::endl - << "in group " << nameGroup.toStdString() << std::endl - << "in document " << nameDoc.toStdString() << "."; + // QString nameProp = ui->lineEditPropNew->text(); + // QString labelVarSet = getValue(selected, ROLE_VARSET_LABEL); + // QString nameDoc = getValue(selected, ROLE_DOC); + // std::stringstream message; + // message << "Adding property " << nameProp.toStdString() << std::endl + // << "of type " << getType() << std::endl + // << "to variable set " << labelVarSet.toStdString() << std::endl + // << "in group " << nameGroup.toStdString() << std::endl + // << "in document " << nameDoc.toStdString() << "."; - reportVarSetInfo(message.str().c_str()); + // reportVarSetInfo(message.str().c_str()); if (checkExpr) { // We have to check the text of the expression as well try { @@ -767,7 +768,6 @@ void DlgExpressionInput::updateVarSetInfo(bool checkExpr) } else { okBtn->setEnabled(false); - reportVarSetInfo("Select a variable set."); } } diff --git a/src/Gui/Dialogs/DlgExpressionInput.h b/src/Gui/Dialogs/DlgExpressionInput.h index 7f6f20220a..2c714750e6 100644 --- a/src/Gui/Dialogs/DlgExpressionInput.h +++ b/src/Gui/Dialogs/DlgExpressionInput.h @@ -94,7 +94,7 @@ private: void checkExpression(const QString& text); void setupVarSets(); std::string getType(); - void reportVarSetInfo(const char* message); + void reportVarSetInfo(const std::string& message); bool reportName(QTreeWidgetItem* item); bool reportGroup(QString& nameGroup); void updateVarSetInfo(bool checkExpr = true); diff --git a/src/Gui/Dialogs/DlgExpressionInput.ui b/src/Gui/Dialogs/DlgExpressionInput.ui index 705ea44a45..04d261470f 100644 --- a/src/Gui/Dialogs/DlgExpressionInput.ui +++ b/src/Gui/Dialogs/DlgExpressionInput.ui @@ -7,7 +7,7 @@ 0 0 414 - 272 + 291 @@ -32,89 +32,6 @@ 0 - - - - Variable Sets - - - - - - - - - - - Group - - - - - - - - 0 - 0 - - - - - 0 - 70 - - - - - - - - - - - Variable set - - - - - - - - - - Info - - - - - - - New property - - - - - - - - 0 - 0 - - - - - - - - - - - - - Show variable sets - - - @@ -131,10 +48,10 @@ true - QFrame::StyledPanel + QFrame::Shape::StyledPanel - QFrame::Raised + QFrame::Shadow::Raised @@ -202,7 +119,7 @@ - Qt::Horizontal + Qt::Orientation::Horizontal @@ -235,7 +152,7 @@ - Qt::Horizontal + Qt::Orientation::Horizontal @@ -250,31 +167,81 @@ - - - Qt::Horizontal + + + + 0 + 0 + - - - 0 - 20 - + + QDialogButtonBox::StandardButton::Ok|QDialogButtonBox::StandardButton::Reset - + - - - QDialogButtonBox::Reset|QDialogButtonBox::Ok + + + Add to variable set... + + + + + + + + + + + + + 0 + 0 + + + + + + + + Name: + + + + + + + Variable Set: + + + + + + + Group: + + + + + + + + + + + + + + - Qt::Vertical + Qt::Orientation::Vertical @@ -296,7 +263,6 @@ expression buttonBox - checkBoxVarSets From a99d90372763912ce1e4d79efe69675736f02d93 Mon Sep 17 00:00:00 2001 From: Pieter Hijma Date: Sun, 20 Jul 2025 11:16:45 +0200 Subject: [PATCH 02/10] Gui: Stop remember state expression dialog Before this commit, the expression dialog remembered the state of whether the varset information should be shown. This is removed according to #17075. --- src/Gui/Dialogs/DlgExpressionInput.cpp | 12 +++--------- src/Gui/Dialogs/DlgExpressionInput.h | 2 +- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/src/Gui/Dialogs/DlgExpressionInput.cpp b/src/Gui/Dialogs/DlgExpressionInput.cpp index d907362751..1843920c02 100644 --- a/src/Gui/Dialogs/DlgExpressionInput.cpp +++ b/src/Gui/Dialogs/DlgExpressionInput.cpp @@ -53,9 +53,6 @@ using namespace Gui::Dialog; FC_LOG_LEVEL_INIT("DlgExpressionInput", true, true) - -bool DlgExpressionInput::varSetsVisible = false; - DlgExpressionInput::DlgExpressionInput(const App::ObjectIdentifier & _path, std::shared_ptr _expression, const Base::Unit & _impliedUnit, QWidget *parent) @@ -66,6 +63,7 @@ DlgExpressionInput::DlgExpressionInput(const App::ObjectIdentifier & _path, , discarded(false) , impliedUnit(_impliedUnit) , minimumWidth(10) + , varSetsVisible(false) { assert(path.getDocumentObject()); @@ -225,15 +223,11 @@ void DlgExpressionInput::initializeVarSets() std::vector varSets = getAllVarSets(); if (!varSets.empty() && typeOkForVarSet()) { ui->checkBoxVarSets->setVisible(true); - ui->checkBoxVarSets->setCheckState(varSetsVisible ? Qt::Checked : Qt::Unchecked); - ui->groupBoxVarSets->setVisible(varSetsVisible); - if (varSetsVisible) { - setupVarSets(); - } + ui->checkBoxVarSets->setCheckState(Qt::Unchecked); + ui->groupBoxVarSets->setVisible(false); } else { // The dialog is shown without any VarSet options. - varSetsVisible = false; ui->checkBoxVarSets->setVisible(false); ui->groupBoxVarSets->setVisible(false); } diff --git a/src/Gui/Dialogs/DlgExpressionInput.h b/src/Gui/Dialogs/DlgExpressionInput.h index 2c714750e6..ccb889f432 100644 --- a/src/Gui/Dialogs/DlgExpressionInput.h +++ b/src/Gui/Dialogs/DlgExpressionInput.h @@ -118,7 +118,7 @@ private: int minimumWidth; - static bool varSetsVisible; + bool varSetsVisible; std::unique_ptr treeWidget; QPushButton* okBtn = nullptr; QPushButton* discardBtn = nullptr; From 2070b1c1c682c0aae09d373b68b700ec9e86c6c4 Mon Sep 17 00:00:00 2001 From: Pieter Hijma Date: Tue, 5 Aug 2025 20:21:52 +0200 Subject: [PATCH 03/10] Gui: Improve presenting VarSets in expr dialog --- src/Gui/Dialogs/DlgAddPropertyVarSet.cpp | 34 +- src/Gui/Dialogs/DlgAddPropertyVarSet.h | 8 +- src/Gui/Dialogs/DlgExpressionInput.cpp | 478 ++++++++++++++--------- src/Gui/Dialogs/DlgExpressionInput.h | 24 +- src/Gui/Dialogs/DlgExpressionInput.ui | 41 +- 5 files changed, 354 insertions(+), 231 deletions(-) diff --git a/src/Gui/Dialogs/DlgAddPropertyVarSet.cpp b/src/Gui/Dialogs/DlgAddPropertyVarSet.cpp index 3ec99ecec7..db221c360e 100644 --- a/src/Gui/Dialogs/DlgAddPropertyVarSet.cpp +++ b/src/Gui/Dialogs/DlgAddPropertyVarSet.cpp @@ -173,9 +173,11 @@ void DlgAddPropertyVarSet::removeExistingWidget(QFormLayout* formLayout, int lab } } } -void DlgAddPropertyVarSet::setWidgetForLabel(const char* labelName, QWidget* widget) + +void DlgAddPropertyVarSet::setWidgetForLabel(const char* labelName, QWidget* widget, + QLayout* layout) { - auto formLayout = qobject_cast(layout()); + auto formLayout = qobject_cast(layout); if (formLayout == nullptr) { FC_ERR("Form layout not found"); return; @@ -191,14 +193,9 @@ void DlgAddPropertyVarSet::setWidgetForLabel(const char* labelName, QWidget* wid formLayout->setWidget(labelRow, QFormLayout::FieldRole, widget); } -void DlgAddPropertyVarSet::initializeGroup() +void DlgAddPropertyVarSet::populateGroup(EditFinishedComboBox& comboBox, + const App::DocumentObject* varSet) { - comboBoxGroup.setObjectName(QStringLiteral("comboBoxGroup")); - comboBoxGroup.setInsertPolicy(QComboBox::InsertAtTop); - comboBoxGroup.setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Preferred); - - setWidgetForLabel("labelGroup", &comboBoxGroup); - std::vector properties; varSet->getPropertyList(properties); @@ -221,10 +218,21 @@ void DlgAddPropertyVarSet::initializeGroup() }); for (const auto& groupName : groupNamesSorted) { - comboBoxGroup.addItem(QString::fromStdString(groupName)); + comboBox.addItem(QString::fromStdString(groupName)); } - comboBoxGroup.setEditText(QString::fromStdString(groupNamesSorted[0])); + comboBox.setEditText(QString::fromStdString(groupNamesSorted[0])); +} + +void DlgAddPropertyVarSet::initializeGroup() +{ + comboBoxGroup.setObjectName(QStringLiteral("comboBoxGroup")); + comboBoxGroup.setInsertPolicy(QComboBox::InsertAtTop); + comboBoxGroup.setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Preferred); + + setWidgetForLabel("labelGroup", &comboBoxGroup, layout()); + populateGroup(comboBoxGroup, varSet); + connComboBoxGroup = connect(&comboBoxGroup, &EditFinishedComboBox::editFinished, this, &DlgAddPropertyVarSet::onGroupFinished); } @@ -335,7 +343,7 @@ void DlgAddPropertyVarSet::addEditor(PropertyItem* propertyItem) editor->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Preferred); editor->setObjectName(QStringLiteral("editor")); - setWidgetForLabel("labelValue", editor.get()); + setWidgetForLabel("labelValue", editor.get(), layout()); QWidget::setTabOrder(ui->comboBoxType, editor.get()); QWidget::setTabOrder(editor.get(), ui->checkBoxAdd); @@ -544,7 +552,7 @@ void DlgAddPropertyVarSet::removeEditor() auto* placeholder = new QWidget(this); placeholder->setObjectName(QStringLiteral("placeholder")); placeholder->setMinimumHeight(comboBoxGroup.height()); - setWidgetForLabel("labelValue", placeholder); + setWidgetForLabel("labelValue", placeholder, layout()); QWidget::setTabOrder(ui->comboBoxType, ui->checkBoxAdd); editor = nullptr; diff --git a/src/Gui/Dialogs/DlgAddPropertyVarSet.h b/src/Gui/Dialogs/DlgAddPropertyVarSet.h index 85c9efff0d..7978f5da77 100644 --- a/src/Gui/Dialogs/DlgAddPropertyVarSet.h +++ b/src/Gui/Dialogs/DlgAddPropertyVarSet.h @@ -86,6 +86,8 @@ public: void changeEvent(QEvent* e) override; void accept() override; void reject() override; + static void populateGroup(EditFinishedComboBox& comboBox, const App::DocumentObject* varSet); + static void setWidgetForLabel(const char* labelName, QWidget* widget, QLayout* layout); public Q_SLOTS: void valueChanged(); @@ -102,9 +104,6 @@ private: Type }; - int findLabelRow(const char* labelName, QFormLayout* layout); - void removeExistingWidget(QFormLayout* layout, int labelRow); - void setWidgetForLabel(const char* labelName, QWidget* widget); void initializeGroup(); std::vector getSupportedTypes(); @@ -153,6 +152,9 @@ private: void clearFields(); void addDocumentation(); + static void removeExistingWidget(QFormLayout* layout, int labelRow); + static int findLabelRow(const char* labelName, QFormLayout* layout); + private: App::VarSet* varSet; std::unique_ptr ui; diff --git a/src/Gui/Dialogs/DlgExpressionInput.cpp b/src/Gui/Dialogs/DlgExpressionInput.cpp index 1843920c02..3f9380597a 100644 --- a/src/Gui/Dialogs/DlgExpressionInput.cpp +++ b/src/Gui/Dialogs/DlgExpressionInput.cpp @@ -1,5 +1,6 @@ /*************************************************************************** * Copyright (c) 2015 Eivind Kvedalen * + * Copyright (c) 2025 Pieter Hijma * * * * This file is part of the FreeCAD CAx development system. * * * @@ -27,6 +28,7 @@ #include #include #include +#include #endif #include @@ -64,6 +66,7 @@ DlgExpressionInput::DlgExpressionInput(const App::ObjectIdentifier & _path, , impliedUnit(_impliedUnit) , minimumWidth(10) , varSetsVisible(false) + , comboBoxGroup(this) { assert(path.getDocumentObject()); @@ -135,7 +138,7 @@ DlgExpressionInput::~DlgExpressionInput() #endif disconnect(ui->comboBoxVarSet, qOverload(&QComboBox::currentIndexChanged), this, &DlgExpressionInput::onVarSetSelected); - disconnect(ui->lineEditGroup, &QLineEdit::textChanged, + disconnect(&comboBoxGroup, &EditFinishedComboBox::currentTextChanged, this, &DlgExpressionInput::onTextChangedGroup); disconnect(ui->lineEditPropNew, &QLineEdit::textChanged, this, &DlgExpressionInput::namePropChanged); @@ -215,11 +218,16 @@ void DlgExpressionInput::initializeVarSets() #endif connect(ui->comboBoxVarSet, qOverload(&QComboBox::currentIndexChanged), this, &DlgExpressionInput::onVarSetSelected); - connect(ui->lineEditGroup, &QLineEdit::textChanged, + connect(&comboBoxGroup, &EditFinishedComboBox::currentTextChanged, this, &DlgExpressionInput::onTextChangedGroup); connect(ui->lineEditPropNew, &QLineEdit::textChanged, this, &DlgExpressionInput::namePropChanged); + comboBoxGroup.setObjectName(QStringLiteral("comboBoxGroup")); + comboBoxGroup.setInsertPolicy(QComboBox::InsertAtTop); + comboBoxGroup.setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Preferred); + DlgAddPropertyVarSet::setWidgetForLabel("labelGroup", &comboBoxGroup, ui->formLayout); + std::vector varSets = getAllVarSets(); if (!varSets.empty() && typeOkForVarSet()) { ui->checkBoxVarSets->setVisible(true); @@ -277,59 +285,59 @@ QPoint DlgExpressionInput::expressionPosition() const void DlgExpressionInput::checkExpression(const QString& text) { - //now handle expression - std::shared_ptr expr(ExpressionParser::parse(path.getDocumentObject(), text.toUtf8().constData())); + //now handle expression + std::shared_ptr expr(ExpressionParser::parse(path.getDocumentObject(), text.toUtf8().constData())); - if (expr) { - std::string error = path.getDocumentObject()->ExpressionEngine.validateExpression(path, expr); + if (expr) { + std::string error = path.getDocumentObject()->ExpressionEngine.validateExpression(path, expr); - if (!error.empty()) - throw Base::RuntimeError(error.c_str()); + if (!error.empty()) + throw Base::RuntimeError(error.c_str()); - std::unique_ptr result(expr->eval()); + std::unique_ptr result(expr->eval()); - expression = expr; - okBtn->setEnabled(true); - ui->msg->clear(); + expression = expr; + okBtn->setEnabled(true); + ui->msg->clear(); - //set default palette as we may have read text right now - ui->msg->setPalette(okBtn->palette()); + //set default palette as we may have read text right now + ui->msg->setPalette(okBtn->palette()); - auto * n = freecad_cast(result.get()); - if (n) { - Base::Quantity value = n->getQuantity(); - if (!value.isValid()) { - throw Base::ValueError("Not a number"); - } - - auto msg = value.getUserString(); - if (impliedUnit != Base::Unit::One) { - if (!value.isDimensionless() && value.getUnit() != impliedUnit) - throw Base::UnitsMismatchError("Unit mismatch between result and required unit"); - - value.setUnit(impliedUnit); - - } - else if (!value.isDimensionless()) { - msg += " (Warning: unit discarded)"; - - QPalette p(ui->msg->palette()); - p.setColor(QPalette::WindowText, Qt::red); - ui->msg->setPalette(p); - } - - numberRange.throwIfOutOfRange(value); - - ui->msg->setText(QString::fromStdString(msg)); - } - else { - ui->msg->setText(QString::fromStdString(result->toString())); + auto * n = freecad_cast(result.get()); + if (n) { + Base::Quantity value = n->getQuantity(); + if (!value.isValid()) { + throw Base::ValueError("Not a number"); } + auto msg = value.getUserString(); + if (impliedUnit != Base::Unit::One) { + if (!value.isDimensionless() && value.getUnit() != impliedUnit) + throw Base::UnitsMismatchError("Unit mismatch between result and required unit"); + + value.setUnit(impliedUnit); + + } + else if (!value.isDimensionless()) { + msg += " (Warning: unit discarded)"; + + QPalette p(ui->msg->palette()); + p.setColor(QPalette::WindowText, Qt::red); + ui->msg->setPalette(p); + } + + numberRange.throwIfOutOfRange(value); + + ui->msg->setText(QString::fromStdString(msg)); } + else { + ui->msg->setText(QString::fromStdString(result->toString())); + } + + } } -static const bool NO_CHECK_EXPR = false; +static const bool NoCheckExpr = false; void DlgExpressionInput::textChanged(const QString &text) { @@ -358,7 +366,7 @@ void DlgExpressionInput::textChanged(const QString &text) // If varsets are visible, check whether the varset info also // agrees that the button should be enabled. // No need to check the expression in that function. - updateVarSetInfo(NO_CHECK_EXPR); + updateVarSetInfo(NoCheckExpr); } } catch (Base::Exception & e) { @@ -378,31 +386,34 @@ void DlgExpressionInput::setDiscarded() void DlgExpressionInput::setExpressionInputSize(int width, int height) { - if (ui->expression->minimumHeight() < height) + if (ui->expression->minimumHeight() < height) { ui->expression->setMinimumHeight(height); + } - if (ui->expression->minimumWidth() < width) + if (ui->expression->minimumWidth() < width) { ui->expression->setMinimumWidth(width); + } minimumWidth = width; } -void DlgExpressionInput::mouseReleaseEvent(QMouseEvent* ev) +void DlgExpressionInput::mouseReleaseEvent(QMouseEvent* event) { - Q_UNUSED(ev); + Q_UNUSED(event); } -void DlgExpressionInput::mousePressEvent(QMouseEvent* ev) +void DlgExpressionInput::mousePressEvent(QMouseEvent* event) { - Q_UNUSED(ev); + Q_UNUSED(event); // The 'FramelessWindowHint' is also set when the background is transparent. if (windowFlags() & Qt::FramelessWindowHint) { //we need to reject the dialog when clicked on the background. As the background is transparent //this is the expected behaviour for the user bool on = ui->expression->completerActive(); - if (!on) + if (!on) { this->reject(); + } } } @@ -459,29 +470,59 @@ static bool isNamePropOk(const QString& nameProp, App::DocumentObject* obj, return true; } -static const int ROLE_DOC = Qt::UserRole; -static const int ROLE_VARSET_NAME = Qt::UserRole + 1; -static const int ROLE_VARSET_LABEL = Qt::UserRole + 2; -static const int ROLE_GROUP = Qt::UserRole + 3; +static const int DocRole = Qt::UserRole; +static const int VarSetNameRole = Qt::UserRole + 1; +static const int VarSetLabelRole = Qt::UserRole + 2; +static const int LevelRole = Qt::UserRole + 3; -static QString getValue(QTreeWidgetItem* item, int role) +static QString getValue(QComboBox* comboBox, int role) { - QVariant variant = item->data(0, role); + QVariant variant = comboBox->currentData(role); return variant.toString(); } +static void storePreferences(const std::string& nameDoc, + const std::string& nameVarSet, + const std::string& nameGroup) +{ + auto paramExpressionEditor = App::GetApplication().GetParameterGroupByPath( + "User parameter:BaseApp/Preferences/ExpressionEditor"); + paramExpressionEditor->SetASCII("LastDocument", nameDoc); + paramExpressionEditor->SetASCII("LastVarSet", nameVarSet); + paramExpressionEditor->SetASCII("LastGroup", nameGroup); +} + +static const App::NumberExpression* toNumberExpr(const App::Expression* expr) +{ + return freecad_cast(expr); +} + +static const App::StringExpression* toStringExpr(const App::Expression* expr) +{ + return freecad_cast(expr); +} + +static const App::OperatorExpression* toUnitNumberExpr(const App::Expression* expr) +{ + auto* opExpr = freecad_cast(expr); + if (opExpr && opExpr->getOperator() == App::OperatorExpression::Operator::UNIT && + toNumberExpr(opExpr->getLeft())) { + return opExpr; + } + return nullptr; +} + void DlgExpressionInput::acceptWithVarSet() { // all checks have been performed in updateVarSetInfo and textChanged that // decide to enable the button // create a property in the VarSet - QTreeWidgetItem *selected = treeWidget->currentItem(); - QString nameVarSet = getValue(selected, ROLE_VARSET_NAME); - QString nameGroup = ui->lineEditGroup->text(); + QString nameVarSet = getValue(ui->comboBoxVarSet, VarSetNameRole); + QString nameGroup = comboBoxGroup.currentText(); QString nameProp = ui->lineEditPropNew->text(); - QString nameDoc = getValue(selected, ROLE_DOC); + QString nameDoc = getValue(ui->comboBoxVarSet, DocRole); App::Document* doc = App::GetApplication().getDocument(nameDoc.toUtf8()); App::DocumentObject* obj = doc->getObject(nameVarSet.toUtf8()); @@ -494,10 +535,8 @@ void DlgExpressionInput::acceptWithVarSet() // // The value of the property is going to be the value that was originally // meant to be the value for the property that this dialog is targeting. - Expression* exprSimplfied = expression->simplify(); - auto ne = freecad_cast(exprSimplfied); - auto se = freecad_cast(exprSimplfied); - if (ne) { + const Expression* expr = expression.get(); + if (const NumberExpression* ne = toNumberExpr(expr)) { // the value is a number: directly assign it to the property instead of // making it an expression in the variable set Gui::Command::doCommand(Gui::Command::Doc, "App.getDocument('%s').getObject('%s').%s = %f", @@ -505,15 +544,22 @@ void DlgExpressionInput::acceptWithVarSet() obj->getNameInDocument(), prop->getName(), ne->getValue()); } - else if (se) { + else if (const StringExpression* se = toStringExpr(expr)) { // the value is a string: directly assign it to the property. Gui::Command::doCommand(Gui::Command::Doc, "App.getDocument('%s').getObject('%s').%s = \"%s\"", obj->getDocument()->getName(), obj->getNameInDocument(), prop->getName(), se->getText().c_str()); } + else if (const OperatorExpression* une = toUnitNumberExpr(expr)) { + // the value is a unit number: directly assign it to the property. + Gui::Command::doCommand(Gui::Command::Doc, "App.getDocument('%s').getObject('%s').%s = \"%s\"", + obj->getDocument()->getName(), + obj->getNameInDocument(), + prop->getName(), une->toString().c_str()); + } else { - // the value is an epxression: make an expression binding in the variable set. + // the value is an expression: make an expression binding in the variable set. ObjectIdentifier objId(*prop); Binding binding; binding.bind(objId); @@ -525,6 +571,8 @@ void DlgExpressionInput::acceptWithVarSet() // for the original property that is the target of this dialog. expression.reset(ExpressionParser::parse(path.getDocumentObject(), prop->getFullName().c_str())); + + storePreferences(nameDoc.toStdString(), nameVarSet.toStdString(), group); } void DlgExpressionInput::accept() { @@ -534,93 +582,149 @@ void DlgExpressionInput::accept() { QDialog::accept(); } -static void addGroupsVarSetComboBox(App::VarSet* varSet, QTreeWidgetItem* varSetItem) +static App::Document* getPreselectedDocument() { - std::vector properties; - std::set namesGroup; - varSet->getPropertyList(properties); - for (auto prop : properties) { - const char* nameGroup = prop->getGroup(); - if (!nameGroup || strcmp(nameGroup, "") == 0) { - namesGroup.insert("Base"); - } - else { - namesGroup.insert(nameGroup); - } + auto paramExpressionEditor = App::GetApplication().GetParameterGroupByPath( + "User parameter:BaseApp/Preferences/ExpressionEditor"); + std::string lastDoc = paramExpressionEditor->GetASCII("LastDocument", ""); + + if (lastDoc.empty()) { + return App::GetApplication().getActiveDocument(); } - for (const auto& nameGroup : namesGroup) { - // the item will be automatically destroyed when the varSetItem will be destroyed - auto item = new QTreeWidgetItem(varSetItem); - item->setText(0, QString::fromStdString(nameGroup)); - item->setData(0, ROLE_GROUP, QString::fromStdString(nameGroup)); - item->setData(0, ROLE_VARSET_NAME, varSetItem->data(0, ROLE_VARSET_NAME)); - item->setData(0, ROLE_VARSET_LABEL, varSetItem->data(0, ROLE_VARSET_LABEL)); - item->setData(0, ROLE_DOC, varSetItem->data(0, ROLE_DOC)); + + App::Document* doc = App::GetApplication().getDocument(lastDoc.c_str()); + if (doc == nullptr) { + return App::GetApplication().getActiveDocument(); } + + return doc; } -static void addVarSetsVarSetComboBox(std::vector& varSets, QTreeWidgetItem* docItem) +int DlgExpressionInput::getVarSetIndex(const App::Document* doc) const { - for (auto varSet : varSets) { - auto vp = freecad_cast( + auto paramExpressionEditor = App::GetApplication().GetParameterGroupByPath( + "User parameter:BaseApp/Preferences/ExpressionEditor"); + std::string lastVarSet = paramExpressionEditor->GetASCII("LastVarSet", "VarSet"); + + auto* model = qobject_cast(ui->comboBoxVarSet->model()); + for (int i = 0; i < model->rowCount(); ++i) { + QStandardItem* item = model->item(i); + if (item->data(DocRole).toString() == QString::fromUtf8(doc->getName()) && + item->data(VarSetNameRole).toString() == QString::fromStdString(lastVarSet)) { + return i; + } + } + + // Select the first varset of the first document (the document is item 0) + return 1; +} + +void DlgExpressionInput::preselectVarSet() +{ + const App::Document* doc = getPreselectedDocument(); + if (doc == nullptr) { + FC_ERR("No active document found"); + } + ui->comboBoxVarSet->setCurrentIndex(getVarSetIndex(doc)); +} + +// Custom delegate to add indentation +class IndentedItemDelegate : public QStyledItemDelegate { +public: + explicit IndentedItemDelegate(QObject *parent = nullptr) : QStyledItemDelegate(parent) {} + + void initStyleOption(QStyleOptionViewItem *option, const QModelIndex &index) const override { + QStyledItemDelegate::initStyleOption(option, index); + + if (index.data(LevelRole) == 1) { + int indentWidth = 20; + option->rect.adjust(indentWidth, 0, 0, 0); + } + } +}; + +static void addVarSetsVarSetComboBox(std::vector& varSets, + QStandardItem* docItem, QStandardItemModel* model) +{ + for (auto* varSet : varSets) { + auto* vp = freecad_cast( Gui::Application::Instance->getViewProvider(varSet)); - // the item will be automatically destroyed when the docItem will be destroyed - auto item = new QTreeWidgetItem(docItem); - item->setIcon(0, vp->getIcon()); - item->setText(0, QString::fromUtf8(varSet->Label.getValue())); - item->setData(0, ROLE_VARSET_LABEL, QString::fromUtf8(varSet->Label.getValue())); - item->setData(0, ROLE_VARSET_NAME, QString::fromUtf8(varSet->getNameInDocument())); - item->setData(0, ROLE_DOC, docItem->data(0, ROLE_DOC)); - addGroupsVarSetComboBox(varSet, item); - } -} - -static void addDocVarSetComboBox(App::Document* doc, QPixmap& docIcon, QTreeWidgetItem* rootItem) -{ - if (!doc->testStatus(App::Document::TempDoc)) { - std::vector varSets; - getVarSetsDocument(varSets, doc); - if (!varSets.empty()) { - // the item will be automatically destroyed when the rootItem will be destroyed - auto item = new QTreeWidgetItem(rootItem); - item->setIcon(0, docIcon); - item->setText(0, QString::fromUtf8(doc->Label.getValue())); - item->setData(0, ROLE_DOC, QByteArray(doc->getName())); - item->setFlags(Qt::ItemIsEnabled); - item->setChildIndicatorPolicy(QTreeWidgetItem::ShowIndicator); - addVarSetsVarSetComboBox(varSets, item); + if (vp == nullptr) { + FC_ERR("No ViewProvider found for VarSet: " << varSet->getNameInDocument()); + continue; } + + // The item will be owned by the model, so no need to delete it manually. + auto item = new QStandardItem(); + item->setIcon(vp->getIcon()); + item->setText(QString::fromUtf8(varSet->Label.getValue())); + item->setData(QString::fromUtf8(varSet->Label.getValue()), VarSetLabelRole); + item->setData(QString::fromUtf8(varSet->getNameInDocument()), VarSetNameRole); + item->setData(docItem->data(DocRole), DocRole); + item->setData(1, LevelRole); + model->appendRow(item); } } -static QTreeWidget* createVarSetTreeWidget() +static void addDocVarSetComboBox(App::Document* doc, QPixmap& docIcon, + QStandardItemModel* model) { - // the caller of the function is responsible of managing the tree widget - auto treeWidget = new QTreeWidget(); - treeWidget->setColumnCount(1); - treeWidget->setHeaderHidden(true); - // the rootItem will be destroyed when the treeWidget will be destroyed - QTreeWidgetItem *rootItem = treeWidget->invisibleRootItem(); + if (doc->testStatus(App::Document::TempDoc)) { + // Do not add temporary documents to the VarSet combo box + return; + } + std::vector varSets; + getVarSetsDocument(varSets, doc); + if (varSets.empty()) { + return; + } + + // The item will be owned by the model, so no need to delete it manually. + auto* item = new QStandardItem(); + item->setIcon(docIcon); + item->setText(QString::fromUtf8(doc->Label.getValue())); + item->setData(QByteArray(doc->getName()), DocRole); + item->setFlags(Qt::ItemIsEnabled); // Make sure this item cannot be selected + item->setData(0, LevelRole); + model->appendRow(item); + + addVarSetsVarSetComboBox(varSets, item, model); +} + +QStandardItemModel* DlgExpressionInput::createVarSetModel() +{ + // Create the model + auto* model = new QStandardItemModel(ui->comboBoxVarSet); + model->setColumnCount(1); + + // Add items to the model QPixmap docIcon(Gui::BitmapFactory().pixmap("Document")); std::vector docs = App::GetApplication().getDocuments(); for (auto doc : docs) { - addDocVarSetComboBox(doc, docIcon, rootItem); + addDocVarSetComboBox(doc, docIcon, model); } - treeWidget->expandAll(); - return treeWidget; + return model; } void DlgExpressionInput::setupVarSets() { ui->comboBoxVarSet->clear(); - // createVarSetTreeWidget returns a dynamically allocated tree widget - // the memory is managed by means of the unique pointer treeWidget. - treeWidget.reset(createVarSetTreeWidget()); - ui->comboBoxVarSet->setModel(treeWidget->model()); - ui->comboBoxVarSet->setView(treeWidget.get()); + + QStandardItemModel* model = createVarSetModel(); + { + QSignalBlocker blocker(ui->comboBoxVarSet); + auto* listView = new QListView(this); + listView->setSelectionMode(QAbstractItemView::SingleSelection); + listView->setModel(model); + ui->comboBoxVarSet->setView(listView); + ui->comboBoxVarSet->setModel(model); + ui->comboBoxVarSet->setItemDelegate(new IndentedItemDelegate(ui->comboBoxVarSet)); + } + + preselectVarSet(); okBtn->setEnabled(false); } @@ -641,22 +745,45 @@ void DlgExpressionInput::onCheckVarSets(int state) { } } -void DlgExpressionInput::onVarSetSelected(int) +void DlgExpressionInput::preselectGroup() { - QTreeWidgetItem* selected = treeWidget->currentItem(); + auto paramExpressionEditor = App::GetApplication().GetParameterGroupByPath( + "User parameter:BaseApp/Preferences/ExpressionEditor"); + std::string lastGroup = paramExpressionEditor->GetASCII("LastGroup", ""); - if (selected) { - // if group is known, fill it in - QVariant variantGroup = selected->data(0, ROLE_GROUP); - if (variantGroup.isValid()) { - ui->lineEditGroup->setText(variantGroup.toString()); - } - else { - ui->lineEditGroup->clear(); - } + if (lastGroup.empty()) { + return; } + int index = comboBoxGroup.findText(QString::fromStdString(lastGroup)); + if (index == -1) { + return; + } + + comboBoxGroup.setCurrentIndex(index); +} + +void DlgExpressionInput::onVarSetSelected([[maybe_unused]] int index) +{ + QString docName = getValue(ui->comboBoxVarSet, DocRole); + QString varSetName = getValue(ui->comboBoxVarSet, VarSetNameRole); + if (docName.isEmpty() || varSetName.isEmpty()) { + FC_ERR("No document or variable set selected"); + return; + } + + App::DocumentObject* varSet = App::GetApplication().getDocument( + docName.toUtf8())->getObject( + varSetName.toUtf8()); + if (varSet == nullptr) { + FC_ERR("Variable set not found: " << varSetName.toStdString()); + return; + } + + DlgAddPropertyVarSet::populateGroup(comboBoxGroup, varSet); + preselectGroup(); updateVarSetInfo(); + ui->lineEditPropNew->setFocus(); } void DlgExpressionInput::onTextChangedGroup(const QString&) @@ -704,11 +831,11 @@ bool DlgExpressionInput::reportGroup(QString& nameGroup) return false; } -bool DlgExpressionInput::reportName(QTreeWidgetItem* item) +bool DlgExpressionInput::reportName() { QString nameProp = ui->lineEditPropNew->text(); - QString nameVarSet = getValue(item, ROLE_VARSET_NAME); - QString nameDoc = getValue(item, ROLE_DOC); + QString nameVarSet = getValue(ui->comboBoxVarSet, VarSetNameRole); + QString nameDoc = getValue(ui->comboBoxVarSet, DocRole); App::Document* doc = App::GetApplication().getDocument(nameDoc.toUtf8()); App::DocumentObject* obj = doc->getObject(nameVarSet.toUtf8()); std::stringstream message; @@ -722,47 +849,30 @@ bool DlgExpressionInput::reportName(QTreeWidgetItem* item) void DlgExpressionInput::updateVarSetInfo(bool checkExpr) { - QTreeWidgetItem* selected = treeWidget->currentItem(); - - if (selected) { - QString nameGroup = ui->lineEditGroup->text(); - if (reportGroup(nameGroup)) { - // needed to report something about the group, so disable the button - okBtn->setEnabled(false); - return; - } - - if (reportName(selected)) { - // needed to report something about the name, so disable the button - okBtn->setEnabled(false); - return; - } - - // QString nameProp = ui->lineEditPropNew->text(); - // QString labelVarSet = getValue(selected, ROLE_VARSET_LABEL); - // QString nameDoc = getValue(selected, ROLE_DOC); - // std::stringstream message; - // message << "Adding property " << nameProp.toStdString() << std::endl - // << "of type " << getType() << std::endl - // << "to variable set " << labelVarSet.toStdString() << std::endl - // << "in group " << nameGroup.toStdString() << std::endl - // << "in document " << nameDoc.toStdString() << "."; - - // reportVarSetInfo(message.str().c_str()); - if (checkExpr) { - // We have to check the text of the expression as well - try { - checkExpression(ui->expression->text()); - okBtn->setEnabled(true); - } - catch (Base::Exception&) { - okBtn->setDisabled(true); - } - } - } - else { + if (reportName()) { + // needed to report something about the name, so disable the button okBtn->setEnabled(false); + return; } + + QString nameGroup = comboBoxGroup.currentText(); + if (reportGroup(nameGroup)) { + // needed to report something about the group, so disable the button + okBtn->setEnabled(false); + return; + } + + if (checkExpr) { + // We have to check the text of the expression as well + try { + checkExpression(ui->expression->text()); + } + catch (Base::Exception&) { + okBtn->setEnabled(false); + } + } + + okBtn->setEnabled(true); } #include "moc_DlgExpressionInput.cpp" diff --git a/src/Gui/Dialogs/DlgExpressionInput.h b/src/Gui/Dialogs/DlgExpressionInput.h index ccb889f432..e50e9ba702 100644 --- a/src/Gui/Dialogs/DlgExpressionInput.h +++ b/src/Gui/Dialogs/DlgExpressionInput.h @@ -25,11 +25,14 @@ #include #include +#include #include #include #include #include +#include "Dialogs/DlgAddPropertyVarSet.h" + namespace Ui { class DlgExpressionInput; } @@ -44,9 +47,7 @@ class Expression; class DocumentObject; } -namespace Gui { - -namespace Dialog { +namespace Gui::Dialog { class GuiExport NumberRange { @@ -83,8 +84,8 @@ public Q_SLOTS: void accept() override; protected: - void mouseReleaseEvent(QMouseEvent*) override; - void mousePressEvent(QMouseEvent*) override; + void mouseReleaseEvent(QMouseEvent* event) override; + void mousePressEvent(QMouseEvent* event) override; private: Base::Type getTypePath(); @@ -92,10 +93,14 @@ private: bool typeOkForVarSet(); void initializeVarSets(); void checkExpression(const QString& text); + int getVarSetIndex(const App::Document* doc) const; + void preselectGroup(); + void preselectVarSet(); + QStandardItemModel* createVarSetModel(); void setupVarSets(); std::string getType(); void reportVarSetInfo(const std::string& message); - bool reportName(QTreeWidgetItem* item); + bool reportName(); bool reportGroup(QString& nameGroup); void updateVarSetInfo(bool checkExpr = true); void acceptWithVarSet(); @@ -104,7 +109,7 @@ private Q_SLOTS: void textChanged(const QString & text); void setDiscarded(); void onCheckVarSets(int state); - void onVarSetSelected(int); + void onVarSetSelected(int index); void onTextChangedGroup(const QString&); void namePropChanged(const QString&); @@ -119,12 +124,13 @@ private: int minimumWidth; bool varSetsVisible; - std::unique_ptr treeWidget; QPushButton* okBtn = nullptr; QPushButton* discardBtn = nullptr; + + EditFinishedComboBox comboBoxGroup; }; } -} + #endif // GUI_DIALOG_EXPRESSIONINPUT_H diff --git a/src/Gui/Dialogs/DlgExpressionInput.ui b/src/Gui/Dialogs/DlgExpressionInput.ui index 04d261470f..a26d728d1d 100644 --- a/src/Gui/Dialogs/DlgExpressionInput.ui +++ b/src/Gui/Dialogs/DlgExpressionInput.ui @@ -195,17 +195,7 @@ - - - - - - 0 - 0 - - - - + @@ -213,13 +203,6 @@ - - - - Variable Set: - - - @@ -227,11 +210,25 @@ - - + + + + Variable Set: + + - - + + + + + 0 + 0 + + + + + + From 15bbf97b635c6274adfedb103566bd14558e3a33 Mon Sep 17 00:00:00 2001 From: Pieter Hijma Date: Fri, 8 Aug 2025 10:11:14 +0200 Subject: [PATCH 04/10] Gui: Process review comments expr dialog Co-authored-by: Max Wilfinger <6246609+maxwxyz@users.noreply.github.com> --- src/Gui/Dialogs/DlgExpressionInput.ui | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Gui/Dialogs/DlgExpressionInput.ui b/src/Gui/Dialogs/DlgExpressionInput.ui index a26d728d1d..cbd3111a36 100644 --- a/src/Gui/Dialogs/DlgExpressionInput.ui +++ b/src/Gui/Dialogs/DlgExpressionInput.ui @@ -182,7 +182,7 @@ - Add to variable set... + Add to Variable Set @@ -199,21 +199,21 @@ - Name: + Name - Group: + Group - Variable Set: + Variable Set From 3f6b0f5b855e63a0c8538a43628492170dc789ca Mon Sep 17 00:00:00 2001 From: Pieter Hijma Date: Mon, 18 Aug 2025 12:10:49 +0200 Subject: [PATCH 05/10] Gui: Process comments DWG expression dialog This commit ensures that the buttons stay at the end of the dialog. --- src/Gui/Dialogs/DlgExpressionInput.cpp | 3 + src/Gui/Dialogs/DlgExpressionInput.ui | 111 +++++++++++-------------- 2 files changed, 51 insertions(+), 63 deletions(-) diff --git a/src/Gui/Dialogs/DlgExpressionInput.cpp b/src/Gui/Dialogs/DlgExpressionInput.cpp index 3f9380597a..831b3ec454 100644 --- a/src/Gui/Dialogs/DlgExpressionInput.cpp +++ b/src/Gui/Dialogs/DlgExpressionInput.cpp @@ -227,6 +227,8 @@ void DlgExpressionInput::initializeVarSets() comboBoxGroup.setInsertPolicy(QComboBox::InsertAtTop); comboBoxGroup.setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Preferred); DlgAddPropertyVarSet::setWidgetForLabel("labelGroup", &comboBoxGroup, ui->formLayout); + setTabOrder(ui->comboBoxVarSet, &comboBoxGroup); + setTabOrder(&comboBoxGroup, ui->lineEditPropNew); std::vector varSets = getAllVarSets(); if (!varSets.empty() && typeOkForVarSet()) { @@ -742,6 +744,7 @@ void DlgExpressionInput::onCheckVarSets(int state) { } else { okBtn->setEnabled(true); // normal expression + adjustSize(); } } diff --git a/src/Gui/Dialogs/DlgExpressionInput.ui b/src/Gui/Dialogs/DlgExpressionInput.ui index cbd3111a36..139a99bfa8 100644 --- a/src/Gui/Dialogs/DlgExpressionInput.ui +++ b/src/Gui/Dialogs/DlgExpressionInput.ui @@ -7,7 +7,7 @@ 0 0 414 - 291 + 252 @@ -29,6 +29,9 @@ 3 + + QLayout::SizeConstraint::SetDefaultConstraint + 0 @@ -166,23 +169,10 @@ - - - - - 0 - 0 - - - - QDialogButtonBox::StandardButton::Ok|QDialogButtonBox::StandardButton::Reset - - - - Add to Variable Set + Store in VarSet... @@ -193,60 +183,56 @@ - - - - - - - Name - - - - - - - Group - - - - - - - Variable Set - - - - - - - - 0 - 0 - - - - - - - - + + + + + Variable Set + + + + + + + + + + Group + + + + + + + Name + + + + + + + + 0 + 0 + + + - - - Qt::Orientation::Vertical + + + + 0 + 0 + - - - 20 - 0 - + + QDialogButtonBox::StandardButton::Ok|QDialogButtonBox::StandardButton::Reset - + @@ -259,7 +245,6 @@ expression - buttonBox From 7eec8b5416fea4d5d74fd7e2ab933dba6f1ca20d Mon Sep 17 00:00:00 2001 From: Pieter Hijma Date: Mon, 18 Aug 2025 17:48:11 +0200 Subject: [PATCH 06/10] Gui: Process comments DWG expression dialog This commit gives an error box on invalid input for the VarSet inputs. --- src/Gui/Dialogs/DlgExpressionInput.cpp | 93 +++++++++++----- src/Gui/Dialogs/DlgExpressionInput.h | 4 +- src/Gui/Dialogs/DlgExpressionInput.ui | 146 ++++++++++++++++++++----- 3 files changed, 189 insertions(+), 54 deletions(-) diff --git a/src/Gui/Dialogs/DlgExpressionInput.cpp b/src/Gui/Dialogs/DlgExpressionInput.cpp index 831b3ec454..15674b0889 100644 --- a/src/Gui/Dialogs/DlgExpressionInput.cpp +++ b/src/Gui/Dialogs/DlgExpressionInput.cpp @@ -207,6 +207,12 @@ bool DlgExpressionInput::typeOkForVarSet() return !determineTypeVarSet().isBad(); } +void DlgExpressionInput::initializeErrorFrame() +{ + ui->errorFrame->setVisible(false); + ui->errorIconLabel->setPixmap(style()->standardIcon(QStyle::SP_MessageBoxCritical).pixmap(QSize(20, 20))); +} + void DlgExpressionInput::initializeVarSets() { #if QT_VERSION >= QT_VERSION_CHECK(6,7,0) @@ -241,6 +247,7 @@ void DlgExpressionInput::initializeVarSets() ui->checkBoxVarSets->setVisible(false); ui->groupBoxVarSets->setVisible(false); } + initializeErrorFrame(); } void NumberRange::setRange(double min, double max) @@ -439,33 +446,45 @@ public: } }; -static bool isNamePropOk(const QString& nameProp, App::DocumentObject* obj, - std::stringstream& message) +constexpr std::string_view InvalidIdentifierMessage = + "must contain only alphanumeric characters, underscore, and must not start with a digit"; + +static bool isPropertyNameValid(const QString& nameProp, App::DocumentObject* obj, + std::stringstream& message) { if (!obj) { message << "Unknown object"; return false; } + const char* prefixText = "Invalid property name: "; + std::string name = nameProp.toStdString(); if (name.empty()) { + message << prefixText << "the name cannot be empty"; return false; } if (name != Base::Tools::getIdentifier(name)) { - message << "Invalid property name (must only contain alphanumericals, underscore, " - << "and must not start with digit"; + message << prefixText << InvalidIdentifierMessage; return false; } - if (ExpressionParser::isTokenAUnit(name) || ExpressionParser::isTokenAConstant(name)) { - message << name << " is a reserved word"; + if (ExpressionParser::isTokenAUnit(name)) { + message << prefixText << name + << " is a unit"; + return false; + } + + if (ExpressionParser::isTokenAConstant(name)) { + message << prefixText << name + << " is a constant"; return false; } auto prop = obj->getPropertyByName(name.c_str()); if (prop && prop->getContainer() == obj) { - message << name << " already exists"; + message << prefixText << name << " already exists"; return false; } @@ -579,6 +598,9 @@ void DlgExpressionInput::acceptWithVarSet() void DlgExpressionInput::accept() { if (varSetsVisible) { + if (needReportOnVarSet()) { + return; + } acceptWithVarSet(); } QDialog::accept(); @@ -766,7 +788,7 @@ void DlgExpressionInput::preselectGroup() comboBoxGroup.setCurrentIndex(index); } -void DlgExpressionInput::onVarSetSelected([[maybe_unused]] int index) +void DlgExpressionInput::onVarSetSelected(int /*index*/) { QString docName = getValue(ui->comboBoxVarSet, DocRole); QString varSetName = getValue(ui->comboBoxVarSet, VarSetNameRole); @@ -799,13 +821,18 @@ void DlgExpressionInput::namePropChanged(const QString&) updateVarSetInfo(); } -static bool isNameGroupOk(const QString& nameGroup, - std::stringstream& message) +static bool isGroupNameValid(const QString& nameGroup, + std::stringstream& message) { + const char* prefixText = "Invalid group name: "; + if(nameGroup.isEmpty()) { + message << prefixText << "the name cannot be empty"; + return false; + } std::string name = nameGroup.toStdString(); - if (name.empty() || name != Base::Tools::getIdentifier(name)) { - message << "Invalid group name (must only contain alphanumericals, underscore, " - << "and must not start with digit"; + + if (name != Base::Tools::getIdentifier(name)) { + message << prefixText << InvalidIdentifierMessage; return false; } @@ -815,18 +842,28 @@ static bool isNameGroupOk(const QString& nameGroup, void DlgExpressionInput::reportVarSetInfo(const std::string& message) { if (!message.empty()) { - FC_ERR(message); + ui->errorFrame->setVisible(true); + ui->errorTextLabel->setText(QString::fromStdString(message)); + ui->errorTextLabel->updateGeometry(); } } -bool DlgExpressionInput::reportGroup(QString& nameGroup) +static QString buildErrorStyle(const char* widgetName) { - if (nameGroup.isEmpty()) { - return true; - } + return QStringLiteral("#%1 {" + " border:1px solid #d93025;" + "}" + "#%1:focus {" + " border:1px solid #ff3b30;" + "}") + .arg(QLatin1String(widgetName)); +} +bool DlgExpressionInput::reportGroup(const QString& nameGroup) +{ std::stringstream message; - if (!isNameGroupOk(nameGroup, message)) { + if (!isGroupNameValid(nameGroup, message)) { + comboBoxGroup.setStyleSheet(buildErrorStyle("comboBoxGroup")); reportVarSetInfo(message.str()); return true; } @@ -842,7 +879,8 @@ bool DlgExpressionInput::reportName() App::Document* doc = App::GetApplication().getDocument(nameDoc.toUtf8()); App::DocumentObject* obj = doc->getObject(nameVarSet.toUtf8()); std::stringstream message; - if (!isNamePropOk(nameProp, obj, message)) { + if (!isPropertyNameValid(nameProp, obj, message)) { + ui->lineEditPropNew->setStyleSheet(buildErrorStyle("lineEditPropNew")); reportVarSetInfo(message.str()); return true; } @@ -852,15 +890,12 @@ bool DlgExpressionInput::reportName() void DlgExpressionInput::updateVarSetInfo(bool checkExpr) { - if (reportName()) { - // needed to report something about the name, so disable the button + if (ui->lineEditPropNew->text().isEmpty()) { okBtn->setEnabled(false); return; } - QString nameGroup = comboBoxGroup.currentText(); - if (reportGroup(nameGroup)) { - // needed to report something about the group, so disable the button + if (comboBoxGroup.currentText().isEmpty()) { okBtn->setEnabled(false); return; } @@ -878,4 +913,12 @@ void DlgExpressionInput::updateVarSetInfo(bool checkExpr) okBtn->setEnabled(true); } +bool DlgExpressionInput::needReportOnVarSet() +{ + ui->lineEditPropNew->setStyleSheet(QString()); + comboBoxGroup.setStyleSheet(QString()); + + return reportGroup(comboBoxGroup.currentText()) || reportName(); +} + #include "moc_DlgExpressionInput.cpp" diff --git a/src/Gui/Dialogs/DlgExpressionInput.h b/src/Gui/Dialogs/DlgExpressionInput.h index e50e9ba702..e8127fa62e 100644 --- a/src/Gui/Dialogs/DlgExpressionInput.h +++ b/src/Gui/Dialogs/DlgExpressionInput.h @@ -91,6 +91,7 @@ private: Base::Type getTypePath(); Base::Type determineTypeVarSet(); bool typeOkForVarSet(); + void initializeErrorFrame(); void initializeVarSets(); void checkExpression(const QString& text); int getVarSetIndex(const App::Document* doc) const; @@ -101,7 +102,7 @@ private: std::string getType(); void reportVarSetInfo(const std::string& message); bool reportName(); - bool reportGroup(QString& nameGroup); + bool reportGroup(const QString& nameGroup); void updateVarSetInfo(bool checkExpr = true); void acceptWithVarSet(); @@ -112,6 +113,7 @@ private Q_SLOTS: void onVarSetSelected(int index); void onTextChangedGroup(const QString&); void namePropChanged(const QString&); + bool needReportOnVarSet(); private: ::Ui::DlgExpressionInput *ui; diff --git a/src/Gui/Dialogs/DlgExpressionInput.ui b/src/Gui/Dialogs/DlgExpressionInput.ui index 139a99bfa8..8736c83604 100644 --- a/src/Gui/Dialogs/DlgExpressionInput.ui +++ b/src/Gui/Dialogs/DlgExpressionInput.ui @@ -7,7 +7,7 @@ 0 0 414 - 252 + 298 @@ -47,6 +47,12 @@ + + + 0 + 0 + + true @@ -183,41 +189,125 @@ - - - - - Variable Set + + + + + true - - - - - - - - - Group - - - - - - - Name - - - - - - + 0 0 + + + 0 + 0 + + + + #errorFrame { border: 1px solid red; border-radius: 5px; background-color: #f8d7da; color: #721c24; } #errorTextLabel { color: #721c24; } + + + QFrame::Shape::StyledPanel + + + QFrame::Shadow::Raised + + + + 5 + + + 5 + + + 5 + + + 5 + + + + + + 0 + 0 + + + + + + + + + + + + 0 + 0 + + + + Error + + + Qt::TextFormat::PlainText + + + Qt::AlignmentFlag::AlignLeading|Qt::AlignmentFlag::AlignLeft|Qt::AlignmentFlag::AlignVCenter + + + true + + + 0 + + + + + + + + + + Variable Set + + + + + + + + + + Group + + + + + + + Name + + + + + + + + 0 + 0 + + + + + + From c0cb0b757e2fc093a7c4ac7a9d90abf46e181da2 Mon Sep 17 00:00:00 2001 From: Pieter Hijma Date: Tue, 26 Aug 2025 16:48:56 +0200 Subject: [PATCH 07/10] Gui: Process minor review comments Co-authored-by: Kacper Donat --- src/Gui/Dialogs/DlgExpressionInput.cpp | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/Gui/Dialogs/DlgExpressionInput.cpp b/src/Gui/Dialogs/DlgExpressionInput.cpp index 15674b0889..af9dff5cd1 100644 --- a/src/Gui/Dialogs/DlgExpressionInput.cpp +++ b/src/Gui/Dialogs/DlgExpressionInput.cpp @@ -210,7 +210,12 @@ bool DlgExpressionInput::typeOkForVarSet() void DlgExpressionInput::initializeErrorFrame() { ui->errorFrame->setVisible(false); - ui->errorIconLabel->setPixmap(style()->standardIcon(QStyle::SP_MessageBoxCritical).pixmap(QSize(20, 20))); + const int size = style()->pixelMetric(QStyle::PM_LargeIconSize); + QIcon icon = Gui::BitmapFactory().iconFromTheme("overlay_error"); + if (icon.isNull()) { + icon = style()->standardIcon(QStyle::SP_MessageBoxCritical); + } + ui->errorIconLabel->setPixmap(icon.pixmap(QSize(size, size))); } void DlgExpressionInput::initializeVarSets() @@ -780,12 +785,9 @@ void DlgExpressionInput::preselectGroup() return; } - int index = comboBoxGroup.findText(QString::fromStdString(lastGroup)); - if (index == -1) { - return; + if (int index = comboBoxGroup.findText(QString::fromStdString(lastGroup)); index != -1) { + comboBoxGroup.setCurrentIndex(index); } - - comboBoxGroup.setCurrentIndex(index); } void DlgExpressionInput::onVarSetSelected(int /*index*/) @@ -797,9 +799,13 @@ void DlgExpressionInput::onVarSetSelected(int /*index*/) return; } - App::DocumentObject* varSet = App::GetApplication().getDocument( - docName.toUtf8())->getObject( - varSetName.toUtf8()); + App::Document* doc = App::GetApplication().getDocument(docName.toUtf8()); + if (doc == nullptr) { + FC_ERR("Document not found: " << docName.toStdString()); + return; + } + + App::DocumentObject* varSet = doc->getObject(varSetName.toUtf8()); if (varSet == nullptr) { FC_ERR("Variable set not found: " << varSetName.toStdString()); return; From 6674a7084be6f591e6d3fa39538912c0bc8052d0 Mon Sep 17 00:00:00 2001 From: Pieter Hijma Date: Tue, 26 Aug 2025 17:09:07 +0200 Subject: [PATCH 08/10] Gui: Add translations expression dialog VarSets --- src/Gui/Dialogs/DlgExpressionInput.cpp | 56 ++++++++++++++------------ src/Gui/Dialogs/DlgExpressionInput.h | 6 ++- 2 files changed, 35 insertions(+), 27 deletions(-) diff --git a/src/Gui/Dialogs/DlgExpressionInput.cpp b/src/Gui/Dialogs/DlgExpressionInput.cpp index af9dff5cd1..ce0b0dc260 100644 --- a/src/Gui/Dialogs/DlgExpressionInput.cpp +++ b/src/Gui/Dialogs/DlgExpressionInput.cpp @@ -451,45 +451,46 @@ public: } }; -constexpr std::string_view InvalidIdentifierMessage = - "must contain only alphanumeric characters, underscore, and must not start with a digit"; +static constexpr const char* InvalidIdentifierMessage = + QT_TR_NOOP("must contain only alphanumeric characters, underscore, and must not start with a digit"); -static bool isPropertyNameValid(const QString& nameProp, App::DocumentObject* obj, - std::stringstream& message) +bool DlgExpressionInput::isPropertyNameValid(const QString& nameProp, + const App::DocumentObject* obj, + QString& message) const { + auto withPrefix = [&](const QString& detail) { + return tr("Invalid property name: %1").arg(detail); + }; + if (!obj) { - message << "Unknown object"; + message = tr("Unknown object"); return false; } - const char* prefixText = "Invalid property name: "; - std::string name = nameProp.toStdString(); if (name.empty()) { - message << prefixText << "the name cannot be empty"; + message = withPrefix(tr("the name cannot be empty")); return false; } if (name != Base::Tools::getIdentifier(name)) { - message << prefixText << InvalidIdentifierMessage; + message = withPrefix(tr(InvalidIdentifierMessage)); return false; } if (ExpressionParser::isTokenAUnit(name)) { - message << prefixText << name - << " is a unit"; + message = withPrefix(tr("%1 is a unit").arg(name)); return false; } if (ExpressionParser::isTokenAConstant(name)) { - message << prefixText << name - << " is a constant"; + message = withPrefix(tr("%1 is a constant").arg(name)); return false; } auto prop = obj->getPropertyByName(name.c_str()); if (prop && prop->getContainer() == obj) { - message << prefixText << name << " already exists"; + message = withPrefix(tr("%1 already exists").arg(name)); return false; } @@ -827,29 +828,32 @@ void DlgExpressionInput::namePropChanged(const QString&) updateVarSetInfo(); } -static bool isGroupNameValid(const QString& nameGroup, - std::stringstream& message) +bool DlgExpressionInput::isGroupNameValid(const QString& nameGroup, + QString& message) const { - const char* prefixText = "Invalid group name: "; + auto withPrefix = [&](const QString& detail) { + return tr("Invalid group name: %1").arg(detail); + }; + if(nameGroup.isEmpty()) { - message << prefixText << "the name cannot be empty"; + message = withPrefix(tr("the name cannot be empty")); return false; } std::string name = nameGroup.toStdString(); if (name != Base::Tools::getIdentifier(name)) { - message << prefixText << InvalidIdentifierMessage; + message = withPrefix(tr(InvalidIdentifierMessage)); return false; } return true; } -void DlgExpressionInput::reportVarSetInfo(const std::string& message) +void DlgExpressionInput::reportVarSetInfo(const QString& message) { - if (!message.empty()) { + if (!message.isEmpty()) { ui->errorFrame->setVisible(true); - ui->errorTextLabel->setText(QString::fromStdString(message)); + ui->errorTextLabel->setText(message); ui->errorTextLabel->updateGeometry(); } } @@ -867,10 +871,10 @@ static QString buildErrorStyle(const char* widgetName) bool DlgExpressionInput::reportGroup(const QString& nameGroup) { - std::stringstream message; + QString message; if (!isGroupNameValid(nameGroup, message)) { comboBoxGroup.setStyleSheet(buildErrorStyle("comboBoxGroup")); - reportVarSetInfo(message.str()); + reportVarSetInfo(message); return true; } @@ -884,10 +888,10 @@ bool DlgExpressionInput::reportName() QString nameDoc = getValue(ui->comboBoxVarSet, DocRole); App::Document* doc = App::GetApplication().getDocument(nameDoc.toUtf8()); App::DocumentObject* obj = doc->getObject(nameVarSet.toUtf8()); - std::stringstream message; + QString message; if (!isPropertyNameValid(nameProp, obj, message)) { ui->lineEditPropNew->setStyleSheet(buildErrorStyle("lineEditPropNew")); - reportVarSetInfo(message.str()); + reportVarSetInfo(message); return true; } diff --git a/src/Gui/Dialogs/DlgExpressionInput.h b/src/Gui/Dialogs/DlgExpressionInput.h index e8127fa62e..ce62aa2d95 100644 --- a/src/Gui/Dialogs/DlgExpressionInput.h +++ b/src/Gui/Dialogs/DlgExpressionInput.h @@ -100,11 +100,15 @@ private: QStandardItemModel* createVarSetModel(); void setupVarSets(); std::string getType(); - void reportVarSetInfo(const std::string& message); + void reportVarSetInfo(const QString& message); bool reportName(); bool reportGroup(const QString& nameGroup); void updateVarSetInfo(bool checkExpr = true); void acceptWithVarSet(); + bool isPropertyNameValid(const QString& nameProp, + const App::DocumentObject* obj, QString& message) const; + bool isGroupNameValid(const QString& nameGroup, + QString& message) const; private Q_SLOTS: void textChanged(const QString & text); From 118cad87deca1e45a82b8776a81ce08dbc63ec21 Mon Sep 17 00:00:00 2001 From: Pieter Hijma Date: Tue, 26 Aug 2025 17:20:21 +0200 Subject: [PATCH 09/10] Gui: Make expr dialog input validation themeable --- src/Gui/Dialogs/DlgExpressionInput.cpp | 21 +++++++++------------ src/Gui/Stylesheets/FreeCAD.qss | 15 +++++++++++++++ 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/src/Gui/Dialogs/DlgExpressionInput.cpp b/src/Gui/Dialogs/DlgExpressionInput.cpp index ce0b0dc260..c623764cd7 100644 --- a/src/Gui/Dialogs/DlgExpressionInput.cpp +++ b/src/Gui/Dialogs/DlgExpressionInput.cpp @@ -858,22 +858,19 @@ void DlgExpressionInput::reportVarSetInfo(const QString& message) } } -static QString buildErrorStyle(const char* widgetName) +static void setErrorState(QWidget* widget, bool on) { - return QStringLiteral("#%1 {" - " border:1px solid #d93025;" - "}" - "#%1:focus {" - " border:1px solid #ff3b30;" - "}") - .arg(QLatin1String(widgetName)); + widget->setProperty("validationState", on ? QStringLiteral("error") : QVariant()); + + widget->style()->unpolish(widget); + widget->style()->polish(widget); } bool DlgExpressionInput::reportGroup(const QString& nameGroup) { QString message; if (!isGroupNameValid(nameGroup, message)) { - comboBoxGroup.setStyleSheet(buildErrorStyle("comboBoxGroup")); + setErrorState(&comboBoxGroup, true); reportVarSetInfo(message); return true; } @@ -890,7 +887,7 @@ bool DlgExpressionInput::reportName() App::DocumentObject* obj = doc->getObject(nameVarSet.toUtf8()); QString message; if (!isPropertyNameValid(nameProp, obj, message)) { - ui->lineEditPropNew->setStyleSheet(buildErrorStyle("lineEditPropNew")); + setErrorState(ui->lineEditPropNew, true); reportVarSetInfo(message); return true; } @@ -925,8 +922,8 @@ void DlgExpressionInput::updateVarSetInfo(bool checkExpr) bool DlgExpressionInput::needReportOnVarSet() { - ui->lineEditPropNew->setStyleSheet(QString()); - comboBoxGroup.setStyleSheet(QString()); + setErrorState(ui->lineEditPropNew, false); + setErrorState(&comboBoxGroup, false); return reportGroup(comboBoxGroup.currentText()) || reportName(); } diff --git a/src/Gui/Stylesheets/FreeCAD.qss b/src/Gui/Stylesheets/FreeCAD.qss index a89192ffc3..902b3f0d8d 100644 --- a/src/Gui/Stylesheets/FreeCAD.qss +++ b/src/Gui/Stylesheets/FreeCAD.qss @@ -2650,3 +2650,18 @@ QMainWindow QHeaderView::section::horizontal::first, QHeaderView::section::horiz Gui--Dialog--DocumentRecovery QTreeView { border: 1px solid @GeneralBorderColor; } + +/*================================================================================================== +Input validation +==================================================================================================*/ +QLineEdit[validationState="error"], +QComboBox[validationState="error"] { + border: 1px solid #d93025; + border-image: none; +} + +QLineEdit[validationState="error"]:focus, +QComboBox[validationState="error"]:focus { + border: 1px solid #ff3b30; + border-image: none; +} From c97305a9fc4c3b78e68524cc4132d05e23ea3cbf Mon Sep 17 00:00:00 2001 From: Pieter Hijma Date: Tue, 26 Aug 2025 17:51:32 +0200 Subject: [PATCH 10/10] Gui: Add more translations to expr dialog This adds translations to the feedback on the expression, for example if there is a unit mismatch. --- src/Gui/Dialogs/DlgExpressionInput.cpp | 31 ++++++++++++++++---------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/src/Gui/Dialogs/DlgExpressionInput.cpp b/src/Gui/Dialogs/DlgExpressionInput.cpp index c623764cd7..78c892f39b 100644 --- a/src/Gui/Dialogs/DlgExpressionInput.cpp +++ b/src/Gui/Dialogs/DlgExpressionInput.cpp @@ -272,13 +272,19 @@ void NumberRange::throwIfOutOfRange(const Base::Quantity& value) const if (!defined) return; + auto toQString = [](const Base::Quantity& v) { + return QString::fromStdString(v.getUserString()); + }; + if (value.getValue() < minimum || value.getValue() > maximum) { Base::Quantity minVal(minimum, value.getUnit()); Base::Quantity maxVal(maximum, value.getUnit()); - auto valStr = value.getUserString(); - auto minStr = minVal.getUserString(); - auto maxStr = maxVal.getUserString(); - throw Base::ValueError(fmt::format("Value out of range ({} out of [{}, {}])", valStr, minStr, maxStr)); + + const QString fmt = QCoreApplication::translate( + "Exceptions", + "Value out of range (%1 out of [%2, %3])"); + const QString msg = fmt.arg(toQString(value), toQString(minVal), toQString(maxVal)); + THROWM(Base::ValueError, msg.toStdString()); } } @@ -321,19 +327,20 @@ void DlgExpressionInput::checkExpression(const QString& text) if (n) { Base::Quantity value = n->getQuantity(); if (!value.isValid()) { - throw Base::ValueError("Not a number"); + THROWMT(Base::ValueError, QT_TRANSLATE_NOOP("Exceptions", "Not a number")); } - auto msg = value.getUserString(); + QString msg = QString::fromStdString(value.getUserString()); if (impliedUnit != Base::Unit::One) { if (!value.isDimensionless() && value.getUnit() != impliedUnit) - throw Base::UnitsMismatchError("Unit mismatch between result and required unit"); + THROWMT(Base::UnitsMismatchError, + QT_TRANSLATE_NOOP("Exceptions", "Unit mismatch between result and required unit")); value.setUnit(impliedUnit); } else if (!value.isDimensionless()) { - msg += " (Warning: unit discarded)"; + msg += tr(" (Warning: unit discarded)"); QPalette p(ui->msg->palette()); p.setColor(QPalette::WindowText, Qt::red); @@ -342,7 +349,7 @@ void DlgExpressionInput::checkExpression(const QString& text) numberRange.throwIfOutOfRange(value); - ui->msg->setText(QString::fromStdString(msg)); + ui->msg->setText(msg); } else { ui->msg->setText(QString::fromStdString(result->toString())); @@ -479,18 +486,18 @@ bool DlgExpressionInput::isPropertyNameValid(const QString& nameProp, } if (ExpressionParser::isTokenAUnit(name)) { - message = withPrefix(tr("%1 is a unit").arg(name)); + message = withPrefix(tr("%1 is a unit").arg(nameProp)); return false; } if (ExpressionParser::isTokenAConstant(name)) { - message = withPrefix(tr("%1 is a constant").arg(name)); + message = withPrefix(tr("%1 is a constant").arg(nameProp)); return false; } auto prop = obj->getPropertyByName(name.c_str()); if (prop && prop->getContainer() == obj) { - message = withPrefix(tr("%1 already exists").arg(name)); + message = withPrefix(tr("%1 already exists").arg(nameProp)); return false; }