Fix review comments from PR #16

- importmbcdialog: give Qt parent to MbcUpdateModel and MbcRegisterFilter;
  remove Qt parent from ActionButtonDelegate (unique_ptr owns it) to fix
  double-delete; call clearAllSelections() after import to deselect all
  source rows, not just proxy-visible ones; always reset update model and
  tab filter before handling new file load
- mainwindow: use suffix() instead of completeSuffix() for extension
  matching; cache to local variable; use QStringLiteral
- mbcheader: render tri-state checkbox correctly using toInt() + bitwise
  state ops; guard toggle on left-click only
- mbcfileimporter: remove Util::showError() GUI calls (breaks GUILESS
  tests); store errors in _lastErrorMessage; caller shows them via
  Util::showError(); validate decimals fit in quint8 range
- mbcregisterfilter: add missing Q_OBJECT macro; add override to
  filterAcceptsRow
- mbcregistermodel: reset _selection in reset(); add clearAllSelections()
- tst_mbcregistermodel: fix include guard to TST_MBCREGISTERMODEL_H
- tst_mbcupdatemodel: replace raw new MbcUpdateModel with unique_ptr

https://claude.ai/code/session_01KvuREfVMg7MnzkpLBg8NVz

Author: claude

src/dialogs/importmbcdialog.cpp

