Merge pull request #2643 from bstoeger/cylinder4

First steps of cylinder-editing undo

CHANGELOG.md

@@ -17,6 +17,7 @@ Mobile: make sure filter header and virtual keyboard are shown when tapping on f
 Mobile: fix issue where in some circumstances changes weren't written to storage
 Mobile: fix issues with filtering while editing dives
 Desktop: implement dive invalidation
+Undo: implement undo of event handling (viz. bookmarks, setpoints, gas switches)
 Desktop: fix tab-order in filter widget
 Desktop: implement fulltext search
 Desktop: add starts-with and exact filter modes for textual search

commands/CMakeLists.txt

@@ -14,6 +14,8 @@ set(SUBSURFACE_GENERIC_COMMANDS_SRCS
 	command_edit.h
 	command_edit_trip.cpp
 	command_edit_trip.h
+	command_event.cpp
+	command_event.h
 )
 add_library(subsurface_commands STATIC ${SUBSURFACE_GENERIC_COMMANDS_SRCS})
 target_link_libraries(subsurface_commands ${QT_LIBRARIES})

commands/command.cpp

@@ -5,6 +5,7 @@
 #include "command_divesite.h"
 #include "command_edit.h"
 #include "command_edit_trip.h"
+#include "command_event.h"
 
 namespace Command {
 
@@ -293,6 +294,21 @@ int editWeight(int index, weightsystem_t ws, bool currentDiveOnly)
 	return execute_edit(new EditWeight(index, ws, currentDiveOnly));
 }
 
+int addCylinder(bool currentDiveOnly)
+{
+	return execute_edit(new AddCylinder(currentDiveOnly));
+}
+
+int removeCylinder(int index, bool currentDiveOnly)
+{
+	return execute_edit(new RemoveCylinder(index, currentDiveOnly));
+}
+
+int editCylinder(int index, cylinder_t cyl, EditCylinderType type, bool currentDiveOnly)
+{
+	return execute_edit(new EditCylinder(index, cyl, type, currentDiveOnly));
+}
+
 // Trip editing related commands
 void editTripLocation(dive_trip *trip, const QString &s)
 {
@@ -311,4 +327,36 @@ void editDive(dive *oldDive, dive *newDive, dive_site *createDs, dive_site *chan
 }
 #endif // SUBSURFACE_MOBILE
 
+// Event commands
+
+void addEventBookmark(struct dive *d, int dcNr, int seconds)
+{
+	execute(new AddEventBookmark(d, dcNr, seconds));
+}
+
+void addEventDivemodeSwitch(struct dive *d, int dcNr, int seconds, int divemode)
+{
+	execute(new AddEventDivemodeSwitch(d, dcNr, seconds, divemode));
+}
+
+void addEventSetpointChange(struct dive *d, int dcNr, int seconds, pressure_t pO2)
+{
+	execute(new AddEventSetpointChange(d, dcNr, seconds, pO2));
+}
+
+void renameEvent(struct dive *d, int dcNr, struct event *ev, const char *name)
+{
+	execute(new RenameEvent(d, dcNr, ev, name));
+}
+
+void removeEvent(struct dive *d, int dcNr, struct event *ev)
+{
+	execute(new RemoveEvent(d, dcNr, ev));
+}
+
+void addGasSwitch(struct dive *d, int dcNr, int seconds, int tank)
+{
+	execute(new AddGasSwitch(d, dcNr, seconds, tank));
+}
+
 } // namespace Command

commands/command.h

@@ -90,6 +90,14 @@ 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);
+int addCylinder(bool currentDiveOnly);
+int removeCylinder(int index, 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!
@@ -101,6 +109,15 @@ void editDive(dive *oldDive, dive *newDive, dive_site *createDs, dive_site *chan
 void editTripLocation(dive_trip *trip, const QString &s);
 void editTripNotes(dive_trip *trip, const QString &s);
 
+// 6) Event commands
+
+void addEventBookmark(struct dive *d, int dcNr, int seconds);
+void addEventDivemodeSwitch(struct dive *d, int dcNr, int seconds, int divemode);
+void addEventSetpointChange(struct dive *d, int dcNr, int seconds, pressure_t pO2);
+void renameEvent(struct dive *d, int dcNr, struct event *ev, const char *name);
+void removeEvent(struct dive *d, int dcNr, struct event *ev);
+void addGasSwitch(struct dive *d, int dcNr, int seconds, int tank);
+
 } // namespace Command
 
 #endif // COMMAND_H

commands/command_base.h

@@ -141,7 +141,7 @@
 // We put everything in a namespace, so that we can shorten names without polluting the global namespace
 namespace Command {
 
-// Classes used to automatically call free_dive()/free_trip for owning pointers that go out of scope.
+// Classes used to automatically call the appropriate free_*() function for owning pointers that go out of scope.
 struct DiveDeleter {
 	void operator()(dive *d) { free_dive(d); }
 };
@@ -151,11 +151,15 @@ struct TripDeleter {
 struct DiveSiteDeleter {
 	void operator()(dive_site *ds) { free_dive_site(ds); }
 };
+struct EventDeleter {
+	void operator()(event *ev) { free(ev); }
+};
 
-// Owning pointers to dive and dive_trip objects.
+// Owning pointers to dive, dive_trip, dive_site and event objects.
 typedef std::unique_ptr<dive, DiveDeleter> OwningDivePtr;
 typedef std::unique_ptr<dive_trip, TripDeleter> OwningTripPtr;
 typedef std::unique_ptr<dive_site, DiveSiteDeleter> OwningDiveSitePtr;
+typedef std::unique_ptr<event, EventDeleter> OwningEventPtr;
 
 // This is the base class of all commands.
 // It defines the Qt-translation functions
@@ -182,4 +186,3 @@ QString getListOfDives(QVector<struct dive *> dives);
 } // namespace Command
 
 #endif // COMMAND_BASE_H
-

commands/command_edit.cpp

@@ -8,6 +8,7 @@
 #include "core/subsurface-string.h"
 #include "core/tag.h"
 #include "qt-models/weightsysteminfomodel.h"
+#include "qt-models/tankinfomodel.h"
 #ifdef SUBSURFACE_MOBILE
 #include "qt-models/divelocationmodel.h"
 #endif
@@ -946,6 +947,7 @@ bool EditWeightBase::workToBeDone()
 	return !dives.empty();
 }
 
+// ***** Remove Weight *****
 RemoveWeight::RemoveWeight(int index, bool currentDiveOnly) :
 	EditWeightBase(index, currentDiveOnly)
 {
@@ -972,6 +974,7 @@ void RemoveWeight::redo()
 	}
 }
 
+// ***** Edit Weight *****
 EditWeight::EditWeight(int index, weightsystem_t wsIn, bool currentDiveOnly) :
 	EditWeightBase(index, currentDiveOnly),
 	new_ws(empty_weightsystem)
@@ -1028,6 +1031,241 @@ void EditWeight::undo()
 	redo();
 }
 
+// ***** Add Cylinder *****
+AddCylinder::AddCylinder(bool currentDiveOnly) :
+	EditDivesBase(currentDiveOnly),
+	cyl(empty_cylinder)
+{
+	if (dives.empty())
+		return;
+	else if (dives.size() == 1)
+		setText(tr("Add cylinder"));
+	else
+		setText(tr("Add cylinder (%n dive(s))", "", dives.size()));
+	cyl = create_new_cylinder(dives[0]);
+}
+
+AddCylinder::~AddCylinder()
+{
+	free_cylinder(cyl);
+}
+
+bool AddCylinder::workToBeDone()
+{
+	return true;
+}
+
+void AddCylinder::undo()
+{
+	for (dive *d: dives) {
+		if (d->cylinders.nr <= 0)
+			continue;
+		remove_cylinder(d, d->cylinders.nr - 1);
+		emit diveListNotifier.cylinderRemoved(d, d->cylinders.nr);
+		invalidate_dive_cache(d); // Ensure that dive is written in git_save()
+	}
+}
+
+void AddCylinder::redo()
+{
+	for (dive *d: dives) {
+		add_cloned_cylinder(&d->cylinders, cyl);
+		emit diveListNotifier.cylinderAdded(d, d->cylinders.nr - 1);
+		invalidate_dive_cache(d); // Ensure that dive is written in git_save()
+	}
+}
+
+static bool same_cylinder_type(const cylinder_t &cyl1, const cylinder_t &cyl2)
+{
+	return same_string(cyl1.type.description, cyl2.type.description) &&
+	       cyl1.cylinder_use == cyl2.cylinder_use;
+}
+
+static bool same_cylinder_size(const cylinder_t &cyl1, const cylinder_t &cyl2)
+{
+	return cyl1.type.size.mliter == cyl2.type.size.mliter &&
+	       cyl1.type.workingpressure.mbar == cyl2.type.workingpressure.mbar;
+}
+
+static bool same_cylinder_pressure(const cylinder_t &cyl1, const cylinder_t &cyl2)
+{
+	return cyl1.start.mbar == cyl2.start.mbar &&
+	       cyl1.end.mbar == cyl2.end.mbar;
+}
+
+// Flags for comparing cylinders
+static const constexpr int SAME_TYPE = 1 << 0;
+static const constexpr int SAME_SIZE = 1 << 1;
+static const constexpr int SAME_PRESS = 1 << 2;
+static const constexpr int SAME_GAS = 1 << 3;
+
+static bool same_cylinder_with_flags(const cylinder_t &cyl1, const cylinder_t &cyl2, int sameCylinderFlags)
+{
+	return (((sameCylinderFlags & SAME_TYPE)  == 0 || same_cylinder_type(cyl1, cyl2)) &&
+		((sameCylinderFlags & SAME_SIZE)  == 0 || same_cylinder_size(cyl1, cyl2)) &&
+		((sameCylinderFlags & SAME_PRESS) == 0 || same_cylinder_pressure(cyl1, cyl2)) &&
+		((sameCylinderFlags & SAME_GAS)   == 0 || same_gasmix(cyl1.gasmix, cyl2.gasmix)));
+}
+
+static int find_cylinder_index(const struct dive *d, const cylinder_t &cyl, int sameCylinderFlags)
+{
+	for (int idx = 0; idx < d->cylinders.nr; ++idx) {
+		if (same_cylinder_with_flags(cyl, d->cylinders.cylinders[idx], sameCylinderFlags))
+			return idx;
+	}
+	return -1;
+}
+
+EditCylinderBase::EditCylinderBase(int index, bool currentDiveOnly, bool nonProtectedOnly, int sameCylinderFlags) :
+	EditDivesBase(currentDiveOnly)
+{
+	// Get the old cylinder, bail if index is invalid
+	if (!current || index < 0 || index >= current->cylinders.nr) {
+		dives.clear();
+		return;
+	}
+	const cylinder_t &orig = current->cylinders.cylinders[index];
+
+	std::vector<dive *> divesNew;
+	divesNew.reserve(dives.size());
+	indexes.reserve(dives.size());
+	cyl.reserve(dives.size());
+
+	for (dive *d: dives) {
+		int idx = d == current ? index : find_cylinder_index(d, orig, sameCylinderFlags);
+		if (idx < 0 || (nonProtectedOnly && is_cylinder_prot(d, idx)))
+			continue;
+		divesNew.push_back(d);
+		indexes.push_back(idx);
+		cyl.push_back(clone_cylinder(d->cylinders.cylinders[idx]));
+	}
+	dives = std::move(divesNew);
+}
+
+EditCylinderBase::~EditCylinderBase()
+{
+	for (cylinder_t c: cyl)
+		free_cylinder(c);
+}
+
+bool EditCylinderBase::workToBeDone()
+{
+	return !dives.empty();
+}
+
+// ***** Remove Cylinder *****
+RemoveCylinder::RemoveCylinder(int index, bool currentDiveOnly) :
+	EditCylinderBase(index, currentDiveOnly, true, SAME_TYPE | SAME_PRESS | SAME_GAS)
+{
+	if (dives.size() == 1)
+		setText(tr("Remove cylinder"));
+	else
+		setText(tr("Remove cylinder (%n dive(s))", "", dives.size()));
+}
+
+void RemoveCylinder::undo()
+{
+	for (size_t i = 0; i < dives.size(); ++i) {
+		std::vector<int> mapping = get_cylinder_map_for_add(dives[i]->cylinders.nr, indexes[i]);
+		add_to_cylinder_table(&dives[i]->cylinders, indexes[i], clone_cylinder(cyl[i]));
+		emit diveListNotifier.cylinderAdded(dives[i], indexes[i]);
+		invalidate_dive_cache(dives[i]); // Ensure that dive is written in git_save()
+	}
+}
+
+void RemoveCylinder::redo()
+{
+	for (size_t i = 0; i < dives.size(); ++i) {
+		std::vector<int> mapping = get_cylinder_map_for_remove(dives[i]->cylinders.nr, indexes[i]);
+		remove_cylinder(dives[i], indexes[i]);
+		cylinder_renumber(dives[i], &mapping[0]);
+		emit diveListNotifier.cylinderRemoved(dives[i], indexes[i]);
+		invalidate_dive_cache(dives[i]); // Ensure that dive is written in git_save()
+	}
+}
+
+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, EditCylinderType typeIn, bool currentDiveOnly) :
+	EditCylinderBase(index, currentDiveOnly, false, editCylinderTypeToFlags(typeIn)),
+	type(typeIn)
+{
+	if (dives.empty())
+		return;
+
+	if (dives.size() == 1)
+		setText(tr("Edit cylinder"));
+	else
+		setText(tr("Edit cylinder (%n dive(s))", "", dives.size()));
+
+	// Try to untranslate the cylinder type
+	QString description = cylIn.type.description;
+	for (int i = 0; i < MAX_TANK_INFO && tank_info[i].name; ++i) {
+		if (gettextFromC::tr(tank_info[i].name) == description) {
+			description = tank_info[i].name;
+			break;
+		}
+	}
+
+	// Update the tank info model
+	TankInfoModel *tim = TankInfoModel::instance();
+	QModelIndexList matches = tim->match(tim->index(0, 0), Qt::DisplayRole, gettextFromC::tr(cylIn.type.description));
+	if (!matches.isEmpty()) {
+		if (cylIn.type.size.mliter != cyl[0].type.size.mliter)
+			tim->setData(tim->index(matches.first().row(), TankInfoModel::ML), cylIn.type.size.mliter);
+		if (cylIn.type.workingpressure.mbar != cyl[0].type.workingpressure.mbar)
+			tim->setData(tim->index(matches.first().row(), TankInfoModel::BAR), cylIn.type.workingpressure.mbar / 1000.0);
+	}
+
+	// The base class copied the cylinders for us, let's edit them
+	for (int i = 0; i < (int)indexes.size(); ++i) {
+		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;
+		}
+	}
+}
+
+void EditCylinder::redo()
+{
+	for (size_t i = 0; i < dives.size(); ++i) {
+		std::swap(dives[i]->cylinders.cylinders[indexes[i]], cyl[i]);
+		emit diveListNotifier.cylinderEdited(dives[i], indexes[i]);
+		invalidate_dive_cache(dives[i]); // Ensure that dive is written in git_save()
+	}
+}
+
+// Undo and redo do the same as just the stored value is exchanged
+void EditCylinder::undo()
+{
+	redo();
+}
+
 #ifdef SUBSURFACE_MOBILE
 
 EditDive::EditDive(dive *oldDiveIn, dive *newDiveIn, dive_site *createDs, dive_site *editDs, location_t dsLocationIn)

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>
@@ -377,6 +378,49 @@ private:
 	void redo() override;
 };
 
+class AddCylinder : public EditDivesBase {
+public:
+	AddCylinder(bool currentDiveOnly);
+	~AddCylinder();
+private:
+	cylinder_t cyl;
+	void undo() override;
+	void redo() override;
+	bool workToBeDone() override;
+};
+
+class EditCylinderBase : public EditDivesBase {
+protected:
+	EditCylinderBase(int index, bool currentDiveOnly, bool nonProtectedOnly, int sameCylinderFlags);
+	~EditCylinderBase();
+
+	std::vector<cylinder_t> cyl;
+	std::vector<int> indexes; // An index for each dive in the dives vector.
+	bool workToBeDone() override;
+};
+
+class RemoveCylinder : public EditCylinderBase {
+public:
+	RemoveCylinder(int index, bool currentDiveOnly);
+private:
+	void undo() override;
+	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, EditCylinderType type, bool currentDiveOnly); // Clones cylinder
+private:
+	EditCylinderType type;
+	void undo() override;
+	void redo() override;
+};
+
 #ifdef SUBSURFACE_MOBILE
 // Edit a full dive. This is used on mobile where we don't have per-field granularity.
 // It may add or edit a dive site.
@@ -406,5 +450,4 @@ private:
 #endif // SUBSURFACE_MOBILE
 
 } // namespace Command
