From 403dd5a8918d4acab33cdd0a400570003244ca5e Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Thu, 19 Jul 2018 14:44:27 +0200 Subject: Undo: fix multi-level undo of delete-dive and remove-dive-from-trip The original undo-code was fundamentally broken. Not only did it leak resources (copied trips were never freed), it also kept references to trips or dives that could be changed by other commands. Thus, anything more than a single undo could lead to crashes. Two ways of fixing this were considered 1) Don't store pointers, but unique dive-ids and trip-ids. Whereas such unique ids exist for dives, they would have to be implemented for trips. 2) Don't free objects in the backend. Instead, take ownership of deleted objects in the undo-object. Thus, all references in previous undo-objects are guaranteed to still exist (unless the objects are deleted elsewhere). After some contemplation, the second method was chosen, because it is significantly less intrusive. While touching the undo-objects, clearly separate backend from ui-code, such that they can ultimately be reused for mobile. Note that if other parts of the code delete dives, crashes can still be provoked. Notable examples are split/merge dives. These will have to be fixed later. Nevertheless, the new code is a significant improvement over the old state. While touching the code, implement proper translation string based on Qt's plural-feature (using %n). Signed-off-by: Berthold Stoeger --- desktop-widgets/simplewidgets.cpp | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) (limited to 'desktop-widgets/simplewidgets.cpp') diff --git a/desktop-widgets/simplewidgets.cpp b/desktop-widgets/simplewidgets.cpp index 64845712d..25345551a 100644 --- a/desktop-widgets/simplewidgets.cpp +++ b/desktop-widgets/simplewidgets.cpp @@ -160,15 +160,15 @@ void RenumberDialog::buttonClicked(QAbstractButton *button) { if (ui.buttonBox->buttonRole(button) == QDialogButtonBox::AcceptRole) { MainWindow::instance()->dive_list()->rememberSelection(); - // we remember a map from dive uuid to a pair of old number / new number - QMap> renumberedDives; + // we remember a list from dive uuid to a new number + QVector> renumberedDives; int i; int newNr = ui.spinBox->value(); struct dive *dive = NULL; for_each_dive (i, dive) { if (!selectedOnly || dive->selected) { invalidate_dive_cache(dive); - renumberedDives.insert(dive->id, QPair(dive->number, newNr++)); + renumberedDives.append(QPair(dive->id, newNr++)); } } UndoRenumberDives *undoCommand = new UndoRenumberDives(renumberedDives); @@ -241,7 +241,7 @@ void ShiftTimesDialog::buttonClicked(QAbstractButton *button) // DANGER, DANGER - this could get our dive_table unsorted... int i; struct dive *dive; - QList affectedDives; + QVector affectedDives; for_each_dive (i, dive) { if (!dive->selected) continue; @@ -249,8 +249,6 @@ void ShiftTimesDialog::buttonClicked(QAbstractButton *button) affectedDives.append(dive->id); } MainWindow::instance()->undoStack->push(new UndoShiftTime(affectedDives, amount)); - sort_table(&dive_table); - mark_divelist_changed(true); MainWindow::instance()->dive_list()->rememberSelection(); MainWindow::instance()->refreshDisplay(); MainWindow::instance()->dive_list()->restoreSelection(); -- cgit v1.2.3-70-g09d2