6 files changed, 99 insertions, 27 deletions

diff --git a/core/subsurface-qt/DiveListNotifier.h b/core/subsurface-qt/DiveListNotifier.h
index 94b0063ea..fe5ea0e22 100644
--- a/core/subsurface-qt/DiveListNotifier.h
+++ b/core/subsurface-qt/DiveListNotifier.h
@@ -30,10 +30,69 @@ signals:
 	void divesChanged(dive_trip *trip, const QVector<dive *> &dives);
 	void divesMovedBetweenTrips(dive_trip *from, dive_trip *to, bool deleteFrom, bool createTo, const QVector<dive *> &dives);
 	void divesTimeChanged(dive_trip *trip, timestamp_t delta, const QVector<dive *> &dives);
+
+	// This signal is sent if the selection of dives and/or the current dive changed
+	void selectionChanged();
+public:
+	// Desktop uses the QTreeView class to present the list of dives. The layout
+	// of this class gives us a very fundamental problem, as we can not easily
+	// distinguish between user-initiated changes of the selection and changes
+	// that are due to actions of the Command-classes. To solve this problem,
+	// the frontend can use this function to query whether a dive list-modifying
+	// command is currently executed. If this function returns true, the
+	// frontend is supposed to not modify the selection.
+	bool inCommand() const;
+
+	// The following class and function are used by divelist-modifying commands
+	// to signalize that are in-flight. If the returned object goes out of scope,
+	// the command-in-flight status is reset to its previous value. Thus, the
+	// function can be called recursively.
+	class InCommandMarker {
+		DiveListNotifier &notifier;
+		bool oldValue;
+		InCommandMarker(DiveListNotifier &);
+		friend DiveListNotifier;
+	public:
+		~InCommandMarker();
+	};
+
+	// Usage:
+	// void doWork()
+	// {
+	// 	auto marker = diveListNotifier.enterCommand();
+	// 	... do work ...
+	// }
+	InCommandMarker enterCommand();
+private:
+	friend InCommandMarker;
+	bool commandExecuting;
 };
 
-// The DiveListNotifier class has no state and no constructor.
+// The DiveListNotifier class has only trivial state.
 // We can simply define it as a global object.
 extern DiveListNotifier diveListNotifier;
 
+// InCommandMarker is so trivial that the functions can be inlined.
+// TODO: perhaps move this into own header-file.
+inline DiveListNotifier::InCommandMarker::InCommandMarker(DiveListNotifier &notifierIn) : notifier(notifierIn),
+	oldValue(notifier.commandExecuting)
+{
+	notifier.commandExecuting = true;
+}
+
+inline DiveListNotifier::InCommandMarker::~InCommandMarker()
+{
+	notifier.commandExecuting = oldValue;
+}
+
+inline bool DiveListNotifier::inCommand() const
+{
+	return commandExecuting;
+}
+
+inline DiveListNotifier::InCommandMarker DiveListNotifier::enterCommand()
+{
+	return InCommandMarker(*this);
+}
+
 #endif