-
 #endif

commands/command_event.cpp

@@ -0,0 +1,210 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include "command_event.h"
+#include "core/dive.h"
+#include "core/selection.h"
+#include "core/subsurface-qt/divelistnotifier.h"
+#include "core/libdivecomputer.h"
+#include "core/gettextfromc.h"
+#include <QVector>
+
+namespace Command {
+
+EventBase::EventBase(struct dive *dIn, int dcNrIn) :
+	d(dIn), dcNr(dcNrIn)
+{
+}
+
+void EventBase::redo()
+{
+	redoit(); // Call actual function in base class
+	updateDive();
+}
+
+void EventBase::undo()
+{
+	undoit(); // Call actual function in base class
+	updateDive();
+}
+
+void EventBase::updateDive()
+{
+	invalidate_dive_cache(d);
+	emit diveListNotifier.eventsChanged(d);
+	dc_number = dcNr;
+	setSelection({ d }, d);
+}
+
+AddEventBase::AddEventBase(struct dive *d, int dcNr, struct event *ev) : EventBase(d, dcNr),
+	eventToAdd(ev)
+{
+}
+
+bool AddEventBase::workToBeDone()
+{
+	return true;
+}
+
+void AddEventBase::redoit()
+{
+	struct divecomputer *dc = get_dive_dc(d, dcNr);
+	eventToRemove = eventToAdd.get();
+	add_event_to_dc(dc, eventToAdd.release()); // return ownership to backend
+}
+
+void AddEventBase::undoit()
+{
+	struct divecomputer *dc = get_dive_dc(d, dcNr);
+	remove_event_from_dc(dc, eventToRemove);
+	eventToAdd.reset(eventToRemove); // take ownership of event
+	eventToRemove = nullptr;
+}
+
+AddEventBookmark::AddEventBookmark(struct dive *d, int dcNr, int seconds) :
+	AddEventBase(d, dcNr, create_event(seconds, SAMPLE_EVENT_BOOKMARK, 0, 0, "bookmark"))
+{
+	setText(tr("Add bookmark"));
+}
+
+AddEventDivemodeSwitch::AddEventDivemodeSwitch(struct dive *d, int dcNr, int seconds, int divemode) :
+	AddEventBase(d, dcNr, create_event(seconds, SAMPLE_EVENT_BOOKMARK, 0, divemode, QT_TRANSLATE_NOOP("gettextFromC", "modechange")))
+{
+	setText(tr("Add dive mode switch to %1").arg(gettextFromC::tr(divemode_text_ui[divemode])));
+}
+
+AddEventSetpointChange::AddEventSetpointChange(struct dive *d, int dcNr, int seconds, pressure_t pO2) :
+	AddEventBase(d, dcNr, create_event(seconds, SAMPLE_EVENT_PO2, 0, pO2.mbar, QT_TRANSLATE_NOOP("gettextFromC", "SP change")))
+{
+	setText(tr("Add set point change")); // TODO: format pO2 value in bar or psi.
+}
+
+RenameEvent::RenameEvent(struct dive *d, int dcNr, struct event *ev, const char *name) : EventBase(d, dcNr),
+	eventToAdd(clone_event_rename(ev, name)),
+	eventToRemove(ev)
+{
+	setText(tr("Rename bookmark to %1").arg(name));
+}
+
+bool RenameEvent::workToBeDone()
+{
+	return true;
+}
+
+void RenameEvent::redoit()
+{
+	struct divecomputer *dc = get_dive_dc(d, dcNr);
+	swap_event(dc, eventToRemove, eventToAdd.get());
+	event *tmp = eventToRemove;
+	eventToRemove = eventToAdd.release();
+	eventToAdd.reset(tmp);
+}
+
+void RenameEvent::undoit()
+{
+	// Undo and redo do the same thing - they simply swap events
+	redoit();
+}
+
+RemoveEvent::RemoveEvent(struct dive *d, int dcNr, struct event *ev) : EventBase(d, dcNr),
+	eventToRemove(ev),
+	cylinder(ev->type == SAMPLE_EVENT_GASCHANGE2 || ev->type == SAMPLE_EVENT_GASCHANGE ?
+		 ev->gas.index : -1)
+{
+	setText(tr("Remove %1 event").arg(ev->name));
+}
+
+bool RemoveEvent::workToBeDone()
+{
+	return true;
+}
+
+void RemoveEvent::redoit()
+{
+	struct divecomputer *dc = get_dive_dc(d, dcNr);
+	remove_event_from_dc(dc, eventToRemove);
+	eventToAdd.reset(eventToRemove); // take ownership of event
+	eventToRemove = nullptr;
+}
+
+void RemoveEvent::undoit()
+{
+	struct divecomputer *dc = get_dive_dc(d, dcNr);
+	eventToRemove = eventToAdd.get();
+	add_event_to_dc(dc, eventToAdd.release()); // return ownership to backend
+}
+
+void RemoveEvent::post() const
+{
+	if (cylinder < 0)
+		return;
+
+	fixup_dive(d);
+	emit diveListNotifier.cylinderEdited(d, cylinder);
+
+	// TODO: This is silly we send a DURATION change event so that the statistics are recalculated.
+	// We should instead define a proper DiveField that expresses the change caused by a gas switch.
+	emit diveListNotifier.divesChanged(QVector<dive *>{ d }, DiveField::DURATION | DiveField::DEPTH);
+}
+
+AddGasSwitch::AddGasSwitch(struct dive *d, int dcNr, int seconds, int tank) : EventBase(d, dcNr)
+{
+	// If there is a gas change at this time stamp, remove it before adding the new one.
+	// There shouldn't be more than one gas change per time stamp. Just in case we'll
+	// support that anyway.
+	struct divecomputer *dc = get_dive_dc(d, dcNr);
+	struct event *gasChangeEvent = dc->events;
+	while ((gasChangeEvent = get_next_event_mutable(gasChangeEvent, "gaschange")) != NULL) {
+		if (gasChangeEvent->time.seconds == seconds) {
+			eventsToRemove.push_back(gasChangeEvent);
+			int idx = gasChangeEvent->gas.index;
+			if (std::find(cylinders.begin(), cylinders.end(), idx) == cylinders.end())
+				cylinders.push_back(idx); // cylinders might have changed their status
+		}
+		gasChangeEvent = gasChangeEvent->next;
+	}
+
+	eventsToAdd.emplace_back(create_gas_switch_event(d, dc, seconds, tank));
+}
+
+bool AddGasSwitch::workToBeDone()
+{
+	return true;
+}
+
+void AddGasSwitch::redoit()
+{
+	std::vector<OwningEventPtr> newEventsToAdd;
+	std::vector<event *> newEventsToRemove;
+	newEventsToAdd.reserve(eventsToRemove.size());
+	newEventsToRemove.reserve(eventsToAdd.size());
+	struct divecomputer *dc = get_dive_dc(d, dcNr);
+
+	for (event *ev: eventsToRemove) {
+		remove_event_from_dc(dc, ev);
+		newEventsToAdd.emplace_back(ev); // take ownership of event
+	}
+	for (OwningEventPtr &ev: eventsToAdd) {
+		newEventsToRemove.push_back(ev.get());
+		add_event_to_dc(dc, ev.release()); // return ownership to backend
+	}
+	eventsToAdd = std::move(newEventsToAdd);
+	eventsToRemove = std::move(newEventsToRemove);
+
+	// this means we potentially have a new tank that is being used and needs to be shown
+	fixup_dive(d);
+
+	for (int idx: cylinders)
+		emit diveListNotifier.cylinderEdited(d, idx);
+
+	// TODO: This is silly we send a DURATION change event so that the statistics are recalculated.
+	// We should instead define a proper DiveField that expresses the change caused by a gas switch.
+	emit diveListNotifier.divesChanged(QVector<dive *>{ d }, DiveField::DURATION | DiveField::DEPTH);
+}
+
+void AddGasSwitch::undoit()
+{
+	// Undo and redo do the same thing, as the dives-to-be-added and dives-to-be-removed are exchanged.
+	redoit();
+}
+
+} // namespace Command

commands/command_event.h

@@ -0,0 +1,104 @@
+// SPDX-License-Identifier: GPL-2.0
+// Note: this header file is used by the undo-machinery and should not be included elsewhere.
+
+#ifndef COMMAND_EVENT_H
+#define COMMAND_EVENT_H
+
+#include "command_base.h"
+
+
+// We put everything in a namespace, so that we can shorten names without polluting the global namespace
+namespace Command {
+
+// Events are a strange thing: they contain there own description which means
+// that on changing the description a new object must be allocated. Moreover,
+// it means that these objects can't be collected in a table.
+// Therefore, the undo commands work on events as they do with dives: using
+// owning pointers. See comments in command_base.h
+
+class EventBase : public Base {
+protected:
+	EventBase(struct dive *d, int dcNr);
+	void undo() override;
+	void redo() override;
+	virtual void redoit() = 0;
+	virtual void undoit() = 0;
+
+	// Note: we store dive and the divecomputer-number instead of a pointer to the divecomputer.
+	// Since one divecomputer is integrated into the dive structure, pointers to divecomputers
+	// are probably not stable.
+	struct dive *d;
+	int dcNr;
+private:
+	void updateDive();
+};
+
+class AddEventBase : public EventBase {
+public:
+	AddEventBase(struct dive *d, int dcNr, struct event *ev); // Takes ownership of event!
+private:
+	bool workToBeDone() override;
+	void undoit() override;
+	void redoit() override;
+
+	OwningEventPtr eventToAdd;	// for redo
+	event *eventToRemove;		// for undo
+};
+
+class AddEventBookmark : public AddEventBase {
+public:
+	AddEventBookmark(struct dive *d, int dcNr, int seconds);
+};
+
+class AddEventDivemodeSwitch : public AddEventBase {
+public:
+	AddEventDivemodeSwitch(struct dive *d, int dcNr, int seconds, int divemode);
+};
+
+class AddEventSetpointChange : public AddEventBase {
+public:
+	AddEventSetpointChange(struct dive *d, int dcNr, int seconds, pressure_t pO2);
+};
+
+class RenameEvent : public EventBase {
+public:
+	RenameEvent(struct dive *d, int dcNr, struct event *ev, const char *name);
+private:
+	bool workToBeDone() override;
+	void undoit() override;
+	void redoit() override;
+
+	OwningEventPtr eventToAdd;	// for undo and redo
+	event *eventToRemove;		// for undo and redo
+};
+
+class RemoveEvent : public EventBase {
+public:
+	RemoveEvent(struct dive *d, int dcNr, struct event *ev);
+private:
+	bool workToBeDone() override;
+	void undoit() override;
+	void redoit() override;
+	void post() const; // Called to fix up dives should a gas-change have happened.
+
+	OwningEventPtr eventToAdd;	// for undo
+	event *eventToRemove;		// for redo
+	int cylinder;			// affected cylinder (if removing gas switch). <0: not a gas switch.
+};
+
+class AddGasSwitch : public EventBase {
+public:
+	AddGasSwitch(struct dive *d, int dcNr, int seconds, int tank);
+private:
+	bool workToBeDone() override;
+	void undoit() override;
+	void redoit() override;
+
+	std::vector<int> cylinders; // cylinders that are modified
+	std::vector<OwningEventPtr> eventsToAdd;
+	std::vector<event *> eventsToRemove;
+};
+
+} // namespace Command
+
+#endif // COMMAND_EVENT_H

core/dive.c

@@ -11,6 +11,7 @@
 #include "device.h"
 #include "divelist.h"
 #include "divesite.h"
+#include "errorhelper.h"
 #include "qthelper.h"
 #include "metadata.h"
 #include "membuffer.h"
@@ -126,10 +127,10 @@ int event_is_gaschange(const struct event *ev)
 		ev->type == SAMPLE_EVENT_GASCHANGE2;
 }
 
-struct event *add_event(struct divecomputer *dc, unsigned int time, int type, int flags, int value, const char *name)
+struct event *create_event(unsigned int time, int type, int flags, int value, const char *name)
 {
 	int gas_index = -1;
-	struct event *ev, **p;
+	struct event *ev;
 	unsigned int size, len = strlen(name);
 
 	size = sizeof(*ev) + len + 1;
@@ -164,18 +165,85 @@ struct event *add_event(struct divecomputer *dc, unsigned int time, int type, in
 		break;
 	}
 
+	return ev;
+}
+
+/* warning: does not test idx for validity */
+struct event *create_gas_switch_event(struct dive *dive, struct divecomputer *dc, int seconds, int idx)
+{
+	/* The gas switch event format is insane for historical reasons */
+	struct gasmix mix = get_cylinder(dive, idx)->gasmix;
+	int o2 = get_o2(mix);
+	int he = get_he(mix);
+	struct event *ev;
+	int value;
+
+	o2 = (o2 + 5) / 10;
+	he = (he + 5) / 10;
+	value = o2 + (he << 16);
+
+	ev = create_event(seconds, he ? SAMPLE_EVENT_GASCHANGE2 : SAMPLE_EVENT_GASCHANGE, 0, value, "gaschange");
+	ev->gas.index = idx;
+	ev->gas.mix = mix;
+	return ev;
+}
+
+struct event *clone_event_rename(const struct event *ev, const char *name)
+{
+	return create_event(ev->time.seconds, ev->type, ev->flags, ev->value, name);
+}
+
+void add_event_to_dc(struct divecomputer *dc, struct event *ev)
+{
+	struct event **p;
+
 	p = &dc->events;
 
 	/* insert in the sorted list of events */
-	while (*p && (*p)->time.seconds <= time)
+	while (*p && (*p)->time.seconds <= ev->time.seconds)
 		p = &(*p)->next;
 	ev->next = *p;
 	*p = ev;
+}
+
+struct event *add_event(struct divecomputer *dc, unsigned int time, int type, int flags, int value, const char *name)
+{
+	struct event *ev = create_event(time, type, flags, value, name);
+
+	if (!ev)
+		return NULL;
+
+	add_event_to_dc(dc, ev);
+
 	remember_event(name);
 	return ev;
 }
 
-static int same_event(const struct event *a, const struct event *b)
+void add_gas_switch_event(struct dive *dive, struct divecomputer *dc, int seconds, int idx)
+{
+	/* sanity check so we don't crash */
+	if (idx < 0 || idx >= dive->cylinders.nr) {
+		report_error("Unknown cylinder index: %d", idx);
+		return;
+	}
+	struct event *ev = create_gas_switch_event(dive, dc, seconds, idx);
+	add_event_to_dc(dc, ev);
+}
+
+/* Substitutes an event in a divecomputer for another. No reordering is performed! */
+void swap_event(struct divecomputer *dc, struct event *from, struct event *to)
+{
+	for (struct event **ep = &dc->events; *ep; ep = &(*ep)->next) {
+		if (*ep == from) {
+			to->next = from->next;
+			*ep = to;
+			from->next = NULL; // For good measure.
+			break;
+		}
+	}
+}
+
+bool same_event(const struct event *a, const struct event *b)
 {
 	if (a->time.seconds != b->time.seconds)
 		return 0;
@@ -188,19 +256,15 @@ static int same_event(const struct event *a, const struct event *b)
 	return !strcmp(a->name, b->name);
 }
 