@@ -25,9 +25,9 @@ ImportMbcDialog::ImportMbcDialog(GuiModel* pGuiModel, GraphDataModel* pGraphData
 {
     _pUi->setupUi(this);
 
-    _pMbcUpdateModel = new MbcUpdateModel(_pGraphDataModel);
+    _pMbcUpdateModel = new MbcUpdateModel(_pGraphDataModel, this);
 
-    _pTabProxyFilter = new MbcRegisterFilter();
+    _pTabProxyFilter = new MbcRegisterFilter(this);
     _pTabProxyFilter->setSourceModel(&_mbcRegisterModel);
 
     /* Disable question mark button */
@@ -69,7 +69,7 @@ ImportMbcDialog::ImportMbcDialog(GuiModel* pGuiModel, GraphDataModel* pGraphData
 
     _pUi->tblMbcUpdate->setFocusPolicy(Qt::NoFocus);
 
-    _pUpdateDelegate = std::make_unique<ActionButtonDelegate>(_pUi->tblMbcUpdate);
+    _pUpdateDelegate = std::make_unique<ActionButtonDelegate>();
     _pUpdateDelegate->setCharacter(QChar(0x2190));
     connect(_pUpdateDelegate.get(), &ActionButtonDelegate::clicked, this, &ImportMbcDialog::handleAcceptUpdate);
 
@@ -152,7 +152,7 @@ void ImportMbcDialog::importSelectedRegisters()
         _pGraphDataModel->add(regList);
     }
 
-    setSelectedSelectionstate(Qt::Unchecked);
+    _mbcRegisterModel.clearAllSelections();
 }
 
 void ImportMbcDialog::visibleItemsDataChanged()
@@ -287,18 +287,23 @@ void ImportMbcDialog::updateMbcRegisters(QString filePath)
 
         /* Clear data from table widget */
         _mbcRegisterModel.reset();
+        _pMbcUpdateModel->setMbcRegisters(QList<MbcRegisterData>());
+        _pUi->cmbTabFilter->clear();
+        _pUi->cmbTabFilter->addItem(MbcRegisterFilter::cTabNoFilter);
         registerDataChanged();
 
-        if (registerList.size() > 0)
+        if (!registerList.isEmpty())
         {
             _mbcRegisterModel.fill(registerList, tabList);
             _pMbcUpdateModel->setMbcRegisters(registerList);
 
             /* Update combo box */
-            _pUi->cmbTabFilter->clear();
-            _pUi->cmbTabFilter->addItem(MbcRegisterFilter::cTabNoFilter);
             _pUi->cmbTabFilter->addItems(tabList);
         }
+        else if (!fileImporter.lastErrorMessage().isEmpty())
+        {
+            Util::showError(fileImporter.lastErrorMessage());
+        }
     }
     else
     {

src/dialogs/mainwindow.cpp

@@ -866,11 +866,12 @@ void MainWindow::handleFileOpen(QString filename)
 {
     QFileInfo fileInfo(filename);
     _pGuiModel->setLastDir(fileInfo.dir().absolutePath());
-    if (fileInfo.completeSuffix().toLower() == QString("mbs"))
+    const QString suffix = fileInfo.suffix().toLower();
+    if (suffix == QStringLiteral("mbs"))
     {
         _pProjectFileHandler->openProjectFile(filename);
     }
-    else if (fileInfo.completeSuffix().toLower() == QString("mbc"))
+    else if (suffix == QStringLiteral("mbc"))
     {
         _pGuiModel->setLastMbcImportedFile(filename);
         showMbcImportDialog();

src/dialogs/mbcheader.h

@@ -32,10 +32,23 @@ class MbcHeader : public QHeaderView
             QRect checkbox_rect = style()->subElementRect(QStyle::SubElement::SE_CheckBoxIndicator, &option, this);
             checkbox_rect.moveCenter(rect.center());
 
-            bool checked = model()->headerData(logicalIndex, orientation(), Qt::CheckStateRole).toBool();
+            const auto headerState = static_cast<Qt::CheckState>(
+                model()->headerData(logicalIndex, orientation(), Qt::CheckStateRole).toInt());
 
             option.rect = checkbox_rect;
-            option.state = checked ? QStyle::State_On : QStyle::State_Off;
+            option.state &= ~(QStyle::State_On | QStyle::State_Off | QStyle::State_NoChange);
+            if (headerState == Qt::Checked)
+            {
+                option.state |= QStyle::State_On;
+            }
+            else if (headerState == Qt::PartiallyChecked)
+            {
+                option.state |= QStyle::State_NoChange;
+            }
+            else
+            {
+                option.state |= QStyle::State_Off;
+            }
 
             style()->drawPrimitive(QStyle::PE_IndicatorCheckBox, &option, painter);
         }
@@ -44,7 +57,7 @@ class MbcHeader : public QHeaderView
     void mouseReleaseEvent(QMouseEvent* event) override
     {
         QHeaderView::mouseReleaseEvent(event);
-        if (model())
+        if (model() && event->button() == Qt::LeftButton)
         {
             int section = logicalIndexAt(event->pos());
             if (section == 0)

src/importexport/mbcfileimporter.cpp

@@ -1,7 +1,8 @@
 #include "mbcfileimporter.h"
 
 #include "importexport/mbcregisterdata.h"
-#include "util/util.h"
+
+#include <limits>
 
 /*!
  * \brief Constructs the importer and immediately parses the given MBC file content.
@@ -28,13 +29,22 @@ QStringList MbcFileImporter::tabList()
     return _tabList;
 }
 
+/*!
+ * \brief Returns the error message from the last parse attempt, or an empty string on success.
+ */
+QString MbcFileImporter::lastErrorMessage()
+{
+    return _lastErrorMessage;
+}
+
 void MbcFileImporter::parseRegisters(QString* pMbcFileContent)
 {
     bool bRet = true;
 
     /* Clear register and tab list */
     _registerList.clear();
     _tabList.clear();
+    _lastErrorMessage.clear();
 
     QDomDocument domDocument;
     QDomDocument::ParseResult result =
@@ -65,16 +75,16 @@ void MbcFileImporter::parseRegisters(QString* pMbcFileContent)
         }
         else
         {
-            Util::showError(tr("The file is not a valid ModbusControl project file."));
+            _lastErrorMessage = tr("The file is not a valid ModbusControl project file.");
             bRet = false;
         }
     }
     else
     {
-        Util::showError(tr("Parse error at line %1, column %2:\n%3")
-                          .arg(result.errorLine)
-                          .arg(result.errorColumn)
-                          .arg(result.errorMessage));
+        _lastErrorMessage = tr("Parse error at line %1, column %2:\n%3")
+                              .arg(result.errorLine)
+                              .arg(result.errorColumn)
+                              .arg(result.errorMessage);
         bRet = false;
     }
 
@@ -112,7 +122,7 @@ bool MbcFileImporter::parseTabTag(const QDomElement &element)
             }
             else
             {
-                Util::showError(tr("No name tag in tab tag"));
+                _lastErrorMessage = tr("No name tag in tab tag");
                 bRet = false;
                 break;
             }
@@ -252,7 +262,15 @@ bool MbcFileImporter::parseVarTag(const QDomElement &element, qint32 tabIdx)
         {
             if (!decimals.isEmpty())
             {
-                modbusRegister.setDecimals(static_cast<quint16>(decimals.toUInt(&bRet)));
+                const auto parsedDecimals = decimals.toUInt(&bRet);
+                if (bRet && parsedDecimals <= std::numeric_limits<quint8>::max())
+                {
+                    modbusRegister.setDecimals(static_cast<quint8>(parsedDecimals));
+                }
+                else
+                {
+                    bRet = false;
+                }
             }
         }
 
@@ -274,9 +292,9 @@ bool MbcFileImporter::parseVarTag(const QDomElement &element, qint32 tabIdx)
         }
         else
         {
-            Util::showError(tr("A tag is not present or value is not valid.\n\nName: %1\nRegister address: %2\nType: %3\nDecimals: %4")
-                                .arg(name, addr, strType, decimals)
-                            );
+            _lastErrorMessage =
+              tr("A tag is not present or value is not valid.\n\nName: %1\nRegister address: %2\nType: %3\nDecimals: %4")
+                .arg(name, addr, strType, decimals);
         }
 
     }

