Desktop: don't connect to remove() slot of model from TableModel

When connecting a model to the TableModel class, it would connect
clicking on an item to the remove() slot of the model.

This breaks the program flow implied by the undo code:
Ui --> Undo-Command --> Model --> UI

Moreover, the naming of the remove() slot is illogical, because
clicks can also have different effects, as for example in the
cylinder-table.

Therefore, move the connect() call from TableModel to the
callers. In the case of TabDiveSite, move the remove() function
from the model to the TabWidget, where it makes more sense.

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

desktop-widgets/diveplanner.cpp

@@ -114,12 +114,14 @@ DivePlannerWidget::DivePlannerWidget(QWidget *parent, Qt::WindowFlags f) : QWidg
 	ui.dateEdit->setDisplayFormat(prefs.date_format);
 	ui.tableWidget->setTitle(tr("Dive planner points"));
 	ui.tableWidget->setModel(plannerModel);
+	connect(ui.tableWidget, &TableView::itemClicked, plannerModel, &DivePlannerPointsModel::remove);
 	plannerModel->setRecalc(true);
 	ui.tableWidget->view()->setItemDelegateForColumn(DivePlannerPointsModel::GAS, new AirTypesDelegate(this));
 	ui.tableWidget->view()->setItemDelegateForColumn(DivePlannerPointsModel::DIVEMODE, new DiveTypesDelegate(this));
 	ui.cylinderTableWidget->setTitle(tr("Available gases"));
 	ui.cylinderTableWidget->setBtnToolTip(tr("Add cylinder"));
 	ui.cylinderTableWidget->setModel(CylindersModel::instance());
+	connect(ui.cylinderTableWidget, &TableView::itemClicked, CylindersModel::instance(), &CylindersModel::remove);
 	ui.waterType->setItemData(0, FRESHWATER_SALINITY);
 	ui.waterType->setItemData(1, SEAWATER_SALINITY);
 	ui.waterType->setItemData(2, EN13319_SALINITY);

desktop-widgets/tab-widgets/TabDiveEquipment.cpp

@@ -33,8 +33,10 @@ TabDiveEquipment::TabDiveEquipment(QWidget *parent) : TabBase(parent),
 	ui.weights->setModel(weightModel);
 
 	connect(&diveListNotifier, &DiveListNotifier::divesChanged, this, &TabDiveEquipment::divesChanged);
-	connect(ui.cylinders->view(), &QTableView::clicked, this, &TabDiveEquipment::editCylinderWidget);
-	connect(ui.weights->view(), &QTableView::clicked, this, &TabDiveEquipment::editWeightWidget);
+	connect(ui.cylinders, &TableView::itemClicked, cylindersModel, &CylindersModel::remove);
+	connect(ui.cylinders, &TableView::itemClicked, this, &TabDiveEquipment::editCylinderWidget);
+	connect(ui.weights, &TableView::itemClicked, weightModel, &WeightModel::remove);
+	connect(ui.weights, &TableView::itemClicked, this, &TabDiveEquipment::editWeightWidget);
 
 	// Current display of things on Gnome3 looks like shit, so
 	// let's fix that.

desktop-widgets/tab-widgets/TabDiveExtraInfo.cpp

@@ -1,8 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 #include "TabDiveExtraInfo.h"
 #include "ui_TabDiveExtraInfo.h"
-
-#include <qt-models/divecomputerextradatamodel.h>
+#include "qt-models/divecomputerextradatamodel.h"
 
 TabDiveExtraInfo::TabDiveExtraInfo(QWidget *parent) :
 	TabBase(parent),

desktop-widgets/tab-widgets/TabDiveSite.cpp

@@ -4,9 +4,10 @@
 #include "core/divesite.h"
 #include "core/divefilter.h"
 #include "qt-models/divelocationmodel.h"
+#include "desktop-widgets/mainwindow.h" // to place message box
 #include "commands/command.h"
 