-void remove_event(struct event *event)
+/* Remove given event from dive computer. Does *not* free the event. */
+void remove_event_from_dc(struct divecomputer *dc, struct event *event)
 {
-	struct event **ep = &current_dc->events;
+	for (struct event **ep = &dc->events; *ep; ep = &(*ep)->next) {
-	while (ep && !same_event(*ep, event))
+		if (*ep == event) {
-		ep = &(*ep)->next;
+			*ep = event->next;
-	if (ep) {
+			event->next = NULL; // For good measure.
-		/* we can't link directly with event->next
+			break;
-		 * because 'event' can be a copy from another
+		}
-		 * dive (for instance the displayed_dive
-		 * that we use on the interface to show things). */
-		struct event *temp = (*ep)->next;
-		free(*ep);
-		*ep = temp;
 	}
 }
 

core/dive.h

@@ -358,10 +358,15 @@ extern void copy_used_cylinders(const struct dive *s, struct dive *d, bool used_
 extern void copy_samples(const struct divecomputer *s, struct divecomputer *d);
 extern bool is_cylinder_used(const struct dive *dive, int idx);
 extern bool is_cylinder_prot(const struct dive *dive, int idx);
-extern void fill_default_cylinder(const struct dive *dive, cylinder_t *cyl);
 extern void add_gas_switch_event(struct dive *dive, struct divecomputer *dc, int time, int idx);
+extern struct event *create_event(unsigned int time, int type, int flags, int value, const char *name);
+extern struct event *create_gas_switch_event(struct dive *dive, struct divecomputer *dc, int seconds, int idx);
+extern struct event *clone_event_rename(const struct event *ev, const char *name);
+extern void add_event_to_dc(struct divecomputer *dc, struct event *ev);
+extern void swap_event(struct divecomputer *dc, struct event *from, struct event *to);
+extern bool same_event(const struct event *a, const struct event *b);
 extern struct event *add_event(struct divecomputer *dc, unsigned int time, int type, int flags, int value, const char *name);
-extern void remove_event(struct event *event);
+extern void remove_event_from_dc(struct divecomputer *dc, struct event *event);
 extern void update_event_name(struct dive *d, struct event *event, const char *name);
 extern void add_extra_data(struct divecomputer *dc, const char *key, const char *value);
 extern void per_cylinder_mean_depth(const struct dive *dive, struct divecomputer *dc, int *mean, int *duration);
@@ -372,15 +377,9 @@ extern int nr_weightsystems(const struct dive *dive);
 
 /* UI related protopypes */
 
-// extern void report_error(GError* error);
-
 extern void remember_event(const char *eventname);
 extern void invalidate_dive_cache(struct dive *dc);
 
-#if WE_DONT_USE_THIS /* this is a missing feature in Qt - selecting which events to display */
-extern int evn_foreach(void (*callback)(const char *, bool *, void *), void *data);
-#endif /* WE_DONT_USE_THIS */
-
 extern void clear_events(void);
 
 extern void set_dc_nickname(struct dive *dive);

core/equipment.c

@@ -29,7 +29,7 @@ void free_weightsystem(weightsystem_t ws)
 	ws.description = NULL;
 }
 
-static void free_cylinder(cylinder_t c)
+void free_cylinder(cylinder_t c)
 {
 	free((void *)c.type.description);
 	c.type.description = NULL;
@@ -137,12 +137,18 @@ void add_cloned_weightsystem_at(struct weightsystem_table *t, weightsystem_t ws)
 	add_to_weightsystem_table(t, t->nr, clone_weightsystem(ws));
 }
 
+cylinder_t clone_cylinder(cylinder_t cyl)
+{
+	cylinder_t res = cyl;
+	res.type.description = copy_string(res.type.description);
+	return res;
+}
+
 /* Add a clone of a cylinder to the end of a cylinder table.
  * Cloned in means that the description-string is copied. */
 void add_cloned_cylinder(struct cylinder_table *t, cylinder_t cyl)
 {
-	cyl.type.description = copy_string(cyl.type.description);
+	add_to_cylinder_table(t, t->nr, clone_cylinder(cyl));
-	add_to_cylinder_table(t, t->nr, cyl);
 }
 
 bool same_weightsystem(weightsystem_t w1, weightsystem_t w2)
@@ -151,15 +157,6 @@ bool same_weightsystem(weightsystem_t w1, weightsystem_t w2)
 	       same_string(w1.description, w2.description);
 }
 
-bool same_cylinder(cylinder_t cyl1, cylinder_t cyl2)
-{
-	return same_string(cyl1.type.description, cyl2.type.description) &&
-	       same_gasmix(cyl1.gasmix, cyl2.gasmix) &&
-	       cyl1.start.mbar == cyl2.start.mbar &&
-	       cyl1.end.mbar == cyl2.end.mbar &&
-	       cyl1.cylinder_use == cyl2.cylinder_use;
-}
-
 void get_gas_string(struct gasmix gasmix, char *text, int len)
 {
 	if (gasmix_is_air(gasmix))
@@ -377,6 +374,46 @@ cylinder_t *get_or_create_cylinder(struct dive *d, int idx)
 	return &d->cylinders.cylinders[idx];
 }
 
+/* if a default cylinder is set, use that */
+void fill_default_cylinder(const struct dive *dive, cylinder_t *cyl)
+{
+	const char *cyl_name = prefs.default_cylinder;
+	struct tank_info_t *ti = tank_info;
+	pressure_t pO2 = {.mbar = lrint(prefs.modpO2 * 1000.0)};
+
+	if (!cyl_name)
+		return;
+	while (ti->name != NULL && ti < tank_info + MAX_TANK_INFO) {
+		if (strcmp(ti->name, cyl_name) == 0)
+			break;
+		ti++;
+	}
+	if (ti->name == NULL)
+		/* didn't find it */
+		return;
+	cyl->type.description = strdup(ti->name);
+	if (ti->ml) {
+		cyl->type.size.mliter = ti->ml;
+		cyl->type.workingpressure.mbar = ti->bar * 1000;
+	} else {
+		cyl->type.workingpressure.mbar = psi_to_mbar(ti->psi);
+		if (ti->psi)
+			cyl->type.size.mliter = lrint(cuft_to_l(ti->cuft) * 1000 / bar_to_atm(psi_to_bar(ti->psi)));
+	}
+	// MOD of air
+	cyl->depth = gas_mod(cyl->gasmix, pO2, dive, 1);
+}
+
+cylinder_t create_new_cylinder(const struct dive *d)
+{
+	cylinder_t cyl = empty_cylinder;
+	fill_default_cylinder(d, &cyl);
+	cyl.start = cyl.type.workingpressure;
+	cyl.manually_added = true;
+	cyl.cylinder_use = OC_GAS;
+	return cyl;
+}
+
 #ifdef DEBUG_CYL
 void dump_cylinders(struct dive *dive, bool verbose)
 {

core/equipment.h

@@ -75,6 +75,8 @@ extern weightsystem_t clone_weightsystem(weightsystem_t ws);
 extern void free_weightsystem(weightsystem_t ws);
 extern void copy_cylinder_types(const struct dive *s, struct dive *d);
 extern void add_cloned_weightsystem(struct weightsystem_table *t, weightsystem_t ws);
+extern cylinder_t clone_cylinder(cylinder_t cyl);
+extern void free_cylinder(cylinder_t cyl);
 extern cylinder_t *add_empty_cylinder(struct cylinder_table *t);
 extern void add_cloned_cylinder(struct cylinder_table *t, cylinder_t cyl);
 extern cylinder_t *get_cylinder(const struct dive *d, int idx);
@@ -82,13 +84,14 @@ extern cylinder_t *get_or_create_cylinder(struct dive *d, int idx);
 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 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);
+extern void fill_default_cylinder(const struct dive *dive, cylinder_t *cyl); /* dive is needed to fill out MOD, which depends on salinity. */
+extern cylinder_t create_new_cylinder(const struct dive *dive); /* dive is needed to fill out MOD, which depends on salinity. */
 #ifdef DEBUG_CYL
 extern void dump_cylinders(struct dive *dive, bool verbose);
 #endif

core/parse-xml.c

@@ -698,31 +698,6 @@ static void try_to_match_autogroup(const char *name, char *buf)
 	nonmatch("autogroup", name, buf);
 }
 
-void add_gas_switch_event(struct dive *dive, struct divecomputer *dc, int seconds, int idx)
-{
-	/* sanity check so we don't crash */
-	if (idx < 0 || idx >= dive->cylinders.nr) {
-		report_error("Unknown cylinder index: %d", idx);
-		return;
-	}
-	/* The gas switch event format is insane for historical reasons */
-	struct gasmix mix = get_cylinder(dive, idx)->gasmix;
-	int o2 = get_o2(mix);
-	int he = get_he(mix);
-	struct event *ev;
-	int value;
-
-	o2 = (o2 + 5) / 10;
-	he = (he + 5) / 10;
-	value = o2 + (he << 16);
-
-	ev = add_event(dc, seconds, he ? SAMPLE_EVENT_GASCHANGE2 : SAMPLE_EVENT_GASCHANGE, 0, value, "gaschange");
-	if (ev) {
-		ev->gas.index = idx;
-		ev->gas.mix = mix;
-	}
-}
-
 static void get_cylinderindex(char *buffer, uint8_t *i, struct parser_state *state)
 {
 	*i = atoi(buffer);

core/planner.c

@@ -175,37 +175,6 @@ static int tissue_at_end(struct deco_state *ds, struct dive *dive, struct deco_s
 	return surface_interval;
 }
 
-
-/* if a default cylinder is set, use that */
-void fill_default_cylinder(const struct dive *dive, cylinder_t *cyl)
-{
-	const char *cyl_name = prefs.default_cylinder;
-	struct tank_info_t *ti = tank_info;
-	pressure_t pO2 = {.mbar = 1600};
-
-	if (!cyl_name)
-		return;
-	while (ti->name != NULL && ti < tank_info + MAX_TANK_INFO) {
-		if (strcmp(ti->name, cyl_name) == 0)
-			break;
-		ti++;
-	}
-	if (ti->name == NULL)
-		/* didn't find it */
-		return;
-	cyl->type.description = strdup(ti->name);
-	if (ti->ml) {
-		cyl->type.size.mliter = ti->ml;
-		cyl->type.workingpressure.mbar = ti->bar * 1000;
-	} else {
-		cyl->type.workingpressure.mbar = psi_to_mbar(ti->psi);
-		if (ti->psi)
-			cyl->type.size.mliter = lrint(cuft_to_l(ti->cuft) * 1000 / bar_to_atm(psi_to_bar(ti->psi)));
-	}
-	// MOD of air
-	cyl->depth = gas_mod(cyl->gasmix, pO2, dive, 1);
-}
-
 /* calculate the new end pressure of the cylinder, based on its current end pressure and the
  * latest segment. */
 static void update_cylinder_pressure(struct dive *d, int old_depth, int new_depth, int duration, int sac, cylinder_t *cyl, bool in_deco, enum divemode_t divemode)
@@ -315,7 +284,7 @@ static void create_dive_from_plan(struct diveplan *diveplan, struct dive *dive,
 		}
 		if (dp->divemode != type) {
 			type = dp->divemode;
-			add_event(dc, lasttime, 8, 0, type, "modechange");
+			add_event(dc, lasttime, SAMPLE_EVENT_BOOKMARK, 0, type, "modechange");
 		}
 
 		/* Create sample */

core/profile.c

@@ -120,19 +120,6 @@ struct ev_select *ev_namelist;
 int evn_allocated;
 int evn_used;
 
-#if WE_DONT_USE_THIS /* we need to implement event filters in Qt */
-int evn_foreach (void (*callback)(const char *, bool *, void *), void *data)
-{
-	int i;
-
-	for (i = 0; i < evn_used; i++) {
-		/* here we display an event name on screen - so translate */
-		callback(translate("gettextFromC", ev_namelist[i].ev_name), &ev_namelist[i].plot_ev, data);
-	}
-	return i;
-}
-#endif /* WE_DONT_USE_THIS */
-
 void clear_events(void)
 {
 	for (int i = 0; i < evn_used; i++)

core/qthelper.cpp

@@ -1662,3 +1662,36 @@ extern "C" char *get_changes_made()
 	else
 		return nullptr;
 }
+
+// Generate a cylinder-renumber map for use when the n-th cylinder
+// of a dive with count cylinders is removed. It fills an int vector
+// with 0..n, -1, n..count-1. Each entry in the vector represents
+// the new id of the cylinder, whereby <0 means that this particular
+// cylinder does not get any new id. This should probably be moved
+// to the C-core, but using std::vector is simply more convenient.
+// The function assumes that n < count!
+std::vector<int> get_cylinder_map_for_remove(int count, int n)
+{
+	// 1) Fill mapping[0]..mapping[n-1] with 0..n-1
+	// 2) Set mapping[n] to -1
+	// 3) Fill mapping[n+1]..mapping[count-1] with n..count-2
+	std::vector<int> mapping(count);
+	std::iota(mapping.begin(), mapping.begin() + n, 0);
+	mapping[n] = -1;
+	std::iota(mapping.begin() + n + 1, mapping.end(), n);
+	return mapping;
+}
+
+// Generate a cylinder-renumber map for use when a cylinder is added
+// before the n-th cylinder. It fills an int vector with
+// with 0..n-1, n+1..count. Each entry in the vector represents
+// the new id of the cylinder. This probably should be moved
+// to the C-core, but using std::vector is simply more convenient.
+// This function assumes that that n <= count!
+std::vector<int> get_cylinder_map_for_add(int count, int n)
+{
+	std::vector<int> mapping(count);
+	std::iota(mapping.begin(), mapping.begin() + n, 0);
+	std::iota(mapping.begin() + n, mapping.end(), n + 1);
+	return mapping;
+}

core/qthelper.h

@@ -83,6 +83,8 @@ QLocale getLocale();
 QVector<QPair<QString, int>> selectedDivesGasUsed();
 QString getUserAgent();
 QString printGPSCoords(const location_t *loc);
+std::vector<int> get_cylinder_map_for_remove(int count, int n);
+std::vector<int> get_cylinder_map_for_add(int count, int n);
 
 extern QString (*changesCallback)();
 void uiNotification(const QString &msg);

core/subsurface-qt/divelistnotifier.h

@@ -87,6 +87,9 @@ signals:
 	void divesTimeChanged(timestamp_t delta, const QVector<dive *> &dives);
 
 	void cylindersReset(const QVector<dive *> &dives);
+	void cylinderAdded(dive *d, int pos);
+	void cylinderRemoved(dive *d, int pos);
+	void cylinderEdited(dive *d, int pos);
 	void weightsystemsReset(const QVector<dive *> &dives);
 	void weightAdded(dive *d, int pos);
 	void weightRemoved(dive *d, int pos);
@@ -110,6 +113,9 @@ signals:
 	void numShownChanged();
 	void filterReset();
 
+	// Event-related signals. Currently, we're very blunt: only one signal for any changes to the events.
+	void eventsChanged(dive *d);
+
 	// This signal is emited every time a command is executed.
 	// This is used to hide an old multi-dives-edited warning message.
 	// This is necessary, so that the user can't click on the "undo" button and undo

desktop-widgets/mainwindow.cpp

@@ -710,6 +710,8 @@ void MainWindow::on_actionPrint_triggered()
 
 void MainWindow::disableShortcuts(bool disablePaste)
 {
+	undoAction->setEnabled(false);
+	redoAction->setEnabled(false);
 	ui.actionPreviousDC->setShortcut(QKeySequence());
 	ui.actionNextDC->setShortcut(QKeySequence());
 	ui.copy->setShortcut(QKeySequence());
@@ -719,6 +721,8 @@ void MainWindow::disableShortcuts(bool disablePaste)
 
 void MainWindow::enableShortcuts()
 {
+	undoAction->setEnabled(true);
+	redoAction->setEnabled(true);
 	ui.actionPreviousDC->setShortcut(Qt::Key_Left);
 	ui.actionNextDC->setShortcut(Qt::Key_Right);
 	ui.copy->setShortcut(QKeySequence(Qt::CTRL + Qt::Key_C));
@@ -918,7 +922,7 @@ void MainWindow::on_actionReplanDive_triggered()
 		divePlannerWidget->setSalinity(current_dive->salinity);
 	DivePlannerPointsModel::instance()->loadFromDive(current_dive);
 	reset_cylinders(&displayed_dive, true);
-	DivePlannerPointsModel::instance()->cylindersModel()->updateDive();
+	DivePlannerPointsModel::instance()->cylindersModel()->updateDive(&displayed_dive);
 }
 
 void MainWindow::on_actionDivePlanner_triggered()
@@ -1088,40 +1092,6 @@ void MainWindow::on_actionViewAll_triggered()
 	ui.bottomSplitter->setCollapsible(1,false);
 }
 
-void MainWindow::enterEditState()
-{
-	undoAction->setEnabled(false);
-	redoAction->setEnabled(false);
-	stateBeforeEdit = state;
-	if (state == VIEWALL || state == INFO_MAXIMIZED)
-		return;
-	toggleCollapsible(true);
-	beginChangeState(EDIT);
-	ui.topSplitter->setSizes({ EXPANDED, EXPANDED });
-	ui.mainSplitter->setSizes({ EXPANDED, COLLAPSED });
-	int appW = qApp->desktop()->size().width();
-	QList<int> infoProfileSizes { round_int(appW * 0.3), round_int(appW * 0.7) };
-
-	QSettings settings;
-	settings.beginGroup("MainWindow");
-	if (settings.value("mainSplitter").isValid()) {
-		ui.topSplitter->restoreState(settings.value("topSplitter").toByteArray());
-		if (ui.topSplitter->sizes().first() == 0 || ui.topSplitter->sizes().last() == 0)
-			ui.topSplitter->setSizes(infoProfileSizes);
-	} else {
-		ui.topSplitter->setSizes(infoProfileSizes);
-	}
-}
-
-void MainWindow::exitEditState()
-{
-	undoAction->setEnabled(true);
-	redoAction->setEnabled(true);
-	if (stateBeforeEdit == state)
-		return;
-	enterState(stateBeforeEdit);
-}
-
 void MainWindow::enterState(CurrentState newState)
 {
 	state = newState;
@@ -1141,8 +1111,6 @@ void MainWindow::enterState(CurrentState newState)
 	case PROFILE_MAXIMIZED:
 		on_actionViewProfile_triggered();
 		break;
-	case EDIT:
-		break;
 	}
 }
 
