Undo: make diverse trip-related operations undo-able

AddDivesToTrip, CreateTrip, AutogroupDives, RemoveAutogenTrips
and MergeTrips basically all did the same thing as RemoveDivesFromTrip,
which was already implemented. Thus, factor our the common functionality
and hook it up to make all these functions undo-able.

Don't do the autogroup-call everytime the dive-list is rebuilt
(that would create innumberable undo-actions), but only on dive-load /
import or if expressly asked by the user [by switching the autogroup
flag].

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

11 files changed, 294 insertions, 74 deletions

diff --git a/core/divelist.c b/core/divelist.c
index 63bb7aa07..f6453e04c 100644
--- a/core/divelist.c
+++ b/core/divelist.c
@@ -22,7 +22,10 @@
  * void remove_dive_from_trip(struct dive *dive, bool was_autogen)
  * void add_dive_to_trip(struct dive *dive, dive_trip_t *trip)
  * dive_trip_t *create_and_hookup_trip_from_dive(struct dive *dive)
+ * dive_trip_t *get_dives_to_autogroup(int start, int *from, int *to, bool *allocated)
  * void autogroup_dives(void)
+ * void combine_trips(struct dive_trip *trip_a, struct dive_trip *trip_b)
+ * dive_trip_t *combine_trips_create(struct dive_trip *trip_a, struct dive_trip *trip_b)
  * struct dive *unregister_dive(int idx)
  * void delete_single_dive(int idx)
  * void add_single_dive(int idx, struct dive *dive)
@@ -877,12 +880,27 @@ void add_dive_to_trip(struct dive *dive, dive_trip_t *trip)
 		trip->when = dive->when;
 }
 
+dive_trip_t *alloc_trip(void)
+{
+	return calloc(1, sizeof(dive_trip_t));
+}
+
+dive_trip_t *create_trip_from_dive(struct dive *dive)
+{
+	dive_trip_t *trip;
+
+	trip = alloc_trip();
+	trip->when = dive->when;
+	trip->location = copy_string(get_dive_location(dive));
+
+	return trip;
+}
+
 dive_trip_t *create_and_hookup_trip_from_dive(struct dive *dive)
 {
-	dive_trip_t *dive_trip = calloc(1, sizeof(dive_trip_t));
+	dive_trip_t *dive_trip = alloc_trip();
 
-	dive_trip->when = dive->when;
-	dive_trip->location = copy_string(get_dive_location(dive));
+	dive_trip = create_trip_from_dive(dive);
 	insert_trip(&dive_trip);
 
 	dive->tripflag = IN_TRIP;
@@ -891,14 +909,23 @@ dive_trip_t *create_and_hookup_trip_from_dive(struct dive *dive)
 }
 
 /*
- * Walk the dives from the oldest dive, and see if we can autogroup them
+ * Collect dives for auto-grouping. Pass in first dive which should be checked.
+ * Returns range of dives that should be autogrouped and trip it should be
+ * associated to. If the returned trip was newly allocated, the last bool
+ * is set to true. Caller still has to register it in the system. Note
+ * whereas this looks complicated - it is needed by the undo-system, which
+ * manually injects the new trips. If there are no dives to be autogrouped,
+ * return NULL.
  */
