Cleanup: remove selectionChanged logic from dive-list commands

Since all commands now fully reset the selection, there is no point
in keeping track of whether the selection changed on addition or
removal of dives. This can be done in the function that sets the
selection.

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

desktop-widgets/command_divelist.cpp

@@ -21,17 +21,10 @@ namespace Command {
 // set and that the trips are added before they are used!
 DiveToAdd DiveListBase::removeDive(struct dive *d, std::vector<OwningTripPtr> &tripsToAdd)
 {
-	// 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) {
+	if (d == current_dive)
-		selectionChanged = true; // Should have been set above, as current dive is always selected.
 		current_dive = nullptr;
-	}
 
 	// remove dive from trip and site - if this is the last dive in the trip
 	// remove the whole trip.
@@ -81,11 +74,6 @@ dive *DiveListBase::addDive(DiveToAdd &d)
 	add_to_dive_table(&dive_table, idx, res);	// Return ownership to backend
 	invalidate_dive_cache(res);		// Ensure that dive is written in git_save()
 
-	// 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;
 }
 
@@ -363,26 +351,14 @@ static void moveDivesBetweenTrips(DivesToTrip &dives)
 
 void DiveListBase::initWork()
 {
-	selectionChanged = false;
 }
 
 void DiveListBase::finishWork()
 {
-	if (selectionChanged) // If the selection changed -> tell the frontend
-		emit diveListNotifier.selectionChanged();
 	for (dive_site *ds: sitesCountChanged)
 		emit diveListNotifier.diveSiteDiveCountChanged(ds);
 }
 
-// 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<dive *> &selection, dive *currentDive)
-{
-	// If anything changed (selection or current dive), send a final signal.
-	selectionChanged |= setSelection(selection, currentDive);
-}
-
 void DiveListBase::undo()
 {
 	auto marker = diveListNotifier.enterCommand();
@@ -452,7 +428,7 @@ void AddDive::redoit()
 	sort_trip_table(&trip_table); // Though unlikely, adding a dive may reorder trips
 
 	// Select the newly added dive
-	restoreSelection(divesAndSitesToRemove.dives, divesAndSitesToRemove.dives[0]);
+	setSelection(divesAndSitesToRemove.dives, divesAndSitesToRemove.dives[0]);
 
 	// Exit from edit mode, but don't recalculate dive list
 	// TODO: Remove edit mode
@@ -466,7 +442,7 @@ void AddDive::undoit()
 	sort_trip_table(&trip_table); // Though unlikely, removing a dive may reorder trips
 
 	// ...and restore the selection
-	restoreSelection(selection, currentDive);
+	setSelection(selection, currentDive);
 
 	// Exit from edit mode, but don't recalculate dive list
 	// TODO: Remove edit mode
@@ -529,7 +505,7 @@ void ImportDives::redoit()
 	divesToAdd = removeDives(divesAndSitesToRemove);
 
 	// Select the newly added dives
-	restoreSelection(divesAndSitesToRemoveNew.dives, divesAndSitesToRemoveNew.dives.back());
+	setSelection(divesAndSitesToRemoveNew.dives, divesAndSitesToRemoveNew.dives.back());
 
 	// Remember dives and sites to remove
 	divesAndSitesToRemove = std::move(divesAndSitesToRemoveNew);
@@ -547,7 +523,7 @@ void ImportDives::undoit()
 	divesAndSitesToRemove = std::move(divesAndSitesToRemoveNew);
 
 	// ...and restore the selection
-	restoreSelection(selection, currentDive);
+	setSelection(selection, currentDive);
 }
 
 DeleteDive::DeleteDive(const QVector<struct dive*> &divesToDeleteIn)
@@ -568,7 +544,7 @@ void DeleteDive::undoit()
 
 	// Select all re-added dives and make the first one current
 	dive *currentDive = !divesToDelete.dives.empty() ? divesToDelete.dives[0] : nullptr;
-	restoreSelection(divesToDelete.dives, currentDive);
+	setSelection(divesToDelete.dives, currentDive);
 }
 
 void DeleteDive::redoit()