-#include <qt-models/divecomputerextradatamodel.h>
+#include <QMessageBox>
 
 TabDiveSite::TabDiveSite(QWidget *parent) : TabBase(parent)
 {
@@ -25,6 +26,7 @@ TabDiveSite::TabDiveSite(QWidget *parent) : TabBase(parent)
 		ui.diveSites->view()->setColumnHidden(i, true);
 
 	connect(ui.diveSites, &TableView::addButtonClicked, this, &TabDiveSite::add);
+	connect(ui.diveSites, &TableView::itemClicked, this, &TabDiveSite::diveSiteClicked);
 	connect(ui.diveSites->view()->selectionModel(), &QItemSelectionModel::selectionChanged, this, &TabDiveSite::selectionChanged);
 
 	// Subtle: We depend on this slot being executed after the slot in the model.
@@ -40,6 +42,26 @@ void TabDiveSite::clear()
 {
 }
 
+void TabDiveSite::diveSiteClicked(const QModelIndex &index)
+{
+	struct dive_site *ds = model.getDiveSite(index);
+	if (!ds)
+		return;
+	switch (index.column()) {
+	case LocationInformationModel::EDIT:
+		MainWindow::instance()->editDiveSite(ds);
+		break;
+	case LocationInformationModel::REMOVE:
+		if (ds->dives.nr > 0 &&
+		    QMessageBox::warning(MainWindow::instance(), tr("Delete dive site?"),
+					 tr("This dive site has %n dive(s). Do you really want to delete it?\n", "", ds->dives.nr),
+					 QMessageBox::Yes|QMessageBox::No) == QMessageBox::No)
+				return;
+		Command::deleteDiveSites(QVector<dive_site *>{ds});
+		break;
+	}
+}
+
 void TabDiveSite::add()
 {
 	// This is mighty dirty: We hook into the "dive site added" signal and

desktop-widgets/tab-widgets/TabDiveSite.h

@@ -16,6 +16,7 @@ private slots:
 	void add();
 	void diveSiteAdded(struct dive_site *, int idx);
 	void diveSiteChanged(struct dive_site *ds, int field);
+	void diveSiteClicked(const QModelIndex &);
 	void on_purgeUnused_clicked();
 	void on_filterText_textChanged(const QString &text);
 	void selectionChanged(const QItemSelection &selected, const QItemSelection &deselected);

desktop-widgets/tableview.cpp

@@ -10,6 +10,8 @@ TableView::TableView(QWidget *parent) : QGroupBox(parent)
 	ui.setupUi(this);
 	ui.tableView->setItemDelegate(new DiveListDelegate(this));
 
+	connect(ui.tableView, &QTableView::clicked, this, &TableView::itemClicked);
+
 	QFontMetrics fm(defaultModelFont());
 	int text_ht = fm.height();
 
@@ -93,7 +95,6 @@ void TableView::setBtnToolTip(const QString &tooltip)
 void TableView::setModel(QAbstractItemModel *model)
 {
 	ui.tableView->setModel(model);
-	connect(ui.tableView, SIGNAL(clicked(QModelIndex)), model, SLOT(remove(QModelIndex)));
 
 	QSettings s;
 	s.beginGroup(objectName());

desktop-widgets/tableview.h

@@ -28,10 +28,6 @@ class TableView : public QGroupBox {
 public:
 	TableView(QWidget *parent = 0);
 	~TableView();
-	/* The model is expected to have a 'remove' slot, that takes a QModelIndex as parameter.
-	 * It's also expected to have the column '1' as a trash icon. I most probably should create a
-	 * proxy model and add that column, will mark that as TODO. see? marked.
-	 */
 	void setModel(QAbstractItemModel *model);
 	void setBtnToolTip(const QString &tooltip);
 	void fixPlusPosition();
@@ -45,6 +41,7 @@ protected:
 
 signals:
 	void addButtonClicked();
+	void itemClicked(const QModelIndex &);
 
 private:
 	Ui::TableView ui;

qt-models/divelocationmodel.cpp

@@ -7,9 +7,7 @@
 #include "core/metrics.h"
 #ifndef SUBSURFACE_MOBILE
 #include "cleanertablemodel.h" // for trashIcon() and editIcon()
-#include "desktop-widgets/mainwindow.h" // to place message box
 #include "commands/command.h"
-#include <QMessageBox>
 #endif
 #include <QLineEdit>
 #include <QIcon>
@@ -262,27 +260,6 @@ bool DiveSiteSortedModel::setData(const QModelIndex &index, const QVariant &valu
 	}
 }
 
-// TODO: Remove or edit. It doesn't make sense to call the model here, which calls the undo command,
-// which in turn calls the model.
-void DiveSiteSortedModel::remove(const QModelIndex &index)
-{
-	struct dive_site *ds = getDiveSite(index);
-	if (!ds)
-		return;
-	switch (index.column()) {
-	case LocationInformationModel::EDIT:
-		MainWindow::instance()->editDiveSite(ds);
-		break;
-	case LocationInformationModel::REMOVE:
-		if (ds->dives.nr > 0 &&
-		    QMessageBox::warning(MainWindow::instance(), tr("Delete dive site?"),
-					 tr("This dive site has %n dive(s). Do you really want to delete it?\n", "", ds->dives.nr),
-					 QMessageBox::Yes|QMessageBox::No) == QMessageBox::No)
-				return;
-		Command::deleteDiveSites(QVector<dive_site *>{ds});
-		break;
-	}
-}
 #endif // SUBSURFACE_MOBILE
 
 void DiveSiteSortedModel::setFilter(const QString &text)

qt-models/divelocationmodel.h

@@ -46,8 +46,6 @@ private:
 	QString fullText;
 #ifndef SUBSURFACE_MOBILE
 	bool setData(const QModelIndex &index, const QVariant &value, int role) override;
-public slots:
-	void remove(const QModelIndex &index);
 #endif // SUBSURFACE_MOBILE
 public:
 	DiveSiteSortedModel();