planner: split DivePlannerPointsModel::remove() in two

There are two cases in this function: with and without holding
the control-key. The former deletes one point, the latter all
points starting with the selected point to the end.

The code was interlaced making it very hard to reason about.
Notably, it was buggy: with control, all points could be
deleted, leading to a crash.

Split the function in two versions, with their own bound
checking. This produces a bit of duplicate code, which
might be broken out later.

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

2 files changed, 37 insertions, 18 deletions

diff --git a/qt-models/diveplannermodel.cpp b/qt-models/diveplannermodel.cpp
index 661caeade..99174bbae 100644
--- a/qt-models/diveplannermodel.cpp
+++ b/qt-models/diveplannermodel.cpp
@@ -902,17 +902,30 @@ divedatapoint DivePlannerPointsModel::at(int row) const
 	return divepoints.at(row);
 }
 
-void DivePlannerPointsModel::remove(const QModelIndex &index)
+void DivePlannerPointsModel::removeControlPressed(const QModelIndex &index)
 {
-	if (index.column() != REMOVE || rowCount() == 1)
-		return;
-
-	divedatapoint dp = at(index.row());
-	if (!dp.entered)
+	// Never delete all points.
+	int rows = rowCount();
+	if (index.column() != REMOVE || index.row() <= 0 || index.row() >= rows)
 		return;
 
 	int old_first_cylid = divepoints[0].cylinderid;
 
+	preserved_until.seconds = divepoints.at(index.row()).time;
+	beginRemoveRows(QModelIndex(), index.row(), rows - 1);
+	divepoints.erase(divepoints.begin() + index.row(), divepoints.end());
+	endRemoveRows();
+
+	cylinders.updateTrashIcon();
+	if (divepoints[0].cylinderid != old_first_cylid)
+		cylinders.moveAtFirst(divepoints[0].cylinderid);
+
+	updateDiveProfile();
+	emitDataChanged();
+}
+
+void DivePlannerPointsModel::remove(const QModelIndex &index)
+{
 /* TODO: this seems so wrong.
  * We can't do this here if we plan to use QML on mobile
  * as mobile has no ControlModifier.
@@ -920,21 +933,26 @@ void DivePlannerPointsModel::remove(const QModelIndex &index)
  * remove method that will pass the first and last index of the
  * removed rows, and remove those in a go.
  */
-	int i;
+	if (QApplication::keyboardModifiers() & Qt::ControlModifier)
+		return removeControlPressed(index);
+
+	// Refuse deleting the last point.
 	int rows = rowCount();
-	if (QApplication::keyboardModifiers() & Qt::ControlModifier) {
-		preserved_until.seconds = divepoints.at(index.row()).time;
-		beginRemoveRows(QModelIndex(), index.row(), rows - 1);
-		for (i = rows - 1; i >= index.row(); i--)
-			divepoints.remove(i);
-	} else {
-		if (index.row() == rows -1)
-			preserved_until.seconds = divepoints.at(rows - 1).time;
-		beginRemoveRows(QModelIndex(), index.row(), index.row());
-		divepoints.remove(index.row());
-	}
+	if (index.column() != REMOVE || index.row() < 0 || index.row() >= rows || rows <= 1)
+		return;
 
+	divedatapoint dp = at(index.row());
+	if (!dp.entered)
+		return;
+
+	int old_first_cylid = divepoints[0].cylinderid;
+
+	if (index.row() == rows)
+		preserved_until.seconds = divepoints.at(rows - 1).time;
+	beginRemoveRows(QModelIndex(), index.row(), index.row());
+	divepoints.remove(index.row());
 	endRemoveRows();
+
 	cylinders.updateTrashIcon();
 	if (divepoints[0].cylinderid != old_first_cylid)
 		cylinders.moveAtFirst(divepoints[0].cylinderid);
diff --git a/qt-models/diveplannermodel.h b/qt-models/diveplannermodel.h
index 52abd4bd6..2a01c2d42 100644
--- a/qt-models/diveplannermodel.h
+++ b/qt-models/diveplannermodel.h
@@ -86,6 +86,7 @@ slots:
 	void savePlan();
 	void saveDuplicatePlan();
 	void remove(const QModelIndex &index);
+	void removeControlPressed(const QModelIndex &index);
 	void cancelPlan();
 	void removeDeco();
 	void deleteTemporaryPlan();