-void autogroup_dives(void)
+dive_trip_t *get_dives_to_autogroup(int start, int *from, int *to, bool *allocated)
 {
 	int i;
 	struct dive *dive, *lastdive = NULL;
 
-	for_each_dive(i, dive) {
+	/* Find first dive that should be merged and remember any previous
+	 * dive that could be merged into.
+	 */
+	for (i = start; (dive = get_dive(i)) != NULL; i++) {
 		dive_trip_t *trip;
 
 		if (dive->divetrip) {
@@ -911,21 +938,53 @@ void autogroup_dives(void)
 			continue;
 		}
 
-		/* Do we have a trip we can combine this into? */
-		if (lastdive && dive->when < lastdive->when + TRIP_THRESHOLD) {
-			dive_trip_t *trip = lastdive->divetrip;
-			add_dive_to_trip(dive, trip);
+		/* We found a dive, let's see if we have to allocate a new trip */
+		if (!lastdive || dive->when >= lastdive->when + TRIP_THRESHOLD) {
+			/* allocate new trip */
+			trip = create_trip_from_dive(dive);
+			trip->autogen = true;
+			*allocated = true;
+		} else {
+			/* use trip of previous dive */
+			trip = lastdive->divetrip;
+			*allocated = false;
+		}
+
+		// Now, find all dives that will be added to this trip
+		lastdive = dive;
+		*from = i;
+		for (*to = *from + 1; (dive = get_dive(*to)) != NULL; (*to)++) {
+			if (dive->divetrip || !DIVE_NEEDS_TRIP(dive) ||
+			    dive->when >= lastdive->when + TRIP_THRESHOLD)
+				break;
 			if (get_dive_location(dive) && !trip->location)
 				trip->location = copy_string(get_dive_location(dive));
 			lastdive = dive;
-			continue;
 		}
-
-		lastdive = dive;
-		trip = create_and_hookup_trip_from_dive(dive);
-		trip->autogen = 1;
+		return trip;
 	}
 
+	/* Did not find anyhting - mark as end */
+	return NULL;
+}
+
+/*
+ * Walk the dives from the oldest dive, and see if we can autogroup them
+ */
+void autogroup_dives(void)
+{
+	int from, to;
+	dive_trip_t *trip;
+	int i, j;
+	bool alloc;
+
+	for(i = 0; (trip = get_dives_to_autogroup(i, &from, &to, &alloc)) != NULL; i = to) {
+		/* If this was newly allocated, add trip to list */
+		if (alloc)
+			insert_trip(&trip);
+		for (j = from; j < to; ++j)
+			add_dive_to_trip(get_dive(j), trip);
+	}
 #ifdef DEBUG_TRIP
 	dump_trip_list();
 #endif
@@ -1209,6 +1268,26 @@ void combine_trips(struct dive_trip *trip_a, struct dive_trip *trip_b)
 		add_dive_to_trip(trip_b->dives, trip_a);
 }
 
+/* Out of two strings, copy the string that is not empty (if any). */
+static char *copy_non_empty_string(const char *a, const char *b)
+{
+	return copy_string(empty_string(b) ? a : b);
+}
+
+/* Combine trips new. This combines two trips, generating a
+ * new trip. To support undo, we have to preserve the old trips. */
+dive_trip_t *combine_trips_create(struct dive_trip *trip_a, struct dive_trip *trip_b)
+{
+	dive_trip_t *trip;
+
+	trip = alloc_trip();
+	trip->when = trip_a->when;
+	trip->location = copy_non_empty_string(trip_a->location, trip_b->location);
+	trip->notes = copy_non_empty_string(trip_a->notes, trip_b->notes);
+
+	return trip;
+}
+
 void mark_divelist_changed(bool changed)
 {
 	if (dive_list_changed == changed)
diff --git a/core/divelist.h b/core/divelist.h
index 2b2ee07a5..8bcf9f55c 100644
--- a/core/divelist.h
+++ b/core/divelist.h
@@ -28,7 +28,10 @@ extern int get_divenr(const struct dive *dive);
 extern int get_divesite_idx(const struct dive_site *ds);
 extern struct dive_trip *unregister_dive_from_trip(struct dive *dive, short was_autogen);
 extern void remove_dive_from_trip(struct dive *dive, short was_autogen);
+extern dive_trip_t *alloc_trip(void);
+extern dive_trip_t *create_trip_from_dive(struct dive *dive);
 extern dive_trip_t *create_and_hookup_trip_from_dive(struct dive *dive);
+extern dive_trip_t *get_dives_to_autogroup(int start, int *from, int *to, bool *allocated);
 extern void autogroup_dives(void);
 extern struct dive *merge_two_dives(struct dive *a, struct dive *b);
 extern bool consecutive_selected();
@@ -38,6 +41,7 @@ extern void select_dives_in_trip(struct dive_trip *trip);
 extern void deselect_dives_in_trip(struct dive_trip *trip);
 extern void filter_dive(struct dive *d, bool shown);
 extern void combine_trips(struct dive_trip *trip_a, struct dive_trip *trip_b);
+extern dive_trip_t *combine_trips_create(struct dive_trip *trip_a, struct dive_trip *trip_b);
 extern void find_new_trip_start_time(dive_trip_t *trip);
 extern struct dive *first_selected_dive();
 extern struct dive *last_selected_dive();
diff --git a/core/load-git.c b/core/load-git.c
index 66f4fd531..0abae4239 100644
--- a/core/load-git.c
+++ b/core/load-git.c
@@ -1208,7 +1208,7 @@ static struct dive *create_new_dive(timestamp_t when)
 
 static dive_trip_t *create_new_trip(int yyyy, int mm, int dd)
 {
-	dive_trip_t *trip = calloc(1, sizeof(dive_trip_t));
+	dive_trip_t *trip = alloc_trip();
 	struct tm tm = { 0 };
 
 	/* We'll fill in the real data from the trip descriptor file */
diff --git a/core/parse.c b/core/parse.c
index 46c041925..c8f3ae89a 100644
--- a/core/parse.c
+++ b/core/parse.c
@@ -282,7 +282,7 @@ void trip_start(void)
 	if (cur_trip)
 		return;
 	dive_end();
-	cur_trip = calloc(1, sizeof(dive_trip_t));
+	cur_trip = alloc_trip();
 	memset(&cur_tm, 0, sizeof(cur_tm));
 }
 
diff --git a/desktop-widgets/divelistview.cpp b/desktop-widgets/divelistview.cpp
index e6cbbc78f..27b7e3ed9 100644
--- a/desktop-widgets/divelistview.cpp
+++ b/desktop-widgets/divelistview.cpp
@@ -671,7 +671,8 @@ void DiveListView::merge_trip(const QModelIndex &a, int offset)
 	dive_trip_t *trip_b = (dive_trip_t *)b.data(DiveTripModel::TRIP_ROLE).value<void *>();
 	if (trip_a == trip_b || !trip_a || !trip_b)
 		return;
-	combine_trips(trip_a, trip_b);
+	UndoMergeTrips *undoCommand = new UndoMergeTrips(trip_a, trip_b);
+	MainWindow::instance()->undoStack->push(undoCommand);
 	rememberSelection();
 	reload(currentLayout, false);
 	restoreSelection();
@@ -694,7 +695,7 @@ void DiveListView::removeFromTrip()
 	//TODO: move this to C-code.
 	int i;
 	struct dive *d;
-	QVector<struct dive *> divesToRemove;
+	QVector<dive *> divesToRemove;
 	for_each_dive (i, d) {
 		if (d->selected && d->divetrip)
 			divesToRemove.append(d);
@@ -717,15 +718,16 @@ void DiveListView::newTripAbove()
 	if (!d) // shouldn't happen as we only are setting up this action if this is a dive
 		return;
 	//TODO: port to c-code.
-	dive_trip_t *trip;
 	int idx;
 	rememberSelection();
-	trip = create_and_hookup_trip_from_dive(d);
+	QVector<dive *> dives;
 	for_each_dive (idx, d) {
 		if (d->selected)
-			add_dive_to_trip(d, trip);
+			dives.append(d);
 	}
-	trip->expanded = 1;
+	UndoCreateTrip *undoCommand = new UndoCreateTrip(dives);
+	MainWindow::instance()->undoStack->push(undoCommand);
+
 	reload(currentLayout, false);
 	mark_divelist_changed(true);
 	restoreSelection();
@@ -764,16 +766,16 @@ void DiveListView::addToTrip(int delta)
 
 	rememberSelection();
 
-	add_dive_to_trip(d, trip);
+	QVector<dive *> dives;
 	if (d->selected) { // there are possibly other selected dives that we should add
 		int idx;
 		for_each_dive (idx, d) {
 			if (d->selected)
-				add_dive_to_trip(d, trip);
+				dives.append(d);
 		}
 	}
-	trip->expanded = 1;
-	mark_divelist_changed(true);
+	UndoAddDivesToTrip *undoEntry = new UndoAddDivesToTrip(dives, trip);
+	MainWindow::instance()->undoStack->push(undoEntry);
 
 	reload(currentLayout, false);
 	restoreSelection();
diff --git a/desktop-widgets/divelogimportdialog.cpp b/desktop-widgets/divelogimportdialog.cpp
index e10f9b7af..151488c0b 100644
--- a/desktop-widgets/divelogimportdialog.cpp
+++ b/desktop-widgets/divelogimportdialog.cpp
@@ -1009,6 +1009,7 @@ void DiveLogImportDialog::on_buttonBox_accepted()
 	}
 
 	process_imported_dives(&table, false, false);
+	autogroup_dives();
 	MainWindow::instance()->undoStack->clear();
 	MainWindow::instance()->refreshDisplay();
 }
diff --git a/desktop-widgets/mainwindow.cpp b/desktop-widgets/mainwindow.cpp
index 1cc82b7c1..06f6b8968 100644
--- a/desktop-widgets/mainwindow.cpp
+++ b/desktop-widgets/mainwindow.cpp
@@ -48,6 +48,7 @@
 #include "desktop-widgets/mapwidget.h"
 #include "desktop-widgets/subsurfacewebservices.h"
 #include "desktop-widgets/tab-widgets/maintab.h"
+#include "desktop-widgets/undocommands.h"
 #include "desktop-widgets/updatemanager.h"
 #include "desktop-widgets/usersurvey.h"
 
@@ -66,7 +67,6 @@
 #include "qt-models/tankinfomodel.h"
 #include "qt-models/weightsysteminfomodel.h"
 #include "qt-models/yearlystatisticsmodel.h"
-
 #include "preferences/preferencesdialog.h"
 
 #ifndef NO_USERMANUAL
@@ -618,6 +618,8 @@ void MainWindow::on_actionCloudstorageopen_triggered()
 	if (!parse_file(fileNamePtr.data(), &dive_table))
 		setCurrentFile(fileNamePtr.data());
 	process_loaded_dives();
+	if (autogroup)
+		autogroup_dives();
 	undoStack->clear();
 	hideProgressBar();
 	refreshDisplay();
@@ -1073,9 +1075,9 @@ void MainWindow::on_actionAutoGroup_triggered()
 {
 	set_autogroup(ui.actionAutoGroup->isChecked());
 	if (autogroup)
-		autogroup_dives();
+		undoStack->push(new UndoAutogroupDives);
 	else
-		remove_autogen_trips();
+		undoStack->push(new UndoRemoveAutogenTrips);
 	refreshDisplay();
 	mark_divelist_changed(true);
 }
@@ -1741,6 +1743,8 @@ void MainWindow::importFiles(const QStringList fileNames)
 		parse_file(fileNamePtr.data(), &table);
 	}
 	process_imported_dives(&table, false, false);
+	if (autogroup)
+		autogroup_dives();
 	undoStack->clear();
 	refreshDisplay();
 }