@@ -1793,20 +1761,18 @@ void MainWindow::editCurrentDive()
 	struct dive *d = current_dive;
 	QString defaultDC(d->dc.model);
 	DivePlannerPointsModel::instance()->clear();
-	disableShortcuts();
 	if (defaultDC == "manually added dive") {
+		disableShortcuts();
 		DivePlannerPointsModel::instance()->setPlanMode(DivePlannerPointsModel::ADD);
 		graphics->setAddState();
 		setApplicationState(ApplicationState::EditDive);
 		DivePlannerPointsModel::instance()->loadFromDive(d);
-		mainTab->enableEdition(MainTab::MANUALLY_ADDED_DIVE);
+		mainTab->enableEdition();
 	} else if (defaultDC == "planned dive") {
+		disableShortcuts();
 		DivePlannerPointsModel::instance()->setPlanMode(DivePlannerPointsModel::PLAN);
 		setApplicationState(ApplicationState::EditPlannedDive);
 		DivePlannerPointsModel::instance()->loadFromDive(d);
-		mainTab->enableEdition(MainTab::MANUALLY_ADDED_DIVE);
-	} else {
-		setApplicationState(ApplicationState::EditDive);
 		mainTab->enableEdition();
 	}
 }

desktop-widgets/mainwindow.h

@@ -58,7 +58,6 @@ public:
 		INFO_MAXIMIZED,
 		PROFILE_MAXIMIZED,
 		LIST_MAXIMIZED,
-		EDIT,
 	};
 
 	MainWindow();
@@ -81,8 +80,6 @@ public:
 	NotificationWidget *getNotificationWidget();
 	void enableDisableCloudActions();
 	void enableDisableOtherDCsActions();
-	void enterEditState();
-	void exitEditState();
 	void editDiveSite(dive_site *ds);
 
 	std::unique_ptr<MainTab> mainTab;

desktop-widgets/modeldelegates.cpp

@@ -38,7 +38,6 @@ QSize DiveListDelegate::sizeHint(const QStyleOptionViewItem&, const QModelIndex&
 // Gets the index of the model in the currentRow and column.
 // currCombo is defined below.
 #define IDX(_XX) mymodel->index(currCombo.currRow, (_XX))
-static bool keyboardFinished = false;
 
 StarWidgetsDelegate::StarWidgetsDelegate(QWidget *parent) : QStyledItemDelegate(parent),
 	parentWidget(parent)
@@ -86,7 +85,6 @@ ComboBoxDelegate::ComboBoxDelegate(QAbstractItemModel *model, QObject *parent, b
 {
 	editable = allowEdit;
 	connect(this, &ComboBoxDelegate::closeEditor, this, &ComboBoxDelegate::editorClosed);
-	connect(this, &ComboBoxDelegate::closeEditor, this, &ComboBoxDelegate::fixTabBehavior);
 }
 
 void ComboBoxDelegate::setEditorData(QWidget *editor, const QModelIndex &index) const
@@ -101,14 +99,6 @@ void ComboBoxDelegate::setEditorData(QWidget *editor, const QModelIndex &index)
 	c->lineEdit()->setSelection(0, c->lineEdit()->text().length());
 }
 
-static struct CurrSelected {
-	QComboBox *comboEditor;
-	int currRow;
-	QString activeText;
-	QAbstractItemModel *model;
-	bool ignoreSelection;
-} currCombo;
-
 QWidget *ComboBoxDelegate::createEditor(QWidget *parent, const QStyleOptionViewItem&, const QModelIndex &index) const
 {
 	QComboBox *comboDelegate = new QComboBox(parent);
@@ -129,7 +119,7 @@ QWidget *ComboBoxDelegate::createEditor(QWidget *parent, const QStyleOptionViewI
 	currCombo.comboEditor = comboDelegate;
 	currCombo.currRow = index.row();
 	currCombo.model = const_cast<QAbstractItemModel *>(index.model());
-	keyboardFinished = false;
+	currCombo.activeText = currCombo.model->data(index).toString();
 
 	// Current display of things on Gnome3 looks like shit, so
 	// let`s fix that.
@@ -177,16 +167,6 @@ void ComboBoxDelegate::fakeActivation()
 	QStyledItemDelegate::eventFilter(currCombo.comboEditor, &ev);
 }
 
-// This 'reverts' the model data to what we actually choosed,
-// becaus e a TAB is being understood by Qt as 'cancel' while
-// we are on a QComboBox ( but not on a QLineEdit.
-void ComboBoxDelegate::fixTabBehavior()
-{
-	if (keyboardFinished) {
-		setModelData(0, 0, QModelIndex());
-	}
-}
-
 bool ComboBoxDelegate::eventFilter(QObject *object, QEvent *event)
 {
 	// Reacts on Key_UP and Key_DOWN to show the QComboBox - list of choices.
@@ -200,10 +180,8 @@ bool ComboBoxDelegate::eventFilter(QObject *object, QEvent *event)
 					return true;
 				}
 			}
-			if (ev->key() == Qt::Key_Tab || ev->key() == Qt::Key_Enter || ev->key() == Qt::Key_Return) {
+			if (ev->key() == Qt::Key_Tab || ev->key() == Qt::Key_Enter || ev->key() == Qt::Key_Return)
 				currCombo.activeText = currCombo.comboEditor->currentText();
-				keyboardFinished = true;
-			}
 		} else { // the 'Drop Down Menu' part.
 			QKeyEvent *ev = static_cast<QKeyEvent *>(event);
 			if (ev->key() == Qt::Key_Enter || ev->key() == Qt::Key_Return ||
@@ -230,12 +208,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,46 +226,30 @@ 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)
 {
-	connect(this, SIGNAL(closeEditor(QWidget *, QAbstractItemDelegate::EndEditHint)),
-		this, SLOT(reenableReplot(QWidget *, QAbstractItemDelegate::EndEditHint)));
-}
-
-void TankInfoDelegate::reenableReplot(QWidget*, QAbstractItemDelegate::EndEditHint)
-{
-	MainWindow::instance()->graphics->setReplot(true);
-	// FIXME: We need to replot after a cylinder is selected but the replot below overwrites
-	//        the newly selected cylinder.
-	//	MainWindow::instance()->graphics->replot();
 }
 
 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);
+	// Ugly hack: We misuse setData() with COMMIT_ROLE or REVERT_ROLE to commit or
-		mymodel->setData(IDX(CylindersModel::WORKINGPRESS), currCylinderData.pressure, CylindersModel::PASS_IN_ROLE);
+	// revert the current row. We send in the type, because we may get multiple
-		mymodel->setData(IDX(CylindersModel::SIZE), currCylinderData.size, CylindersModel::PASS_IN_ROLE);
+	// 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<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;
 }

desktop-widgets/modeldelegates.h

@@ -44,12 +44,18 @@ slots:
 	void testActivation(const QModelIndex &currIndex);
 	//HACK: try to get rid of this in the future.
 	void fakeActivation();
-	void fixTabBehavior();
 	virtual void editorClosed(QWidget *widget, QAbstractItemDelegate::EndEditHint hint) = 0;
 private:
 	bool editable;
 protected:
 	QAbstractItemModel *model;
