Undo: make weight editing undoable

Implement the EditWeight undo command. Since there is common code
(storage of the old weight), this creates a common base class for
RemoveWeight and EditWeight. The model calls directly into the undo
command, which is somewhat unfortunate as it feels like a layering
violation. It's the easy thing to do for now.

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

commands/command.cpp

@@ -278,6 +278,11 @@ int removeWeight(int index, bool currentDiveOnly)
 	return execute_edit(new RemoveWeight(index, currentDiveOnly));
 }
 
+int editWeight(int index, weightsystem_t ws, bool currentDiveOnly)
+{
+	return execute_edit(new EditWeight(index, ws, currentDiveOnly));
+}
+
 // Trip editing related commands
 void editTripLocation(dive_trip *trip, const QString &s)
 {

commands/command.h

@@ -83,6 +83,7 @@ void replanDive(dive *d); // dive computer(s) and cylinder(s) will be reset!
 void editProfile(dive *d); // dive computer(s) and cylinder(s) will be reset!
 int addWeight(bool currentDiveOnly);
 int removeWeight(int index, bool currentDiveOnly);
+int editWeight(int index, weightsystem_t ws, bool currentDiveOnly);
 
 // 5) Trip editing commands
 

commands/command_edit.cpp

@@ -6,6 +6,7 @@
 #include "core/selection.h"
 #include "core/subsurface-string.h"
 #include "core/tag.h"
+#include "qt-models/weightsysteminfomodel.h"
 
 namespace Command {
 
@@ -993,7 +994,7 @@ static int find_weightsystem_index(const struct dive *d, weightsystem_t ws)
 	return -1;
 }
 
-RemoveWeight::RemoveWeight(int index, bool currentDiveOnly) :
+EditWeightBase::EditWeightBase(int index, bool currentDiveOnly) :
 	EditDivesBase(currentDiveOnly),
 	ws(empty_weightsystem)
 {
@@ -1024,21 +1025,25 @@ RemoveWeight::RemoveWeight(int index, bool currentDiveOnly) :
 		}
 	}
 	dives = std::move(divesNew);
-
-	//: remove the part in parentheses for %n = 1
-	setText(tr("Remove weight (%n dive(s))", "", dives.size()));
 }
 
-RemoveWeight::~RemoveWeight()
+EditWeightBase::~EditWeightBase()
 {
 	free_weightsystem(ws);
 }
 
-bool RemoveWeight::workToBeDone()
+bool EditWeightBase::workToBeDone()
 {
 	return !dives.empty();
 }
 
+RemoveWeight::RemoveWeight(int index, bool currentDiveOnly) :
+	EditWeightBase(index, currentDiveOnly)
+{
+	//: remove the part in parentheses for %n = 1
+	setText(tr("Remove weight (%n dive(s))", "", dives.size()));
+}
+
 void RemoveWeight::undo()
 {
 	for (size_t i = 0; i < dives.size(); ++i) {
@@ -1055,4 +1060,57 @@ void RemoveWeight::redo()
 	}
 }
 
+EditWeight::EditWeight(int index, weightsystem_t wsIn, bool currentDiveOnly) :
+	EditWeightBase(index, currentDiveOnly),
+	new_ws(empty_weightsystem)
+{
+	if (dives.empty())
+		return;
+
+	//: remove the part in parentheses for %n = 1
+	setText(tr("Edit weight (%n dive(s))", "", dives.size()));
+
+	// Try to untranslate the weightsystem name
+	new_ws = clone_weightsystem(wsIn);
+	QString vString(new_ws.description);
+	for (int i = 0; i < MAX_WS_INFO && ws_info[i].name; ++i) {
+		if (gettextFromC::tr(ws_info[i].name) == vString) {
+			free_weightsystem(new_ws);
+			new_ws.description = copy_string(ws_info[i].name);
+			break;
+		}
+	}
+
+	// If that doesn't change anything, do nothing
+	if (same_weightsystem(ws, new_ws)) {
+		dives.clear();
+		return;
+	}
+
+	WSInfoModel *wsim = WSInfoModel::instance();
+	QModelIndexList matches = wsim->match(wsim->index(0, 0), Qt::DisplayRole, gettextFromC::tr(new_ws.description));
+	if (!matches.isEmpty())
+		wsim->setData(wsim->index(matches.first().row(), WSInfoModel::GR), new_ws.weight.grams);
+}
+
+EditWeight::~EditWeight()
+{
+	free_weightsystem(new_ws);
+}
+
+void EditWeight::redo()
+{
+	for (size_t i = 0; i < dives.size(); ++i) {
+		set_weightsystem(dives[i], indexes[i], new_ws);
+		emit diveListNotifier.weightEdited(dives[i], indexes[i]);
+	}
+	std::swap(ws, new_ws);
+}
+
+// Undo and redo do the same as just the stored value is exchanged
+void EditWeight::undo()
+{
+	redo();
+}
+
 } // namespace Command

