Undo: unify selection behavior in dive-list commands

Some commands tried to retain the current selection on undo/redo,
others set the selection to the modified dives.

The latter was introduced because it was easier in some cases, but
it is probably more user-friendly because the user gets feedback
on the change.

Therefore, unify to always select the affected dives on undo()/redo().

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

4 files changed, 31 insertions, 18 deletions

diff --git a/desktop-widgets/command_divelist.cpp b/desktop-widgets/command_divelist.cpp
index 4c699fdd6..26708e07d 100644
--- a/desktop-widgets/command_divelist.cpp
+++ b/desktop-widgets/command_divelist.cpp
@@ -614,6 +614,9 @@ void ShiftTime::redoit()
 	emit diveListNotifier.divesTimeChanged(timeChanged, diveList);
 	emit diveListNotifier.divesChanged(diveList, DiveField::DATETIME);
 
+	// Select the changed dives
+	restoreSelection(diveList.toStdVector(), diveList[0]);
+
 	// Negate the time-shift so that the next call does the reverse
 	timeChanged = -timeChanged;
 }
@@ -638,6 +641,13 @@ RenumberDives::RenumberDives(const QVector<QPair<dive *, int>> &divesToRenumberI
 void RenumberDives::undoit()
 {
 	renumberDives(divesToRenumber);
+
+	// Select the changed dives
+	std::vector<dive *> dives;
+	dives.reserve(divesToRenumber.size());
+	for (const QPair<dive *, int> &item: divesToRenumber)
+		dives.push_back(item.first);
+	restoreSelection(dives, dives[0]);
 }
 
 bool RenumberDives::workToBeDone()
@@ -660,6 +670,13 @@ void TripBase::redoit()
 {
 	moveDivesBetweenTrips(divesToMove);
 	sort_trip_table(&trip_table); // Though unlikely, moving dives may reorder trips
+
+	// Select the moved dives
+	std::vector<dive *> dives;
+	dives.reserve(divesToMove.divesToMove.size());
+	for (const DiveToTrip &item: divesToMove.divesToMove)
+		dives.push_back(item.dive);
+	restoreSelection(dives, dives[0]);
 }
 
 void TripBase::undoit()
diff --git a/desktop-widgets/command_private.cpp b/desktop-widgets/command_private.cpp
index 7e604dfc9..6dac645b5 100644
--- a/desktop-widgets/command_private.cpp
+++ b/desktop-widgets/command_private.cpp
@@ -47,12 +47,13 @@ bool setSelection(const std::vector<dive *> &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.
 	QVector<dive *> divesToSelect;
-	QVector<dive *> divesToDeselect;
+	divesToSelect.reserve(selection.size());
 
 	// 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
+	bool selectionChanged = false;
 	for_each_dive(i, d) {
 		// We only modify dives that are currently visible.
 		if (d->hidden_by_filter) {
@@ -65,24 +66,21 @@ bool setSelection(const std::vector<dive *> &selection, dive *currentDive)
 		// 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();
 
+		if (newState) {
+			++amount_selected;
+			divesToSelect.push_back(d);
+		}
 		// 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);
-		} else if (!newState && d->selected) {
-			d->selected = false;
-			divesToDeselect.push_back(d);
-		}
+		selectionChanged |= d->selected != newState;
+		d->selected = newState;
 	}
 
 	// Send the select and deselect signals
 	emit diveListNotifier.divesSelected(divesToSelect);
-	emit diveListNotifier.divesDeselected(divesToDeselect);
 
 	bool currentDiveChanged = false;
 	if (!currentDive) {
@@ -105,7 +103,7 @@ bool setSelection(const std::vector<dive *> &selection, dive *currentDive)
 	}
 
 	// return true if selection of current dive changed
-	return !divesToSelect.empty() || !divesToDeselect.empty() || currentDiveChanged;
+	return selectionChanged || currentDiveChanged;
 }
 
 // Turn current selection into a vector.
diff --git a/desktop-widgets/divelistview.cpp b/desktop-widgets/divelistview.cpp
index 055737252..c010c0010 100644
--- a/desktop-widgets/divelistview.cpp
+++ b/desktop-widgets/divelistview.cpp
@@ -192,13 +192,11 @@ void DiveListView::reset()
 }
 
 // If items were selected, inform the selection model
-void DiveListView::diveSelectionChanged(const QVector<QModelIndex> &indexes, bool select)
+void DiveListView::diveSelectionChanged(const QVector<QModelIndex> &indexes)
 {
+	clearSelection();
 	MultiFilterSortModel *m = MultiFilterSortModel::instance();
 	QItemSelectionModel *s = selectionModel();
-	auto flags = select ?
-		QItemSelectionModel::Rows | QItemSelectionModel::Select :
-		QItemSelectionModel::Rows | QItemSelectionModel::Deselect;
 	for (const QModelIndex &index: indexes) {
 		// We have to transform the indices into local indices, since
 		// there might be sorting or filtering in effect.
@@ -210,10 +208,10 @@ void DiveListView::diveSelectionChanged(const QVector<QModelIndex> &indexes, boo
 		if (!localIndex.isValid())
 			continue;
 
-		s->select(localIndex, flags);
+		s->select(localIndex, QItemSelectionModel::Rows | QItemSelectionModel::Select);
 
 		// If an item of a not-yet expanded trip is selected, expand the trip.
-		if (select && localIndex.parent().isValid() && !isExpanded(localIndex.parent())) {
+		if (localIndex.parent().isValid() && !isExpanded(localIndex.parent())) {
 			setAnimated(false);
 			expand(localIndex.parent());
 			setAnimated(true);
diff --git a/desktop-widgets/divelistview.h b/desktop-widgets/divelistview.h
index 7a3757adb..abf9ce8a2 100644
--- a/desktop-widgets/divelistview.h
+++ b/desktop-widgets/divelistview.h
@@ -63,7 +63,7 @@ slots:
 	void shiftTimes();
 	void loadImages();
 	void loadWebImages();
-	void diveSelectionChanged(const QVector<QModelIndex> &indexes, bool select);
+	void diveSelectionChanged(const QVector<QModelIndex> &indexes);
 	void currentDiveChanged(QModelIndex index);
 	void filterFinished();
 	void tripChanged(dive_trip *trip, TripField);