undo: more fine-grained editing of cylinder

Don't overwrite the full cylinder when editing a single field.
Implement three "modes": editing of type, pressure and gasmix.

Don't consider individual fields, because some of them are
related. E.g. you can change the gasmix by setting the MOD.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>

commands/command.cpp

@@ -304,9 +304,9 @@ int removeCylinder(int index, bool currentDiveOnly)
 	return execute_edit(new RemoveCylinder(index, currentDiveOnly));
 }
 
-int editCylinder(int index, cylinder_t cyl, bool currentDiveOnly)
+int editCylinder(int index, cylinder_t cyl, EditCylinderType type, bool currentDiveOnly)
 {
-	return execute_edit(new EditCylinder(index, cyl, currentDiveOnly));
+	return execute_edit(new EditCylinder(index, cyl, type, currentDiveOnly));
 }
 
 // Trip editing related commands

commands/command.h

@@ -92,7 +92,12 @@ int removeWeight(int index, bool currentDiveOnly);
 int editWeight(int index, weightsystem_t ws, bool currentDiveOnly);
 int addCylinder(bool currentDiveOnly);
 int removeCylinder(int index, bool currentDiveOnly);
-int editCylinder(int index, cylinder_t cyl, bool currentDiveOnly);
+enum class EditCylinderType {
+	TYPE,
+	PRESSURE,
+	GASMIX
+};
+int editCylinder(int index, cylinder_t cyl, EditCylinderType type, bool currentDiveOnly);
 #ifdef SUBSURFACE_MOBILE
 // Edits a dive and creates a divesite (if createDs != NULL) or edits a divesite (if changeDs != NULL).
 // Takes ownership of newDive and createDs!

commands/command_edit.cpp

@@ -1151,9 +1151,23 @@ void RemoveCylinder::redo()
 	}
 }
 
+static int editCylinderTypeToFlags(EditCylinderType type)
+{
+	switch (type) {
+	default:
+	case EditCylinderType::TYPE:
+		return SAME_TYPE | SAME_SIZE;
+	case EditCylinderType::PRESSURE:
+		return SAME_PRESS;
+	case EditCylinderType::GASMIX:
+		return SAME_GAS;
+	}
+}
+
 // ***** Edit Cylinder *****
-EditCylinder::EditCylinder(int index, cylinder_t cylIn, bool currentDiveOnly) :
-	EditCylinderBase(index, currentDiveOnly, false, SAME_TYPE | SAME_PRESS | SAME_GAS)
+EditCylinder::EditCylinder(int index, cylinder_t cylIn, EditCylinderType typeIn, bool currentDiveOnly) :
+	EditCylinderBase(index, currentDiveOnly, false, editCylinderTypeToFlags(typeIn)),
+	type(typeIn)
 {
 	if (dives.empty())
 		return;
@@ -1184,9 +1198,23 @@ EditCylinder::EditCylinder(int index, cylinder_t cylIn, bool currentDiveOnly) :
 
 	// The base class copied the cylinders for us, let's edit them
 	for (int i = 0; i < (int)indexes.size(); ++i) {
-		free_cylinder(cyl[i]);
-		cyl[i] = cylIn;
+		switch (type) {
+		case EditCylinderType::TYPE:
+			free((void *)cyl[i].type.description);
+			cyl[i].type = cylIn.type;
 			cyl[i].type.description = copy_qstring(description);
+			cyl[i].cylinder_use = cylIn.cylinder_use;
+			break;
+		case EditCylinderType::PRESSURE:
+			cyl[i].start.mbar = cylIn.start.mbar;
+			cyl[i].end.mbar = cylIn.end.mbar;
+			break;
+		case EditCylinderType::GASMIX:
+			cyl[i].gasmix = cylIn.gasmix;
+			cyl[i].bestmix_o2 = cylIn.bestmix_o2;
+			cyl[i].bestmix_he = cylIn.bestmix_he;
+			break;
+		}
 	}
 }
 

commands/command_edit.h

