From 7067e335964de97d51381ca9e9b09f7236ffc619 Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Fri, 3 Aug 2018 11:35:43 +0200 Subject: Undo: select dives after add, remove, merge, split dive commands Select the proper dives after the add, remove, split and merge dives commands on undo *and* redo. Generally, select the added dives. For undo of add, remember the pre-addition selection. For redo of remove, select the closest dive to the first removed dive. The biggest part of the commit is the signal-interface between the dive commands and the dive-list model and dive-list view. This is done in two steps: 1) To the DiveTripModel in batches of trips. The dive trip model transforms the dives into indices. 2) To the DiveListView. The DiveListView has to translate the DiveTripModel indexes to actual indexes via its QSortFilterProxy- model. For code-reuse, derive all divelist-changing commands from a new base-class, which has a flag that describes whether the divelist changed. The helper functions which add and remove dives are made members of the base class and set the flag is a selected dive is added or removed. To properly detect when the current dive was deleted it became necessary to turn the current dive from an index to a pointer, because indices are not stable. Unfortunately, in some cases an index was expected and these places now have to transform the dive into an index. These should be converted in due course. Signed-off-by: Berthold Stoeger --- desktop-widgets/command_divelist.cpp | 271 +++++++++++++++++++++++++++++------ 1 file changed, 228 insertions(+), 43 deletions(-) (limited to 'desktop-widgets/command_divelist.cpp') diff --git a/desktop-widgets/command_divelist.cpp b/desktop-widgets/command_divelist.cpp index a75004bea..9cf61eff9 100644 --- a/desktop-widgets/command_divelist.cpp +++ b/desktop-widgets/command_divelist.cpp @@ -4,6 +4,7 @@ #include "desktop-widgets/mainwindow.h" #include "desktop-widgets/divelistview.h" #include "core/divelist.h" +#include "core/display.h" // for amount_selected #include "core/subsurface-qt/DiveListNotifier.h" namespace Command { @@ -42,8 +43,20 @@ void processByTrip(std::vector> &dives, Function // This helper function removes a dive, takes ownership of the dive and adds it to a DiveToAdd structure. // It is crucial that dives are added in reverse order of deletion, so the the indices are correctly // set and that the trips are added before they are used! -static DiveToAdd removeDive(struct dive *d) -{ +DiveToAdd DiveListBase::removeDive(struct dive *d) +{ + // If the dive to be removed is selected, we will inform the frontend + // later via a signal that the dive changed. + if (d->selected) + selectionChanged = true; + + // If the dive was the current dive, reset the current dive. The calling + // command is responsible of finding a new dive. + if (d == current_dive) { + selectionChanged = true; // Should have been set above, as current dive is always selected. + current_dive = nullptr; + } + DiveToAdd res; res.idx = get_divenr(d); if (res.idx < 0) @@ -65,7 +78,7 @@ static DiveToAdd removeDive(struct dive *d) // This helper function adds a dive and returns ownership to the backend. It may also add a dive trip. // It is crucial that dives are added in reverse order of deletion (see comment above)! // Returns pointer to added dive (which is owned by the backend!) -static dive *addDive(DiveToAdd &d) +dive *DiveListBase::addDive(DiveToAdd &d) { if (d.tripToAdd) insert_trip_dont_merge(d.tripToAdd.release()); // Return ownership to backend @@ -74,13 +87,18 @@ static dive *addDive(DiveToAdd &d) dive *res = d.dive.release(); // Give up ownership of dive add_single_dive(d.idx, res); // Return ownership to backend + // If the dive to be removed is selected, we will inform the frontend + // later via a signal that the dive changed. + if (res->selected) + selectionChanged = true; + return res; } // This helper function calls removeDive() on a list of dives to be removed and // returns a vector of corresponding DiveToAdd objects, which can later be readded. // The passed in vector is cleared. -static std::vector removeDives(std::vector &divesToDelete) +std::vector DiveListBase::removeDives(std::vector &divesToDelete) { std::vector res; res.reserve(divesToDelete.size()); @@ -90,7 +108,7 @@ static std::vector removeDives(std::vector &divesToDelete) divesToDelete.clear(); // We send one dives-deleted signal per trip (see comments in DiveListNotifier.h). - // Therefore, collect all dives in a array and sort by trip. + // Therefore, collect all dives in an array and sort by trip. std::vector> dives; dives.reserve(res.size()); for (const DiveToAdd &entry: res) @@ -112,7 +130,7 @@ static std::vector removeDives(std::vector &divesToDelete) // of dives to be (re)added and returns a vector of the added dives. It does this in reverse // order, so that trips are created appropriately and indexing is correct. // The passed in vector is cleared. -static std::vector addDives(std::vector &divesToAdd) +std::vector DiveListBase::addDives(std::vector &divesToAdd) { std::vector res; res.reserve(divesToAdd.size()); @@ -289,6 +307,154 @@ static void moveDivesBetweenTrips(DivesToTrip &dives) std::reverse(dives.divesToMove.begin(), dives.divesToMove.end()); } +// When we initialize the command we don't have to roll-back any selection change +DiveListBase::DiveListBase() : firstExecution(true) +{ +} + +// Turn current selection into a vector. +// TODO: This could be made much more efficient if we kept a sorted list of selected dives! +static std::vector getDiveSelection() +{ + std::vector res; + res.reserve(amount_selected); + + int i; + dive *d; + for_each_dive(i, d) { + if (d->selected) + res.push_back(d); + } + return res; +} + +void DiveListBase::initWork() +{ + selectionChanged = false; +} + +void DiveListBase::finishWork() +{ + if (selectionChanged) // If the selection changed -> tell the frontend + emit diveListNotifier.selectionChanged(); +} + +// Set the current dive either from a list of selected dives, +// or a newly selected dive. In both cases, try to select the +// dive that is newer that is newer than the given date. +// This mimics the old behavior when the current dive changed. +static void setClosestCurrentDive(timestamp_t when, const std::vector &selection) +{ + // Start from back until we get the first dive that is before + // the supposed-to-be selected dive. (Note: this mimics the + // old behavior when the current dive changed). + for (auto it = selection.rbegin(); it < selection.rend(); ++it) { + if ((*it)->when > when && !(*it)->hidden_by_filter) { + current_dive = *it; + return; + } + } + + // We didn't find a more recent selected dive -> try to + // find *any* visible selected dive. + for (dive *d: selection) { + if (!d->hidden_by_filter) { + current_dive = d; + return; + } + } + + // No selected dive is visible! Take the closest dive. Note, this might + // return null, but that just means unsetting the current dive (as no + // dive is visible anyway). + current_dive = find_next_visible_dive(when); +} + +// Rese the selection to the dives of the "selection" vector and send the appropriate signals. +// Set the current dive to "currentDive". "currentDive" must be an element of "selection" (or +// null if "seletion" is empty). +void DiveListBase::restoreSelection(const std::vector &selection, dive *currentDive) +{ + // To do so, generate vectors of dives to be selected and deselected. + // We send signals batched by trip, so keep track of trip/dive pairs. + std::vector> divesToSelect; + std::vector> divesToDeselect; + + // TODO: We might want to keep track of selected dives in a more efficient way! + int i; + dive *d; + amount_selected = 0; // We recalculate amount_selected + for_each_dive(i, d) { + // We only modify dives that are currently visible. + if (d->hidden_by_filter) { + d->selected = false; // Note, not necessary, just to be sure + // that we get amount_selected right + continue; + } + + // Search the dive in the list of selected dives. + // TODO: By sorting the list in the same way as the backend, this could be made more efficient. + bool newState = std::find(selection.begin(), selection.end(), d) != selection.end(); + + // TODO: Instead of using select_dive() and deselect_dive(), we set selected directly. + // The reason is that deselect() automatically sets a new current dive, which we + // don't want, as we set it later anyway. + // There is other parts of the C++ code that touches the innards directly, but + // ultimately this should be pushed down to C. + if (newState && !d->selected) { + d->selected = true; + ++amount_selected; + divesToSelect.push_back({ d->divetrip, d }); + } else if (!newState && d->selected) { + d->selected = false; + divesToDeselect.push_back({ d->divetrip, d }); + } + } + + // Send the select and deselect signals + processByTrip(divesToSelect, [&](dive_trip *trip, const QVector &divesInTrip) { + emit diveListNotifier.divesSelected(trip, divesInTrip); + }); + processByTrip(divesToDeselect, [&](dive_trip *trip, const QVector &divesInTrip) { + emit diveListNotifier.divesDeselected(trip, divesInTrip); + }); + + bool currentDiveChanged = false; + if (current_dive != currentDive) { + currentDiveChanged = true; + + // We cannot simply change the currentd dive to the given dive. + // It might be hidden by a filter and thus not be selected. + if (currentDive->selected) + // Current dive is visible and selected. Excellent. + current_dive = currentDive; + else + // Current not visible -> find a different dive. + setClosestCurrentDive(currentDive->when, selection); + emit diveListNotifier.currentDiveChanged(); + } + + // If anything changed (selection or current dive), send a final signal. + if (!divesToSelect.empty() || !divesToDeselect.empty() || currentDiveChanged) + selectionChanged = true; +} + +void DiveListBase::undo() +{ + auto marker = diveListNotifier.enterCommand(); + initWork(); + undoit(); + finishWork(); +} + +void DiveListBase::redo() +{ + auto marker = diveListNotifier.enterCommand(); + initWork(); + redoit(); + finishWork(); +} + AddDive::AddDive(dive *d, bool autogroup) { setText(tr("add dive")); @@ -321,30 +487,31 @@ bool AddDive::workToBeDone() return true; } -void AddDive::redo() +void AddDive::redoit() { - auto marker = diveListNotifier.enterCommand(); + // Remember selection so that we can undo it + selection = getDiveSelection(); + currentDive = current_dive; - int idx = divesToAdd[0].idx; divesToRemove = addDives(divesToAdd); mark_divelist_changed(true); - // Finally, do the UI stuff: - MainWindow::instance()->dive_list()->unselectDives(); - MainWindow::instance()->dive_list()->selectDive(idx, true); + // Select the newly added dive + restoreSelection(divesToRemove, divesToRemove[0]); // Exit from edit mode, but don't recalculate dive list // TODO: Remove edit mode MainWindow::instance()->refreshDisplay(false); } -void AddDive::undo() +void AddDive::undoit() { - auto marker = diveListNotifier.enterCommand(); - - // Simply remove the dive that was previously added + // Simply remove the dive that was previously added... divesToAdd = removeDives(divesToRemove); + // ...and restore the selection + restoreSelection(selection, currentDive); + // Exit from edit mode, but don't recalculate dive list // TODO: Remove edit mode MainWindow::instance()->refreshDisplay(false); @@ -360,18 +527,31 @@ bool DeleteDive::workToBeDone() return !divesToDelete.empty(); } -void DeleteDive::undo() +void DeleteDive::undoit() { - auto marker = diveListNotifier.enterCommand(); divesToDelete = addDives(divesToAdd); mark_divelist_changed(true); + + // Select all re-added dives and make the first one current + dive *currentDive = !divesToDelete.empty() ? divesToDelete[0] : nullptr; + restoreSelection(divesToDelete, currentDive); } -void DeleteDive::redo() +void DeleteDive::redoit() { - auto marker = diveListNotifier.enterCommand(); divesToAdd = removeDives(divesToDelete); mark_divelist_changed(true); + + // Deselect all dives and select dive that was close to the first deleted dive + dive *newCurrent = nullptr; + if (!divesToAdd.empty()) { + timestamp_t when = divesToAdd[0].dive->when; + newCurrent = find_next_visible_dive(when); + } + if (newCurrent) + restoreSelection(std::vector{ newCurrent }, newCurrent); + else + restoreSelection(std::vector(), nullptr); } @@ -381,9 +561,8 @@ ShiftTime::ShiftTime(const QVector &changedDives, int amount) setText(tr("shift time of %n dives", "", changedDives.count())); } -void ShiftTime::redo() +void ShiftTime::redoit() { - auto marker = diveListNotifier.enterCommand(); for (dive *d: diveList) d->when -= timeChanged; @@ -413,10 +592,10 @@ bool ShiftTime::workToBeDone() return !diveList.isEmpty(); } -void ShiftTime::undo() +void ShiftTime::undoit() { - // Same as redo(), since after redo() we reversed the timeOffset - redo(); + // Same as redoit(), since after redoit() we reversed the timeOffset + redoit(); } @@ -425,9 +604,8 @@ RenumberDives::RenumberDives(const QVector> &divesToRenumberI setText(tr("renumber %n dive(s)", "", divesToRenumber.count())); } -void RenumberDives::undo() +void RenumberDives::undoit() { - auto marker = diveListNotifier.enterCommand(); renumberDives(divesToRenumber); mark_divelist_changed(true); } @@ -437,10 +615,10 @@ bool RenumberDives::workToBeDone() return !divesToRenumber.isEmpty(); } -void RenumberDives::redo() +void RenumberDives::redoit() { // Redo and undo do the same thing! - undo(); + undoit(); } bool TripBase::workToBeDone() @@ -448,18 +626,17 @@ bool TripBase::workToBeDone() return !divesToMove.divesToMove.empty(); } -void TripBase::redo() +void TripBase::redoit() { - auto marker = diveListNotifier.enterCommand(); moveDivesBetweenTrips(divesToMove); mark_divelist_changed(true); } -void TripBase::undo() +void TripBase::undoit() { // Redo and undo do the same thing! - redo(); + redoit(); } RemoveDivesFromTrip::RemoveDivesFromTrip(const QVector &divesToRemove) @@ -540,7 +717,7 @@ SplitDives::SplitDives(dive *d, duration_t time) split_dive_dont_insert(d, &new1, &new2) : split_dive_at_time_dont_insert(d, time, &new1, &new2); - // If this didn't work, reset pointers so that redo() and undo() do nothing + // If this didn't work, reset pointers as a mark that nothing is to be done. if (idx < 0) { diveToSplit = nullptr; divesToUnsplit[0] = divesToUnsplit[1]; @@ -561,23 +738,27 @@ bool SplitDives::workToBeDone() return !!diveToSplit; } -void SplitDives::redo() +void SplitDives::redoit() { - auto marker = diveListNotifier.enterCommand(); divesToUnsplit[0] = addDive(splitDives[0]); divesToUnsplit[1] = addDive(splitDives[1]); unsplitDive = removeDive(diveToSplit); mark_divelist_changed(true); + + // Select split dives and make first dive current + restoreSelection(std::vector{ divesToUnsplit[0], divesToUnsplit[1] }, divesToUnsplit[0]); } -void SplitDives::undo() +void SplitDives::undoit() { - // Note: reverse order with respect to redo() - auto marker = diveListNotifier.enterCommand(); + // Note: reverse order with respect to redoit() diveToSplit = addDive(unsplitDive); splitDives[1] = removeDive(divesToUnsplit[1]); splitDives[0] = removeDive(divesToUnsplit[0]); mark_divelist_changed(true); + + // Select unsplit dive and make it current + restoreSelection(std::vector{ diveToSplit }, diveToSplit); } MergeDives::MergeDives(const QVector &dives) @@ -664,20 +845,24 @@ bool MergeDives::workToBeDone() return !!mergedDive.dive; } -void MergeDives::redo() +void MergeDives::redoit() { - auto marker = diveListNotifier.enterCommand(); renumberDives(divesToRenumber); diveToUnmerge = addDive(mergedDive); unmergedDives = removeDives(divesToMerge); + + // Select merged dive and make it current + restoreSelection(std::vector{ diveToUnmerge }, diveToUnmerge); } -void MergeDives::undo() +void MergeDives::undoit() { - auto marker = diveListNotifier.enterCommand(); divesToMerge = addDives(unmergedDives); mergedDive = removeDive(diveToUnmerge); renumberDives(divesToRenumber); + + // Select unmerged dives and make first one current + restoreSelection(divesToMerge, divesToMerge[0]); } } // namespace Command -- cgit v1.2.3-70-g09d2