@@ -583,9 +559,9 @@ void DeleteDive::redoit()
 		newCurrent = find_next_visible_dive(when);
 	}
 	if (newCurrent)
-		restoreSelection(std::vector<dive *>{ newCurrent }, newCurrent);
+		setSelection(std::vector<dive *>{ newCurrent }, newCurrent);
 	else
-		restoreSelection(std::vector<dive *>(), nullptr);
+		setSelection(std::vector<dive *>(), nullptr);
 }
 
 
@@ -615,7 +591,7 @@ void ShiftTime::redoit()
 	emit diveListNotifier.divesChanged(diveList, DiveField::DATETIME);
 
 	// Select the changed dives
-	restoreSelection(diveList.toStdVector(), diveList[0]);
+	setSelection(diveList.toStdVector(), diveList[0]);
 
 	// Negate the time-shift so that the next call does the reverse
 	timeChanged = -timeChanged;
@@ -647,7 +623,7 @@ void RenumberDives::undoit()
 	dives.reserve(divesToRenumber.size());
 	for (const QPair<dive *, int> &item: divesToRenumber)
 		dives.push_back(item.first);
-	restoreSelection(dives, dives[0]);
+	setSelection(dives, dives[0]);
 }
 
 bool RenumberDives::workToBeDone()
@@ -676,7 +652,7 @@ void TripBase::redoit()
 	dives.reserve(divesToMove.divesToMove.size());
 	for (const DiveToTrip &item: divesToMove.divesToMove)
 		dives.push_back(item.dive);
-	restoreSelection(dives, dives[0]);
+	setSelection(dives, dives[0]);
 }
 
 void TripBase::undoit()
@@ -799,7 +775,7 @@ void SplitDivesBase::redoit()
 	unsplitDive = removeDives(diveToSplit);
 
 	// Select split dives and make first dive current
-	restoreSelection(divesToUnsplit.dives, divesToUnsplit.dives[0]);
+	setSelection(divesToUnsplit.dives, divesToUnsplit.dives[0]);
 }
 
 void SplitDivesBase::undoit()
@@ -809,7 +785,7 @@ void SplitDivesBase::undoit()
 	splitDives = removeDives(divesToUnsplit);
 
 	// Select unsplit dive and make it current
-	restoreSelection(diveToSplit.dives, diveToSplit.dives[0] );
+	setSelection(diveToSplit.dives, diveToSplit.dives[0] );
 }
 
 static std::array<dive *, 2> doSplitDives(const dive *d, duration_t time)
@@ -883,7 +859,7 @@ void DiveComputerBase::redoit()
 	diveToRemove = std::move(addedDive);
 
 	// Select added dive and make it current
-	restoreSelection(diveToRemove.dives, diveToRemove.dives[0]);
+	setSelection(diveToRemove.dives, diveToRemove.dives[0]);
 
 	// Update the profile to show the first dive computer
 	dc_number = dc_nr_after;
@@ -1006,7 +982,7 @@ void MergeDives::redoit()
 	unmergedDives = removeDives(divesToMerge);
 
 	// Select merged dive and make it current
-	restoreSelection(diveToUnmerge.dives, diveToUnmerge.dives[0]);
+	setSelection(diveToUnmerge.dives, diveToUnmerge.dives[0]);
 }
 
 void MergeDives::undoit()
@@ -1016,7 +992,7 @@ void MergeDives::undoit()
 	renumberDives(divesToRenumber);
 
 	// Select unmerged dives and make first one current
-	restoreSelection(divesToMerge.dives, divesToMerge.dives[0]);
+	setSelection(divesToMerge.dives, divesToMerge.dives[0]);
 }
 
 } // namespace Command

desktop-widgets/command_divelist.h

@@ -49,17 +49,13 @@ struct DivesToTrip
 	std::vector<OwningTripPtr> tripsToAdd;
 };
 
