selection: trickle down trip selection

The trip selection code was an awkward layering violation.
Whereas dive selections due to dive undo-commands trickled
down via DiveTripModel-->MultiFilterSortModel-->DiveListView,
for trip editing, the DiveListView directly intercepted the
TripEdited signal.

Instead, mimic the dive-selection code. This is a bit longer
but more consistent and logical. The undo/redo of trip changes
is now also a "programmatical" change of the selection.

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

commands/command_edit_trip.cpp

@@ -7,6 +7,7 @@
 namespace Command {
 
 EditTripBase::EditTripBase(dive_trip *tripIn, const QString &newValue) : trip(tripIn),
+	current(current_dive),
 	value(newValue)
 {
 }
@@ -26,6 +27,7 @@ void EditTripBase::undo()
 	value = old;
 
 	emit diveListNotifier.tripChanged(trip, fieldId());
+	setTripSelection(trip, current);
 }
 
 // Undo and redo do the same as just the stored value is exchanged

commands/command_edit_trip.h

@@ -20,6 +20,7 @@ class EditTripBase : public Base {
 	bool workToBeDone() override;
 
 	dive_trip *trip; // Trip to be edited.
+	dive *current;
 public:
 	EditTripBase(dive_trip *trip, const QString &newValue);
 

core/selection.cpp

@@ -224,6 +224,27 @@ bool setSelectionKeepCurrent(const std::vector<dive *> &selection)
 	return current_dive != oldCurrent;
 }
 
+void setTripSelection(dive_trip *trip, dive *currentDive)
+{
+	if (!trip)
+		return;
+
+	current_dive = currentDive;
+	for (int i = 0; i < divelog.dives->nr; ++i) {
+		dive &d = *divelog.dives->dives[i];
+		d.selected = d.divetrip == trip;
+	}
+	for (int i = 0; i < divelog.trips->nr; ++i) {
+		dive_trip *t = divelog.trips->trips[i];
+		t->selected = t == trip;
+	}
+
+	amount_selected = trip->dives.nr;
+	amount_trips_selected = 1;
+
+	emit diveListNotifier.tripSelected(trip, currentDive);
+}
+
 extern "C" void select_single_dive(dive *d)
 {
 	if (d)
@@ -314,16 +335,3 @@ extern "C" struct dive_trip *single_selected_trip()
 	fprintf(stderr, "warning: found no selected trip even though one should be selected\n");
 	return NULL; // shouldn't happen
 }
-
-extern "C" void clear_selection(void)
-{
-	current_dive = nullptr;
-	amount_selected = 0;
-	amount_trips_selected = 0;
-	int i;
-	struct dive *dive;
-	for_each_dive (i, dive)
-		dive->selected = false;
-	for (int i = 0; i < divelog.trips->nr; ++i)
-		divelog.trips->trips[i]->selected = false;
-}

core/selection.h