@@ -5,6 +5,7 @@
 #define COMMAND_EDIT_H
 
 #include "command_base.h"
+#include "command.h" // for EditCylinderType
 #include "core/subsurface-qt/divelistnotifier.h"
 
 #include <QVector>
@@ -406,10 +407,16 @@ private:
 	void redo() override;
 };
 
+// Instead of implementing an undo command for every single field in a cylinder,
+// we only have one and pass an edit "type". We either edit the type, pressure
+// or gasmix fields. This has mostly historical reasons rooted in the way the
+// CylindersModel code works. The model works for undo and also in the planner
+// without undo. Having a single undo-command simplifies the code there.
 class EditCylinder : public EditCylinderBase {
 public:
-	EditCylinder(int index, cylinder_t cyl, bool currentDiveOnly); // Clones cylinder
+	EditCylinder(int index, cylinder_t cyl, EditCylinderType type, bool currentDiveOnly); // Clones cylinder
 private:
+	EditCylinderType type;
 	void undo() override;
 	void redo() override;
 };

core/equipment.c

@@ -282,15 +282,6 @@ void remove_cylinder(struct dive *dive, int idx)
 	remove_from_cylinder_table(&dive->cylinders, idx);
 }
 
-// cyl is cloned.
-void set_cylinder(struct dive *dive, int idx, cylinder_t cyl)
-{
-	if (idx < 0 || idx >= dive->cylinders.nr)
-		return;
-	free_cylinder(dive->cylinders.cylinders[idx]);
-	dive->cylinders.cylinders[idx] = clone_cylinder(cyl);
-}
-
 void remove_weightsystem(struct dive *dive, int idx)
 {
 	remove_from_weightsystem_table(&dive->weightsystems, idx);

core/equipment.h

@@ -85,7 +85,6 @@ extern void add_cylinder_description(const cylinder_type_t *);
 extern void add_weightsystem_description(const weightsystem_t *);
 extern bool same_weightsystem(weightsystem_t w1, weightsystem_t w2);
 extern void remove_cylinder(struct dive *dive, int idx);
-extern void set_cylinder(struct dive *dive, int idx, cylinder_t ws);
 extern void remove_weightsystem(struct dive *dive, int idx);
 extern void set_weightsystem(struct dive *dive, int idx, weightsystem_t ws);
 extern void reset_cylinders(struct dive *dive, bool track_gas);

qt-models/cylindermodel.cpp

@@ -296,7 +296,6 @@ cylinder_t *CylindersModel::cylinderAt(const QModelIndex &index)
 
 bool CylindersModel::setData(const QModelIndex &index, const QVariant &value, int role)
 {
-
 	if (!d)
 		return false;
 
@@ -324,6 +323,7 @@ bool CylindersModel::setData(const QModelIndex &index, const QVariant &value, in
 				tempCyl.type.description = strdup(qPrintable(type));
 				dataChanged(index, index);
 			}
+			return true;
 		}
 		case SIZE:
 			if (tempCyl.type.size.mliter != value.toInt()) {
@@ -362,10 +362,12 @@ bool CylindersModel::setData(const QModelIndex &index, const QVariant &value, in
 	if (index.column() != TYPE && !changed)
 		return false;
 
+	Command::EditCylinderType type = Command::EditCylinderType::TYPE;
 	switch (index.column()) {
 	case TYPE:
 		newType = qPrintable(vString);
 		cyl.type.description = newType.c_str();
+		type = Command::EditCylinderType::TYPE;
 		break;
 	case SIZE: {
 			TankInfoModel *tanks = TankInfoModel::instance();
@@ -375,6 +377,7 @@ bool CylindersModel::setData(const QModelIndex &index, const QVariant &value, in
 			if (!matches.isEmpty())
 				tanks->setData(tanks->index(matches.first().row(), TankInfoModel::ML), cyl.type.size.mliter);
 		}
+		type = Command::EditCylinderType::TYPE;
 		break;
 	case WORKINGPRESS: {
 			TankInfoModel *tanks = TankInfoModel::instance();
@@ -383,13 +386,16 @@ bool CylindersModel::setData(const QModelIndex &index, const QVariant &value, in
 			if (!matches.isEmpty())
 				tanks->setData(tanks->index(matches.first().row(), TankInfoModel::BAR), cyl.type.workingpressure.mbar / 1000.0);
 		}
+		type = Command::EditCylinderType::TYPE;
 		break;
 	case START:
 		cyl.start = string_to_pressure(qPrintable(vString));
+		type = Command::EditCylinderType::PRESSURE;
 		break;
 	case END:
 		//if (!cyl->start.mbar || string_to_pressure(qPrintable(vString)).mbar <= cyl->start.mbar) {
 		cyl.end = string_to_pressure(qPrintable(vString));
+		type = Command::EditCylinderType::PRESSURE;
 		break;
 	case O2: {
 			cyl.gasmix.o2 = string_to_fraction(qPrintable(vString));
@@ -405,6 +411,7 @@ bool CylindersModel::setData(const QModelIndex &index, const QVariant &value, in
 			cyl.depth = gas_mod(cyl.gasmix, modpO2, d, M_OR_FT(3, 10));
 			cyl.bestmix_o2 = false;
 		}
+		type = Command::EditCylinderType::GASMIX;
 		break;
 	case HE:
 		cyl.gasmix.he = string_to_fraction(qPrintable(vString));
@@ -412,9 +419,11 @@ bool CylindersModel::setData(const QModelIndex &index, const QVariant &value, in
 		if (get_o2(cyl.gasmix) + get_he(cyl.gasmix) > 1000)
 			cyl.gasmix.o2.permille = 1000 - get_he(cyl.gasmix);
 		cyl.bestmix_he = false;
+		type = Command::EditCylinderType::GASMIX;
 		break;
 	case DEPTH:
 		cyl.depth = string_to_depth(qPrintable(vString));
+		type = Command::EditCylinderType::GASMIX;
 		break;
 	case MOD: {
 			if (QString::compare(qPrintable(vString), "*") == 0) {
@@ -430,6 +439,7 @@ bool CylindersModel::setData(const QModelIndex &index, const QVariant &value, in
 			modpO2.mbar = prefs.decopo2;
 			cyl.depth = gas_mod(cyl.gasmix, modpO2, d, M_OR_FT(3, 10));
 		}
+		type = Command::EditCylinderType::GASMIX;
 		break;
 	case MND:
 		if (QString::compare(qPrintable(vString), "*") == 0) {
@@ -441,6 +451,7 @@ bool CylindersModel::setData(const QModelIndex &index, const QVariant &value, in
 			// Calculate fHe for input depth
 			cyl.gasmix.he = best_he(string_to_depth(qPrintable(vString)), d, prefs.o2narcotic, cyl.gasmix.o2);
 		}
+		type = Command::EditCylinderType::GASMIX;
 		break;
 	case USE: {
 			int use = vString.toInt();
@@ -448,6 +459,7 @@ bool CylindersModel::setData(const QModelIndex &index, const QVariant &value, in
 				use = 0;
 			cyl.cylinder_use = (enum cylinderuse)use;
 		}
+		type = Command::EditCylinderType::TYPE;
 		break;
 	}
 
@@ -461,7 +473,7 @@ bool CylindersModel::setData(const QModelIndex &index, const QVariant &value, in
 	} else {
 #ifndef SUBSURFACE_MOBILE
 		// On the EquipmentTab - place an editCylinder command.
-		Command::editCylinder(index.row(), cyl, false);
+		Command::editCylinder(index.row(), cyl, type, false);
 #endif
 	}
 	return true;
@@ -709,7 +721,7 @@ void CylindersModel::commitTempCyl(int row)
 		if (inPlanner)
 			std::swap(*cyl, tempCyl);
 		else
-			Command::editCylinder(tempRow, tempCyl, false);
+			Command::editCylinder(tempRow, tempCyl, Command::EditCylinderType::TYPE, false);
 	}
 	free_cylinder(tempCyl);
 	tempRow = -1;