Dive list: split reload() in reload() and setSortOrder()

The DiveListView code had a very fundamental problem with its
header: Each had its own idea of who is responsible for sorting.
Since we can't easily change QHeaderView, accept QHeaderView
as the authority on sort-column and order.

To make this possible, split the reload() function in two
distinct functions:
 - reload() reloads the model and sorts according to the
   current sort criterion.
 - setSortOrder() tells the header to display a certain
   sort criterion. If this is a new criterion, it will then
   emit a signal. In this signal, resort according to that
   criterion.

Thus, the actual sorting code has to be moved from the
headerClicked() to a new sortIndicatorChanged() slot.
Morover, the sorting of the QHeaderView has to be used.

Reported-by: Stefan Fuchs <sfuchs@gmx.de>
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>

4 files changed, 22 insertions, 25 deletions

diff --git a/desktop-widgets/divelistview.cpp b/desktop-widgets/divelistview.cpp
index a31813697..227ae502e 100644
--- a/desktop-widgets/divelistview.cpp
+++ b/desktop-widgets/divelistview.cpp
@@ -27,7 +27,7 @@
 #include "core/subsurface-qt/DiveListNotifier.h"
 
 DiveListView::DiveListView(QWidget *parent) : QTreeView(parent), mouseClickSelection(false), sortColumn(DiveTripModel::NR),
-	currentOrder(Qt::AscendingOrder), dontEmitDiveChangedSignal(false), selectionSaved(false),
+	currentOrder(Qt::AscendingOrder), currentLayout(DiveTripModel::TREE), dontEmitDiveChangedSignal(false), selectionSaved(false),
 	initialColumnWidths(DiveTripModel::COLUMNS, 50)	// Set up with default length 50
 {
 	setItemDelegate(new DiveListDelegate(this));
@@ -48,7 +48,7 @@ DiveListView::DiveListView(QWidget *parent) : QTreeView(parent), mouseClickSelec
 	header()->setStretchLastSection(true);
 	header()->setSortIndicatorShown(true);
 	header()->setSectionsClickable(true);
-	connect(header(), &QHeaderView::sectionPressed, this, &DiveListView::headerClicked);
+	connect(header(), &QHeaderView::sortIndicatorChanged, this, &DiveListView::sortIndicatorChanged);
 
 	installEventFilter(this);
 
@@ -459,18 +459,12 @@ bool DiveListView::eventFilter(QObject *, QEvent *event)
 	return true;
 }
 
-void DiveListView::headerClicked(int i)
+void DiveListView::sortIndicatorChanged(int i, Qt::SortOrder order)
 {
 	DiveTripModel::Layout newLayout = i == (int)DiveTripModel::NR ? DiveTripModel::TREE : DiveTripModel::LIST;
 	/* No layout change? Just re-sort, and scroll to first selection, making sure all selections are expanded */
 	if (currentLayout == newLayout) {
-		// If this is the same column as before, change sort order. Otherwise, choose a default
-		// sort order (ascending).
-		if (sortColumn == i)
-			currentOrder = (currentOrder == Qt::DescendingOrder) ? Qt::AscendingOrder : Qt::DescendingOrder;
-		else
-			currentOrder = Qt::AscendingOrder;
-		sortByColumn(i, currentOrder);
+		sortByColumn(i, order);
 	} else {
 		// clear the model, repopulate with new indexes.
 		rememberSelection();
@@ -478,27 +472,28 @@ void DiveListView::headerClicked(int i)
 		if (currentLayout == DiveTripModel::TREE)
 			backupExpandedRows();
 		currentLayout = newLayout;
-		currentOrder = Qt::AscendingOrder;
 		MultiFilterSortModel::instance()->setLayout(newLayout);
-		sortByColumn(i, currentOrder);
+		sortByColumn(i, order);
 		if (newLayout == DiveTripModel::TREE)
 			restoreExpandedRows();
 		restoreSelection();
 	}
 	// remember the new sort column
 	sortColumn = i;
+	currentOrder = order;
 }
 