@@ -57,6 +57,9 @@ void setSelection(const std::vector<dive *> &selection, dive *currentDive, int c
 // Does not send a signal.
 bool setSelectionKeepCurrent(const std::vector<dive *> &selection);
 
+// Select all dive in a trip and sends a trip-selected signal
+void setTripSelection(dive_trip *trip, dive *currentDive);
+
 // Get currently selected dives
 std::vector<dive *> getDiveSelection();
 bool diveInSelection(const std::vector<dive *> &selection, const dive *d);

core/subsurface-qt/divelistnotifier.h

@@ -112,6 +112,7 @@ signals:
 
 	// Selection changes
 	void divesSelected(const QVector<dive *> &dives, dive *currentDive);
+	void tripSelected(dive_trip *trip, dive *currentDive);
 
 	// Dive site signals. Add and delete events are sent per dive site and
 	// provide an index into the global dive site table.

desktop-widgets/divelistview.cpp

@@ -40,6 +40,7 @@ DiveListView::DiveListView(QWidget *parent) : QTreeView(parent),
 	MultiFilterSortModel *m = MultiFilterSortModel::instance();
 	setModel(m);
 	connect(m, &MultiFilterSortModel::selectionChanged, this, &DiveListView::diveSelectionChanged);
+	connect(m, &MultiFilterSortModel::tripSelected, this, &DiveListView::tripSelected);
 	connect(&diveListNotifier, &DiveListNotifier::settingsChanged, this, &DiveListView::settingsChanged);
 
 	setSortingEnabled(true);
@@ -49,8 +50,6 @@ DiveListView::DiveListView(QWidget *parent) : QTreeView(parent),
 
 	resetModel();
 
-	connect(&diveListNotifier, &DiveListNotifier::tripChanged, this, &DiveListView::tripChanged);
-
 	header()->setStretchLastSection(true);
 	header()->setSortIndicatorShown(true);
 	header()->setSectionsClickable(true);
@@ -286,42 +285,28 @@ void DiveListView::rowsInserted(const QModelIndex &parent, int start, int end)
 	}
 }
 
-// This is a bit ugly: we hook directly into the tripChanged signal to
+void DiveListView::tripSelected(QModelIndex trip, QModelIndex currentDive)
-// select the trip if it was edited. This feels like a layering violation:
-// Shouldn't the core-layer call us?
-void DiveListView::tripChanged(dive_trip *trip, TripField)
 {
-	// First check if the trip is already selected (and only this trip, as only then is it displayed).
+	programmaticalSelectionChange = true;
-	if (single_selected_trip() == trip)
-		return;
 
-	unselectDives();
-	selectTrip(trip);
-	selectionChangeDone();
-}
-
-void DiveListView::selectTrip(dive_trip_t *trip)
-{
-	if (!trip)
-		return;
-
-	QAbstractItemModel *m = model();
-	QModelIndexList match = m->match(m->index(0, 0), DiveTripModelBase::TRIP_ROLE, QVariant::fromValue(trip), 2, Qt::MatchRecursive);
-	QItemSelectionModel::SelectionFlags flags;
-	if (!match.count())
-		return;
-	QModelIndex idx = match.first();
-	flags = QItemSelectionModel::Select;
-	flags |= QItemSelectionModel::Rows;
-	selectionModel()->select(idx, flags);
-	expand(idx);
-}
-
-void DiveListView::unselectDives()
-{
-	clear_selection();
 	// clear the Qt selection
 	selectionModel()->clearSelection();
+
+	if (trip.isValid()) {
+		QItemSelectionModel::SelectionFlags flags = QItemSelectionModel::Select | QItemSelectionModel::Rows;
+		selectionModel()->select(trip, flags);
+		selectTripItems(trip);
+	}
+
+	// Set the currently activated row.
+	// Note, we have to use the QItemSelectionModel::Current mode to avoid
+	// changing our selection (in contrast to Qt's documentation, which
+	// instructs to use QItemSelectionModel::NoUpdate, which results in
+	// funny side-effects).
+	selectionModel()->setCurrentIndex(currentDive, QItemSelectionModel::Current);
+
+	selectionChangeDone();
+	programmaticalSelectionChange = false;
 }
 
 bool DiveListView::eventFilter(QObject *, QEvent *event)
@@ -527,6 +512,18 @@ void DiveListView::selectionChangeDone()
 	emit divesSelected();
 }
 