+	mutable struct CurrSelected {
+		QComboBox *comboEditor;
+		int currRow;
+		QString activeText;
+		QAbstractItemModel *model;
+		bool ignoreSelection;
+	} currCombo;
 };
 
 class TankInfoDelegate : public ComboBoxDelegate {
@@ -61,7 +67,6 @@ public:
 public
 slots:
 	void editorClosed(QWidget *widget, QAbstractItemDelegate::EndEditHint hint);
-	void reenableReplot(QWidget *widget, QAbstractItemDelegate::EndEditHint hint);
 };
 
 class TankUseDelegate : public QStyledItemDelegate {

desktop-widgets/simplewidgets.cpp

@@ -23,7 +23,6 @@
 #include "commands/command.h"
 #include "core/metadata.h"
 #include "core/tag.h"
-#include "core/divelist.h" // for mark_divelist_changed
 
 double MinMaxAvgWidget::average() const
 {
@@ -174,38 +173,21 @@ RenumberDialog::RenumberDialog(QWidget *parent) : QDialog(parent), selectedOnly(
 	connect(quit, SIGNAL(activated()), parent, SLOT(close()));
 }
 
-SetpointDialog *SetpointDialog::instance()
-{
-	static SetpointDialog *self = new SetpointDialog(MainWindow::instance());
-	return self;
-}
-
-void SetpointDialog::setpointData(struct divecomputer *divecomputer, int second)
-{
-	dc = divecomputer;
-	time = second < 0 ? 0 : second;
-}
-
 void SetpointDialog::buttonClicked(QAbstractButton *button)
 {
-	if (ui.buttonBox->buttonRole(button) == QDialogButtonBox::AcceptRole && dc) {
+	if (ui.buttonBox->buttonRole(button) == QDialogButtonBox::AcceptRole)
-		add_event(dc, time, SAMPLE_EVENT_PO2, 0, (int)(1000.0 * ui.spinbox->value()),
+		Command::addEventSetpointChange(d, dcNr, time, pressure_t { (int)(1000.0 * ui.spinbox->value()) });
-			QT_TRANSLATE_NOOP("gettextFromC", "SP change"));
-		invalidate_dive_cache(current_dive);
-	}
-	mark_divelist_changed(true);
-	MainWindow::instance()->graphics->replot();
 }
 
-SetpointDialog::SetpointDialog(QWidget *parent) : QDialog(parent),
+SetpointDialog::SetpointDialog(struct dive *dIn, int dcNrIn, int seconds) : QDialog(MainWindow::instance()),
-	dc(0), time(0)
+	d(dIn), dcNr(dcNrIn), time(seconds < 0 ? 0 : seconds)
 {
 	ui.setupUi(this);
-	connect(ui.buttonBox, SIGNAL(clicked(QAbstractButton *)), this, SLOT(buttonClicked(QAbstractButton *)));
+	connect(ui.buttonBox, &QDialogButtonBox::clicked, this, &SetpointDialog::buttonClicked);
 	QShortcut *close = new QShortcut(QKeySequence(Qt::CTRL + Qt::Key_W), this);
-	connect(close, SIGNAL(activated()), this, SLOT(close()));
+	connect(close, &QShortcut::activated, this, &QDialog::close);
 	QShortcut *quit = new QShortcut(QKeySequence(Qt::CTRL + Qt::Key_Q), this);
-	connect(quit, SIGNAL(activated()), parent, SLOT(close()));
+	connect(quit, &QShortcut::activated, MainWindow::instance(), &QWidget::close);
 }
 
 ShiftTimesDialog *ShiftTimesDialog::instance()

desktop-widgets/simplewidgets.h

@@ -65,16 +65,15 @@ private:
 class SetpointDialog : public QDialog {
 	Q_OBJECT
 public:
-	static SetpointDialog *instance();
+	SetpointDialog(struct dive *d, int dcNr, int time);
-	void setpointData(struct divecomputer *divecomputer, int time);
 private
 slots:
 	void buttonClicked(QAbstractButton *button);
 
 private:
-	explicit SetpointDialog(QWidget *parent);
 	Ui::SetpointDialog ui;
-	struct divecomputer *dc;
+	struct dive *d;
+	int dcNr;
 	int time;
 };
 

desktop-widgets/tab-widgets/TabDiveEquipment.cpp

@@ -1,18 +1,13 @@
 // SPDX-License-Identifier: GPL-2.0
 #include "TabDiveEquipment.h"
 #include "maintab.h"
-#include "desktop-widgets/mainwindow.h" // TODO: Only used temporarilly for edit mode changes
 #include "desktop-widgets/simplewidgets.h" // For isGnome3Session()
 #include "desktop-widgets/modeldelegates.h"
 #include "commands/command.h"
-#include "profile-widget/profilewidget2.h"
 
 #include "qt-models/cylindermodel.h"
 #include "qt-models/weightmodel.h"
 
-#include "core/subsurface-string.h"
-#include "core/divelist.h"
-
 #include <QSettings>
 #include <QCompleter>
 
@@ -33,9 +28,10 @@ TabDiveEquipment::TabDiveEquipment(QWidget *parent) : TabBase(parent),
 	ui.weights->setModel(weightModel);
 
 	connect(&diveListNotifier, &DiveListNotifier::divesChanged, this, &TabDiveEquipment::divesChanged);
-	connect(ui.cylinders, &TableView::itemClicked, cylindersModel, &CylindersModelFiltered::remove);
 	connect(ui.cylinders, &TableView::itemClicked, this, &TabDiveEquipment::editCylinderWidget);
 	connect(ui.weights, &TableView::itemClicked, this, &TabDiveEquipment::editWeightWidget);
+	connect(cylindersModel->model(), &CylindersModel::divesEdited, this, &TabDiveEquipment::divesEdited);
+	connect(weightModel, &WeightModel::divesEdited, this, &TabDiveEquipment::divesEdited);
 
 	// Current display of things on Gnome3 looks like shit, so
 	// let's fix that.
@@ -129,7 +125,7 @@ void TabDiveEquipment::toggleTriggeredColumn()
 
 void TabDiveEquipment::updateData()
 {
-	cylindersModel->updateDive();
+	cylindersModel->updateDive(current_dive);
 	weightModel->updateDive(current_dive);
 	suitModel.updateModel();
 
@@ -153,8 +149,7 @@ void TabDiveEquipment::clear()
 
 void TabDiveEquipment::addCylinder_clicked()
 {
-	MainWindow::instance()->mainTab->enableEdition();
+	divesEdited(Command::addCylinder(false));
-	cylindersModel->add();
 }
 
 void TabDiveEquipment::addWeight_clicked()
@@ -164,15 +159,14 @@ void TabDiveEquipment::addWeight_clicked()
 
 void TabDiveEquipment::editCylinderWidget(const QModelIndex &index)
 {
-	if (cylindersModel->model()->changed && !MainWindow::instance()->mainTab->isEditing()) {
+	if (!index.isValid())
-		MainWindow::instance()->mainTab->enableEdition();
 		return;
-	}
+
-	if (index.isValid() && index.column() != CylindersModel::REMOVE) {
+	if (index.column() == CylindersModel::REMOVE)
-		MainWindow::instance()->mainTab->enableEdition();
+		divesEdited(Command::removeCylinder(cylindersModel->mapToSource(index).row(), false));
+	else
 		ui.cylinders->edit(index);
 }
-}
 
 void TabDiveEquipment::editWeightWidget(const QModelIndex &index)
 {
@@ -185,87 +179,6 @@ void TabDiveEquipment::editWeightWidget(const QModelIndex &index)
 		ui.weights->edit(index);
 }
 
-// tricky little macro to edit all the selected dives
-// loop ove all DIVES and do WHAT.
-#define MODIFY_DIVES(DIVES, WHAT)                            \
-	do {                                                 \
-		for (dive *mydive: DIVES) {                  \
-			invalidate_dive_cache(mydive);       \
-			WHAT;                                \
-		}					     \
-		mark_divelist_changed(true);                 \
-	} while (0)
-
-// Get the list of selected dives, but put the current dive at the last position of the vector
-static QVector<dive *> getSelectedDivesCurrentLast()
-{
-	QVector<dive *> res;
-	struct dive *d;
-	int i;
-	for_each_dive (i, d) {
-		if (d->selected && d != current_dive)
-			res.append(d);
-	}
-	res.append(current_dive);
-	return res;
-}
-
-// TODO: This is a temporary functions until undo of cylinders is implemented.
-// Therefore it is not worth putting it in a header.
-extern bool cylinders_equal(const dive *d1, const dive *d2);
-
-void TabDiveEquipment::acceptChanges()
-{
-	bool do_replot = false;
-
-	// now check if something has changed and if yes, edit the selected dives that
-	// were identical with the master dive shown (and mark the divelist as changed)
-	struct dive *cd = current_dive;
-
-	// Get list of selected dives, but put the current dive last;
-	// this is required in case the invocation wants to compare things
-	// to the original value in current_dive like it should
-	QVector<dive *> selectedDives = getSelectedDivesCurrentLast();
-
-	if (cylindersModel->model()->changed) {
-		mark_divelist_changed(true);
-		MODIFY_DIVES(selectedDives,
-			// if we started out with the same cylinder description (for multi-edit) or if we do copt & paste
-			// make sure that we have the same cylinder type and copy the gasmix, but DON'T copy the start
-			// and end pressures (those are per dive after all)
-			if (cylinders_equal(mydive, cd) && mydive != cd)
-				copy_cylinder_types(&displayed_dive, cd);
-			copy_cylinders(&displayed_dive.cylinders, &cd->cylinders);
-		);
-		/* if cylinders changed we may have changed gas change events
-		 * and sensor idx in samples as well
-		 * - so far this is ONLY supported for a single selected dive */
-		struct divecomputer *tdc = &current_dive->dc;
-		struct divecomputer *sdc = &displayed_dive.dc;
-		while(tdc && sdc) {
-			free_events(tdc->events);
-			copy_events(sdc, tdc);
-			free(tdc->sample);
-			copy_samples(sdc, tdc);
-			tdc = tdc->next;
-			sdc = sdc->next;
-		}
-		do_replot = true;
-	}
-
-	if (do_replot)
-		MainWindow::instance()->graphics->replot();
-
-	cylindersModel->model()->changed = false;
-}
-
-void TabDiveEquipment::rejectChanges()
-{
-	cylindersModel->model()->changed = false;
-	cylindersModel->updateDive();
-	weightModel->updateDive(current_dive);
-}
-
 void TabDiveEquipment::divesEdited(int i)
 {
 	// No warning if only one dive was edited

desktop-widgets/tab-widgets/TabDiveEquipment.h

@@ -21,9 +21,6 @@ public:
 	~TabDiveEquipment();
 	void updateData() override;
 	void clear() override;
-	void acceptChanges();
-	void rejectChanges();
-	void divesEdited(int i);
 	void closeWarning();
 
 private slots:
@@ -34,6 +31,7 @@ private slots:
 	void editCylinderWidget(const QModelIndex &index);
 	void editWeightWidget(const QModelIndex &index);
 	void on_suit_editingFinished();
+	void divesEdited(int count);
 
 private:
 	Ui::TabDiveEquipment ui;

desktop-widgets/tab-widgets/maintab.cpp

@@ -49,7 +49,8 @@ struct Completers {
 };
 
 MainTab::MainTab(QWidget *parent) : QTabWidget(parent),
-	editMode(NONE),
+	editMode(false),
+	ignoreInput(false),
 	lastSelectedDive(true),
 	lastTabSelectedDive(0),
 	lastTabSelectedDiveTrip(0),
@@ -206,41 +207,19 @@ void MainTab::displayMessage(QString str)
 	ui.diveNotesMessage->animatedShow();
 }
 
-void MainTab::enableEdition(EditMode newEditMode)
+void MainTab::enableEdition()
 {
-	if (((newEditMode == DIVE || newEditMode == NONE) && current_dive == NULL) || editMode != NONE)
+	if (current_dive == NULL || editMode)
 		return;
-	if ((newEditMode == DIVE || newEditMode == NONE) &&
-	    current_dive->dc.model &&
-	    strcmp(current_dive->dc.model, "manually added dive") == 0) {
-		// editCurrentDive will call enableEdition with newEditMode == MANUALLY_ADDED_DIVE
-		// so exit this function here after editCurrentDive() returns
-
-
-
-		// FIXME : can we get rid of this recursive crap?
-
-
-
-		MainWindow::instance()->editCurrentDive();
-		return;
-	}
 
 	ui.editDiveSiteButton->setEnabled(false);
 	MainWindow::instance()->diveList->setEnabled(false);
 	MainWindow::instance()->setEnabledToolbar(false);
-	MainWindow::instance()->enterEditState();
-	ui.tabWidget->setTabEnabled(2, false);
-	ui.tabWidget->setTabEnabled(3, false);
-	ui.tabWidget->setTabEnabled(5, false);
 
 	ui.dateEdit->setEnabled(true);
-	if (amount_selected > 1) {
-		displayMessage(tr("Multiple dives are being edited."));
-	} else {
 	displayMessage(tr("This dive is being edited."));
-	}
+
-	editMode = newEditMode != NONE ? newEditMode : DIVE;
+	editMode = true;
 }
 
 // This function gets called if a field gets updated by an undo command.
@@ -305,7 +284,7 @@ void MainTab::nextInputField(QKeyEvent *event)
 
 bool MainTab::isEditing()
 {
-	return editMode != NONE;
+	return editMode;
 }
 
 static bool isHtml(const QString &s)
@@ -363,9 +342,8 @@ void MainTab::updateDiveSite(struct dive *d)
 void MainTab::updateDiveInfo()
 {
 	ui.location->refreshDiveSiteCache();
-	EditMode rememberEM = editMode;
 	// don't execute this while adding / planning a dive
-	if (editMode == MANUALLY_ADDED_DIVE || MainWindow::instance()->graphics->isPlanner())
+	if (editMode || MainWindow::instance()->graphics->isPlanner())
 		return;
 
 	// If there is no current dive, disable all widgets except the last, which is the dive site tab.
@@ -375,7 +353,7 @@ void MainTab::updateDiveInfo()
 	for (int i = 0; i < extraWidgets.size() - 1; ++i)
 		extraWidgets[i]->setEnabled(enabled);
 
-	editMode = IGNORE_MODE; // don't trigger on changes to the widgets
+	ignoreInput = true; // don't trigger on changes to the widgets
 
 	for (TabBase *widget: extraWidgets)
 		widget->updateData();
@@ -393,9 +371,6 @@ void MainTab::updateDiveInfo()
 			if (lastSelectedDive && !onDiveSiteTab)
 				lastTabSelectedDive = ui.tabWidget->currentIndex();
 			ui.tabWidget->setTabText(0, tr("Trip notes"));
-			ui.tabWidget->setTabEnabled(1, false);
-			ui.tabWidget->setTabEnabled(2, false);
-			ui.tabWidget->setTabEnabled(5, false);
 			// Recover the tab selected for last dive trip but only if we're not on the dive site tab
 			if (lastSelectedDive && !onDiveSiteTab)
 				ui.tabWidget->setCurrentIndex(lastTabSelectedDiveTrip);
@@ -434,11 +409,6 @@ void MainTab::updateDiveInfo()
 			if (!lastSelectedDive && !onDiveSiteTab)
 				lastTabSelectedDiveTrip = ui.tabWidget->currentIndex();
 			ui.tabWidget->setTabText(0, tr("Notes"));
-			ui.tabWidget->setTabEnabled(1, true);
-			ui.tabWidget->setTabEnabled(2, true);
-			ui.tabWidget->setTabEnabled(3, true);
-			ui.tabWidget->setTabEnabled(4, true);
-			ui.tabWidget->setTabEnabled(5, true);
 			// Recover the tab selected for last dive but only if we're not on the dive site tab
 			if (!lastSelectedDive && !onDiveSiteTab)
 				ui.tabWidget->setCurrentIndex(lastTabSelectedDive);
@@ -506,7 +476,7 @@ void MainTab::updateDiveInfo()
 		ui.timeEdit->setTime(QTime(0, 0, 0, 0));
 		ui.tagWidget->clear();
 	}
-	editMode = rememberEM;
+	ignoreInput = false;
 
 	if (verbose && current_dive && current_dive->dive_site)
 		qDebug() << "Set the current dive site:" << current_dive->dive_site->uuid;
@@ -529,22 +499,17 @@ void MainTab::acceptChanges()
 	if (ui.location->hasFocus())
 		stealFocus();
 
-	EditMode lastMode = editMode;
+	ignoreInput = true;
-	editMode = IGNORE_MODE;
 	ui.dateEdit->setEnabled(true);
 	hideMessage();
 
-	// TODO: This is a temporary hack until the equipment tab is included in the undo system:
+	if (editMode) {
-	// The equipment tab is hardcoded at the first place of the "extra widgets".
-	((TabDiveEquipment *)extraWidgets[0])->acceptChanges();
-
-	if (lastMode == MANUALLY_ADDED_DIVE) {
 		MainWindow::instance()->showProfile();
 		DivePlannerPointsModel::instance()->setPlanMode(DivePlannerPointsModel::NOTHING);
 		Command::editProfile(&displayed_dive);
 	}
 	int scrolledBy = MainWindow::instance()->diveList->verticalScrollBar()->sliderPosition();
-	if (lastMode == MANUALLY_ADDED_DIVE) {
+	if (editMode) {
 		MainWindow::instance()->diveList->reload();
 		MainWindow::instance()->refreshDisplay();
 		MainWindow::instance()->graphics->replot();
@@ -554,39 +519,15 @@ void MainTab::acceptChanges()
 	DivePlannerPointsModel::instance()->setPlanMode(DivePlannerPointsModel::NOTHING);
 	MainWindow::instance()->diveList->verticalScrollBar()->setSliderPosition(scrolledBy);
 	MainWindow::instance()->diveList->setFocus();
-	MainWindow::instance()->exitEditState();
 	MainWindow::instance()->setEnabledToolbar(true);
 	ui.editDiveSiteButton->setEnabled(!ui.location->text().isEmpty());
-	editMode = NONE;
+	ignoreInput = false;
-}
+	editMode = false;
-
-bool weightsystems_equal(const dive *d1, const dive *d2)
-{
-	if (d1->weightsystems.nr != d2->weightsystems.nr)
-		return false;
-	for (int i = 0; i < d1->weightsystems.nr; ++i) {
-		if (!same_weightsystem(d1->weightsystems.weightsystems[i], d2->weightsystems.weightsystems[i]))
-			return false;
-	}
-	return true;
-}
-
-bool cylinders_equal(const dive *d1, const dive *d2)
-{
-	if (d1->cylinders.nr != d2->cylinders.nr)
-		return false;
-	for (int i = 0; i < d1->cylinders.nr; ++i) {
-		if (!same_cylinder(*get_cylinder(d1, i), *get_cylinder(d2, i)))
-			return false;
-	}
-	return true;
 }
 
 void MainTab::rejectChanges()
 {
-	EditMode lastMode = editMode;
+	if (editMode && current_dive) {
-
-	if (lastMode != NONE && current_dive && !cylinders_equal(current_dive, &displayed_dive)) {
 		if (QMessageBox::warning(MainWindow::instance(), TITLE_OR_TEXT(tr("Discard the changes?"),
 									       tr("You are about to discard your changes.")),
 					 QMessageBox::Discard | QMessageBox::Cancel, QMessageBox::Discard) != QMessageBox::Discard) {
@@ -594,7 +535,7 @@ void MainTab::rejectChanges()
 		}
 	}
 	ui.dateEdit->setEnabled(true);
-	editMode = NONE;
+	editMode = false;
 	hideMessage();
 	// no harm done to call cancelPlan even if we were not PLAN mode...
 	DivePlannerPointsModel::instance()->cancelPlan();
@@ -606,17 +547,9 @@ void MainTab::rejectChanges()
 		clear_dive(&displayed_dive);
 	updateDiveInfo();
 
-	// TODO: This is a temporary hack until the equipment tab is included in the undo system:
-	// The equipment tab is hardcoded at the first place of the "extra widgets".
-	((TabDiveEquipment *)extraWidgets[0])->rejectChanges();
-
-	// the user could have edited the location and then canceled the edit
-	// let's get the correct location back in view
-	MapWidget::instance()->centerOnDiveSite(current_dive ? current_dive->dive_site : nullptr);
 	// show the profile and dive info
 	MainWindow::instance()->graphics->replot();
 	MainWindow::instance()->setEnabledToolbar(true);
-	MainWindow::instance()->exitEditState();
 	ui.editDiveSiteButton->setEnabled(!ui.location->text().isEmpty());
 }
 
@@ -632,7 +565,7 @@ void MainTab::divesEdited(int i)
 
 void MainTab::on_buddy_editingFinished()
 {
-	if (editMode == IGNORE_MODE || !current_dive)
+	if (ignoreInput || !current_dive)
 		return;
 
 	divesEdited(Command::editBuddies(stringToList(ui.buddy->toPlainText()), false));
@@ -640,7 +573,7 @@ void MainTab::on_buddy_editingFinished()
 
 void MainTab::on_divemaster_editingFinished()
 {
-	if (editMode == IGNORE_MODE || !current_dive)
+	if (ignoreInput || !current_dive)
 		return;
 
 	divesEdited(Command::editDiveMaster(stringToList(ui.divemaster->toPlainText()), false));
@@ -648,7 +581,7 @@ void MainTab::on_divemaster_editingFinished()
 
 void MainTab::on_duration_editingFinished()
 {
-	if (editMode == IGNORE_MODE || !current_dive)
+	if (ignoreInput || !current_dive)
 		return;
 
 	// Duration editing is special: we only edit the current dive.
@@ -657,7 +590,7 @@ void MainTab::on_duration_editingFinished()
 
 void MainTab::on_depth_editingFinished()
 {
-	if (editMode == IGNORE_MODE || !current_dive)
+	if (ignoreInput || !current_dive)
 		return;
 
 	// Depth editing is special: we only edit the current dive.
@@ -677,7 +610,7 @@ static void shiftTime(QDateTime &dateTime)
 
 void MainTab::on_dateEdit_dateChanged(const QDate &date)
 {
-	if (editMode == IGNORE_MODE || !current_dive)
+	if (ignoreInput || !current_dive)
 		return;
 	QDateTime dateTime = QDateTime::fromMSecsSinceEpoch(1000*current_dive->when, Qt::UTC);
 	dateTime.setTimeSpec(Qt::UTC);
@@ -687,7 +620,7 @@ void MainTab::on_dateEdit_dateChanged(const QDate &date)
 
 void MainTab::on_timeEdit_timeChanged(const QTime &time)
 {
-	if (editMode == IGNORE_MODE || !current_dive)
+	if (ignoreInput || !current_dive)
 		return;
 	QDateTime dateTime = QDateTime::fromMSecsSinceEpoch(1000*current_dive->when, Qt::UTC);
 	dateTime.setTimeSpec(Qt::UTC);
@@ -697,7 +630,7 @@ void MainTab::on_timeEdit_timeChanged(const QTime &time)
 
 void MainTab::on_tagWidget_editingFinished()
 {
-	if (editMode == IGNORE_MODE || !current_dive)
+	if (ignoreInput || !current_dive)
 		return;
 
 	divesEdited(Command::editTags(ui.tagWidget->getBlockStringList(), false));
@@ -705,7 +638,7 @@ void MainTab::on_tagWidget_editingFinished()
 
 void MainTab::on_location_diveSiteSelected()
 {
-	if (editMode == IGNORE_MODE || !current_dive)
+	if (ignoreInput || !current_dive)
 		return;
 
 	struct dive_site *newDs = ui.location->currDiveSite();
@@ -743,7 +676,7 @@ void MainTab::on_notes_editingFinished()
 
 void MainTab::on_rating_valueChanged(int value)
 {
-	if (editMode == IGNORE_MODE || !current_dive)
+	if (ignoreInput || !current_dive)
 		return;
 
 	divesEdited(Command::editRating(value, false));
@@ -760,7 +693,7 @@ void MainTab::escDetected()
 {
 	// In edit mode, pressing escape cancels the current changes.
 	// In standard mode, remove focus of any active widget to
-	if (editMode != NONE)
+	if (editMode)
 		rejectChanges();
 	else
 		stealFocus();

desktop-widgets/tab-widgets/maintab.h

@@ -26,13 +26,6 @@ class TabBase;
 class MainTab : public QTabWidget {
 	Q_OBJECT
 public:
-	enum EditMode {
-		NONE,
-		DIVE,
-		MANUALLY_ADDED_DIVE,
-		IGNORE_MODE
-	};
-
 	MainTab(QWidget *parent = 0);
 	~MainTab();
 	void clearTabs();
@@ -72,11 +65,12 @@ slots:
 	void closeMessage();
 	void closeWarning();
 	void displayMessage(QString str);
-	void enableEdition(EditMode newEditMode = NONE);
+	void enableEdition();
 	void escDetected(void);
 private:
 	Ui::MainTab ui;
-	EditMode editMode;
+	bool editMode;
+	bool ignoreInput; // When computionally editing fields, we have to ignore changed-signals
 	BuddyCompletionModel buddyModel;
 	DiveMasterCompletionModel diveMasterModel;
 	TagCompletionModel tagModel;

profile-widget/profilewidget2.cpp

@@ -30,6 +30,7 @@
 #include "core/qthelper.h"
 #include "core/gettextfromc.h"
 #include "core/imagedownloader.h"
+#include "core/subsurface-qt/divelistnotifier.h"
 #endif
 
 #include <libdivecomputer/parser.h>
@@ -170,6 +171,8 @@ ProfileWidget2::ProfileWidget2(QWidget *parent) : QGraphicsView(parent),
 	connect(DivePictureModel::instance(), &DivePictureModel::rowsInserted, this, &ProfileWidget2::plotPictures);
 	connect(DivePictureModel::instance(), &DivePictureModel::picturesRemoved, this, &ProfileWidget2::removePictures);
 	connect(DivePictureModel::instance(), &DivePictureModel::modelReset, this, &ProfileWidget2::plotPictures);
+	connect(&diveListNotifier, &DiveListNotifier::cylinderEdited, this, &ProfileWidget2::profileChanged);
+	connect(&diveListNotifier, &DiveListNotifier::eventsChanged, this, &ProfileWidget2::profileChanged);
 #endif // SUBSURFACE_MOBILE
 
 #if !defined(QT_NO_DEBUG) && defined(SHOW_PLOT_INFO_TABLE)
@@ -1449,69 +1452,37 @@ void ProfileWidget2::contextMenuEvent(QContextMenuEvent *event)
 	if (current_dive && current_dive->cylinders.nr > 1) {
 		// if we have more than one gas, offer to switch to another one
 		QMenu *gasChange = m.addMenu(tr("Add gas change"));
-		for (int i = 0; i < current_dive->cylinders.nr; i++) {
+		for (int i = 0; i < current_dive->cylinders.nr; i++)
-			QAction *action = new QAction(&m);
+			gasChange->addAction(QString(current_dive->cylinders.cylinders[i].type.description) + tr(" (cyl. %1)").arg(i + 1),
-			action->setText(QString(current_dive->cylinders.cylinders[i].type.description) + tr(" (cyl. %1)").arg(i + 1));
+					     [this, i, seconds] { changeGas(i, seconds); });
-			connect(action, &QAction::triggered, [this, i, seconds] { changeGas(i, seconds); } );
-			gasChange->addAction(action);
 	}
-	}
+	m.addAction(tr("Add setpoint change"), [this, seconds]() { ProfileWidget2::addSetpointChange(seconds); });
-	QAction *setpointAction = m.addAction(tr("Add setpoint change"), this, &ProfileWidget2::addSetpointChange);
+	m.addAction(tr("Add bookmark"), [this, seconds]() { addBookmark(seconds); });
-	setpointAction->setData(event->globalPos());
+	m.addAction(tr("Split dive into two"), [this, seconds]() { splitDive(seconds); });
-	QAction *action = m.addAction(tr("Add bookmark"), this, &ProfileWidget2::addBookmark);
-	action->setData(event->globalPos());
-	QAction *splitAction = m.addAction(tr("Split dive into two"), this, &ProfileWidget2::splitDive);
-	splitAction->setData(event->globalPos());
 	const struct event *ev = NULL;
 	enum divemode_t divemode = UNDEF_COMP_TYPE;
-	QString gas = action->text();
 
 	get_current_divemode(current_dc, seconds, &ev, &divemode);
 	QMenu *changeMode = m.addMenu(tr("Change divemode"));
-	if (divemode != OC) {
+	if (divemode != OC)
-		QAction *action = new QAction(&m);
+		changeMode->addAction(gettextFromC::tr(divemode_text_ui[OC]),
-		action->setText(gettextFromC::tr(divemode_text_ui[OC]));
+				      [this, seconds](){ addDivemodeSwitch(seconds, OC); });
-		connect(action, SIGNAL(triggered(bool)), this, SLOT(addDivemodeSwitch()));
+	if (divemode != CCR)
-		action->setData(event->globalPos());
+		changeMode->addAction(gettextFromC::tr(divemode_text_ui[CCR]),
-		changeMode->addAction(action);
+				      [this, seconds](){ addDivemodeSwitch(seconds, CCR); });
-	}
+	if (divemode != PSCR)
-	if (divemode != CCR) {
+		changeMode->addAction(gettextFromC::tr(divemode_text_ui[PSCR]),
-		QAction *action = new QAction(&m);
+				      [this, seconds](){ addDivemodeSwitch(seconds, PSCR); });
-		action->setText(gettextFromC::tr(divemode_text_ui[CCR]));
-		connect(action, SIGNAL(triggered(bool)), this, SLOT(addDivemodeSwitch()));
-		action->setData(event->globalPos());
-		changeMode->addAction(action);
-	}
-	if (divemode != PSCR) {
-		QAction *action = new QAction(&m);
-		action->setText(gettextFromC::tr(divemode_text_ui[PSCR]));
-		connect(action, SIGNAL(triggered(bool)), this, SLOT(addDivemodeSwitch()));
-		action->setData(event->globalPos());
-		changeMode->addAction(action);
-	}
 
 	if (same_string(current_dc->model, "manually added dive"))
 		m.addAction(tr("Edit the profile"), this, SIGNAL(editCurrentDive()));
 
 	if (DiveEventItem *item = dynamic_cast<DiveEventItem *>(sceneItem)) {
-		action = new QAction(&m);
+		m.addAction(tr("Remove event"), [this,item] { removeEvent(item); });
-		action->setText(tr("Remove event"));
+		m.addAction(tr("Hide similar events"), [this, item] { hideEvents(item); });
-		action->setData(QVariant::fromValue<void *>(item)); // so we know what to remove.
-		connect(action, SIGNAL(triggered(bool)), this, SLOT(removeEvent()));
-		m.addAction(action);
-		action = new QAction(&m);
-		action->setText(tr("Hide similar events"));
-		action->setData(QVariant::fromValue<void *>(item));
-		connect(action, SIGNAL(triggered(bool)), this, SLOT(hideEvents()));
-		m.addAction(action);
 		struct event *dcEvent = item->getEvent();
-		if (dcEvent->type == SAMPLE_EVENT_BOOKMARK) {
+		if (dcEvent->type == SAMPLE_EVENT_BOOKMARK)
-			action = new QAction(&m);
+			m.addAction(tr("Edit name"), [this, item] { editName(item); });
-			action->setText(tr("Edit name"));
-			action->setData(QVariant::fromValue<void *>(item));
-			connect(action, SIGNAL(triggered(bool)), this, SLOT(editName()));
-			m.addAction(action);
-		}
 #if 0 // TODO::: FINISH OR DISABLE
 		QPointF scenePos = mapToScene(event->pos());
 		int idx = getEntryFromPos(scenePos);
@@ -1561,10 +1532,8 @@ void ProfileWidget2::contextMenuEvent(QContextMenuEvent *event)
 			break;
 		}
 	}
-	if (some_hidden) {
+	if (some_hidden)
-		action = m.addAction(tr("Unhide all events"), this, &ProfileWidget2::unhideEvents);
+		m.addAction(tr("Unhide all events"), this, &ProfileWidget2::unhideEvents);
-		action->setData(event->globalPos());
-	}
 	m.exec(event->globalPos());
 }
 
@@ -1583,10 +1552,8 @@ void ProfileWidget2::makeFirstDC()
 	Command::moveDiveComputerToFront(current_dive, dc_number);
 }
 
-void ProfileWidget2::hideEvents()
+void ProfileWidget2::hideEvents(DiveEventItem *item)
 {
-	QAction *action = qobject_cast<QAction *>(sender());
-	DiveEventItem *item = static_cast<DiveEventItem *>(action->data().value<void *>());
 	struct event *event = item->getEvent();
 
 	if (QMessageBox::question(this,
@@ -1618,67 +1585,59 @@ void ProfileWidget2::unhideEvents()
 		item->show();
 }
 
-void ProfileWidget2::removeEvent()
+// The profile displays a copy of the current_dive, namely displayed_dive.
+// Therefore, the events we get are likewise copies. This function finds
+// the original event. TODO: Remove function once the profile can display
+// arbitrary dives.
+static event *find_event(const struct event *ev)
 {
-	QAction *action = qobject_cast<QAction *>(sender());
+	struct divecomputer *dc = current_dc;
-	DiveEventItem *item = static_cast<DiveEventItem *>(action->data().value<void *>());
+	if (!dc)
-	struct event *event = item->getEvent();
+		return nullptr;
+	for (struct event *act = current_dc->events; act; act = act->next) {
+		if (same_event(act, ev))
+			return act;
+	}
+	return nullptr;
+}
+
+void ProfileWidget2::removeEvent(DiveEventItem *item)
+{
+	struct event *event = find_event(item->getEvent());
+	if (!event)
+		return;
 
 	if (QMessageBox::question(this, TITLE_OR_TEXT(
 					  tr("Remove the selected event?"),
 					  tr("%1 @ %2:%3").arg(event->name).arg(event->time.seconds / 60).arg(event->time.seconds % 60, 2, 10, QChar('0'))),
-				  QMessageBox::Ok | QMessageBox::Cancel) == QMessageBox::Ok) {
+				  QMessageBox::Ok | QMessageBox::Cancel) == QMessageBox::Ok)
-		remove_event(event);
+		Command::removeEvent(current_dive, dc_number, event);
-		invalidate_dive_cache(current_dive);
-		mark_divelist_changed(true);
-		replot();
-	}
 }
 
-void ProfileWidget2::addBookmark()
+void ProfileWidget2::addBookmark(int seconds)
 {
-	QAction *action = qobject_cast<QAction *>(sender());
+	Command::addEventBookmark(current_dive, dc_number, seconds);
-	QPointF scenePos = mapToScene(mapFromGlobal(action->data().toPoint()));
-	add_event(current_dc, lrint(timeAxis->valueAt(scenePos)), SAMPLE_EVENT_BOOKMARK, 0, 0, "bookmark");
-	invalidate_dive_cache(current_dive);
-	mark_divelist_changed(true);
-	replot();
 }
 
-void ProfileWidget2::addDivemodeSwitch()
+void ProfileWidget2::addDivemodeSwitch(int seconds, int divemode)
 {
-	int i;
+	Command::addEventDivemodeSwitch(current_dive, dc_number, seconds, divemode);
-	QAction *action = qobject_cast<QAction *>(sender());
-	QPointF scenePos = mapToScene(mapFromGlobal(action->data().toPoint()));
-	for (i = 0; i < NUM_DIVEMODE; i++)
-		if (gettextFromC::tr(divemode_text_ui[i]) == action->text())
-			add_event(current_dc, lrint(timeAxis->valueAt(scenePos)), 8, 0, i,
-				QT_TRANSLATE_NOOP("gettextFromC", "modechange"));
-	invalidate_dive_cache(current_dive);
-	mark_divelist_changed(true);
-	replot();
 }
 
-void ProfileWidget2::addSetpointChange()
+void ProfileWidget2::addSetpointChange(int seconds)
 {
-	QAction *action = qobject_cast<QAction *>(sender());
+	SetpointDialog dialog(current_dive, dc_number, seconds);
-	QPointF scenePos = mapToScene(mapFromGlobal(action->data().toPoint()));
+	dialog.exec();
-	SetpointDialog::instance()->setpointData(current_dc, lrint(timeAxis->valueAt(scenePos)));
-	SetpointDialog::instance()->show();
 }
 
-void ProfileWidget2::splitDive()
+void ProfileWidget2::splitDive(int seconds)
 {
 #ifndef SUBSURFACE_MOBILE
 	// Make sure that this is an actual dive and we're not in add mode
 	dive *d = get_dive_by_uniq_id(displayed_dive.id);
 	if (!d)
 		return;
-	QAction *action = qobject_cast<QAction *>(sender());
+	Command::splitDives(d, duration_t{ seconds });
-	QPointF scenePos = mapToScene(mapFromGlobal(action->data().toPoint()));
-	duration_t time;
-	time.seconds = lrint(timeAxis->valueAt(scenePos));
-	Command::splitDives(d, time);
 #endif
 }
 
@@ -1687,26 +1646,7 @@ void ProfileWidget2::changeGas(int tank, int seconds)
 	if (!current_dive || tank < 0 || tank >= current_dive->cylinders.nr)
 		return;
 
-	// if there is a gas change at this time stamp, remove it before adding the new one
+	Command::addGasSwitch(current_dive, dc_number, seconds, tank);
-	struct event *gasChangeEvent = current_dc->events;
-	while ((gasChangeEvent = get_next_event_mutable(gasChangeEvent, "gaschange")) != NULL) {
-		if (gasChangeEvent->time.seconds == seconds) {
-			remove_event(gasChangeEvent);
-			gasChangeEvent = current_dc->events;
-		} else {
-			gasChangeEvent = gasChangeEvent->next;
-		}
-	}
-	add_gas_switch_event(current_dive, current_dc, seconds, tank);
-	// this means we potentially have a new tank that is being used and needs to be shown
-	fixup_dive(current_dive);
-	invalidate_dive_cache(current_dive);
-
-	// FIXME - this no longer gets written to the dive list - so we need to enableEdition() here
-
-	emit updateDiveInfo();
-	mark_divelist_changed(true);
-	replot();
 }
 #endif
 
@@ -1752,11 +1692,11 @@ double ProfileWidget2::getFontPrintScale()
 }
 
 #ifndef SUBSURFACE_MOBILE
-void ProfileWidget2::editName()
+void ProfileWidget2::editName(DiveEventItem *item)
 {
-	QAction *action = qobject_cast<QAction *>(sender());
+	struct event *event = find_event(item->getEvent());
-	DiveEventItem *item = static_cast<DiveEventItem *>(action->data().value<void *>());
+	if (!event)
-	struct event *event = item->getEvent();
+		return;
 	bool ok;
 	QString newName = QInputDialog::getText(this, tr("Edit name of bookmark"),
 						tr("Custom name:"), QLineEdit::Normal,
@@ -1768,14 +1708,7 @@ void ProfileWidget2::editName()
 			lengthWarning.exec();
 			return;
 		}
-		// order is important! first update the current dive (by matching the unchanged event),
+		Command::renameEvent(current_dive, dc_number, event, qPrintable(newName));
-		// then update the displayed dive (as event is part of the events on displayed dive
-		// and will be freed as part of changing the name!
-		update_event_name(current_dive, event, qPrintable(newName));
-		update_event_name(&displayed_dive, event, qPrintable(newName));
-		invalidate_dive_cache(current_dive);
-		mark_divelist_changed(true);
-		replot();
 	}
 }
 #endif
@@ -2245,6 +2178,13 @@ void ProfileWidget2::removePictures(const QVector<QString> &fileUrls)
 	calculatePictureYPositions();
 }
 
+void ProfileWidget2::profileChanged(dive *d)
+{
+	if (!d || d->id != displayed_dive.id)
+		return; // Cylinders of a differnt dive than the shown one changed.
+	replot();
+}
+
 #endif
 
 void ProfileWidget2::dropEvent(QDropEvent *event)

profile-widget/profilewidget2.h

@@ -115,20 +115,10 @@ slots: // Necessary to call from QAction's signals.
 	void removePictures(const QVector<QString> &fileUrls);
 	void setPlanState();
 	void setAddState();
-	void addSetpointChange();
-	void splitDive();
-	void addBookmark();
-	void addDivemodeSwitch();
-	void hideEvents();
-	void unhideEvents();
-	void removeEvent();
-	void editName();
-	void makeFirstDC();
-	void deleteCurrentDC();
-	void splitCurrentDC();
 	void pointInserted(const QModelIndex &parent, int start, int end);
 	void pointsRemoved(const QModelIndex &, int start, int end);
 	void updateThumbnail(QString filename, QImage thumbnail, duration_t duration);
+	void profileChanged(dive *d);
 
 	/* this is called for every move on the handlers. maybe we can speed up this a bit? */
 	void recreatePlannedDive();
@@ -174,6 +164,17 @@ private:
 			 const double *thresholdSettingsMin, const double *thresholdSettingsMax);
 	void clearPictures();
 	void plotPicturesInternal(const struct dive *d, bool synchronous);
+	void addDivemodeSwitch(int seconds, int divemode);
+	void addBookmark(int seconds);
+	void splitDive(int seconds);
+	void addSetpointChange(int seconds);
+	void removeEvent(DiveEventItem *item);
+	void hideEvents(DiveEventItem *item);
+	void editName(DiveEventItem *item);
+	void unhideEvents();
+	void makeFirstDC();
+	void deleteCurrentDC();
+	void splitCurrentDC();
 private:
 	DivePlotDataModel *dataModel;
 	int zoomLevel;

qt-models/cleanertablemodel.h

@@ -31,8 +31,4 @@ private:
 	QStringList headers;
 };
 
-/* Has the string value changed */
-#define CHANGED() \
-	(vString = value.toString()) != data(index, role).toString()
-
 #endif

qt-models/cylindermodel.cpp

@@ -2,29 +2,34 @@
 #include "cylindermodel.h"
 #include "tankinfomodel.h"
 #include "models.h"
+#include "commands/command.h"
 #include "core/qthelper.h"
-#include "core/divelist.h" // for mark_divelist_changed()
 #include "core/color.h"
 #include "qt-models/diveplannermodel.h"
 #include "core/gettextfromc.h"
 #include "core/subsurface-qt/divelistnotifier.h"
 #include "core/subsurface-string.h"
+#include <string>
 
-CylindersModel::CylindersModel(QObject *parent) :
+CylindersModel::CylindersModel(bool planner, QObject *parent) : CleanerTableModel(parent),
-	CleanerTableModel(parent),
+	d(nullptr),
-	changed(false),
+	inPlanner(planner),
-	rows(0)
+	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%")
 						 << tr("Deco switch at") <<tr("Bot. MOD") <<tr("MND") << tr("Use"));
 
 	connect(&diveListNotifier, &DiveListNotifier::cylindersReset, this, &CylindersModel::cylindersReset);
+	connect(&diveListNotifier, &DiveListNotifier::cylinderAdded, this, &CylindersModel::cylinderAdded);
+	connect(&diveListNotifier, &DiveListNotifier::cylinderRemoved, this, &CylindersModel::cylinderRemoved);
+	connect(&diveListNotifier, &DiveListNotifier::cylinderEdited, this, &CylindersModel::cylinderEdited);
 }
 
 QVariant CylindersModel::headerData(int section, Qt::Orientation orientation, int role) const
 {
-	if (role == Qt::DisplayRole && orientation == Qt::Horizontal && in_planner() && section == WORKINGPRESS)
+	if (role == Qt::DisplayRole && orientation == Qt::Horizontal && inPlanner && section == WORKINGPRESS)
 		return tr("Start press.");
 	else
 		return CleanerTableModel::headerData(section, orientation, role);
@@ -126,13 +131,12 @@ static QVariant percent_string(fraction_t fraction)
 
 bool CylindersModel::cylinderUsed(int i) const
 {
-	const struct dive *dive = &displayed_dive;
+	if (i < 0 || i >= d->cylinders.nr)
-	if (i < 0 || i >= dive->cylinders.nr)
 		return false;
-	if (is_cylinder_used(dive, i))
+	if (is_cylinder_used(d, i))
 		return true;
 
-	cylinder_t *cyl = get_cylinder(dive, i);
+	cylinder_t *cyl = get_cylinder(d, i);
 	if (cyl->start.mbar || cyl->sample_start.mbar ||
 	    cyl->end.mbar || cyl->sample_end.mbar)
 		return true;
@@ -148,15 +152,15 @@ bool CylindersModel::cylinderUsed(int i) const
 
 QVariant CylindersModel::data(const QModelIndex &index, int role) const
 {
-	if (!index.isValid() || index.row() >= rows)
+	if (!d || !index.isValid() || index.row() >= d->cylinders.nr)
 		return QVariant();
 
-	if (index.row() >= displayed_dive.cylinders.nr) {
+	if (index.row() >= d->cylinders.nr) {
-		qWarning("CylindersModel and displayed_dive are out of sync!");
+		qWarning("CylindersModel and dive are out of sync!");
 		return QVariant();
 	}
 
-	const cylinder_t *cyl = get_cylinder(&displayed_dive, index.row());
+	const cylinder_t *cyl = index.row() == tempRow ? &tempCyl : get_cylinder(d, index.row());
 
 	switch (role) {
 	case Qt::BackgroundRole: {
@@ -227,13 +231,13 @@ QVariant CylindersModel::data(const QModelIndex &index, int role) const
 			} else {
 				pressure_t modpO2;
 				modpO2.mbar = prefs.bottompo2;
-				return get_depth_string(gas_mod(cyl->gasmix, modpO2, &displayed_dive, M_OR_FT(1,1)), true);
+				return get_depth_string(gas_mod(cyl->gasmix, modpO2, d, M_OR_FT(1,1)), true);
 			}
 		case MND:
 			if (cyl->bestmix_he)
 				return QStringLiteral("*");
 			else
-				return get_depth_string(gas_mnd(cyl->gasmix, prefs.bestmixend, &displayed_dive, M_OR_FT(1,1)), true);
+				return get_depth_string(gas_mnd(cyl->gasmix, prefs.bestmixend, d, M_OR_FT(1,1)), true);
 			break;
 		case USE:
 			return gettextFromC::tr(cylinderuse_text[cyl->cylinder_use]);
@@ -246,8 +250,8 @@ QVariant CylindersModel::data(const QModelIndex &index, int role) const
 	case Qt::DecorationRole:
 	case Qt::SizeHintRole:
 		if (index.column() == REMOVE) {
-			if ((in_planner() && DivePlannerPointsModel::instance()->tankInUse(index.row())) ||
+			if ((inPlanner && DivePlannerPointsModel::instance()->tankInUse(index.row())) ||
-				(!in_planner() && is_cylinder_prot(&displayed_dive, index.row()))) {
+				(!inPlanner && is_cylinder_prot(d, index.row()))) {
 					return trashForbiddenIcon();
 			}
 			return trashIcon();
@@ -256,8 +260,8 @@ QVariant CylindersModel::data(const QModelIndex &index, int role) const
 	case Qt::ToolTipRole:
 		switch (index.column()) {
 		case REMOVE:
-			if ((in_planner() && DivePlannerPointsModel::instance()->tankInUse(index.row())) ||
+			if ((inPlanner && DivePlannerPointsModel::instance()->tankInUse(index.row())) ||
-				(!in_planner() && is_cylinder_prot(&displayed_dive, index.row()))) {
+				(!inPlanner && is_cylinder_prot(d, index.row()))) {
 					return tr("This gas is in use. Only cylinders that are not used in the dive can be removed.");
 			}
 			return tr("Clicking here will remove this cylinder.");
@@ -285,197 +289,227 @@ QVariant CylindersModel::data(const QModelIndex &index, int role) const
 
 cylinder_t *CylindersModel::cylinderAt(const QModelIndex &index)
 {
-	return get_cylinder(&displayed_dive, index.row());
+	if (!d)
+		return nullptr;
+	return get_cylinder(d, index.row());
 }
 
 bool CylindersModel::setData(const QModelIndex &index, const QVariant &value, int role)
 {
-	QString vString;
+	if (!d)
-
-	cylinder_t *cyl = cylinderAt(index);
-	if (!cyl)
 		return false;
 
-	if (role == PASS_IN_ROLE) {
+	int row = index.row();
-		// this is our magic 'pass data in' function that allows the delegate to get
+	if (row < 0 || row >= d->cylinders.nr)
-		// the data here without silly unit conversions;
-		// so we only implement the two columns we care about
-		switch (index.column()) {
-		case SIZE:
-			if (cyl->type.size.mliter != value.toInt()) {
-				cyl->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();
-				dataChanged(index, index);
-			}
-			return true;
-		}
 		return false;
-	}
+
+	// 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), cyl->type.description)) {
+			if (!same_string(qPrintable(type), tempCyl.type.description)) {
-				free((void *)cyl->type.description);
+				free((void *)tempCyl.type.description);
-				cyl->type.description = strdup(qPrintable(type));
+				tempCyl.type.description = strdup(qPrintable(type));
-				changed = true;
+				dataChanged(index, index);
 			}
+			return true;
 		}
-		break;
 		case SIZE:
-		if (CHANGED()) {
+			if (tempCyl.type.size.mliter != value.toInt()) {
-			TankInfoModel *tanks = TankInfoModel::instance();
+				tempCyl.type.size.mliter = value.toInt();
-			QModelIndexList matches = tanks->match(tanks->index(0, 0), Qt::DisplayRole, cyl->type.description);
+				dataChanged(index, index);
-
-			cyl->type.size = string_to_volume(qPrintable(vString), cyl->type.workingpressure);
-			mark_divelist_changed(true);
-			if (!matches.isEmpty())
-				tanks->setData(tanks->index(matches.first().row(), TankInfoModel::ML), cyl->type.size.mliter);
-			changed = true;
 			}
-		break;
+			return true;
 		case WORKINGPRESS:
-		if (CHANGED()) {
+			if (tempCyl.type.workingpressure.mbar != value.toInt()) {
-			TankInfoModel *tanks = TankInfoModel::instance();
+				tempCyl.type.workingpressure.mbar = value.toInt();
-			QModelIndexList matches = tanks->match(tanks->index(0, 0), Qt::DisplayRole, cyl->type.description);
+				dataChanged(index, index);
-			cyl->type.workingpressure = string_to_pressure(qPrintable(vString));
-			if (!matches.isEmpty())
-				tanks->setData(tanks->index(matches.first().row(), TankInfoModel::BAR), cyl->type.workingpressure.mbar / 1000.0);
-			changed = true;
 			}
+			return true;
+		}
+		return false;
+	case COMMIT_ROLE:
+		commitTempCyl(index.row());
+		return true;
+	case REVERT_ROLE:
+		clearTempCyl();
+		return true;
+	default:
+		break;
+	}
+
+	QString vString = value.toString();
+	bool changed = vString != data(index, role).toString();
+
+	std::string newType; // If we allocate a new type string, this makes sure that it is freed at the end of the function
+
+	// First, we make a shallow copy of the old cylinder. Then we modify the fields inside that copy.
+	// At the end, we either place an EditCylinder undo command (EquipmentTab) or copy the cylinder back (planner).
+	// Yes, this is not ideal, but the pragmatic thing to do for now.
+	cylinder_t cyl = d->cylinders.cylinders[row];
+
+	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();
+			QModelIndexList matches = tanks->match(tanks->index(0, 0), Qt::DisplayRole, cyl.type.description);
+
+			cyl.type.size = string_to_volume(qPrintable(vString), cyl.type.workingpressure);
+			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();
+			QModelIndexList matches = tanks->match(tanks->index(0, 0), Qt::DisplayRole, cyl.type.description);
+			cyl.type.workingpressure = string_to_pressure(qPrintable(vString));
+			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:
-		if (CHANGED()) {
+		cyl.start = string_to_pressure(qPrintable(vString));
-			cyl->start = string_to_pressure(qPrintable(vString));
+		type = Command::EditCylinderType::PRESSURE;
-			changed = true;
-		}
 		break;
 	case END:
-		if (CHANGED()) {
+		//if (!cyl->start.mbar || string_to_pressure(qPrintable(vString)).mbar <= cyl->start.mbar) {
-			//&& (!cyl->start.mbar || string_to_pressure(qPrintable(vString)).mbar <= cyl->start.mbar)) {
+		cyl.end = string_to_pressure(qPrintable(vString));
-			cyl->end = string_to_pressure(qPrintable(vString));
+		type = Command::EditCylinderType::PRESSURE;
-			changed = true;
-		}
 		break;
-	case O2:
+	case O2: {
-		if (CHANGED()) {
+			cyl.gasmix.o2 = string_to_fraction(qPrintable(vString));
-			cyl->gasmix.o2 = string_to_fraction(qPrintable(vString));
 			// fO2 + fHe must not be greater than 1
-			if (get_o2(cyl->gasmix) + get_he(cyl->gasmix) > 1000)
+			if (get_o2(cyl.gasmix) + get_he(cyl.gasmix) > 1000)
-				cyl->gasmix.he.permille = 1000 - get_o2(cyl->gasmix);
+				cyl.gasmix.he.permille = 1000 - get_o2(cyl.gasmix);
 			pressure_t modpO2;
-			if (displayed_dive.dc.divemode == PSCR)
+			if (d->dc.divemode == PSCR)
-				modpO2.mbar = prefs.decopo2 + (1000 - get_o2(cyl->gasmix)) * SURFACE_PRESSURE *
+				modpO2.mbar = prefs.decopo2 + (1000 - get_o2(cyl.gasmix)) * SURFACE_PRESSURE *
 						prefs.o2consumption / prefs.decosac / prefs.pscr_ratio;
 			else
 				modpO2.mbar = prefs.decopo2;
-			cyl->depth = gas_mod(cyl->gasmix, modpO2, &displayed_dive, M_OR_FT(3, 10));
+			cyl.depth = gas_mod(cyl.gasmix, modpO2, d, M_OR_FT(3, 10));
-			cyl->bestmix_o2 = false;
+			cyl.bestmix_o2 = false;
-			changed = true;
 		}
+		type = Command::EditCylinderType::GASMIX;
 		break;
 	case HE:
-		if (CHANGED()) {
+		cyl.gasmix.he = string_to_fraction(qPrintable(vString));
-			cyl->gasmix.he = string_to_fraction(qPrintable(vString));
 		// fO2 + fHe must not be greater than 1
-			if (get_o2(cyl->gasmix) + get_he(cyl->gasmix) > 1000)
+		if (get_o2(cyl.gasmix) + get_he(cyl.gasmix) > 1000)
-				cyl->gasmix.o2.permille = 1000 - get_he(cyl->gasmix);
+			cyl.gasmix.o2.permille = 1000 - get_he(cyl.gasmix);
-			cyl->bestmix_he = false;
+		cyl.bestmix_he = false;
-			changed = true;
+		type = Command::EditCylinderType::GASMIX;
-		}
 		break;
 	case DEPTH:
-		if (CHANGED()) {
+		cyl.depth = string_to_depth(qPrintable(vString));
-			cyl->depth = string_to_depth(qPrintable(vString));
+		type = Command::EditCylinderType::GASMIX;
-			changed = true;
-		}
 		break;
-	case MOD:
+	case MOD: {
-		if (CHANGED()) {
 			if (QString::compare(qPrintable(vString), "*") == 0) {
-				cyl->bestmix_o2 = true;
+				cyl.bestmix_o2 = true;
 				// Calculate fO2 for max. depth
-				cyl->gasmix.o2 = best_o2(displayed_dive.maxdepth, &displayed_dive);
+				cyl.gasmix.o2 = best_o2(d->maxdepth, d);
 			} else {
-				cyl->bestmix_o2 = false;
+				cyl.bestmix_o2 = false;
 				// Calculate fO2 for input depth
-				cyl->gasmix.o2 = best_o2(string_to_depth(qPrintable(vString)), &displayed_dive);
+				cyl.gasmix.o2 = best_o2(string_to_depth(qPrintable(vString)), d);
 			}
 			pressure_t modpO2;
 			modpO2.mbar = prefs.decopo2;
-			cyl->depth = gas_mod(cyl->gasmix, modpO2, &displayed_dive, M_OR_FT(3, 10));
+			cyl.depth = gas_mod(cyl.gasmix, modpO2, d, M_OR_FT(3, 10));
-			changed = true;
 		}
+		type = Command::EditCylinderType::GASMIX;
 		break;
 	case MND:
-		if (CHANGED()) {
 		if (QString::compare(qPrintable(vString), "*") == 0) {
-				cyl->bestmix_he = true;
+			cyl.bestmix_he = true;
 			// Calculate fO2 for max. depth
-				cyl->gasmix.he = best_he(displayed_dive.maxdepth, &displayed_dive, prefs.o2narcotic, cyl->gasmix.o2);
+			cyl.gasmix.he = best_he(d->maxdepth, d, prefs.o2narcotic, cyl.gasmix.o2);
 		} else {
-				cyl->bestmix_he = false;
+			cyl.bestmix_he = false;
 			// Calculate fHe for input depth
-				cyl->gasmix.he = best_he(string_to_depth(qPrintable(vString)), &displayed_dive, prefs.o2narcotic, cyl->gasmix.o2);
+			cyl.gasmix.he = best_he(string_to_depth(qPrintable(vString)), d, prefs.o2narcotic, cyl.gasmix.o2);
-			}
-			changed = true;
 		}
+		type = Command::EditCylinderType::GASMIX;
 		break;
-	case USE:
+	case USE: {
-		if (CHANGED()) {
 			int use = vString.toInt();
 			if (use > NUM_GAS_USE - 1 || use < 0)
 				use = 0;
-			cyl->cylinder_use = (enum cylinderuse)use;
+			cyl.cylinder_use = (enum cylinderuse)use;
-			changed = true;
 		}
+		type = Command::EditCylinderType::TYPE;
 		break;
 	}
+
+	if (inPlanner) {
+		// In the planner - simply overwrite the cylinder in the dive with the modified cylinder.
+		// We have only made a shallow copy, therefore copy the new cylinder first.
+		cylinder_t copy = clone_cylinder(cyl);
+		std::swap(copy, d->cylinders.cylinders[row]);
+		free_cylinder(copy);
 		dataChanged(index, index);
+	} else {
+		// On the EquipmentTab - place an editCylinder command.
+		int count = Command::editCylinder(index.row(), cyl, type, false);
+		emit divesEdited(count);
+	}
 	return true;
 }
 
 int CylindersModel::rowCount(const QModelIndex&) const
 {
-	return rows;
+	return d ? d->cylinders.nr : 0;
 }
 
 void CylindersModel::add()
 {
-	int row = rows;
+	if (!d)
-	cylinder_t cyl = empty_cylinder;
+		return;
-	fill_default_cylinder(&displayed_dive, &cyl);
+	int row = d->cylinders.nr;
-	cyl.start = cyl.type.workingpressure;
+	cylinder_t cyl = create_new_cylinder(d);
-	cyl.manually_added = true;
-	cyl.cylinder_use = OC_GAS;
 	beginInsertRows(QModelIndex(), row, row);
-	add_to_cylinder_table(&displayed_dive.cylinders, row, cyl);
+	add_to_cylinder_table(&d->cylinders, row, cyl);
-	rows++;
-	changed = true;
 	endInsertRows();
 	emit dataChanged(createIndex(row, 0), createIndex(row, COLUMNS - 1));
 }
 
 void CylindersModel::clear()
 {
-	if (rows > 0) {
+	beginResetModel();
-		beginRemoveRows(QModelIndex(), 0, rows - 1);
+	d = nullptr;
-		endRemoveRows();
+	endResetModel();
-	}
 }
 
-void CylindersModel::updateDive()
+void CylindersModel::updateDive(dive *dIn)
 {
 #ifdef DEBUG_CYL
-	dump_cylinders(&displayed_dive, true);
+	if (d)
+		dump_cylinders(dIn, true);
 #endif
 	beginResetModel();
-	rows = displayed_dive.cylinders.nr;
+	d = dIn;
 	endResetModel();
 }
 
@@ -486,15 +520,19 @@ Qt::ItemFlags CylindersModel::flags(const QModelIndex &index) const
 	return QAbstractItemModel::flags(index) | Qt::ItemIsEditable;
 }
 
+// This function is only invoked from the planner! Therefore, there is
+// no need to check whether we are in the planner. Perhaps move some
+// of this functionality to the planner itself.
 void CylindersModel::remove(QModelIndex index)
 {
+	if (!d)
+		return;
 	if (index.column() == USE) {
 		cylinder_t *cyl = cylinderAt(index);
 		if (cyl->cylinder_use == OC_GAS)
 			cyl->cylinder_use = NOT_USED;
 		else if (cyl->cylinder_use == NOT_USED)
 			cyl->cylinder_use = OC_GAS;
-		changed = true;
 		dataChanged(index, index);
 		return;
 	}
@@ -502,94 +540,120 @@ void CylindersModel::remove(QModelIndex index)
 	if (index.column() != REMOVE)
 		return;
 
-	if ((in_planner() && DivePlannerPointsModel::instance()->tankInUse(index.row())) ||
+	if (DivePlannerPointsModel::instance()->tankInUse(index.row()))
-		(!in_planner() && is_cylinder_prot(&displayed_dive, index.row())))
 			return;
 
 	beginRemoveRows(QModelIndex(), index.row(), index.row());
-	rows--;
+	remove_cylinder(d, index.row());
-	remove_cylinder(&displayed_dive, index.row());
-	changed = true;
 	endRemoveRows();
 
-	// Create a mapping of cylinder indices:
+	std::vector<int> mapping = get_cylinder_map_for_remove(d->cylinders.nr + 1, index.row());
-	// 1) Fill mapping[0]..mapping[index-1] with 0..index
+	cylinder_renumber(d, &mapping[0]);
-	// 2) Set mapping[index] to -1
-	// 3) Fill mapping[index+1]..mapping[end] with index..
-	std::vector<int> mapping(displayed_dive.cylinders.nr + 1);
-	std::iota(mapping.begin(), mapping.begin() + index.row(), 0);
-	mapping[index.row()] = -1;
-	std::iota(mapping.begin() + index.row() + 1, mapping.end(), index.row());
-
-	cylinder_renumber(&displayed_dive, &mapping[0]);
-	if (in_planner())
 	DivePlannerPointsModel::instance()->cylinderRenumber(&mapping[0]);
-	changed = true;
+}
+
+void CylindersModel::cylinderAdded(struct dive *changed, int pos)
+{
+	if (d != changed)
+		return;
+
+	// The row was already inserted by the undo command. Just inform the model.
+	beginInsertRows(QModelIndex(), pos, pos);
+	endInsertRows();
+}
+
+void CylindersModel::cylinderRemoved(struct dive *changed, int pos)
+{
+	if (d != changed)
+		return;
+
+	// The row was already deleted by the undo command. Just inform the model.
+	beginRemoveRows(QModelIndex(), pos, pos);
+	endRemoveRows();
+}
+
+void CylindersModel::cylinderEdited(struct dive *changed, int pos)
+{
+	if (d != changed)
+		return;
+
+	dataChanged(index(pos, TYPE), index(pos, USE));
 }
 
 void CylindersModel::moveAtFirst(int cylid)
 {
+	if (!d)
+		return;
+
 	cylinder_t temp_cyl;
 
 	beginMoveRows(QModelIndex(), cylid, cylid, QModelIndex(), 0);
-	memmove(&temp_cyl, get_cylinder(&displayed_dive, cylid), sizeof(temp_cyl));
+	memmove(&temp_cyl, get_cylinder(d, cylid), sizeof(temp_cyl));
 	for (int i = cylid - 1; i >= 0; i--)
-		memmove(get_cylinder(&displayed_dive, i + 1), get_cylinder(&displayed_dive, i), sizeof(temp_cyl));
+		memmove(get_cylinder(d, i + 1), get_cylinder(d, i), sizeof(temp_cyl));
-	memmove(get_cylinder(&displayed_dive, 0), &temp_cyl, sizeof(temp_cyl));
+	memmove(get_cylinder(d, 0), &temp_cyl, sizeof(temp_cyl));
 
 	// Create a mapping of cylinder indices:
 	// 1) Fill mapping[0]..mapping[cyl] with 0..index
 	// 2) Set mapping[cyl] to 0
 	// 3) Fill mapping[cyl+1]..mapping[end] with cyl..
-	std::vector<int> mapping(displayed_dive.cylinders.nr);
+	std::vector<int> mapping(d->cylinders.nr);
 	std::iota(mapping.begin(), mapping.begin() + cylid, 1);
 	mapping[cylid] = 0;
 	std::iota(mapping.begin() + (cylid + 1), mapping.end(), cylid);
-	cylinder_renumber(&displayed_dive, &mapping[0]);
+	cylinder_renumber(d, &mapping[0]);
-	if (in_planner())
+	if (inPlanner)
 		DivePlannerPointsModel::instance()->cylinderRenumber(&mapping[0]);
-	changed = true;
 	endMoveRows();
 }
 
 void CylindersModel::updateDecoDepths(pressure_t olddecopo2)
 {
+	if (!d)
+		return;
+
 	pressure_t decopo2;
 	decopo2.mbar = prefs.decopo2;
-	for (int i = 0; i < displayed_dive.cylinders.nr; i++) {
+	for (int i = 0; i < d->cylinders.nr; i++) {
-		cylinder_t *cyl = get_cylinder(&displayed_dive, i);
+		cylinder_t *cyl = get_cylinder(d, i);
 		/* If the gas's deco MOD matches the old pO2, it will have been automatically calculated and should be updated.
 		 * If they don't match, we should leave the user entered depth as it is */
-		if (cyl->depth.mm == gas_mod(cyl->gasmix, olddecopo2, &displayed_dive, M_OR_FT(3, 10)).mm) {
+		if (cyl->depth.mm == gas_mod(cyl->gasmix, olddecopo2, d, M_OR_FT(3, 10)).mm) {
-			cyl->depth = gas_mod(cyl->gasmix, decopo2, &displayed_dive, M_OR_FT(3, 10));
+			cyl->depth = gas_mod(cyl->gasmix, decopo2, d, M_OR_FT(3, 10));
 		}
 	}
-	emit dataChanged(createIndex(0, 0), createIndex(displayed_dive.cylinders.nr - 1, COLUMNS - 1));
+	emit dataChanged(createIndex(0, 0), createIndex(d->cylinders.nr - 1, COLUMNS - 1));
 }
 
 void CylindersModel::updateTrashIcon()
 {
-	emit dataChanged(createIndex(0, 0), createIndex(displayed_dive.cylinders.nr - 1, 0));
+	if (!d)
+		return;
+
+	emit dataChanged(createIndex(0, 0), createIndex(d->cylinders.nr - 1, 0));
 }
 
 bool CylindersModel::updateBestMixes()
 {
+	if (!d)
+		return false;
+
 	// Check if any of the cylinders are best mixes, update if needed
 	bool gasUpdated = false;
-	for (int i = 0; i < displayed_dive.cylinders.nr; i++) {
+	for (int i = 0; i < d->cylinders.nr; i++) {
-		cylinder_t *cyl = get_cylinder(&displayed_dive, i);
+		cylinder_t *cyl = get_cylinder(d, i);
 		if (cyl->bestmix_o2) {
-			cyl->gasmix.o2 = best_o2(displayed_dive.maxdepth, &displayed_dive);
+			cyl->gasmix.o2 = best_o2(d->maxdepth, d);
 			// fO2 + fHe must not be greater than 1
 			if (get_o2(cyl->gasmix) + get_he(cyl->gasmix) > 1000)
 				cyl->gasmix.he.permille = 1000 - get_o2(cyl->gasmix);
 			pressure_t modpO2;
 			modpO2.mbar = prefs.decopo2;
-			cyl->depth = gas_mod(cyl->gasmix, modpO2, &displayed_dive, M_OR_FT(3, 10));
+			cyl->depth = gas_mod(cyl->gasmix, modpO2, d, M_OR_FT(3, 10));
 			gasUpdated = true;
 		}
 		if (cyl->bestmix_he) {
-			cyl->gasmix.he = best_he(displayed_dive.maxdepth, &displayed_dive, prefs.o2narcotic, cyl->gasmix.o2);
+			cyl->gasmix.he = best_he(d->maxdepth, d, prefs.o2narcotic, cyl->gasmix.o2);
 			// fO2 + fHe must not be greater than 1
 			if (get_o2(cyl->gasmix) + get_he(cyl->gasmix) > 1000)
 				cyl->gasmix.o2.permille = 1000 - get_he(cyl->gasmix);
@@ -599,7 +663,7 @@ bool CylindersModel::updateBestMixes()
 	/* This slot is called when the bottom pO2 and END preferences are updated, we want to
 	 * emit dataChanged so MOD and MND are refreshed, even if the gas mix hasn't been changed */
 	if (gasUpdated)
-		emit dataChanged(createIndex(0, 0), createIndex(displayed_dive.cylinders.nr - 1, COLUMNS - 1));
+		emit dataChanged(createIndex(0, 0), createIndex(d->cylinders.nr - 1, COLUMNS - 1));
 	return gasUpdated;
 }
 
@@ -607,24 +671,71 @@ void CylindersModel::cylindersReset(const QVector<dive *> &dives)
 {
 	// This model only concerns the currently displayed dive. If this is not among the
 	// dives that had their cylinders reset, exit.
-	if (!current_dive || std::find(dives.begin(), dives.end(), current_dive) == dives.end())
+	if (!d || std::find(dives.begin(), dives.end(), d) == dives.end())
 		return;
 
-	// Copy the cylinders from the current dive to the displayed dive.
+	// And update the model (the actual change was already performed in the backend)..
-	copy_cylinders(&current_dive->cylinders, &displayed_dive.cylinders);
+	beginResetModel();
-
+	endResetModel();
-	// And update the model..
-	updateDive();
 }
 
-CylindersModelFiltered::CylindersModelFiltered(QObject *parent) : QSortFilterProxyModel(parent)
+// 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)
+{
+	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 (inPlanner) {
+			std::swap(*cyl, tempCyl);
+		} else {
+			int count = Command::editCylinder(tempRow, tempCyl, Command::EditCylinderType::TYPE, false);
+			emit divesEdited(count);
+		}
+	}
+	free_cylinder(tempCyl);
+	tempRow = -1;
+}
+
+CylindersModelFiltered::CylindersModelFiltered(QObject *parent) : QSortFilterProxyModel(parent),
+	source(false) // Currently, only the EquipmentTab uses the filtered model.
 {
 	setSourceModel(&source);
 }
 
-void CylindersModelFiltered::updateDive()
+void CylindersModelFiltered::updateDive(dive *d)
 {
-	source.updateDive();
+	source.updateDive(d);
 }
 
 void CylindersModelFiltered::clear()
@@ -632,21 +743,11 @@ void CylindersModelFiltered::clear()
 	source.clear();
 }
 
-void CylindersModelFiltered::add()
-{
-	source.add();
-}
-
 CylindersModel *CylindersModelFiltered::model()
 {
 	return &source;
 }
 
-void CylindersModelFiltered::remove(QModelIndex index)
-{
-	source.remove(mapToSource(index));
-}
-
 bool CylindersModelFiltered::filterAcceptsRow(int source_row, const QModelIndex &source_parent) const
 {
 	return prefs.display_unused_tanks || source.cylinderUsed(source_row);

qt-models/cylindermodel.h

@@ -31,9 +31,11 @@ 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);
+	explicit CylindersModel(bool planner, QObject *parent = 0); // First argument: true if this model is used for the planner
 	QVariant data(const QModelIndex &index, int role = Qt::DisplayRole) const override;
 	int rowCount(const QModelIndex &parent = QModelIndex()) const override;
 	Qt::ItemFlags flags(const QModelIndex &index) const override;
@@ -41,23 +43,36 @@ public:
 
 	void add();
 	void clear();
-	void updateDive();
+	void updateDive(dive *d);
 	void updateDecoDepths(pressure_t olddecopo2);
 	void updateTrashIcon();
 	void moveAtFirst(int cylid);
-	bool changed;
 	QVariant headerData(int section, Qt::Orientation orientation, int role) const override;
 	bool updateBestMixes();
 	bool cylinderUsed(int i) const;
 
+signals:
+	void divesEdited(int num);
+
 public
 slots:
 	void remove(QModelIndex index);
 	void cylindersReset(const QVector<dive *> &dives);
+	void cylinderAdded(dive *d, int pos);
+	void cylinderRemoved(dive *d, int pos);
+	void cylinderEdited(dive *d, int pos);
 
 private:
-	int rows;
+	dive *d;
+	bool inPlanner;
+	// 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
@@ -68,11 +83,7 @@ public:
 	CylindersModel *model(); // Access to unfiltered base model
 
 	void clear();
-	void add();
+	void updateDive(dive *d);
-	void updateDive();
-public
-slots:
-	void remove(QModelIndex index);
 private:
 	CylindersModel source;
 	bool filterAcceptsRow(int source_row, const QModelIndex &source_parent) const override;

qt-models/diveplannermodel.cpp

@@ -92,7 +92,7 @@ void DivePlannerPointsModel::loadFromDive(dive *d)
 	const struct event *evd = NULL;
 	enum divemode_t current_divemode = UNDEF_COMP_TYPE;
 	recalc = false;
-	cylinders.updateDive();
+	cylinders.updateDive(&displayed_dive);
 	duration_t lasttime = { 0 };
 	duration_t lastrecordedtime = {};
 	duration_t newtime = {};
@@ -165,7 +165,7 @@ void DivePlannerPointsModel::setupCylinders()
 		reset_cylinders(&displayed_dive, true);
 
 		if (displayed_dive.cylinders.nr > 0) {
-			cylinders.updateDive();
+			cylinders.updateDive(&displayed_dive);
 			return;		// We have at least one cylinder
 		}
 	}
@@ -183,7 +183,7 @@ void DivePlannerPointsModel::setupCylinders()
 		add_to_cylinder_table(&displayed_dive.cylinders, 0, cyl);
 	}
 	reset_cylinders(&displayed_dive, false);
-	cylinders.updateDive();
+	cylinders.updateDive(&displayed_dive);
 }
 
 // Update the dive's maximum depth.  Returns true if max. depth changed
@@ -415,6 +415,7 @@ int DivePlannerPointsModel::rowCount(const QModelIndex&) const
 }
 
 DivePlannerPointsModel::DivePlannerPointsModel(QObject *parent) : QAbstractTableModel(parent),
+	cylinders(true),
 	mode(NOTHING),
 	recalc(false)
 {
@@ -908,7 +909,7 @@ void DivePlannerPointsModel::clear()
 {
 	bool oldRecalc = setRecalc(false);
 
-	cylinders.updateDive();
+	cylinders.updateDive(&displayed_dive);
 	if (rowCount() > 0) {
 		beginRemoveRows(QModelIndex(), 0, rowCount() - 1);
 		divepoints.clear();

qt-models/weightmodel.cpp

@@ -112,8 +112,10 @@ void WeightModel::commitTempWS()
 		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))
+	if (!same_string(ws.description, tempWS.description) || gettextFromC::tr(ws.description) != QString(tempWS.description)) {
-		Command::editWeight(tempRow, tempWS, false);
+		int count = Command::editWeight(tempRow, tempWS, false);
+		emit divesEdited(count);
+	}
 	tempRow = -1;
 #endif
 }
@@ -126,7 +128,8 @@ bool WeightModel::setData(const QModelIndex &index, const QVariant &value, int r
 	switch (index.column()) {
 	case WEIGHT:
 		ws.weight = string_to_weight(qPrintable(vString));
-		Command::editWeight(index.row(), ws, false);
+		int count = Command::editWeight(index.row(), ws, false);
+		emit divesEdited(count);
 		return true;
 	}
 	return false;

qt-models/weightmodel.h

@@ -29,6 +29,9 @@ public:
 	void updateDive(dive *d);
 	weightsystem_t weightSystemAt(const QModelIndex &index) const;
 
+signals:
+	void divesEdited(int num);
+
 public
 slots:
 	void weightsystemsReset(const QVector<dive *> &dives);