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>

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();

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();
 }

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();

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);
 
-void UndoRemoveDivesFromTrip::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
-	}
-	tripsToAdd.clear();
-
-	for (auto &pair: divesToAdd)
-		add_dive_to_trip(pair.first, pair.second);
-	divesToAdd.clear();
 	mark_divelist_changed(true);
 
 	// Finally, do the UI stuff:
 	MainWindow::instance()->refreshDisplay();
 }
 
-void UndoRemoveDivesFromTrip::redo()
+void UndoTripBase::undo()
 {
-	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);
-	}
-	mark_divelist_changed(true);
+	// Redo and undo do the same thing!
+	redo();
+}
 
-	// Finally, do the UI stuff:
-	MainWindow::instance()->refreshDisplay();
+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} );
+	}
+}
+
+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} );
+}
+
+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} );
+}
+
+UndoAutogroupDives::UndoAutogroupDives()
+{
+	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 } );
+	}
+}
+
+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)

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 {