From 4f9526ef8122f3496ee0ead986fcc10c500e0584 Mon Sep 17 00:00:00 2001
From: Tomaz Canabrava <tcanabrava@kde.org>
Date: Sat, 16 Nov 2013 18:41:47 -0200
Subject: [PATCH] Fix wrong handling of Dive Table, and revert some wrong
 changes.

This patch just reverts some wrong changes that I'v done on a
past commit ( sorry ) and correctly handles the selectDive,
by using a IDX instead of the dive pointer, as dirk told me
it's extremely error-prone since the pointer can change.

Signed-off-by: Tomaz Canabrava <tcanabrava@kde.org>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
---
 qt-ui/divelistview.cpp | 76 ++++++++++++++----------------------------
 qt-ui/divelistview.h   |  2 +-
 qt-ui/globe.cpp        |  2 +-
 qt-ui/maintab.cpp      |  2 +-
 qt-ui/models.cpp       | 20 ++---------
 qt-ui/models.h         |  4 +--
 6 files changed, 32 insertions(+), 74 deletions(-)

diff --git a/qt-ui/divelistview.cpp b/qt-ui/divelistview.cpp
index 7689f06a9..a34484dba 100644
--- a/qt-ui/divelistview.cpp
+++ b/qt-ui/divelistview.cpp
@@ -110,8 +110,10 @@ void DiveListView::backupExpandedRows(){
 }
 
 void DiveListView::restoreExpandedRows(){
+	setAnimated(false);
 	Q_FOREACH(const int &i, expandedRows)
 		setExpanded( model()->index(i, 0), true );
+	setAnimated(true);
 }
 void DiveListView::fixMessyQtModelBehaviour()
 {
@@ -139,9 +141,7 @@ void DiveListView::restoreSelection()
 {
 	unselectDives();
 	Q_FOREACH(int i, selectedDives) {
-		struct dive *d = get_dive(i);
-		if (d)
-			selectDive(d);
+		selectDive(i);
 	}
 }
 
@@ -150,25 +150,18 @@ void DiveListView::unselectDives()
 	selectionModel()->clearSelection();
 }
 
-void DiveListView::selectDive(struct dive *dive, bool scrollto, bool toggle)
+void DiveListView::selectDive(int i, bool scrollto, bool toggle)
 {
-	if (dive == NULL)
-		return;
 	QSortFilterProxyModel *m = qobject_cast<QSortFilterProxyModel*>(model());
-	QModelIndexList match = m->match(m->index(0,0), DiveTripModel::NR, dive->number, 1, Qt::MatchRecursive);
+	QModelIndexList match = m->match(m->index(0,0), DiveTripModel::DIVE_IDX, i, 2, Qt::MatchRecursive);
 	QItemSelectionModel::SelectionFlags flags;
 	QModelIndex idx = match.first();
-
-	QModelIndex parent = idx.parent();
-	if (parent.isValid())
-		expand(parent);
 	flags = toggle ? QItemSelectionModel::Toggle : QItemSelectionModel::Select;
 	flags |= QItemSelectionModel::Rows;
 	selectionModel()->select(idx, flags);
 	if (scrollto)
 		scrollTo(idx, PositionAtCenter);
 }
