Undo: use QUndoStack::isClean() to determine unsaved changes

Properly implement the unsaved-changes flag(s). Since we currently have
two kinds of changes, there are two flags:
1) dive_list_changed in divelist.c marks non-undoable changes. This flag
   is only cleared on save or load.
2) QUndoStack::isClean() is used to determine the state of undoable
   changes. Every time the user returns to the state where they saved,
   this flag is cleared.

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

core/divelist.c

@@ -20,6 +20,8 @@
 #include "git-access.h"
 #include "table.h"
 
+/* This flag is set to true by operations that are not implemented in the
+ * undo system. It is therefore only cleared on save and load. */
 static bool dive_list_changed = false;
 
 bool autogroup = false;

core/divesitehelpers.cpp

@@ -108,7 +108,6 @@ void reverseGeoLookup(degrees_t latitude, degrees_t longitude, taxonomy_data *ta
 				taxonomy->category[ri].category = TC_ADMIN_L3;
 				taxonomy->nr++;
 			}
-			mark_divelist_changed(true);
 		} else {
 			report_error("geonames.org did not provide reverse lookup information");
 			qDebug() << "no reverse geo lookup; geonames returned\n" << fullReply;
@@ -157,7 +156,6 @@ void reverseGeoLookup(degrees_t latitude, degrees_t longitude, taxonomy_data *ta
 				if (idx == taxonomy->nr)
 					taxonomy->nr++;
 			}
-			mark_divelist_changed(true);
 		}
 	} else {
 		report_error("timeout accessing geonames.org");

desktop-widgets/command.h

@@ -10,7 +10,10 @@
 namespace Command {
 
 // 1) General commands
+void init();				// Setup signals to inform frontend of clean status.
 void clear();				// Reset the undo stack. Delete all commands.
+void setClean();			// Call after save - this marks a state where no changes need to be saved.
+bool isClean();				// Any changes need to be saved?
 QAction *undoAction(QObject *parent);	// Create an undo action.
 QAction *redoAction(QObject *parent);	// Create an redo action.
 

desktop-widgets/command_base.cpp

@@ -1,17 +1,33 @@
 // SPDX-License-Identifier: GPL-2.0
 
 #include "command_base.h"
+#include "core/qthelper.h" // for updateWindowTitle()
 
 namespace Command {
 
 static QUndoStack undoStack;
 
 // General commands
+void init()
+{
+	QObject::connect(&undoStack, &QUndoStack::cleanChanged, &updateWindowTitle);
+}
+
 void clear()
 {
 	undoStack.clear();
 }
 
+void setClean()
+{
+	undoStack.setClean();
+}
+
+bool isClean()
+{
+	return undoStack.isClean();
+}
+
 QAction *undoAction(QObject *parent)
 {
 	return undoStack.createUndoAction(parent, QCoreApplication::translate("Command", "&Undo"));

desktop-widgets/command_divelist.cpp

@@ -411,7 +411,6 @@ void AddDive::redoit()
 
 	divesAndSitesToRemove = addDives(divesToAdd);
 	sort_trip_table(&trip_table); // Though unlikely, adding a dive may reorder trips
-	mark_divelist_changed(true);
 
 	// Select the newly added dive
 	restoreSelection(divesAndSitesToRemove.dives, divesAndSitesToRemove.dives[0]);
@@ -500,8 +499,6 @@ void ImportDives::redoit()
 
 	// Remember dives and sites to remove
 	divesAndSitesToRemove = std::move(divesAndSitesToRemoveNew);
-
-	mark_divelist_changed(true);
 }
 
 void ImportDives::undoit()
@@ -517,8 +514,6 @@ void ImportDives::undoit()
 
 	// ...and restore the selection
 	restoreSelection(selection, currentDive);
-
-	mark_divelist_changed(true);
 }
 
 DeleteDive::DeleteDive(const QVector<struct dive*> &divesToDeleteIn)
@@ -536,7 +531,6 @@ void DeleteDive::undoit()
 {
 	divesToDelete = addDives(divesToAdd);
 	sort_trip_table(&trip_table); // Though unlikely, removing a dive may reorder trips
-	mark_divelist_changed(true);
 
 	// Select all re-added dives and make the first one current
 	dive *currentDive = !divesToDelete.dives.empty() ? divesToDelete.dives[0] : nullptr;
@@ -547,7 +541,6 @@ void DeleteDive::redoit()
 {
 	divesToAdd = removeDives(divesToDelete);
 	sort_trip_table(&trip_table); // Though unlikely, adding a dive may reorder trips
-	mark_divelist_changed(true);
 
 	// Deselect all dives and select dive that was close to the first deleted dive
 	dive *newCurrent = nullptr;
@@ -587,8 +580,6 @@ void ShiftTime::redoit()
 
 	// Negate the time-shift so that the next call does the reverse
 	timeChanged = -timeChanged;
-
-	mark_divelist_changed(true);
 }
 
 bool ShiftTime::workToBeDone()
@@ -611,7 +602,6 @@ RenumberDives::RenumberDives(const QVector<QPair<dive *, int>> &divesToRenumberI
 void RenumberDives::undoit()
 {
 	renumberDives(divesToRenumber);
-	mark_divelist_changed(true);
 }
 
 bool RenumberDives::workToBeDone()
@@ -634,8 +624,6 @@ void TripBase::redoit()
 {
 	moveDivesBetweenTrips(divesToMove);
 	sort_trip_table(&trip_table); // Though unlikely, moving dives may reorder trips
-
-	mark_divelist_changed(true);
 }
 
 void TripBase::undoit()
@@ -764,7 +752,6 @@ void SplitDivesBase::redoit()
 {
 	divesToUnsplit = addDives(splitDives);
 	unsplitDive = removeDives(diveToSplit);
-	mark_divelist_changed(true);
 
 	// Select split dives and make first dive current
 	restoreSelection(divesToUnsplit.dives, divesToUnsplit.dives[0]);
@@ -775,7 +762,6 @@ void SplitDivesBase::undoit()
 	// Note: reverse order with respect to redoit()
 	diveToSplit = addDives(unsplitDive);
 	splitDives = removeDives(divesToUnsplit);
-	mark_divelist_changed(true);
 
 	// Select unsplit dive and make it current
 	restoreSelection(diveToSplit.dives, diveToSplit.dives[0] );

desktop-widgets/command_edit.cpp

@@ -92,8 +92,6 @@ void EditBase<T>::undo()
 
 	if (setSelection(selectedDives, current))
 		emit diveListNotifier.selectionChanged(); // If the selection changed -> tell the frontend
-
-	mark_divelist_changed(true);
 }
 
 // We have to manually instantiate the constructors of the EditBase class,
@@ -509,8 +507,6 @@ void EditTagsBase::undo()
 
 	if (setSelection(selectedDives, current))
 		emit diveListNotifier.selectionChanged(); // If the selection changed -> tell the frontend
-
-	mark_divelist_changed(true);
 }
 
 // Undo and redo do the same as just the stored value is exchanged

desktop-widgets/command_edit_trip.cpp

@@ -2,7 +2,6 @@
 
 #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 {
@@ -27,7 +26,6 @@ void EditTripBase::undo()
 	value = old;
 
 	emit diveListNotifier.tripChanged(trip, fieldId());
-	mark_divelist_changed(true);
 }
 
 // Undo and redo do the same as just the stored value is exchanged

desktop-widgets/mainwindow.cpp

@@ -128,6 +128,7 @@ MainWindow::MainWindow() : QMainWindow(),
 	m_Instance = this;
 	ui.setupUi(this);
 	read_hashes();
+	Command::init();
 	// Define the States of the Application Here, Currently the states are situations where the different
 	// widgets will change on the mainwindow.
 
@@ -589,7 +590,15 @@ void MainWindow::on_actionCloudstoragesave_triggered()
 		return;
 
 	setCurrentFile(qPrintable(filename));
-	mark_divelist_changed(false);
+	setFileClean();
+}
+
+// Currently we have two markers for unsaved changes:
+// 1) unsaved_changes() returns true for non-undoable changes.
+// 2) Command::isClean() returns false for undoable changes.
+static bool unsavedChanges()
+{
+	return unsaved_changes() || !Command::isClean();
 }
 
 void MainWindow::on_actionCloudOnline_triggered()
@@ -611,7 +620,7 @@ void MainWindow::on_actionCloudOnline_triggered()
 	git_local_only = isOffline;
 	if (!isOffline) {
 		// User requests to go online. Try to sync cloud storage
-		if (unsaved_changes()) {
+		if (unsavedChanges()) {
 			// If there are unsaved changes, ask the user if they want to save them.
 			// If they don't, they have to sync manually.
 			if (QMessageBox::warning(this, tr("Save changes?"),
@@ -653,12 +662,18 @@ bool MainWindow::okToClose(QString message)
 		QMessageBox::warning(this, tr("Warning"), message);
 		return false;
 	}
-	if (unsaved_changes() && askSaveChanges() == false)
+	if (unsavedChanges() && askSaveChanges() == false)
 		return false;
 
 	return true;
 }
 
+void MainWindow::setFileClean()
+{
+	mark_divelist_changed(false);
+	Command::setClean();
+}
+
 void MainWindow::closeCurrentFile()
 {
 	graphics->setEmptyState();
@@ -667,7 +682,7 @@ void MainWindow::closeCurrentFile()
 	clear_dive_file_data();
 	setCurrentFile(nullptr);
 	cleanUpEmpty();
-	mark_divelist_changed(false);
+	setFileClean();
 
 	clear_events();
 
@@ -763,7 +778,7 @@ void MainWindow::on_actionQuit_triggered()
 			return;
 	}
 
-	if (unsaved_changes() && (askSaveChanges() == false))
+	if (unsavedChanges() && (askSaveChanges() == false))
 		return;
 	writeSettings();
 	QApplication::quit();
@@ -1421,7 +1436,7 @@ void MainWindow::closeEvent(QCloseEvent *event)
 		return;
 	}
 
-	if (unsaved_changes() && (askSaveChanges() == false)) {
+	if (unsavedChanges() && (askSaveChanges() == false)) {
 		event->ignore();
 		return;
 	}
@@ -1555,7 +1570,7 @@ int MainWindow::file_save_as(void)
 		return -1;
 
 	setCurrentFile(qPrintable(filename));
-	mark_divelist_changed(false);
+	setFileClean();
 	addRecentFile(filename, true);
 	return 0;
 }
@@ -1592,7 +1607,7 @@ int MainWindow::file_save(void)
 	}
 	if (is_cloud)
 		hideProgressBar();
-	mark_divelist_changed(false);
+	setFileClean();
 	addRecentFile(QString(existing_filename), true);
 	return 0;
 }
@@ -1631,7 +1646,7 @@ void MainWindow::setTitle()
 		return;
 	}
 
-	QString unsaved = (unsaved_changes() ? " *" : "");
+	QString unsaved = (unsavedChanges() ? " *" : "");
 	QString shown = QString(" (%1)").arg(filterWidget2.shownText());
 	setWindowTitle("Subsurface: " + displayedFilename(existing_filename) + unsaved + shown);
 }

desktop-widgets/mainwindow.h

@@ -203,6 +203,7 @@ private:
 	void writeSettings();
 	int file_save();
 	int file_save_as();
+	void setFileClean();
 	void beginChangeState(CurrentState s);
 	void saveSplitterSizes();
 	void toggleCollapsible(bool toggle);