@@ -1764,6 +1768,8 @@ void MainWindow::loadFiles(const QStringList fileNames)
 	hideProgressBar();
 	updateRecentFiles();
 	process_loaded_dives();
+	if (autogroup)
+		autogroup_dives();
 	undoStack->clear();
 
 	refreshDisplay();
diff --git a/desktop-widgets/undocommands.cpp b/desktop-widgets/undocommands.cpp
index 16f87fa23..8470e4f2d 100644
--- a/desktop-widgets/undocommands.cpp
+++ b/desktop-widgets/undocommands.cpp
@@ -88,6 +88,60 @@ static void renumberDives(QVector<QPair<int, int>> &divesToRenumber)
 	}
 }
 
+// This helper function moves a dive to a trip. The old trip is recorded in the
+// passed-in structure. This means that calling the function twice on the same
+// object is a no-op concerning the dive. If the old trip was deleted from the
+// core, an owning pointer to the removed trip is returned, otherwise a null pointer.
+static OwningTripPtr moveDiveToTrip(DiveToTrip &diveToTrip)
+{
+	// Firstly, check if we move to the same trip and bail if this is a no-op.
+	if (diveToTrip.trip == diveToTrip.dive->divetrip)
+		return {};
+
+	// Remove from old trip
+	OwningTripPtr res;
+
+	// Remove dive from trip - if this is the last dive in the trip, remove the whole trip.
+	dive_trip *trip = unregister_dive_from_trip(diveToTrip.dive, false);
+	if (trip && trip->nrdives == 0) {
+		unregister_trip(trip);		// Remove trip from backend
+		res.reset(trip);
+	}
+
+	// Store old trip and get new trip we should associate this dive with
+	std::swap(trip, diveToTrip.trip);
+	add_dive_to_trip(diveToTrip.dive, trip);
+	return res;
+}
+
+// This helper function moves a set of dives between trips using the
+// moveDiveToTrip function. Before doing so, it adds the necessary trips to
+// the core. Trips that are removed from the core because they are empty
+// are recorded in the passed in struct. The vectors of trips and dives
+// are reversed. Thus, calling the function twice on the same object is
+// a no-op.
+static void moveDivesBetweenTrips(DivesToTrip &dives)
+{
+	// first bring back the trip(s)
+	for (OwningTripPtr &trip: dives.tripsToAdd) {
+		dive_trip *t = trip.release();	// Give up ownership
+		insert_trip(&t);		// Return ownership to backend
+	}
+	dives.tripsToAdd.clear();
+
+	for (DiveToTrip &dive: dives.divesToMove) {
+		OwningTripPtr tripToAdd = moveDiveToTrip(dive);
+		// register trips that we'll have to readd
+		if (tripToAdd)
+			dives.tripsToAdd.push_back(std::move(tripToAdd));
+	}
+
+	// Reverse the tripsToAdd and the divesToAdd, so that on undo/redo the operations
+	// will be performed in reverse order.
+	std::reverse(dives.tripsToAdd.begin(), dives.tripsToAdd.end());
+	std::reverse(dives.divesToMove.begin(), dives.divesToMove.end());
+}
+
 UndoAddDive::UndoAddDive(dive *d)
 {
 	setText(gettextFromC::tr("add dive"));
@@ -196,47 +250,90 @@ void UndoRenumberDives::redo()
 	undo();
 }
 