+void DiveListView::selectTripItems(QModelIndex index)
+{
+	const QAbstractItemModel *model = index.model();
+	if (model->rowCount(index)) {
+		QItemSelection selection;
+		selection.select(model->index(0, 0, index), model->index(model->rowCount(index) - 1, 0, index));
+		selectionModel()->select(selection, QItemSelectionModel::Select | QItemSelectionModel::Rows);
+		if (!isExpanded(index))
+			expand(index);
+	}
+}
+
 void DiveListView::selectionChanged(const QItemSelection &selected, const QItemSelection &deselected)
 {
 	if (programmaticalSelectionChange) {
@@ -570,14 +567,7 @@ void DiveListView::selectionChanged(const QItemSelection &selected, const QItemS
 			select_trip(trip);
 			for (int i = 0; i < trip->dives.nr; ++i)
 				addToSelection.push_back(trip->dives.dives[i]);
-			if (model->rowCount(index)) {
+			selectTripItems(index);
-				QItemSelection selection;
-				selection.select(model->index(0, 0, index), model->index(model->rowCount(index) - 1, 0, index));
-				selectionModel()->select(selection, QItemSelectionModel::Select | QItemSelectionModel::Rows);
-				selectionModel()->setCurrentIndex(index, QItemSelectionModel::Select | QItemSelectionModel::NoUpdate);
-				if (!isExpanded(index))
-					expand(index);
-			}
 		}
 	}
 

desktop-widgets/divelistview.h

@@ -52,17 +52,17 @@ slots:
 	void addDivesToTrip();
 	void shiftTimes();
 	void diveSelectionChanged(const QVector<QModelIndex> &indices, QModelIndex currentDive);
-	void tripChanged(dive_trip *trip, TripField);
+	void tripSelected(QModelIndex trip, QModelIndex currentDive);
 private:
 	void rowsInserted(const QModelIndex &parent, int start, int end) override;
 	void reset() override;
 	void setSelection(const QRect &rect, QItemSelectionModel::SelectionFlags flags) override;
-	void unselectDives();
 	void mouseReleaseEvent(QMouseEvent *event) override;
 	void keyPressEvent(QKeyEvent *event) override;
 	void selectAll() override;
 	void selectionChanged(const QItemSelection &selected, const QItemSelection &deselected) override;
 	void selectionChangeDone();
+	void selectTripItems(QModelIndex index);
 	DiveTripModelBase::Layout currentLayout;
 	QModelIndex contextMenuIndex;
 	// Remember the initial column widths, to avoid writing unchanged widths to the settings

qt-models/divetripmodel.cpp

@@ -683,6 +683,7 @@ 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::tripSelected, this, &DiveTripModelTree::tripSelected);
 	connect(&diveListNotifier, &DiveListNotifier::tripChanged, this, &DiveTripModelTree::tripChanged);
 	connect(&diveListNotifier, &DiveListNotifier::filterReset, this, &DiveTripModelTree::filterReset);
 	connect(&diveListNotifier, &DiveListNotifier::cylinderAdded, this, &DiveTripModelTree::diveChanged);
@@ -1425,6 +1426,24 @@ void DiveTripModelTree::divesSelectedTrip(dive_trip *trip, const QVector<dive *>
 	}
 }
 
+void DiveTripModelTree::tripSelected(dive_trip *trip, dive *currentDive)
+{
+	if (!trip)
+		return;
+
+	// Find the trip.
+	int idx = findTripIdx(trip);
+	if (idx < 0) {
+		// We don't know the trip - this shouldn't happen. We seem to have
+		// missed some signals!
+		qWarning() << "DiveTripModelTree::tripSelected(): unknown trip";
+		return;
+	}
+
+	QModelIndex tripIdx = createIndex(idx, 0, noParent);
+	emit DiveTripModelBase::tripSelected(tripIdx, diveToIdx(currentDive));
+}
+
 bool DiveTripModelTree::lessThan(const QModelIndex &i1, const QModelIndex &i2) const
 {
 	// In tree mode we don't support any sorting!
@@ -1445,6 +1464,7 @@ 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::tripSelected, this, &DiveTripModelList::tripSelected);
 	connect(&diveListNotifier, &DiveListNotifier::filterReset, this, &DiveTripModelList::filterReset);
 	connect(&diveListNotifier, &DiveListNotifier::cylinderAdded, this, &DiveTripModelList::diveChanged);
 	connect(&diveListNotifier, &DiveListNotifier::cylinderEdited, this, &DiveTripModelList::diveChanged);
