From 1dcc885bb257d6c8c64074f8788b468397c34aaa Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Fri, 28 Feb 2020 20:38:04 +0100 Subject: [PATCH] undo/cylinders: Implement editing of the type This one is tricky, as when browsing through the types-combobox, the user is presented with presets without actually changing the dive. We do not want an undo-command for every change-event in the combo-box. Therefore, implement a scheme analoguous to the weight-editing: A temporary row can be set / committed or reset. Sadly, the code is more complex because we have to consider the planner, which is not included in the undo system. Firstly, the planner uses a different model, therefore all interactions are channeled through setData() with special roles. Secondly, in the planner we shouldn't place an undo command, but simply overwrite the dive. Signed-off-by: Berthold Stoeger --- desktop-widgets/modeldelegates.cpp | 34 ++++------- qt-models/cylindermodel.cpp | 95 ++++++++++++++++++++++++++---- qt-models/cylindermodel.h | 11 +++- 3 files changed, 106 insertions(+), 34 deletions(-) diff --git a/desktop-widgets/modeldelegates.cpp b/desktop-widgets/modeldelegates.cpp index ed7278b4c..65e8f377e 100644 --- a/desktop-widgets/modeldelegates.cpp +++ b/desktop-widgets/modeldelegates.cpp @@ -230,12 +230,6 @@ void ComboBoxDelegate::updateEditorGeometry(QWidget *editor, const QStyleOptionV editor->setGeometry(defaultRect); } -static struct RevertCylinderData { - QString type; - int pressure; - int size; -} currCylinderData; - void TankInfoDelegate::setModelData(QWidget*, QAbstractItemModel*, const QModelIndex&) const { QAbstractItemModel *mymodel = currCombo.model; @@ -254,9 +248,9 @@ void TankInfoDelegate::setModelData(QWidget*, QAbstractItemModel*, const QModelI int tankSize = tanks->data(tanks->index(row, TankInfoModel::ML)).toInt(); int tankPressure = tanks->data(tanks->index(row, TankInfoModel::BAR)).toInt(); - mymodel->setData(IDX(CylindersModel::TYPE), cylinderName, Qt::EditRole); - mymodel->setData(IDX(CylindersModel::WORKINGPRESS), tankPressure, CylindersModel::PASS_IN_ROLE); - mymodel->setData(IDX(CylindersModel::SIZE), tankSize, CylindersModel::PASS_IN_ROLE); + mymodel->setData(IDX(CylindersModel::TYPE), cylinderName, CylindersModel::TEMP_ROLE); + mymodel->setData(IDX(CylindersModel::WORKINGPRESS), tankPressure, CylindersModel::TEMP_ROLE); + mymodel->setData(IDX(CylindersModel::SIZE), tankSize, CylindersModel::TEMP_ROLE); } TankInfoDelegate::TankInfoDelegate(QObject *parent) : ComboBoxDelegate(TankInfoModel::instance(), parent, true) @@ -275,25 +269,19 @@ void TankInfoDelegate::reenableReplot(QWidget*, QAbstractItemDelegate::EndEditHi void TankInfoDelegate::editorClosed(QWidget*, QAbstractItemDelegate::EndEditHint hint) { - if (hint == QAbstractItemDelegate::NoHint || - hint == QAbstractItemDelegate::RevertModelCache) { - QAbstractItemModel *mymodel = currCombo.model; - mymodel->setData(IDX(CylindersModel::TYPE), currCylinderData.type, Qt::EditRole); - mymodel->setData(IDX(CylindersModel::WORKINGPRESS), currCylinderData.pressure, CylindersModel::PASS_IN_ROLE); - mymodel->setData(IDX(CylindersModel::SIZE), currCylinderData.size, CylindersModel::PASS_IN_ROLE); - } + QAbstractItemModel *mymodel = currCombo.model; + // Ugly hack: We misuse setData() with COMMIT_ROLE or REVERT_ROLE to commit or + // revert the current row. We send in the type, because we may get multiple + // end events and thus can prevent multiple commits. + if (hint == QAbstractItemDelegate::RevertModelCache) + mymodel->setData(IDX(CylindersModel::TYPE), currCombo.activeText, CylindersModel::REVERT_ROLE); + else + mymodel->setData(IDX(CylindersModel::TYPE), currCombo.activeText, CylindersModel::COMMIT_ROLE); } QWidget *TankInfoDelegate::createEditor(QWidget *parent, const QStyleOptionViewItem &option, const QModelIndex &index) const { - // ncreate editor needs to be called before because it will populate a few - // things in the currCombo global var. QWidget *delegate = ComboBoxDelegate::createEditor(parent, option, index); - QAbstractItemModel *model = currCombo.model; - int row = index.row(); - currCylinderData.type = model->data(model->index(row, CylindersModel::TYPE)).value(); - currCylinderData.pressure = model->data(model->index(row, CylindersModel::WORKINGPRESS_INT)).value(); - currCylinderData.size = model->data(model->index(row, CylindersModel::SIZE_INT)).value(); MainWindow::instance()->graphics->setReplot(false); return delegate; } diff --git a/qt-models/cylindermodel.cpp b/qt-models/cylindermodel.cpp index d76cdb344..f2ee0c12e 100644 --- a/qt-models/cylindermodel.cpp +++ b/qt-models/cylindermodel.cpp @@ -2,6 +2,7 @@ #include "cylindermodel.h" #include "tankinfomodel.h" #include "models.h" +#include "commands/command.h" #include "core/qthelper.h" #include "core/color.h" #include "qt-models/diveplannermodel.h" @@ -10,7 +11,9 @@ #include "core/subsurface-string.h" CylindersModel::CylindersModel(QObject *parent) : CleanerTableModel(parent), - d(nullptr) + d(nullptr), + tempRow(-1), + tempCyl(empty_cylinder) { // enum {REMOVE, TYPE, SIZE, WORKINGPRESS, START, END, O2, HE, DEPTH, MOD, MND, USE, IS_USED}; setHeaderDataStrings(QStringList() << "" << tr("Type") << tr("Size") << tr("Work press.") << tr("Start press.") << tr("End press.") << tr("O₂%") << tr("He%") @@ -155,7 +158,7 @@ QVariant CylindersModel::data(const QModelIndex &index, int role) const return QVariant(); } - const cylinder_t *cyl = get_cylinder(d, index.row()); + const cylinder_t *cyl = index.row() == tempRow ? &tempCyl : get_cylinder(d, index.row()); switch (role) { case Qt::BackgroundRole: { @@ -299,25 +302,49 @@ bool CylindersModel::setData(const QModelIndex &index, const QVariant &value, in if (!cyl) return false; - if (role == PASS_IN_ROLE) { - // this is our magic 'pass data in' function that allows the delegate to get - // the data here without silly unit conversions; - // so we only implement the two columns we care about + // Here we handle a few cases that allow us to set / commit / revert + // a temporary row. This is a horribly misuse of the model/view system. + // The reason it is done this way is that the planner and the equipment + // tab use different model-classes which are not in a superclass / subclass + // relationship. + switch (role) { + case TEMP_ROLE: + // TEMP_ROLE means that we are not supposed to write through to the + // actual dive, but a temporary cylinder that is displayed while the + // user browses throught the cylinder types. + initTempCyl(index.row()); + switch (index.column()) { + case TYPE: { + QString type = value.toString(); + if (!same_string(qPrintable(type), tempCyl.type.description)) { + free((void *)tempCyl.type.description); + tempCyl.type.description = strdup(qPrintable(type)); + dataChanged(index, index); + } + } case SIZE: - if (cyl->type.size.mliter != value.toInt()) { - cyl->type.size.mliter = value.toInt(); + if (tempCyl.type.size.mliter != value.toInt()) { + tempCyl.type.size.mliter = value.toInt(); dataChanged(index, index); } return true; case WORKINGPRESS: - if (cyl->type.workingpressure.mbar != value.toInt()) { - cyl->type.workingpressure.mbar = value.toInt(); + if (tempCyl.type.workingpressure.mbar != value.toInt()) { + tempCyl.type.workingpressure.mbar = value.toInt(); dataChanged(index, index); } return true; } return false; + case COMMIT_ROLE: + commitTempCyl(index.row()); + return true; + case REVERT_ROLE: + clearTempCyl(); + return true; + default: + break; } QString vString = value.toString(); @@ -621,6 +648,54 @@ void CylindersModel::cylindersReset(const QVector &dives) endResetModel(); } +// Save the cylinder in the given row so that we can revert if the user cancels a type-editing action. +void CylindersModel::initTempCyl(int row) +{ + if (!d || tempRow == row) + return; + clearTempCyl(); + const cylinder_t *cyl = get_cylinder(d, row); + if (!cyl) + return; + + tempRow = row; + tempCyl = clone_cylinder(*cyl); + + dataChanged(index(row, TYPE), index(row, USE)); +} + +void CylindersModel::clearTempCyl() +{ + if (tempRow < 0) + return; + int oldRow = tempRow; + tempRow = -1; + free_cylinder(tempCyl); + dataChanged(index(oldRow, TYPE), index(oldRow, USE)); +} + +void CylindersModel::commitTempCyl(int row) +{ +#ifndef SUBSURFACE_MOBILE + if (tempRow < 0) + return; + if (row != tempRow) + return clearTempCyl(); // Huh? We are supposed to commit a different row than the one we stored? + cylinder_t *cyl = get_cylinder(d, tempRow); + if (!cyl) + return; + // Only submit a command if the type changed + if (!same_string(cyl->type.description, tempCyl.type.description) || gettextFromC::tr(cyl->type.description) != QString(tempCyl.type.description)) { + if (in_planner()) + std::swap(*cyl, tempCyl); + else + Command::editCylinder(tempRow, tempCyl, false); + } + free_cylinder(tempCyl); + tempRow = -1; +#endif +} + CylindersModelFiltered::CylindersModelFiltered(QObject *parent) : QSortFilterProxyModel(parent) { setSourceModel(&source); diff --git a/qt-models/cylindermodel.h b/qt-models/cylindermodel.h index d3d87b900..e4585b0ac 100644 --- a/qt-models/cylindermodel.h +++ b/qt-models/cylindermodel.h @@ -31,7 +31,9 @@ public: }; enum Roles { - PASS_IN_ROLE = Qt::UserRole + 1 // For setting data: don't do any conversions + TEMP_ROLE = Qt::UserRole + 1, // Temporarily set data, but don't store in dive + COMMIT_ROLE, // Save the temporary data to the dive. Must be set with Column == TYPE. + REVERT_ROLE // Revert to original data from dive. Must be set with Column == TYPE. }; explicit CylindersModel(QObject *parent = 0); QVariant data(const QModelIndex &index, int role = Qt::DisplayRole) const override; @@ -59,7 +61,14 @@ slots: private: dive *d; + // Used if we temporarily change a line because the user is selecting a weight type + int tempRow; + cylinder_t tempCyl; + cylinder_t *cylinderAt(const QModelIndex &index); + void initTempCyl(int row); + void clearTempCyl(); + void commitTempCyl(int row); }; // Cylinder model that hides unused cylinders if the pref.show_unused_cylinders flag is not set