commands/command_edit.h

@@ -339,16 +339,32 @@ private:
 	bool workToBeDone() override;
 };
 
-class RemoveWeight : public EditDivesBase {
+class EditWeightBase : public EditDivesBase {
-public:
+protected:
-	RemoveWeight(int index, bool currentDiveOnly);
+	EditWeightBase(int index, bool currentDiveOnly);
-	~RemoveWeight();
+	~EditWeightBase();
-private:
+
 	weightsystem_t ws;
 	std::vector<int> indexes; // An index for each dive in the dives vector.
+	bool workToBeDone() override;
+};
+
+class RemoveWeight : public EditWeightBase {
+public:
+	RemoveWeight(int index, bool currentDiveOnly);
+private:
+	void undo() override;
+	void redo() override;
+};
+
+class EditWeight : public EditWeightBase {
+public:
+	EditWeight(int index, weightsystem_t ws, bool currentDiveOnly); // Clones ws
+	~EditWeight();
+private:
+	weightsystem_t new_ws;
 	void undo() override;
 	void redo() override;
-	bool workToBeDone() override;
 };
 
 } // namespace Command

core/equipment.c

@@ -288,6 +288,15 @@ void remove_weightsystem(struct dive *dive, int idx)
 	remove_from_weightsystem_table(&dive->weightsystems, idx);
 }
 
+// ws is cloned.
+void set_weightsystem(struct dive *dive, int idx, weightsystem_t ws)
+{
+	if (idx < 0 || idx >= dive->weightsystems.nr)
+		return;
+	free_weightsystem(dive->weightsystems.weightsystems[idx]);
+	dive->weightsystems.weightsystems[idx] = clone_weightsystem(ws);
+}
+
 /* when planning a dive we need to make sure that all cylinders have a sane depth assigned
  * and if we are tracking gas consumption the pressures need to be reset to start = end = workingpressure */
 void reset_cylinders(struct dive *dive, bool track_gas)

core/equipment.h

@@ -83,6 +83,7 @@ extern bool same_weightsystem(weightsystem_t w1, weightsystem_t w2);
 extern bool same_cylinder(cylinder_t cyl1, cylinder_t cyl2);
 extern void remove_cylinder(struct dive *dive, int idx);
 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);
 extern int gas_volume(const cylinder_t *cyl, pressure_t p); /* Volume in mliter of a cylinder at pressure 'p' */
 extern int find_best_gasmix_match(struct gasmix mix, const struct cylinder_table *cylinders);

core/subsurface-qt/DiveListNotifier.h

@@ -88,6 +88,7 @@ signals:
 	void weightsystemsReset(const QVector<dive *> &dives);
 	void weightAdded(dive *d, int pos);
 	void weightRemoved(dive *d, int pos);
+	void weightEdited(dive *d, int pos);
 
 	// Trip edited signal
 	void tripChanged(dive_trip *trip, TripField field);

desktop-widgets/tab-widgets/TabDiveEquipment.cpp

@@ -179,12 +179,10 @@ void TabDiveEquipment::editWeightWidget(const QModelIndex &index)
 	if (!index.isValid())
 		return;
 
-	if (index.column() == WeightModel::REMOVE) {
+	if (index.column() == WeightModel::REMOVE)
 		divesEdited(Command::removeWeight(index.row(), false));
-	} else {
+	else
-		MainWindow::instance()->mainTab->enableEdition();
 		ui.weights->edit(index);
-	}
 }
 
 // tricky little macro to edit all the selected dives
@@ -256,15 +254,6 @@ void TabDiveEquipment::acceptChanges()
 		do_replot = true;
 	}
 
-	if (weightModel->changed) {
-		mark_divelist_changed(true);
-		MODIFY_DIVES(selectedDives,
-			if (weightsystems_equal(mydive, cd))
-				copy_weights(&displayed_dive.weightsystems, &mydive->weightsystems);
-		);
-		copy_weights(&displayed_dive.weightsystems, &cd->weightsystems);
-	}
-
 	if (do_replot)
 		MainWindow::instance()->graphics->replot();
 

qt-models/weightmodel.cpp