src/importexport/mbcfileimporter.h

@@ -31,6 +31,7 @@ class MbcFileImporter : public QObject
 
     QList <MbcRegisterData> registerList();
     QStringList tabList();
+    QString lastErrorMessage();
 
 signals:
 
@@ -45,6 +46,7 @@ public slots:
 
     QList <MbcRegisterData> _registerList;
     QStringList _tabList;
+    QString _lastErrorMessage;
 };
 
 #endif // MBCFILEIMPORTER_H

src/models/mbcregisterfilter.h

@@ -5,10 +5,11 @@
 
 class MbcRegisterFilter : public QSortFilterProxyModel
 {
+    Q_OBJECT
 
 public:
     MbcRegisterFilter(QObject* parent = nullptr);
-    bool filterAcceptsRow(int source_row, const QModelIndex &source_parent) const;
+    bool filterAcceptsRow(int source_row, const QModelIndex &source_parent) const override;
 
     static const QString cTabNoFilter;
 

src/models/mbcregistermodel.cpp

@@ -257,6 +257,7 @@ void MbcRegisterModel::reset()
 
     _mbcRegisterList.clear();
     _tabList.clear();
+    _selection = Qt::Unchecked;
 
     endResetModel();
 }
@@ -363,3 +364,19 @@ quint32 MbcRegisterModel::selectedRegisterCount()
 
     return cnt;
 }
+
+/*!
+ * \brief Unchecks every row in the source model regardless of current filter state.
+ */
+void MbcRegisterModel::clearAllSelections()
+{
+    for (MbcRegister& row : _mbcRegisterList)
+    {
+        row.bSelected = false;
+    }
+
+    if (!_mbcRegisterList.isEmpty())
+    {
+        emit dataChanged(this->index(0, 0), this->index(rowCount() - 1, 0));
+    }
+}

src/models/mbcregistermodel.h

@@ -30,6 +30,7 @@ class MbcRegisterModel : public QAbstractTableModel
     bool setData(const QModelIndex & index, const QVariant & value, int role) override;
 
     void setSelectionstate(QList<QModelIndex>& indexList, Qt::CheckState state);
+    void clearAllSelections();
 
     void reset();
 

tests/models/tst_mbcregistermodel.h

@@ -1,5 +1,5 @@
-#ifndef TEST_MBCREGISTERMODEL_H
-#define TEST_MBCREGISTERMODEL_H
+#ifndef TST_MBCREGISTERMODEL_H
+#define TST_MBCREGISTERMODEL_H
 
 #include <QObject>
 
@@ -46,4 +46,4 @@ private slots:
 
 };
 
-#endif /* TEST_MBCREGISTERMODEL_H */
+#endif /* TST_MBCREGISTERMODEL_H */