From 58f2e5f77c2faaf4c2f75767ee8fde67cc0931ac Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Sat, 30 Mar 2019 18:39:27 +0100 Subject: [PATCH] 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 --- core/divelist.c | 2 ++ core/divesitehelpers.cpp | 2 -- desktop-widgets/command.h | 3 +++ desktop-widgets/command_base.cpp | 16 +++++++++++++ desktop-widgets/command_divelist.cpp | 14 ------------ desktop-widgets/command_edit.cpp | 4 ---- desktop-widgets/command_edit_trip.cpp | 2 -- desktop-widgets/mainwindow.cpp | 33 +++++++++++++++++++-------- desktop-widgets/mainwindow.h | 1 + 9 files changed, 46 insertions(+), 31 deletions(-) diff --git a/core/divelist.c b/core/divelist.c index 635fa0b6e..8881aceeb 100644 --- a/core/divelist.c +++ b/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; diff --git a/core/divesitehelpers.cpp b/core/divesitehelpers.cpp index 10aab071b..6093c890c 100644 --- a/core/divesitehelpers.cpp +++ b/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"); diff --git a/desktop-widgets/command.h b/desktop-widgets/command.h index 82cb06f31..9e08b9a5e 100644 --- a/desktop-widgets/command.h +++ b/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. diff --git a/desktop-widgets/command_base.cpp b/desktop-widgets/command_base.cpp index 24ce81225..26a587f7c 100644 --- a/desktop-widgets/command_base.cpp +++ b/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")); diff --git a/desktop-widgets/command_divelist.cpp b/desktop-widgets/command_divelist.cpp index 4e1359ea7..31696f2ff 100644 --- a/desktop-widgets/command_divelist.cpp +++ b/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 &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> &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] ); diff --git a/desktop-widgets/command_edit.cpp b/desktop-widgets/command_edit.cpp index 7ff714e8f..31d34a89b 100644 --- a/desktop-widgets/command_edit.cpp +++ b/desktop-widgets/command_edit.cpp @@ -92,8 +92,6 @@ void EditBase::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 diff --git a/desktop-widgets/command_edit_trip.cpp b/desktop-widgets/command_edit_trip.cpp index 4dc680e2d..b25e3fcc5 100644 --- a/desktop-widgets/command_edit_trip.cpp +++ b/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 diff --git a/desktop-widgets/mainwindow.cpp b/desktop-widgets/mainwindow.cpp index cc1adc3b2..09af6414f 100644 --- a/desktop-widgets/mainwindow.cpp +++ b/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); } diff --git a/desktop-widgets/mainwindow.h b/desktop-widgets/mainwindow.h index 196ceb4dc..efac8610e 100644 --- a/desktop-widgets/mainwindow.h +++ b/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);