Cleanup: unify selection signals

For historic reasons, there where three distinct signals concerning
dive-selection from the undo-machinery:
1) divesSelected: sent newly selected dives
2) currentDiveChanged: sent if the current dive changed
3) selectionChanged: sent at the end of a command if either the selection
   or the current dive changed

Since now the undo-commands do a full reset of the selection, merge these
three signals into a single signal.

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

7 files changed, 59 insertions, 93 deletions

diff --git a/core/subsurface-qt/DiveListNotifier.h b/core/subsurface-qt/DiveListNotifier.h
index 7f97cbf8c..64ef71481 100644
--- a/core/subsurface-qt/DiveListNotifier.h
+++ b/core/subsurface-qt/DiveListNotifier.h
@@ -52,15 +52,7 @@ signals:
 	// Trip edited signal
 	void tripChanged(dive_trip *trip, TripField field);
 
-	// Selection-signals come in two kinds:
-	//  - divesSelected and currentDiveChanged are are used by the dive-list
-	//    model and view to correctly highlight the correct dives.
-	//  - selectionChanged() is called once at the end of commands if either the selection
-	//    or the current dive changed. It is used by the main-window / profile to update
-	//    their data.
-	void divesSelected(const QVector<dive *> &dives);
-	void currentDiveChanged();
-	void selectionChanged();
+	void divesSelected(const QVector<dive *> &dives, dive *current);
 
 	// Dive site signals. Add and delete events are sent per dive site and
 	// provide an index into the global dive site table.
diff --git a/desktop-widgets/command_private.cpp b/desktop-widgets/command_private.cpp
index 0dfd5eea4..714ffd9b6 100644
--- a/desktop-widgets/command_private.cpp
+++ b/desktop-widgets/command_private.cpp
@@ -53,7 +53,6 @@ void setSelection(const std::vector<dive *> &selection, dive *currentDive)
 	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) {
@@ -75,36 +74,19 @@ void setSelection(const std::vector<dive *> &selection, dive *currentDive)
 		// 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.
-		selectionChanged |= d->selected != newState;
 		d->selected = newState;
 	}
 
-	// Send the select and deselect signals
-	emit diveListNotifier.divesSelected(divesToSelect);
-
-	bool currentDiveChanged = false;
-	if (!currentDive) {
-		// If currentDive is null, we have no current dive. In such a case always
-		// notify the frontend.
-		currentDiveChanged = true;
-		emit diveListNotifier.currentDiveChanged();
-	} else 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();
+	// We cannot simply change the current dive to the given dive.
+	// It might be hidden by a filter and thus not be selected.
+	current_dive = currentDive;
+	if (current_dive && !currentDive->selected) {
+		// Current not visible -> find a different dive.
+		setClosestCurrentDive(currentDive->when, selection);
 	}
 
-	// If the selection changed -> tell the frontend
-	if (selectionChanged || currentDiveChanged)
-		emit diveListNotifier.selectionChanged();
+	// Send the new selection
+	emit diveListNotifier.divesSelected(divesToSelect, current_dive);
 }
 
 // Turn current selection into a vector.
diff --git a/desktop-widgets/divelistview.cpp b/desktop-widgets/divelistview.cpp
index c010c0010..09c69c2a8 100644
--- a/desktop-widgets/divelistview.cpp
+++ b/desktop-widgets/divelistview.cpp
@@ -448,7 +448,7 @@ void DiveListView::selectDives(const QList<int> &newDiveSelection)
 		scrollTo(idx);
 	}
 	// now that everything is up to date, update the widgets
-	emit diveListNotifier.selectionChanged();
+	emit divesSelected();
 	dontEmitDiveChangedSignal = false;
 	return;
 }
@@ -663,7 +663,7 @@ void DiveListView::selectionChanged(const QItemSelection &selected, const QItemS
 		}
 	}
 	if (!dontEmitDiveChangedSignal)
-		emit diveListNotifier.selectionChanged();
+		emit divesSelected();
 
 	// Display the new, processed, selection
 	QTreeView::selectionChanged(selectionModel()->selection(), newDeselected);
@@ -1063,7 +1063,7 @@ void DiveListView::filterFinished()
 	// If there are no more selected dives, select the first visible dive
 	if (!selectionModel()->hasSelection())
 		selectFirstDive();
-	emit diveListNotifier.selectionChanged();
+	emit divesSelected();
 }
 
 QString DiveListView::lastUsedImageDir()
diff --git a/desktop-widgets/divelistview.h b/desktop-widgets/divelistview.h
index abf9ce8a2..0d5117ca3 100644
--- a/desktop-widgets/divelistview.h
+++ b/desktop-widgets/divelistview.h
@@ -42,6 +42,8 @@ public:
 	QList<dive_trip *> selectedTrips();
 	static QString lastUsedImageDir();
 	static void updateLastUsedImageDir(const QString &s);
+signals:
+	void divesSelected();
 public
 slots:
 	void toggleColumnVisibilityByIndex();