@@ -1665,6 +1685,21 @@ void DiveTripModelList::divesSelected(const QVector<dive *> &divesIn, dive *curr
 	currentChanged(currentDive);
 }
 
+void DiveTripModelList::tripSelected(dive_trip *trip, dive *currentDive)
+{
+	if (!trip)
+		return;
+
+	// In the list view, there are no trips, so simply transform this into
+	// a dive selection.
+	QVector<dive *> dives;
+	dives.reserve(trip->dives.nr);
+	for (int i = 0; i < trip->dives.nr; ++i)
+		dives.push_back(trip->dives.dives[i]);
+
+	divesSelected(dives, currentDive);
+}
+
 // Simple sorting helper for sorting against a criterium and if
 // that is undefined against a different criterium.
 // Return true if diff1 < 0, false if diff1 > 0.

qt-models/divetripmodel.h

@@ -85,6 +85,7 @@ signals:
 	// indices into local indices according to current sorting/filtering and instructs the QSelectionModel to
 	// perform the appropriate actions.
 	void selectionChanged(const QVector<QModelIndex> &indices, QModelIndex currentDive);
+	void tripSelected(QModelIndex trip, QModelIndex currentDive);
 protected:
 	dive *oldCurrent;
 	QBrush invalidForeground;
@@ -116,6 +117,7 @@ public slots:
 	void diveChanged(dive *d);
 	void divesTimeChanged(timestamp_t delta, const QVector<dive *> &dives);
 	void divesSelected(const QVector<dive *> &dives, dive *currentDive);
+	void tripSelected(dive_trip *trip, dive *currentDive);
 	void tripChanged(dive_trip *trip, TripField);
 	void filterReset();
 
@@ -194,6 +196,7 @@ public slots:
 	// 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, dive *currentDive);
+	void tripSelected(dive_trip *trip, dive *currentDive);
 	void filterReset();
 
 public:

qt-models/filtermodels.cpp

@@ -27,6 +27,7 @@ void MultiFilterSortModel::resetModel(DiveTripModelBase::Layout layout)
 
 	setSourceModel(model.get());
 	connect(model.get(), &DiveTripModelBase::selectionChanged, this, &MultiFilterSortModel::selectionChangedSlot);
+	connect(model.get(), &DiveTripModelBase::tripSelected, this, &MultiFilterSortModel::tripSelectedSlot);
 	model->initSelection();
 	LocationInformationModel::instance()->update();
 }
@@ -45,6 +46,16 @@ void MultiFilterSortModel::selectionChangedSlot(const QVector<QModelIndex> &indi
 	emit selectionChanged(indicesLocal, mapFromSource(currentDive));
 }
 
+// Translate selection into local indices and re-emit signal
+void MultiFilterSortModel::tripSelectedSlot(QModelIndex trip, QModelIndex currentDive)
+{
+	QModelIndex local = mapFromSource(trip);
+	if (!local.isValid())
+		return;
+
+	emit tripSelected(local, mapFromSource(currentDive));
+}
+
 bool MultiFilterSortModel::filterAcceptsRow(int source_row, const QModelIndex &source_parent) const
 {
 	return true;

qt-models/filtermodels.h

@@ -19,8 +19,10 @@ public:
 	void resetModel(DiveTripModelBase::Layout layout);
 signals:
 	void selectionChanged(const QVector<QModelIndex> &indices, QModelIndex currentDive);
+	void tripSelected(QModelIndex trip, QModelIndex currentDive);
 private slots:
 	void selectionChangedSlot(const QVector<QModelIndex> &indices, QModelIndex currentDive);
+	void tripSelectedSlot(QModelIndex trip, QModelIndex currentDive);
 private:
 	MultiFilterSortModel(QObject *parent = 0);
 	std::unique_ptr<DiveTripModelBase> model;