@@ -6,6 +6,9 @@
 #include "core/qthelper.h"
 #include "core/subsurface-qt/DiveListNotifier.h"
 #include "qt-models/weightsysteminfomodel.h"
+#ifndef SUBSURFACE_MOBILE
+#include "commands/command.h"
+#endif
 
 WeightModel::WeightModel(QObject *parent) : CleanerTableModel(parent),
 	changed(false),
@@ -18,6 +21,7 @@ WeightModel::WeightModel(QObject *parent) : CleanerTableModel(parent),
 	connect(&diveListNotifier, &DiveListNotifier::weightsystemsReset, this, &WeightModel::weightsystemsReset);
 	connect(&diveListNotifier, &DiveListNotifier::weightAdded, this, &WeightModel::weightAdded);
 	connect(&diveListNotifier, &DiveListNotifier::weightRemoved, this, &WeightModel::weightRemoved);
+	connect(&diveListNotifier, &DiveListNotifier::weightEdited, this, &WeightModel::weightEdited);
 }
 
 weightsystem_t WeightModel::weightSystemAt(const QModelIndex &index) const
@@ -105,50 +109,30 @@ void WeightModel::clearTempWS()
 
 void WeightModel::commitTempWS()
 {
-	if (tempRow < 0)
+#ifndef SUBSURFACE_MOBILE
+	if (tempRow < 0 || !d || tempRow > d->weightsystems.nr)
 		return;
+	// Only submit a command if the type changed
+	weightsystem_t ws = d->weightsystems.weightsystems[tempRow];
+	if (!same_string(ws.description, tempWS.description) || gettextFromC::tr(ws.description) != QString(tempWS.description))
+		Command::editWeight(tempRow, tempWS, false);
 	tempRow = -1;
-	setData(index(tempRow, TYPE), QVariant::fromValue(QString(tempWS.description)), Qt::EditRole);
+#endif
-	setData(index(tempRow, WEIGHT), QVariant::fromValue(get_weight_string(tempWS.weight, true)), Qt::EditRole);
 }
 
 bool WeightModel::setData(const QModelIndex &index, const QVariant &value, int role)
 {
+#ifndef SUBSURFACE_MOBILE
 	QString vString = value.toString();
-	weightsystem_t *ws = &d->weightsystems.weightsystems[index.row()];
+	weightsystem_t ws = weightSystemAt(index);
 	switch (index.column()) {
-	case TYPE:
-		if (!value.isNull()) {
-			//TODO: C-function weight_system_set_description ?
-			if (!ws->description || gettextFromC::tr(ws->description) != vString) {
-				// loop over translations to see if one matches
-				int i = -1;
-				while (i < MAX_WS_INFO && ws_info[++i].name) {
-					if (gettextFromC::tr(ws_info[i].name) == vString) {
-						ws->description = copy_string(ws_info[i].name);
-						break;
-					}
-				}
-				if (i == MAX_WS_INFO || ws_info[i].name == NULL) // didn't find a match
-					ws->description = copy_qstring(vString);
-				changed = true;
-			}
-		}
-		break;
 	case WEIGHT:
-		if (CHANGED()) {
+		ws.weight = string_to_weight(qPrintable(vString));
-			ws->weight = string_to_weight(qPrintable(vString));
+		Command::editWeight(index.row(), ws, false);
-			// now update the ws_info
+		return true;
-			changed = true;
-			WSInfoModel *wsim = WSInfoModel::instance();
-			QModelIndexList matches = wsim->match(wsim->index(0, 0), Qt::DisplayRole, gettextFromC::tr(ws->description));
-			if (!matches.isEmpty())
-				wsim->setData(wsim->index(matches.first().row(), WSInfoModel::GR), ws->weight.grams);
-		}
-		break;
 	}
-	dataChanged(index, index);
+	return false;
-	return true;
+#endif
 }
 
 Qt::ItemFlags WeightModel::flags(const QModelIndex &index) const
@@ -200,3 +184,11 @@ void WeightModel::weightRemoved(struct dive *changed, int pos)
 	beginRemoveRows(QModelIndex(), pos, pos);
 	endRemoveRows();
 }
+
+void WeightModel::weightEdited(struct dive *changed, int pos)
+{
+	if (d != changed)
+		return;
+
+	dataChanged(index(pos, TYPE), index(pos, WEIGHT));
+}

qt-models/weightmodel.h

@@ -35,6 +35,7 @@ slots:
 	void weightsystemsReset(const QVector<dive *> &dives);
 	void weightAdded(dive *d, int pos);
 	void weightRemoved(dive *d, int pos);
+	void weightEdited(dive *d, int pos);
 
 private:
 	dive *d;