mifr/subsurface
1
0
Fork 0
mirror of https://github.com/subsurface/subsurface.git synced 2024-11-28 05:00:20 +00:00
Code Issues Projects Releases Packages Wiki Activity

planner: split DivePlannerPointsModel::remove() in two

Browse source 
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>
This commit is contained in:
Berthold Stoeger 2021-03-02 10:54:36 +01:00 committed by Dirk Hohndel
parent a0f6b4d0b4
commit 11c54b85f6
2 changed files with 41 additions and 22 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

62
qt-models/diveplannermodel.cpp
View file

@ -902,9 +902,43 @@ divedatapoint DivePlannerPointsModel::at(int row) const
return divepoints.at(row);
}
void DivePlannerPointsModel::removeControlPressed(const QModelIndex &index)
{
// 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)
{
if (index.column() != REMOVE || rowCount() == 1)
/* TODO: this seems so wrong.
* We can't do this here if we plan to use QML on mobile
* as mobile has no ControlModifier.
* The correct thing to do is to create a new method
* remove method that will pass the first and last index of the
* removed rows, and remove those in a go.
*/
if (QApplication::keyboardModifiers() & Qt::ControlModifier)
return removeControlPressed(index);
// Refuse deleting the last point.
int rows = rowCount();
if (index.column() != REMOVE || index.row() < 0 || index.row() >= rows || rows <= 1)
return;
divedatapoint dp = at(index.row());
@ -913,28 +947,12 @@ void DivePlannerPointsModel::remove(const QModelIndex &index)
int old_first_cylid = divepoints[0].cylinderid;
/* TODO: this seems so wrong.
* We can't do this here if we plan to use QML on mobile
* as mobile has no ControlModifier.
* The correct thing to do is to create a new method
* remove method that will pass the first and last index of the
* removed rows, and remove those in a go.
*/
int i;
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.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);

1
qt-models/diveplannermodel.h
View file

@ -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();