-void DiveListView::reload(DiveTripModel::Layout layout)
+void DiveListView::setSortOrder(int i, Qt::SortOrder order)
 {
-	if (layout == DiveTripModel::CURRENT)
-		layout = currentLayout;
-	else
-		currentLayout = layout;
+	// The QHeaderView will call our signal if the sort order changed
+	header()->setSortIndicator(i, order);
+}
 
-	MultiFilterSortModel::instance()->setLayout(layout);
+void DiveListView::reload()
+{
+	// A side-effect of setting the layout is reloading the model data
+	MultiFilterSortModel::instance()->setLayout(currentLayout);
 
-	sortByColumn(sortColumn, currentOrder);
 	if (amount_selected && current_dive != NULL)
 		selectDive(get_divenr(current_dive), true);
 	else
diff --git a/desktop-widgets/divelistview.h b/desktop-widgets/divelistview.h
index 44d6fdc24..79049d754 100644
--- a/desktop-widgets/divelistview.h
+++ b/desktop-widgets/divelistview.h
@@ -25,7 +25,8 @@ public:
 	void mouseDoubleClickEvent(QMouseEvent * event);
 	void selectionChanged(const QItemSelection &selected, const QItemSelection &deselected);
 	void currentChanged(const QModelIndex &current, const QModelIndex &previous);
-	void reload(DiveTripModel::Layout layout);
+	void setSortOrder(int i, Qt::SortOrder order); // Call to set sort order
+	void reload(); // Call to reload model data
 	bool eventFilter(QObject *, QEvent *);
 	void unselectDives();
 	void clearTripSelection();
@@ -44,7 +45,7 @@ public
 slots:
 	void toggleColumnVisibilityByIndex();
 	void reloadHeaderActions();
-	void headerClicked(int);
+	void sortIndicatorChanged(int index, Qt::SortOrder order);
 	void removeFromTrip();
 	void deleteDive();
 	void markDiveInvalid();
diff --git a/desktop-widgets/mainwindow.cpp b/desktop-widgets/mainwindow.cpp
index ee38bda28..33d954f8d 100644
--- a/desktop-widgets/mainwindow.cpp
+++ b/desktop-widgets/mainwindow.cpp
@@ -245,7 +245,7 @@ MainWindow::MainWindow() : QMainWindow(),
 	graphics->setEmptyState();
 	initialUiSetup();
 	readSettings();
-	diveList->reload(DiveTripModel::TREE);
+	diveList->reload();
 	diveList->reloadHeaderActions();
 	diveList->setFocus();
 	MapWidget::instance()->reload();
@@ -494,7 +494,7 @@ void MainWindow::refreshDisplay(bool doRecreateDiveList)
 
 void MainWindow::recreateDiveList()
 {
-	diveList->reload(DiveTripModel::CURRENT);
+	diveList->reload();
 	TagFilterModel::instance()->repopulate();
 	BuddyFilterModel::instance()->repopulate();
 	LocationFilterModel::instance()->repopulate();
@@ -704,7 +704,8 @@ void MainWindow::cleanUpEmpty()
 	mainTab->clearTabs();
 	mainTab->updateDiveInfo(true);
 	graphics->setEmptyState();
-	diveList->reload(DiveTripModel::TREE);
+	diveList->reload();
+	diveList->setSortOrder(DiveTripModel::NR, Qt::AscendingOrder);
 	MapWidget::instance()->reload();
 	if (!existing_filename)
 		setTitle();
diff --git a/desktop-widgets/tab-widgets/maintab.cpp b/desktop-widgets/tab-widgets/maintab.cpp
index d27a4730d..b61c9ab78 100644
--- a/desktop-widgets/tab-widgets/maintab.cpp
+++ b/desktop-widgets/tab-widgets/maintab.cpp
@@ -926,7 +926,7 @@ void MainTab::acceptChanges()
 	int scrolledBy = MainWindow::instance()->diveList->verticalScrollBar()->sliderPosition();
 	resetPallete();
 	if (editMode == MANUALLY_ADDED_DIVE) {
-		MainWindow::instance()->diveList->reload(DiveTripModel::CURRENT);
+		MainWindow::instance()->diveList->reload();
 		int newDiveNr = get_divenr(get_dive_by_uniq_id(addedId));
 		MainWindow::instance()->diveList->unselectDives();
 		MainWindow::instance()->diveList->selectDive(newDiveNr, true);