-// All divelist commands derive from a common base class, which has a flag
+// All divelist commands derive from a common base class. It keeps track
-// for when then selection changed. In such a case, in the redo() and undo()
+// of dive site counts that may have changed.
-// methods a signal will be sent. The base-class implements redo() and undo(),
+// Derived classes implement the virtual methods redoit() and undoit()
-// which resets the flag and sends a signal. Derived classes implement the
+// [Yes, the names could be more expressive].
-// virtual methods redoit() and undoit() [Yes, the names could be more expressive].
-// Moreover, the class implements helper methods, which set the selectionChanged
-// flag accordingly.
 class DiveListBase : public Base {
 protected:
-	// These are helper functions to add / remove dive from the C-core structures,
+	// These are helper functions to add / remove dive from the C-core structures.
-	// which set the selectionChanged flag if the added / removed dive was selected.
 	DiveToAdd removeDive(struct dive *d, std::vector<OwningTripPtr> &tripsToAdd);
 	dive *addDive(DiveToAdd &d);
 	DivesAndTripsToAdd removeDives(DivesAndSitesToRemove &divesAndSitesToDelete);
@@ -68,20 +64,11 @@ protected:
 	// Register dive sites where counts changed so that we can signal the frontend later.
 	void diveSiteCountChanged(struct dive_site *ds);
 
-	// Set the selection to a given state. Set the selectionChanged flag if anything changed.
-	void restoreSelection(const std::vector<dive *> &selection, dive *currentDive);
-
-	// Commands set this flag if the selection changed on first execution.
-	// Only then, a new the divelist will be scanned again after the command.
-	// If this flag is set on first execution, a selectionChanged signal will
-	// be sent.
-	bool selectionChanged;
-
 private:
 	// Keep track of dive sites where the number of dives changed
 	std::vector<dive_site *> sitesCountChanged;
-	void initWork(); // reset selectionChanged flag
+	void initWork();
-	void finishWork(); // emit signals if selection or dive site counts changed
+	void finishWork(); // update dive site counts
 	void undo() override;
 	void redo() override;
 	virtual void redoit() = 0;

desktop-widgets/command_edit.cpp

@@ -99,8 +99,7 @@ void EditBase<T>::undo()
 	DiveField id = fieldId();
 	emit diveListNotifier.divesChanged(QVector<dive *>::fromStdVector(dives), id);
 
-	if (setSelection(selectedDives, current))
+	setSelection(selectedDives, current);
-		emit diveListNotifier.selectionChanged(); // If the selection changed -> tell the frontend
 }
 
 // We have to manually instantiate the constructors of the EditBase class,
@@ -539,8 +538,7 @@ void EditTagsBase::undo()
 	DiveField id = fieldId();
 	emit diveListNotifier.divesChanged(QVector<dive *>::fromStdVector(dives), id);
 
-	if (setSelection(selectedDives, current))
+	setSelection(selectedDives, current);
-		emit diveListNotifier.selectionChanged(); // If the selection changed -> tell the frontend
 }
 
 // Undo and redo do the same as just the stored value is exchanged

desktop-widgets/command_private.cpp

@@ -42,7 +42,7 @@ static void setClosestCurrentDive(timestamp_t when, const std::vector<dive *> &s
 // Reset 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). Return true if the selection or current dive changed.
-bool setSelection(const std::vector<dive *> &selection, dive *currentDive)
+void 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.
@@ -102,8 +102,9 @@ bool setSelection(const std::vector<dive *> &selection, dive *currentDive)
 		emit diveListNotifier.currentDiveChanged();
 	}
 
-	// return true if selection of current dive changed
+	// If the selection changed -> tell the frontend
-	return selectionChanged || currentDiveChanged;
+	if (selectionChanged || currentDiveChanged)
+		emit diveListNotifier.selectionChanged();
 }
 
 // Turn current selection into a vector.

desktop-widgets/command_private.h

@@ -15,7 +15,7 @@ namespace Command {
 // Reset 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). Return true if the selection or current dive changed.
-bool setSelection(const std::vector<dive *> &selection, dive *currentDive);
+void setSelection(const std::vector<dive *> &selection, dive *currentDive);
 
 // Get currently selectd dives
 std::vector<dive *> getDiveSelection();