-
 void DiveListView::showSearchEdit()
 {
 	searchBox->show();
@@ -196,25 +189,9 @@ bool DiveListView::eventFilter(QObject* , QEvent* event)
 // index. TRIP_ROLE vs DIVE_ROLE?
 void DiveListView::headerClicked(int i)
 {
-	sortColumn = i;
-	QItemSelection oldSelection = selectionModel()->selection();
-	QList<struct dive*> currentSelectedDives;
-	DiveTripModel::Layout newLayout;
-	bool first = true;
-
-	newLayout = i == (int) DiveTripModel::NR ? DiveTripModel::TREE : DiveTripModel::LIST;
-
-	Q_FOREACH(const QModelIndex& index , oldSelection.indexes()) {
-		if (index.column() != 0) // We only care about the dives, so, let's stick to rows and discard columns.
-			continue;
-
-		struct dive *d = (struct dive *) index.data(DiveTripModel::DIVE_ROLE).value<void*>();
-		if (d)
-			currentSelectedDives.push_back(d);
-	}
-
+	DiveTripModel::Layout newLayout = i == (int) DiveTripModel::NR ? DiveTripModel::TREE : DiveTripModel::LIST;
+	rememberSelection();
 	unselectDives();
-
 	/* No layout change? Just re-sort, and scroll to first selection, making sure all selections are expanded */
 	if (currentLayout == newLayout) {
 		currentOrder = (currentOrder == Qt::DescendingOrder) ? Qt::AscendingOrder : Qt::DescendingOrder;
@@ -231,12 +208,7 @@ void DiveListView::headerClicked(int i)
 			restoreExpandedRows();
 		}
 	}
-
-	// repopulate the selections.
-	Q_FOREACH(struct dive *d, currentSelectedDives) {
-		selectDive(d, first);
-		first = false;
-	}
+	restoreSelection();
 }
 
 void DiveListView::reload(DiveTripModel::Layout layout, bool forceSort)
@@ -263,7 +235,7 @@ void DiveListView::reload(DiveTripModel::Layout layout, bool forceSort)
 
 	sortByColumn(sortColumn, currentOrder);
 	if (amount_selected && current_dive != NULL) {
-		selectDive(current_dive, true);
+		selectDive(selected_dive, true);
 	} else {
 		QModelIndex firstDiveOrTrip = m->index(0,0);
 		if (firstDiveOrTrip.isValid()) {
@@ -495,23 +467,25 @@ void DiveListView::deleteDive()
 	struct dive *d = (struct dive *) contextMenuIndex.data(DiveTripModel::DIVE_ROLE).value<void*>();
 	if (!d)
 		return;
-
-	QSortFilterProxyModel *proxy = qobject_cast<QSortFilterProxyModel*>(model());
-	DiveTripModel *realModel = qobject_cast<DiveTripModel*>(proxy->sourceModel());
-	realModel->deleteSelectedDives();
-
-	struct dive* next_dive = 0;
+	// after a dive is deleted the ones following it move forward in the dive_table
+	// so instead of using the for_each_dive macro I'm using an explicit for loop
+	// to make this easier to understand
+	for (i = 0; i < dive_table.nr; i++) {
+		d = get_dive(i);
+		if (!d->selected)
+			continue;
+		delete_single_dive(i);
+		i--; // so the next dive isn't skipped... it's now #i
+	}
 	if (amount_selected == 0) {
-		if (i > 0){
-			next_dive = get_dive(nr -1);
-		}
-		else{
+		if (i > 0)
+			select_dive(nr - 1);
+		else
 			mainWindow()->cleanUpEmpty();
-		}
 	}
 	mark_divelist_changed(TRUE);
-	if (next_dive)
-		selectDive(next_dive);
+	mainWindow()->refreshDisplay();
+	reload(currentLayout, false);
 }
 
 void DiveListView::testSlot()
@@ -563,7 +537,7 @@ void DiveListView::contextMenuEvent(QContextMenuEvent *event)
 	QAction * actionTaken = popup.exec(event->globalPos());
 	if (actionTaken == collapseAction && collapseAction) {
 		this->setAnimated(false);
-		selectDive(current_dive, true);
+		selectDive(selected_dive, true);
 		scrollTo(selectedIndexes().first());
 		this->setAnimated(true);
 	}
diff --git a/qt-ui/divelistview.h b/qt-ui/divelistview.h
index d03167635..f257d3b38 100644
--- a/qt-ui/divelistview.h
+++ b/qt-ui/divelistview.h
@@ -25,7 +25,7 @@ public:
 	void reload(DiveTripModel::Layout layout, bool forceSort = true);
 	bool eventFilter(QObject* , QEvent* );
 	void unselectDives();
-	void selectDive(struct dive *, bool scrollto = false, bool toggle = false);
+	void selectDive(int dive_table_idx, bool scrollto = false, bool toggle = false);
 	void rememberSelection();
 	void restoreSelection();
 	void contextMenuEvent(QContextMenuEvent *event);
diff --git a/qt-ui/globe.cpp b/qt-ui/globe.cpp
index 9e733cefc..aeaab2903 100644
--- a/qt-ui/globe.cpp
+++ b/qt-ui/globe.cpp
@@ -115,7 +115,7 @@ void GlobeGPS::mouseClicked(qreal lon, qreal lat, GeoDataCoordinates::Unit unit)
 			mainWindow()->dive_list()->unselectDives();
 			clear = false;
 		}