diff --git a/desktop-widgets/command_divelist.cpp b/desktop-widgets/command_divelist.cpp
index 17d5f00e4..a75004bea 100644
--- a/desktop-widgets/command_divelist.cpp
+++ b/desktop-widgets/command_divelist.cpp
@@ -323,6 +323,8 @@ bool AddDive::workToBeDone()
 
 void AddDive::redo()
 {
+	auto marker = diveListNotifier.enterCommand();
+
 	int idx = divesToAdd[0].idx;
 	divesToRemove = addDives(divesToAdd);
 	mark_divelist_changed(true);
@@ -338,6 +340,8 @@ void AddDive::redo()
 
 void AddDive::undo()
 {
+	auto marker = diveListNotifier.enterCommand();
+
 	// Simply remove the dive that was previously added
 	divesToAdd = removeDives(divesToRemove);
 
@@ -358,12 +362,14 @@ bool DeleteDive::workToBeDone()
 
 void DeleteDive::undo()
 {
+	auto marker = diveListNotifier.enterCommand();
 	divesToDelete = addDives(divesToAdd);
 	mark_divelist_changed(true);
 }
 
 void DeleteDive::redo()
 {
+	auto marker = diveListNotifier.enterCommand();
 	divesToAdd = removeDives(divesToDelete);
 	mark_divelist_changed(true);
 }
@@ -377,6 +383,7 @@ ShiftTime::ShiftTime(const QVector<dive *> &changedDives, int amount)
 
 void ShiftTime::redo()
 {
+	auto marker = diveListNotifier.enterCommand();
 	for (dive *d: diveList)
 		d->when -= timeChanged;
 
@@ -420,6 +427,7 @@ RenumberDives::RenumberDives(const QVector<QPair<dive *, int>> &divesToRenumberI
 
 void RenumberDives::undo()
 {
+	auto marker = diveListNotifier.enterCommand();
 	renumberDives(divesToRenumber);
 	mark_divelist_changed(true);
 }
@@ -442,6 +450,7 @@ bool TripBase::workToBeDone()
 
 void TripBase::redo()
 {
+	auto marker = diveListNotifier.enterCommand();
 	moveDivesBetweenTrips(divesToMove);
 
 	mark_divelist_changed(true);
@@ -554,8 +563,7 @@ bool SplitDives::workToBeDone()
 
 void SplitDives::redo()
 {
-	if (!diveToSplit)
-		return;
+	auto marker = diveListNotifier.enterCommand();
 	divesToUnsplit[0] = addDive(splitDives[0]);
 	divesToUnsplit[1] = addDive(splitDives[1]);
 	unsplitDive = removeDive(diveToSplit);
@@ -564,9 +572,8 @@ void SplitDives::redo()
 
 void SplitDives::undo()
 {
-	if (!unsplitDive.dive)
-		return;
 	// Note: reverse order with respect to redo()
+	auto marker = diveListNotifier.enterCommand();
 	diveToSplit = addDive(unsplitDive);
 	splitDives[1] = removeDive(divesToUnsplit[1]);
 	splitDives[0] = removeDive(divesToUnsplit[0]);
@@ -659,6 +666,7 @@ bool MergeDives::workToBeDone()
 
 void MergeDives::redo()
 {
+	auto marker = diveListNotifier.enterCommand();
 	renumberDives(divesToRenumber);
 	diveToUnmerge = addDive(mergedDive);
 	unmergedDives = removeDives(divesToMerge);
@@ -666,6 +674,7 @@ void MergeDives::redo()
 
 void MergeDives::undo()
 {
+	auto marker = diveListNotifier.enterCommand();
 	divesToMerge = addDives(unmergedDives);
 	mergedDive = removeDive(diveToUnmerge);
 	renumberDives(divesToRenumber);
diff --git a/desktop-widgets/divelistview.cpp b/desktop-widgets/divelistview.cpp
index ebeb1f6ef..2a0d7028c 100644
--- a/desktop-widgets/divelistview.cpp
+++ b/desktop-widgets/divelistview.cpp
@@ -24,6 +24,7 @@
 #include "desktop-widgets/divelistview.h"
 #include "qt-models/divepicturemodel.h"
 #include "core/metrics.h"
+#include "core/subsurface-qt/DiveListNotifier.h"
 
 DiveListView::DiveListView(QWidget *parent) : QTreeView(parent), mouseClickSelection(false), sortColumn(0),
 	currentOrder(Qt::DescendingOrder), dontEmitDiveChangedSignal(false), selectionSaved(false),
@@ -267,19 +268,17 @@ void DiveListView::selectTrip(dive_trip_t *trip)
 // works as expected
 void DiveListView::clearTripSelection()
 {
-	// we want to make sure no trips are selected
-	disconnect(selectionModel(), SIGNAL(selectionChanged(QItemSelection, QItemSelection)), this, SLOT(selectionChanged(QItemSelection, QItemSelection)));
-	disconnect(selectionModel(), SIGNAL(currentChanged(QModelIndex, QModelIndex)), this, SLOT(currentChanged(QModelIndex, QModelIndex)));
+	// This marks the selection change as being internal - ie. we don't process it further.
+	// TODO: This should probably be sold differently.
+	auto marker = diveListNotifier.enterCommand();
 
+	// we want to make sure no trips are selected
 	Q_FOREACH (const QModelIndex &index, selectionModel()->selectedRows()) {
 		dive_trip_t *trip = static_cast<dive_trip_t *>(index.data(DiveTripModel::TRIP_ROLE).value<void *>());
 		if (!trip)
 			continue;
 		selectionModel()->select(index, QItemSelectionModel::Deselect);
 	}
-
-	connect(selectionModel(), SIGNAL(selectionChanged(QItemSelection, QItemSelection)), this, SLOT(selectionChanged(QItemSelection, QItemSelection)));
-	connect(selectionModel(), SIGNAL(currentChanged(QModelIndex, QModelIndex)), this, SLOT(currentChanged(QModelIndex, QModelIndex)));
 }
 
 void DiveListView::unselectDives()
@@ -374,7 +373,7 @@ void DiveListView::selectDives(const QList<int> &newDiveSelection)
 		scrollTo(idx);
 	}
 	// now that everything is up to date, update the widgets
-	emit currentDiveChanged();
+	emit diveListNotifier.selectionChanged();
 	dontEmitDiveChangedSignal = false;
 	return;
 }
@@ -545,11 +544,20 @@ void DiveListView::currentChanged(const QModelIndex &current, const QModelIndex&
 
 void DiveListView::selectionChanged(const QItemSelection &selected, const QItemSelection &deselected)
 {
-	QItemSelection newSelected = selected.size() ? selected : selectionModel()->selection();
-	QItemSelection newDeselected = deselected;
+	if (diveListNotifier.inCommand()) {
+		// This is a programmatical change of the selection.
+		// Call the QTreeView base function to reflect the selection in the display,
+		// but don't process it any further.
+		QTreeView::selectionChanged(selected, deselected);
+		return;
+	}
 
-	disconnect(selectionModel(), SIGNAL(selectionChanged(QItemSelection, QItemSelection)), this, SLOT(selectionChanged(QItemSelection, QItemSelection)));
-	disconnect(selectionModel(), SIGNAL(currentChanged(QModelIndex, QModelIndex)), this, SLOT(currentChanged(QModelIndex, QModelIndex)));
+	// This is a manual selection change. This means that the core does not yet know
+	// of the new selection. Update the core-structures accordingly and select/deselect
+	// all dives of a trip if a trip is selected/deselected.
+	const QItemSelection &newDeselected = deselected;
+
+	QItemSelection newSelected = selected.size() ? selected : selectionModel()->selection();
 
 	Q_FOREACH (const QModelIndex &index, newDeselected.indexes()) {
 		if (index.column() != 0)
@@ -581,11 +589,11 @@ void DiveListView::selectionChanged(const QItemSelection &selected, const QItemS
 			select_dive(get_divenr(dive));
 		}
 	}
-	QTreeView::selectionChanged(selectionModel()->selection(), newDeselected);
-	connect(selectionModel(), SIGNAL(selectionChanged(QItemSelection, QItemSelection)), this, SLOT(selectionChanged(QItemSelection, QItemSelection)));
-	connect(selectionModel(), SIGNAL(currentChanged(QModelIndex, QModelIndex)), this, SLOT(currentChanged(QModelIndex, QModelIndex)));
 	if (!dontEmitDiveChangedSignal)
-		emit currentDiveChanged();
+		emit diveListNotifier.selectionChanged();
+
+	// Display the new, processed, selection
+	QTreeView::selectionChanged(selectionModel()->selection(), newDeselected);
 }
 
 enum asked_user {NOTYET, MERGE, DONTMERGE};
diff --git a/desktop-widgets/divelistview.h b/desktop-widgets/divelistview.h
index 2d7699d1d..9e6857312 100644
--- a/desktop-widgets/divelistview.h
+++ b/desktop-widgets/divelistview.h
@@ -58,10 +58,6 @@ slots:
 	void shiftTimes();
 	void loadImages();
 	void loadWebImages();
-
-signals:
-	void currentDiveChanged();
-
 private:
 	bool mouseClickSelection;
 	QList<int> expandedRows;
diff --git a/desktop-widgets/mainwindow.cpp b/desktop-widgets/mainwindow.cpp
index 376d2e7f3..5ec086195 100644
--- a/desktop-widgets/mainwindow.cpp
+++ b/desktop-widgets/mainwindow.cpp
@@ -209,7 +209,7 @@ MainWindow::MainWindow() : QMainWindow(),
 	if (!QIcon::hasThemeIcon("window-close")) {
 		QIcon::setThemeName("subsurface");
 	}
-	connect(dive_list(), &DiveListView::currentDiveChanged, this, &MainWindow::current_dive_changed);
+	connect(&diveListNotifier, &DiveListNotifier::selectionChanged, this, &MainWindow::selectionChanged);
 	connect(PreferencesDialog::instance(), SIGNAL(settingsChanged()), this, SLOT(readSettings()));
 	connect(PreferencesDialog::instance(), SIGNAL(settingsChanged()), diveListView, SLOT(update()));
 	connect(PreferencesDialog::instance(), SIGNAL(settingsChanged()), diveListView, SLOT(reloadHeaderActions()));
@@ -528,7 +528,7 @@ void MainWindow::configureToolbar() {
 	}
 }
 
-void MainWindow::current_dive_changed()
+void MainWindow::selectionChanged()
 {
 	graphics()->plotDive(nullptr, false, true);
 	information()->updateDiveInfo();
diff --git a/desktop-widgets/mainwindow.h b/desktop-widgets/mainwindow.h
index 2f2b91a83..ae3b081a4 100644
--- a/desktop-widgets/mainwindow.h
+++ b/desktop-widgets/mainwindow.h
@@ -132,7 +132,7 @@ slots:
 	void on_action_Check_for_Updates_triggered();
 
 	void on_actionDiveSiteEdit_triggered();
-	void current_dive_changed();
+	void selectionChanged();
 	void initialUiSetup();
 
 	void on_actionImportDiveLog_triggered();