-UndoRemoveDivesFromTrip::UndoRemoveDivesFromTrip(const QVector<dive *> &divesToRemoveIn) : divesToRemove(divesToRemoveIn)
+void UndoTripBase::redo()
 {
-	setText(tr("remove %n dive(s) from trip", "", divesToRemove.size()));
+	moveDivesBetweenTrips(divesToMove);
+
+	mark_divelist_changed(true);
+
+	// Finally, do the UI stuff:
+	MainWindow::instance()->refreshDisplay();
 }
 
-void UndoRemoveDivesFromTrip::undo()
+void UndoTripBase::undo()
 {
-	// first bring back the trip(s)
-	for (auto &trip: tripsToAdd) {
-		dive_trip *t = trip.release();	// Give up ownership
-		insert_trip(&t);		// Return ownership to backend
+	// Redo and undo do the same thing!
+	redo();
+}
+
+UndoRemoveDivesFromTrip::UndoRemoveDivesFromTrip(const QVector<dive *> &divesToRemove)
+{
+	setText(divesToRemove.size() == 1 ? gettextFromC::tr("remove dive from trip")
+					  : gettextFromC::tr("remove %1 dives from trip").arg(divesToRemove.size()));
+	divesToMove.divesToMove.reserve(divesToRemove.size());
+	for (dive *d: divesToRemove)
+		divesToMove.divesToMove.push_back( {d, nullptr} );
+}
+
+UndoRemoveAutogenTrips::UndoRemoveAutogenTrips()
+{
+	setText(gettextFromC::tr("remove autogenerated trips"));
+	// TODO: don't touch core-innards directly
+	int i;
+	struct dive *dive;
+	for_each_dive(i, dive) {
+		if (dive->divetrip && dive->divetrip->autogen)
+			divesToMove.divesToMove.push_back( {dive, nullptr} );
 	}
-	tripsToAdd.clear();
+}
 
-	for (auto &pair: divesToAdd)
-		add_dive_to_trip(pair.first, pair.second);
-	divesToAdd.clear();
-	mark_divelist_changed(true);
+UndoAddDivesToTrip::UndoAddDivesToTrip(const QVector<dive *> &divesToAddIn, dive_trip *trip)
+{
+	setText(divesToAddIn.size() == 1 ? gettextFromC::tr("add dives to trip")
+					 : gettextFromC::tr("add %1 dives to trip").arg(divesToAddIn.size()));
+	for (dive *d: divesToAddIn)
+		divesToMove.divesToMove.push_back( {d, trip} );
+}
 
-	// Finally, do the UI stuff:
-	MainWindow::instance()->refreshDisplay();
+UndoCreateTrip::UndoCreateTrip(const QVector<dive *> &divesToAddIn)
+{
+	setText(gettextFromC::tr("create trip"));
+
+	if (divesToAddIn.isEmpty())
+		return;
+
+	dive_trip *trip = create_trip_from_dive(divesToAddIn[0]);
+	divesToMove.tripsToAdd.emplace_back(trip);
+	for (dive *d: divesToAddIn)
+		divesToMove.divesToMove.push_back( {d, trip} );
 }
 
-void UndoRemoveDivesFromTrip::redo()
+UndoAutogroupDives::UndoAutogroupDives()
 {
-	for (dive *d: divesToRemove) {
-		// remove dive from trip - if this is the last dive in the trip
-		// remove the whole trip.
-		dive_trip *trip = unregister_dive_from_trip(d, false);
-		if (!trip)
-			continue;				// This was not part of a trip
-		if (trip->nrdives == 0) {
-			unregister_trip(trip);			// Remove trip from backend
-			tripsToAdd.emplace_back(trip);		// Take ownership of trip
-		}
-		divesToAdd.emplace_back(d, trip);
+	setText(gettextFromC::tr("autogroup dives"));
+
+	dive_trip *trip;
+	bool alloc;
+	int from, to;
+	for(int i = 0; (trip = get_dives_to_autogroup(i, &from, &to, &alloc)) != NULL; i = to) {
+		// If this is an allocated trip, take ownership
+		if (alloc)
+			divesToMove.tripsToAdd.emplace_back(trip);
+		for (int j = from; j < to; ++j)
+			divesToMove.divesToMove.push_back( { get_dive(j), trip } );
 	}
-	mark_divelist_changed(true);
+}
 
-	// Finally, do the UI stuff:
-	MainWindow::instance()->refreshDisplay();
+UndoMergeTrips::UndoMergeTrips(dive_trip *trip1, dive_trip *trip2)
+{
+	if (trip1 == trip2)
+		return;
+	dive_trip *newTrip = combine_trips_create(trip1, trip2);
+	divesToMove.tripsToAdd.emplace_back(newTrip);
+	for (dive *d = trip1->dives; d; d = d->next)
+		divesToMove.divesToMove.push_back( { d, newTrip } );
+	for (dive *d = trip2->dives; d; d = d->next)
+		divesToMove.divesToMove.push_back( { d, newTrip } );
 }
 
 UndoSplitDives::UndoSplitDives(dive *d, duration_t time)
diff --git a/desktop-widgets/undocommands.h b/desktop-widgets/undocommands.h
index da30c27a0..11c35bb40 100644
--- a/desktop-widgets/undocommands.h
+++ b/desktop-widgets/undocommands.h
@@ -157,9 +157,27 @@ struct DiveToAdd {
 	int		 idx;		// Position in divelist
 };
 
+// This helper structure describes a dive that should be moved to / removed from
+// a trip. If the "trip" member is null, the dive is removed from its trip (if
+// it is in a trip, that is)
+struct DiveToTrip
+{
+	struct dive	*dive;
+	dive_trip	*trip;
+};
+
+// This helper structure describes a number of dives to add to /remove from /
+// move between trips.
+// It has ownership of the trips (if any) that have to be added before hand.
+struct DivesToTrip
+{
+	std::vector<DiveToTrip> divesToMove;		// If dive_trip is null, remove from trip
+	std::vector<OwningTripPtr> tripsToAdd;
+};
+
 class UndoAddDive : public QUndoCommand {
 public:
-	UndoAddDive(dive *dive);	// Warning: old dive will be erased (moved in C++-speak)!
+	UndoAddDive(dive *dive);
 private:
 	void undo() override;
 	void redo() override;
@@ -211,20 +229,36 @@ private:
 	QVector<QPair<int, int>> divesToRenumber;
 };
 
-class UndoRemoveDivesFromTrip : public QUndoCommand {
+// The classes UndoRemoveDivesFromTrip, UndoRemoveAutogenTrips, UndoCreateTrip,
+// UndoAutogroupDives and UndoMergeTrips all do the same thing, just the intialization
+// differs. Therefore, define a base class with the proper data-structures, redo()
+// and undo() functions and derive to specialize the initialization.
+class UndoTripBase : public QUndoCommand {
 	Q_DECLARE_TR_FUNCTIONS(Command)
-public:
-	UndoRemoveDivesFromTrip(const QVector<dive *> &divesToRemove);
-private:
+protected:
 	void undo() override;
 	void redo() override;
 
-	// For redo
-	QVector<dive *> divesToRemove;
-
-	// For undo
-	std::vector<std::pair<dive *, dive_trip *>> divesToAdd;
-	std::vector<OwningTripPtr> tripsToAdd;
+	// For redo and undo
+	DivesToTrip divesToMove;
+};
+struct UndoRemoveDivesFromTrip : public UndoTripBase {
+	UndoRemoveDivesFromTrip(const QVector<dive *> &divesToRemove);
+};
+struct UndoRemoveAutogenTrips : public UndoTripBase {
+	UndoRemoveAutogenTrips();
+};
+struct UndoAddDivesToTrip : public UndoTripBase {
+	UndoAddDivesToTrip(const QVector<dive *> &divesToAdd, dive_trip *trip);
+};
+struct UndoCreateTrip : public UndoTripBase {
+	UndoCreateTrip(const QVector<dive *> &divesToAdd);
+};
+struct UndoAutogroupDives : public UndoTripBase {
+	UndoAutogroupDives();
+};
+struct UndoMergeTrips : public UndoTripBase {
+	UndoMergeTrips(dive_trip *trip1, dive_trip *trip2);
 };
 
 class UndoSplitDives : public QUndoCommand {
diff --git a/mobile-widgets/qmlmanager.cpp b/mobile-widgets/qmlmanager.cpp
index 9254230f9..22431de30 100644
--- a/mobile-widgets/qmlmanager.cpp
+++ b/mobile-widgets/qmlmanager.cpp
@@ -1300,7 +1300,7 @@ void QMLManager::deleteDive(int id)
 		deletedDive = alloc_dive();
 	copy_dive(d, deletedDive);
 	if (!deletedTrip) {
-		deletedTrip = (struct dive_trip *)calloc(1, sizeof(struct dive_trip));
+		deletedTrip = alloc_trip();
 	} else {
 		free(deletedTrip->location);
 		free(deletedTrip->notes);
diff --git a/qt-models/divetripmodel.cpp b/qt-models/divetripmodel.cpp
index 58ccc0fd2..406c5eee6 100644
--- a/qt-models/divetripmodel.cpp
+++ b/qt-models/divetripmodel.cpp
@@ -643,8 +643,6 @@ void DiveTripModel::setupModelData()
 
 	beginResetModel();
 
-	if (autogroup)
-		autogroup_dives();
 	items.clear();
 	while (--i >= 0) {
 		dive *d = get_dive(i);
@@ -665,7 +663,6 @@ void DiveTripModel::setupModelData()
 		if (it == items.end()) {
 			// We didn't find an entry for this trip -> add one
 			items.emplace_back(trip, d);
-
 		} else {
 			// We found the trip -> simply add the dive
 			it->dives.push_back(d);