-		mainWindow()->dive_list()->selectDive(dive, first, toggle);
+		mainWindow()->dive_list()->selectDive(idx, first, toggle);
 		first = false;
 	}
 }
diff --git a/qt-ui/maintab.cpp b/qt-ui/maintab.cpp
index 970f90a2f..60b4add3a 100644
--- a/qt-ui/maintab.cpp
+++ b/qt-ui/maintab.cpp
@@ -511,7 +511,7 @@ void MainTab::acceptChanges()
 		// unselectDives() doesn't mess with the dive_table at all
 		struct dive *addedDive = current_dive;
 		mainWindow()->dive_list()->unselectDives();
-		mainWindow()->dive_list()->selectDive(addedDive, true, true);
+		mainWindow()->dive_list()->selectDive(selected_dive, true, true);
 		mainWindow()->showProfile();
 		mark_divelist_changed(TRUE);
 		DivePlannerPointsModel::instance()->setPlanMode(DivePlannerPointsModel::NOTHING);
diff --git a/qt-ui/models.cpp b/qt-ui/models.cpp
index dd69e45c2..2e4a0860e 100644
--- a/qt-ui/models.cpp
+++ b/qt-ui/models.cpp
@@ -989,6 +989,9 @@ QVariant DiveItem::data(int column, int role) const
 	if (role == DiveTripModel::DIVE_ROLE)
 		retVal = QVariant::fromValue<void*>(dive);
 
+	if(role == DiveTripModel::DIVE_IDX){
+		retVal = get_divenr(dive);
+	}
 	return retVal;
 }
 
@@ -1198,23 +1201,6 @@ QVariant DiveComputerModel::data(const QModelIndex& index, int role) const
 	return ret;
 }
 
-void DiveTripModel::deleteSelectedDives()
-{
-	// after a dive is deleted the ones following it move forward in the dive_table
-	// so instead of using the for_each_dive macro I'm using an explicit for loop
-	// to make this easier to understand
-	beginRemoveRows(index(0,0), 0, rowCount()-1);
-	for (int i = 0; i < dive_table.nr; i++) {
-		struct dive *d = get_dive(i);
-		if (!d->selected)
-			continue;
-		delete_single_dive(i);
-		i--; // so the next dive isn't skipped... it's now #i
-	}
-	endRemoveRows();
-	setupModelData();
-}
-
 int DiveComputerModel::rowCount(const QModelIndex& parent) const
 {
 	return numRows;
diff --git a/qt-ui/models.h b/qt-ui/models.h
index f212a18aa..de9aab5ce 100644
--- a/qt-ui/models.h
+++ b/qt-ui/models.h
@@ -192,7 +192,7 @@ public:
 	enum Column {NR, DATE, RATING, DEPTH, DURATION, TEMPERATURE, TOTALWEIGHT,
 		SUIT, CYLINDER, NITROX, SAC, OTU, MAXCNS, LOCATION, COLUMNS };
 
-	enum ExtraRoles{STAR_ROLE = Qt::UserRole + 1, DIVE_ROLE, TRIP_ROLE, SORT_ROLE};
+	enum ExtraRoles{STAR_ROLE = Qt::UserRole + 1, DIVE_ROLE, TRIP_ROLE, SORT_ROLE, DIVE_IDX};
 	enum Layout{TREE, LIST, CURRENT};
 
 	Qt::ItemFlags flags(const QModelIndex &index) const;
@@ -200,8 +200,6 @@ public:
 	DiveTripModel(QObject* parent = 0);
 	Layout layout() const;
 	void setLayout(Layout layout);
-	void deleteSelectedDives();
-
 private:
 	void setupModelData();
 	QMap<dive_trip_t*, TripItem*> trips;