Gui: Process comments DWG expression dialog

This commit gives an error box on invalid input for the VarSet inputs.

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"

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;

src/Gui/Dialogs/DlgExpressionInput.ui

@@ -7,7 +7,7 @@
     <x>0</x>
     <y>0</y>
     <width>414</width>
-    <height>252</height>
+    <height>298</height>
    </rect>
   </property>
   <property name="sizePolicy">
@@ -47,6 +47,12 @@
        </property>
        <item>
         <widget class="QFrame" name="ctrlArea">
+         <property name="sizePolicy">
+          <sizepolicy hsizetype="Preferred" vsizetype="Fixed">
+           <horstretch>0</horstretch>
+           <verstretch>0</verstretch>
+          </sizepolicy>
+         </property>
          <property name="autoFillBackground">
           <bool>true</bool>
          </property>
@@ -183,41 +189,125 @@
      <property name="title">
       <string/>
      </property>
-     <layout class="QFormLayout" name="formLayout">
-      <item row="0" column="0">
-       <widget class="QLabel" name="labelVarSet">
-        <property name="text">
-         <string>Variable Set</string>
+     <layout class="QVBoxLayout" name="verticalLayout_3">
+      <item>
+       <widget class="QFrame" name="errorFrame">
+        <property name="enabled">
+         <bool>true</bool>
         </property>
-       </widget>
-      </item>
-      <item row="0" column="1">
-       <widget class="QComboBox" name="comboBoxVarSet"/>
-      </item>
-      <item row="1" column="0">
-       <widget class="QLabel" name="labelGroup">
-        <property name="text">
-         <string>Group</string>
-        </property>
-       </widget>
-      </item>
-      <item row="2" column="0">
-       <widget class="QLabel" name="LabelPropNew">
-        <property name="text">
-         <string>Name</string>
-        </property>
-       </widget>
-      </item>
-      <item row="2" column="1">
-       <widget class="QLineEdit" name="lineEditPropNew">
         <property name="sizePolicy">
-         <sizepolicy hsizetype="Preferred" vsizetype="Fixed">
+         <sizepolicy hsizetype="Preferred" vsizetype="Preferred">
           <horstretch>0</horstretch>
           <verstretch>0</verstretch>
          </sizepolicy>
         </property>
+        <property name="minimumSize">
+         <size>
+          <width>0</width>
+          <height>0</height>
+         </size>
+        </property>
+        <property name="styleSheet">
+         <string notr="true">#errorFrame { border: 1px solid red; border-radius: 5px; background-color: #f8d7da; color: #721c24; } #errorTextLabel { color: #721c24; }</string>
+        </property>
+        <property name="frameShape">
+         <enum>QFrame::Shape::StyledPanel</enum>
+        </property>
+        <property name="frameShadow">
+         <enum>QFrame::Shadow::Raised</enum>
+        </property>
+        <layout class="QHBoxLayout" name="horizontalLayout">
+         <property name="leftMargin">
+          <number>5</number>
+         </property>
+         <property name="topMargin">
+          <number>5</number>
+         </property>
+         <property name="rightMargin">
+          <number>5</number>
+         </property>
+         <property name="bottomMargin">
+          <number>5</number>
+         </property>
+         <item>
+          <widget class="QLabel" name="errorIconLabel">
+           <property name="sizePolicy">
+            <sizepolicy hsizetype="Fixed" vsizetype="Minimum">
+             <horstretch>0</horstretch>
+             <verstretch>0</verstretch>
+            </sizepolicy>
+           </property>
+           <property name="text">
+            <string/>
+           </property>
+          </widget>
+         </item>
+         <item>
+          <widget class="QLabel" name="errorTextLabel">
+           <property name="sizePolicy">
+            <sizepolicy hsizetype="Preferred" vsizetype="MinimumExpanding">
+             <horstretch>0</horstretch>
+             <verstretch>0</verstretch>
+            </sizepolicy>
+           </property>
+           <property name="text">
+            <string>Error</string>
+           </property>
+           <property name="textFormat">
+            <enum>Qt::TextFormat::PlainText</enum>
+           </property>
+           <property name="alignment">
+            <set>Qt::AlignmentFlag::AlignLeading|Qt::AlignmentFlag::AlignLeft|Qt::AlignmentFlag::AlignVCenter</set>
+           </property>
+           <property name="wordWrap">
+            <bool>true</bool>
+           </property>
+           <property name="margin">
+            <number>0</number>
+           </property>
+          </widget>
+         </item>
+        </layout>
        </widget>
       </item>
+      <item>
+       <layout class="QFormLayout" name="formLayout">
+        <item row="0" column="0">
+         <widget class="QLabel" name="labelVarSet">
+          <property name="text">
+           <string>Variable Set</string>
+          </property>
+         </widget>
+        </item>
+        <item row="0" column="1">
+         <widget class="QComboBox" name="comboBoxVarSet"/>
+        </item>
+        <item row="1" column="0">
+         <widget class="QLabel" name="labelGroup">
+          <property name="text">
+           <string>Group</string>
+          </property>
+         </widget>
+        </item>
+        <item row="2" column="0">
+         <widget class="QLabel" name="LabelPropNew">
+          <property name="text">
+           <string>Name</string>
+          </property>
+         </widget>
+        </item>
+        <item row="2" column="1">
+         <widget class="QLineEdit" name="lineEditPropNew">
+          <property name="sizePolicy">
+           <sizepolicy hsizetype="Preferred" vsizetype="Fixed">
+            <horstretch>0</horstretch>
+            <verstretch>0</verstretch>
+           </sizepolicy>
+          </property>
+         </widget>
+        </item>
+       </layout>
+      </item>
      </layout>
     </widget>
    </item>