diff --git a/desktop-widgets/mainwindow.cpp b/desktop-widgets/mainwindow.cpp
index 36cabb8cb..8dc0f94b5 100644
--- a/desktop-widgets/mainwindow.cpp
+++ b/desktop-widgets/mainwindow.cpp
@@ -191,7 +191,8 @@ MainWindow::MainWindow() : QMainWindow(),
 	if (!QIcon::hasThemeIcon("window-close")) {
 		QIcon::setThemeName("subsurface");
 	}
-	connect(&diveListNotifier, &DiveListNotifier::selectionChanged, this, &MainWindow::selectionChanged);
+	connect(&diveListNotifier, &DiveListNotifier::divesSelected, this, &MainWindow::selectionChanged);
+	connect(diveList, &DiveListView::divesSelected, this, &MainWindow::selectionChanged);
 	connect(PreferencesDialog::instance(), SIGNAL(settingsChanged()), this, SLOT(readSettings()));
 	connect(PreferencesDialog::instance(), SIGNAL(settingsChanged()), diveList, SLOT(update()));
 	connect(PreferencesDialog::instance(), SIGNAL(settingsChanged()), diveList, SLOT(reloadHeaderActions()));
diff --git a/qt-models/divetripmodel.cpp b/qt-models/divetripmodel.cpp
index a0aa1f5ba..3956ab6e2 100644
--- a/qt-models/divetripmodel.cpp
+++ b/qt-models/divetripmodel.cpp
@@ -551,7 +551,6 @@ DiveTripModelTree::DiveTripModelTree(QObject *parent) : DiveTripModelBase(parent
 	connect(&diveListNotifier, &DiveListNotifier::divesMovedBetweenTrips, this, &DiveTripModelTree::divesMovedBetweenTrips);
 	connect(&diveListNotifier, &DiveListNotifier::divesTimeChanged, this, &DiveTripModelTree::divesTimeChanged);
 	connect(&diveListNotifier, &DiveListNotifier::divesSelected, this, &DiveTripModelTree::divesSelected);
-	connect(&diveListNotifier, &DiveListNotifier::currentDiveChanged, this, &DiveTripModelTree::currentDiveChanged);
 	connect(&diveListNotifier, &DiveListNotifier::tripChanged, this, &DiveTripModelTree::tripChanged);
 
 	// Fill model
@@ -1051,7 +1050,7 @@ void DiveTripModelTree::divesTimeChangedTrip(dive_trip *trip, timestamp_t delta,
 	divesAdded(trip, false, dives);
 }
 
-void DiveTripModelTree::divesSelected(const QVector<dive *> &dives)
+void DiveTripModelTree::divesSelected(const QVector<dive *> &dives, dive *current)
 {
 	// We got a number of dives that have been selected. Turn this into QModelIndexes and
 	// emit a signal, so that views can change the selection.
@@ -1062,6 +1061,41 @@ void DiveTripModelTree::divesSelected(const QVector<dive *> &dives)
 		      { divesSelectedTrip(trip, divesInTrip, indexes); });
 
 	emit selectionChanged(indexes);
+
+	// The current dive has changed. Transform the current dive into an index and pass it on to the view.
+	if (!current) {
+		emit newCurrentDive(QModelIndex()); // No current dive -> tell view to clear current index with an invalid index
+		return;
+	}
+
+	dive_trip *trip = current->divetrip;
+	if (!trip) {
+		// Outside of a trip - search top-level.
+		int idx = findDiveIdx(current);
+		if (idx < 0) {
+			// We don't know this dive. Something is wrong. Warn and bail.
+			qWarning() << "DiveTripModelTree::diveSelected(): unknown top-level dive";
+			emit newCurrentDive(QModelIndex());
+			return;
+		}
+		emit newCurrentDive(createIndex(idx, 0, noParent));
+	} else {
+		int idx = findTripIdx(trip);
+		if (idx < 0) {
+			// We don't know the trip - this shouldn't happen. Warn and bail.
+			qWarning() << "DiveTripModelTree::diveSelected(): unknown trip";
+			emit newCurrentDive(QModelIndex());
+			return;
+		}
+		int diveIdx = findDiveInTrip(idx, current);
+		if (diveIdx < 0) {
+			// We don't know this dive. Something is wrong. Warn and bail.
+			qWarning() << "DiveTripModelTree::diveSelected(): unknown dive";
+			emit newCurrentDive(QModelIndex());
+			return;
+		}
+		emit newCurrentDive(createIndex(diveIdx, 0, idx));
+	}
 }
 
 void DiveTripModelTree::divesSelectedTrip(dive_trip *trip, const QVector<dive *> &dives, QVector<QModelIndex> &indexes)
@@ -1102,44 +1136,6 @@ void DiveTripModelTree::divesSelectedTrip(dive_trip *trip, const QVector<dive *>
 	}
 }
 
