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 <bstoeger@mail.tuwien.ac.at>

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::TYPE), cylinderName, CylindersModel::TEMP_ROLE);
-	mymodel->setData(IDX(CylindersModel::WORKINGPRESS), tankPressure, CylindersModel::PASS_IN_ROLE);
+	mymodel->setData(IDX(CylindersModel::WORKINGPRESS), tankPressure, CylindersModel::TEMP_ROLE);
-	mymodel->setData(IDX(CylindersModel::SIZE), tankSize, CylindersModel::PASS_IN_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 ||
+	QAbstractItemModel *mymodel = currCombo.model;
-	    hint == QAbstractItemDelegate::RevertModelCache) {
+	// Ugly hack: We misuse setData() with COMMIT_ROLE or REVERT_ROLE to commit or
-		QAbstractItemModel *mymodel = currCombo.model;
+	// revert the current row. We send in the type, because we may get multiple
-		mymodel->setData(IDX(CylindersModel::TYPE), currCylinderData.type, Qt::EditRole);
+	// end events and thus can prevent multiple commits.
-		mymodel->setData(IDX(CylindersModel::WORKINGPRESS), currCylinderData.pressure, CylindersModel::PASS_IN_ROLE);
+	if (hint == QAbstractItemDelegate::RevertModelCache)
-		mymodel->setData(IDX(CylindersModel::SIZE), currCylinderData.size, CylindersModel::PASS_IN_ROLE);
+		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<QString>();
-	currCylinderData.pressure = model->data(model->index(row, CylindersModel::WORKINGPRESS_INT)).value<int>();
-	currCylinderData.size = model->data(model->index(row, CylindersModel::SIZE_INT)).value<int>();
 	MainWindow::instance()->graphics->setReplot(false);
 	return delegate;
 }

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) {
+	// Here we handle a few cases that allow us to set / commit / revert
-		// this is our magic 'pass data in' function that allows the delegate to get
+	// a temporary row. This is a horribly misuse of the model/view system.
-		// the data here without silly unit conversions;
+	// The reason it is done this way is that the planner and the equipment
-		// so we only implement the two columns we care about
+	// 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()) {
+			if (tempCyl.type.size.mliter != value.toInt()) {
-				cyl->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()) {
+			if (tempCyl.type.workingpressure.mbar != value.toInt()) {
-				cyl->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<dive *> &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);

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