Undo: implement undo of dive trip editing

This is copying the dive editing code. It uses an OO design with
virtual functions for getting and setting the values. It doesn't
use templates though, as both fields of strig type. This feels
a bit over-engineered, but it is 1) consistent with the dive edit
code and 2) the number / types of dive trip fields might increase.

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

core/subsurface-qt/DiveListNotifier.h

@@ -10,7 +10,7 @@
 
 #include <QObject>
 
-// Dive fields that can be edited.
+// Dive and trip fields that can be edited.
 // Use "enum class" to not polute the global name space.
 enum class DiveField {
 	NR,
@@ -29,6 +29,10 @@ enum class DiveField {
 	MODE,
 	NOTES,
 };
+enum class TripField {
+	LOCATION,
+	NOTES
+};
 
 class DiveListNotifier : public QObject {
 	Q_OBJECT
@@ -54,6 +58,9 @@ signals:
 	void cylindersReset(dive_trip *trip, const QVector<dive *> &dives);
 	void weightsystemsReset(dive_trip *trip, const QVector<dive *> &dives);
 
+	// Trip edited signal
+	void tripChanged(dive_trip *trip, TripField field);
+
 	// Selection-signals come in two kinds:
 	//  - divesSelected, divesDeselected and currentDiveChanged are finer grained and are
 	//    called batch-wise per trip (except currentDiveChanged, of course). These signals

desktop-widgets/CMakeLists.txt

@@ -66,6 +66,8 @@ set(SUBSURFACE_INTERFACE
 	command_divesite.h
 	command_edit.cpp
 	command_edit.h
+	command_edit_trip.cpp
+	command_edit_trip.h
 	command_private.cpp
 	command_private.h
 	configuredivecomputerdialog.cpp

desktop-widgets/command.cpp

@@ -4,6 +4,7 @@
 #include "command_divelist.h"
 #include "command_divesite.h"
 #include "command_edit.h"
+#include "command_edit_trip.h"
 
 namespace Command {
 
@@ -205,4 +206,14 @@ void pasteDives(const dive *d, dive_components what)
 	execute(new PasteDives(d, what));
 }
 
+void editTripLocation(dive_trip *trip, const QString &s)
+{
+	execute(new EditTripLocation(trip, s));
+}
+
+void editTripNotes(dive_trip *trip, const QString &s)
+{
+	execute(new EditTripNotes(trip, s));
+}
+
 } // namespace Command

desktop-widgets/command.h

@@ -68,6 +68,10 @@ void editBuddies(const QStringList &newList, bool currentDiveOnly);
 void editDiveMaster(const QStringList &newList, bool currentDiveOnly);
 void pasteDives(const dive *d, dive_components what);
 
+// 4) Trip editing commands
+void editTripLocation(dive_trip *trip, const QString &s);
+void editTripNotes(dive_trip *trip, const QString &s);
+
 } // namespace Command
 
 #endif // COMMAND_H

desktop-widgets/command_edit_trip.cpp

@@ -0,0 +1,85 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include "command_edit_trip.h"
+#include "command_private.h"
+#include "core/divelist.h" // for mark_divelist_changed(). TODO: remove
+#include "core/qthelper.h"
+
+namespace Command {
+
+EditTripBase::EditTripBase(dive_trip *tripIn, const QString &newValue) : trip(tripIn),
+	value(newValue)
+{
+}
+
+// Note: Virtual functions cannot be called in the constructor.
+// Therefore, setting of the title is done here.
+bool EditTripBase::workToBeDone()
+{
+	setText(tr("Edit %1").arg(fieldName()));
+	return data(trip) != value;
+}
+
+void EditTripBase::undo()
+{
+	QString old = data(trip);
+	set(trip, value);
+	value = old;
+
+	emit diveListNotifier.tripChanged(trip, fieldId());
+	mark_divelist_changed(true);
+}
+
+// Undo and redo do the same as just the stored value is exchanged
+void EditTripBase::redo()
+{
+	undo();
+}
+
+// Implementation of virtual functions
+
+// ***** Location *****
+void EditTripLocation::set(dive_trip *t, const QString &s) const
+{
+	free(t->location);
+	t->location = copy_qstring(s);
+}
+
+QString EditTripLocation::data(dive_trip *t) const
+{
+	return QString(t->location);
+}
+
+QString EditTripLocation::fieldName() const
+{
+	return tr("trip location");
+}
+
+TripField EditTripLocation::fieldId() const
+{
+	return TripField::LOCATION;
+}
+
+// ***** Notes *****
+void EditTripNotes::set(dive_trip *t, const QString &s) const
+{
+	free(t->notes);
+	t->notes = copy_qstring(s);
+}
+
+QString EditTripNotes::data(dive_trip *t) const
+{
+	return QString(t->notes);
+}
+
+QString EditTripNotes::fieldName() const
+{
+	return tr("trip notes");
+}
+
+TripField EditTripNotes::fieldId() const
+{
+	return TripField::NOTES;
+}
+
+} // namespace Command

desktop-widgets/command_edit_trip.h

@@ -0,0 +1,58 @@
+// SPDX-License-Identifier: GPL-2.0
+// Note: this header file is used by the undo-machinery and should not be included elsewhere.
+
+#ifndef COMMAND_EDIT_TRIP_H
+#define COMMAND_EDIT_TRIP_H
+
+#include "command_base.h"
+#include "core/subsurface-qt/DiveListNotifier.h"
+
+#include <QVector>
+
+// These are commands that edit individual fields of a a trip.
+// The implementation follows the (rather verbose) OO-style of the dive-edit commands.
+// But here, no template is used as the two fields are both of string type.
+
+// We put everything in a namespace, so that we can shorten names without polluting the global namespace
+namespace Command {
+
+class EditTripBase : public Base {
+	bool workToBeDone() override;
+
+	dive_trip *trip; // Trip to be edited.
+public:
+	EditTripBase(dive_trip *trip, const QString &newValue);
+
+protected:
+	QString value; // Value to be set
+	void undo() override;
+	void redo() override;
+
+	// Get and set functions to be overriden by sub-classes.
+	virtual void set(struct dive_trip *t, const QString &) const = 0;
+	virtual QString data(struct dive_trip *t) const = 0;
+	virtual QString fieldName() const = 0;	// Name of the field, used to create the undo menu-entry
+	virtual TripField fieldId() const = 0;
+};
+
+class EditTripLocation : public EditTripBase {
+public:
+	using EditTripBase::EditTripBase;	// Use constructor of base class.
+	void set(dive_trip *t, const QString &s) const override;
+	QString data(dive_trip *t) const override;
+	QString fieldName() const override;
+	TripField fieldId() const override;
+};
+
+class EditTripNotes : public EditTripBase {
+public:
+	using EditTripBase::EditTripBase;	// Use constructor of base class.
+	void set(dive_trip *t, const QString &s) const override;
+	QString data(dive_trip *t) const override;
+	QString fieldName() const override;
+	TripField fieldId() const override;
+};
+
+} // namespace Command
+
+#endif

desktop-widgets/tab-widgets/maintab.cpp

@@ -69,7 +69,6 @@ MainTab::MainTab(QWidget *parent) : QTabWidget(parent),
 	ui.timeEdit->setDisplayFormat(prefs.time_format);
 
 	memset(&displayed_dive, 0, sizeof(displayed_dive));
-	memset(&displayedTrip, 0, sizeof(displayedTrip));
 
 	// This makes sure we only delete the models
 	// after the destructor of the tables,
@@ -82,6 +81,8 @@ MainTab::MainTab(QWidget *parent) : QTabWidget(parent),
 	closeMessage();
 
 	connect(&diveListNotifier, &DiveListNotifier::divesChanged, this, &MainTab::divesChanged);
+	connect(&diveListNotifier, &DiveListNotifier::tripChanged, this, &MainTab::tripChanged);
+
 	connect(ui.editDiveSiteButton, &QToolButton::clicked, MainWindow::instance(), &MainWindow::startDiveSiteEdit);
 	connect(ui.location, &DiveLocationLineEdit::entered, MapWidget::instance(), &MapWidget::centerOnIndex);
 	connect(ui.location, &DiveLocationLineEdit::currentChanged, MapWidget::instance(), &MapWidget::centerOnIndex);
@@ -281,14 +282,10 @@ void MainTab::updateTextLabels(bool showUnits)
 
 void MainTab::enableEdition(EditMode newEditMode)
 {
-	const bool isTripEdit = MainWindow::instance() &&
-		MainWindow::instance()->diveList->selectedTrips().count() == 1;
-
 	if (((newEditMode == DIVE || newEditMode == NONE) && current_dive == NULL) || editMode != NONE)
 		return;
 	modified = false;
 	if ((newEditMode == DIVE || newEditMode == NONE) &&
-	    !isTripEdit &&
 	    current_dive->dc.model &&
 	    strcmp(current_dive->dc.model, "manually added dive") == 0) {
 		// editCurrentDive will call enableEdition with newEditMode == MANUALLY_ADDED_DIVE
@@ -312,21 +309,13 @@ void MainTab::enableEdition(EditMode newEditMode)
 	ui.tabWidget->setTabEnabled(3, false);
 	ui.tabWidget->setTabEnabled(5, false);
 
-	if (isTripEdit) {
+	ui.dateEdit->setEnabled(true);
-		// we are editing trip location and notes
+	if (amount_selected > 1) {
-		displayMessage(tr("This trip is being edited."));
+		displayMessage(tr("Multiple dives are being edited."));
-		currentTrip = current_dive->divetrip;
-		ui.dateEdit->setEnabled(false);
-		editMode = TRIP;
 	} else {
-		ui.dateEdit->setEnabled(true);
+		displayMessage(tr("This dive is being edited."));
-		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 = newEditMode != NONE ? newEditMode : DIVE;
 }
 
 static void profileFromDive(struct dive *d)
@@ -397,6 +386,26 @@ void MainTab::divesChanged(dive_trip *trip, const QVector<dive *> &dives, DiveFi
 	}
 }
 
+// This function gets called if a trip-field gets updated by an undo command.
+// Refresh the corresponding UI field.
+void MainTab::tripChanged(dive_trip *trip, TripField field)
+{
+	// If the current dive is not in list of changed dives, do nothing
+	if (currentTrip != trip)
+		return;
+
+	switch(field) {
+	case TripField::NOTES:
+		ui.notes->setText(currentTrip->notes);
+		break;
+	case TripField::LOCATION:
+		ui.diveTripLocation->setText(currentTrip->location);
+		break;
+	default:
+		break;
+	}
+}
+
 void MainTab::clearEquipment()
 {
 	cylindersModel->clear();
@@ -767,18 +776,6 @@ void MainTab::acceptChanges()
 		resetPallete();
 		displayed_dive.divetrip = nullptr; // Should not be necessary, just in case!
 		return;
-	} else if (MainWindow::instance() && MainWindow::instance()->diveList->selectedTrips().count() == 1) {
-		/* now figure out if things have changed */
-		if (displayedTrip.notes && !same_string(displayedTrip.notes, currentTrip->notes)) {
-			currentTrip->notes = copy_string(displayedTrip.notes);
-			mark_divelist_changed(true);
-		}
-		if (displayedTrip.location && !same_string(displayedTrip.location, currentTrip->location)) {
-			currentTrip->location = copy_string(displayedTrip.location);
-			mark_divelist_changed(true);
-		}
-		currentTrip = NULL;
-		ui.dateEdit->setEnabled(true);
 	} else {
 		// Get list of selected dives, but put the current dive last;
 		// this is required in case the invocation wants to compare things
@@ -1102,13 +1099,11 @@ void MainTab::on_location_diveSiteSelected()
 		Command::editDiveSite(newDs, false);
 }
 
-void MainTab::on_diveTripLocation_textEdited(const QString& text)
+void MainTab::on_diveTripLocation_editingFinished()
 {
-	if (currentTrip) {
+	if (!currentTrip)
-		free(displayedTrip.location);
+		return;
-		displayedTrip.location = copy_qstring(text);
+	Command::editTripLocation(currentTrip, ui.diveTripLocation->text());
-		markChangedWidget(ui.diveTripLocation);
-	}
 }
 
 void MainTab::on_suit_editingFinished()
@@ -1119,28 +1114,18 @@ void MainTab::on_suit_editingFinished()
 	Command::editSuit(ui.suit->text(), false);
 }
 
-void MainTab::on_notes_textChanged()
-{
-	if (editMode == IGNORE)
-		return;
-	if (currentTrip) {
-		if (same_string(displayedTrip.notes, qPrintable(ui.notes->toPlainText())))
-			return;
-		free(displayedTrip.notes);
-		displayedTrip.notes = copy_qstring(ui.notes->toPlainText());
-		markChangedWidget(ui.notes);
-	}
-}
-
 void MainTab::on_notes_editingFinished()
 {
-	if (currentTrip || !current_dive)
+	if (!currentTrip && !current_dive)
-		return; // Trip-note editing is done via on_notes_textChanged()
+		return;
 
 	QString notes = ui.notes->toHtml().indexOf("<div") != -1 ?
 		ui.notes->toHtml() : ui.notes->toPlainText();
 
-	Command::editNotes(notes, false);
+	if (currentTrip)
+		Command::editTripNotes(currentTrip, notes);
+	else
+		Command::editNotes(notes, false);
 }
 
 void MainTab::on_rating_valueChanged(int value)

desktop-widgets/tab-widgets/maintab.h

@@ -39,7 +39,6 @@ public:
 	enum EditMode {
 		NONE,
 		DIVE,
-		TRIP,
 		ADD,
 		MANUALLY_ADDED_DIVE,
 		IGNORE
@@ -62,6 +61,7 @@ signals:
 public
 slots:
 	void divesChanged(dive_trip *trip, const QVector<dive *> &dives, DiveField field);
+	void tripChanged(dive_trip *trip, TripField field);
 	void addCylinder_clicked();
 	void addWeight_clicked();
 	void updateDiveInfo(bool clear = false);
@@ -76,8 +76,7 @@ slots:
 	void on_divemaster_editingFinished();
 	void on_buddy_editingFinished();
 	void on_suit_editingFinished();
-	void on_diveTripLocation_textEdited(const QString& text);
+	void on_diveTripLocation_editingFinished();
-	void on_notes_textChanged();
 	void on_notes_editingFinished();
 	void on_airtemp_editingFinished();
 	void on_duration_editingFinished();
@@ -119,7 +118,6 @@ private:
 	void copyTagsToDisplayedDive();
 	void markChangedWidget(QWidget *w);
 	dive_trip_t *currentTrip;
-	dive_trip_t displayedTrip;
 	QList<TabBase*> extraWidgets;
 };