-void DiveTripModelTree::currentDiveChanged()
-{
-	// The current dive has changed. Transform the current dive into an index and pass it on to the view.
-	if (!current_dive) {
-		emit newCurrentDive(QModelIndex()); // No current dive -> tell view to clear current index with an invalid index
-		return;
-	}
-
-	dive_trip *trip = current_dive->divetrip;
-	if (!trip) {
-		// Outside of a trip - search top-level.
-		int idx = findDiveIdx(current_dive);
-		if (idx < 0) {
-			// We don't know this dive. Something is wrong. Warn and bail.
-			qWarning() << "DiveTripModelTree::currentDiveChanged(): unknown top-level dive";
-			emit newCurrentDive(QModelIndex());
-			return;
-		}
-		emit newCurrentDive(createIndex(idx, 0, noParent));
-	} else {
-		int idx = findTripIdx(trip);
-		if (idx < 0) {
-			// We don't know the trip - this shouldn't happen. Warn and bail.
-			qWarning() << "DiveTripModelTree::currentDiveChanged(): unknown trip";
-			emit newCurrentDive(QModelIndex());
-			return;
-		}
-		int diveIdx = findDiveInTrip(idx, current_dive);
-		if (diveIdx < 0) {
-			// We don't know this dive. Something is wrong. Warn and bail.
-			qWarning() << "DiveTripModelTree::currentDiveChanged(): unknown dive";
-			emit newCurrentDive(QModelIndex());
-			return;
-		}
-		emit newCurrentDive(createIndex(diveIdx, 0, idx));
-	}
-}
-
 void DiveTripModelTree::filterFinished()
 {
 	// If the filter finished, update all trip items to show the correct number of displayed dives
@@ -1169,7 +1165,6 @@ DiveTripModelList::DiveTripModelList(QObject *parent) : DiveTripModelBase(parent
 	//connect(&diveListNotifier, &DiveListNotifier::divesMovedBetweenTrips, this, &DiveTripModelList::divesMovedBetweenTrips);
 	connect(&diveListNotifier, &DiveListNotifier::divesTimeChanged, this, &DiveTripModelList::divesTimeChanged);
 	connect(&diveListNotifier, &DiveListNotifier::divesSelected, this, &DiveTripModelList::divesSelected);
-	connect(&diveListNotifier, &DiveListNotifier::currentDiveChanged, this, &DiveTripModelList::currentDiveChanged);
 
 	// Fill model
 	items.reserve(dive_table.nr);
@@ -1285,10 +1280,9 @@ void DiveTripModelList::divesTimeChanged(timestamp_t delta, const QVector<dive *
 	QVector<dive *> selectedDives = filterSelectedDives(dives);
 	divesDeleted(nullptr, false, dives);
 	divesAdded(nullptr, false, dives);
-	divesSelected(selectedDives);
 }
 
-void DiveTripModelList::divesSelected(const QVector<dive *> &dives)
+void DiveTripModelList::divesSelected(const QVector<dive *> &dives, dive *current)
 {
 	// We got a number of dives that have been selected. Turn this into QModelIndexes and
 	// emit a signal, so that views can change the selection.
@@ -1307,20 +1301,17 @@ void DiveTripModelList::divesSelected(const QVector<dive *> &dives)
 	}
 
 	emit selectionChanged(indexes);
-}
 
-void DiveTripModelList::currentDiveChanged()
-{
-	// The current dive has changed. Transform the current dive into an index and pass it on to the view.
-	if (!current_dive) {
+	// Transform the current dive into an index and pass it on to the view.
+	if (!current) {
 		emit newCurrentDive(QModelIndex()); // No current dive -> tell view to clear current index with an invalid index
 		return;
 	}
 
-	auto it = std::find(items.begin(), items.end(), current_dive);
+	auto it = std::find(items.begin(), items.end(), current);
 	if (it == items.end()) {
 		// We don't know this dive. Something is wrong. Warn and bail.
-		qWarning() << "DiveTripModelList::currentDiveChanged(): unknown top-level dive";
+		qWarning() << "DiveTripModelList::divesSelected(): unknown dive";
 		emit newCurrentDive(QModelIndex());
 		return;
 	}
diff --git a/qt-models/divetripmodel.h b/qt-models/divetripmodel.h
index 734612ff7..8ddcf9615 100644
--- a/qt-models/divetripmodel.h
+++ b/qt-models/divetripmodel.h
@@ -108,8 +108,7 @@ public slots:
 	void divesMovedBetweenTrips(dive_trip *from, dive_trip *to, bool deleteFrom, bool createTo, const QVector<dive *> &dives);
 	void divesChanged(const QVector<dive *> &dives);
 	void divesTimeChanged(timestamp_t delta, const QVector<dive *> &dives);
-	void divesSelected(const QVector<dive *> &dives);
-	void currentDiveChanged();
+	void divesSelected(const QVector<dive *> &dives, dive *current);
 	void tripChanged(dive_trip *trip, TripField);
 
 public:
@@ -175,8 +174,7 @@ public slots:
 	void divesTimeChanged(timestamp_t delta, const QVector<dive *> &dives);
 	// Does nothing in list view.
 	//void divesMovedBetweenTrips(dive_trip *from, dive_trip *to, bool deleteFrom, bool createTo, const QVector<dive *> &dives);
-	void divesSelected(const QVector<dive *> &dives);
-	void currentDiveChanged();
+	void divesSelected(const QVector<dive *> &dives, dive *current);
 
 public:
 	DiveTripModelList(QObject *parent = nullptr);