undo: more fine-grained undo of profile editing

Place undo commands for every change of the profile, not
only on "saving". Move the edit-mode from the mainwindow
and the maintab to the profile widget.

This is still very rough. For example, the only way to exit
the edit mode is changing the current dive.

The undo-commands are placed by the desktop-profile widget.
We might think about moving that down to the profile-view so
that this will be useable on mobile.

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

CHANGELOG.md

@@ -1,3 +1,4 @@
+- profile: include profile editing in undo system
 - core: avoid crash with corrupted cloud storage
 - mobile: fix profile scaling issue on high DPR devices
 - mobile/Android: add logfiles as attachment to support emails

commands/command_base.cpp

@@ -80,7 +80,6 @@ QString getListOfDives(QVector<struct dive *> dives)
 	return getListOfDives(std::vector<struct dive *>(dives.begin(), dives.end()));
 }
 
-
 // return a string that can be used for the commit message and should list the changes that
 // were made to the dive list
 QString changesMade()

desktop-widgets/mainwindow.cpp

@@ -145,8 +145,6 @@ MainWindow::MainWindow() : QMainWindow(),
 
 	registerApplicationState(ApplicationState::Default, { true, { mainTab.get(), FLAG_NONE },  { profile.get(), FLAG_NONE },
 								    { diveList.get(), FLAG_NONE }, { mapWidget.get(), FLAG_NONE } });
-	registerApplicationState(ApplicationState::EditDive, { false, { mainTab.get(), FLAG_NONE },  { profile.get(), FLAG_NONE },
-								      { diveList.get(), FLAG_NONE }, { mapWidget.get(), FLAG_NONE } });
 	registerApplicationState(ApplicationState::PlanDive, { false, { &plannerWidgets->plannerWidget, FLAG_NONE },         { profile.get(), FLAG_NONE },
 								      { &plannerWidgets->plannerSettingsWidget, FLAG_NONE }, { &plannerWidgets->plannerDetails, FLAG_NONE } });
 	registerApplicationState(ApplicationState::EditPlannedDive, { true, { &plannerWidgets->plannerWidget, FLAG_NONE }, { profile.get(), FLAG_NONE },
@@ -246,8 +244,6 @@ MainWindow::MainWindow() : QMainWindow(),
 	set_git_update_cb(&updateProgress);
 	set_error_cb(&showErrorFromC);
 
-	connect(profile->view.get(), &ProfileWidget2::editCurrentDive, this, &MainWindow::editCurrentDive);
-
 // full screen support is buggy on Windows and Ubuntu.
 // require the FULLSCREEN_SUPPORT macro to enable it!
 #ifndef FULLSCREEN_SUPPORT
@@ -1408,25 +1404,6 @@ void MainWindow::on_actionImportDiveSites_triggered()
 	divesiteImport.exec();
 }
 
-void MainWindow::editCurrentDive()
-{
-	// We only allow editing of the profile for manually added dives.
-	if (!current_dive || (!same_string(current_dive->dc.model, "manually added dive") && current_dive->dc.samples) || !userMayChangeAppState())
-		return;
-
-	// This shouldn't be possible, but let's make sure no weird "double editing" takes place.
-	if (mainTab->isEditing() || DivePlannerPointsModel::instance()->currentMode() != DivePlannerPointsModel::NOTHING)
-		return;
-
-	disableShortcuts(false);
-	copy_dive(current_dive, &displayed_dive); // Work on a copy of the dive
-	DivePlannerPointsModel::instance()->setPlanMode(DivePlannerPointsModel::ADD);
-	DivePlannerPointsModel::instance()->loadFromDive(&displayed_dive);
-	profile->setEditState(&displayed_dive, 0);
-	setApplicationState(ApplicationState::EditDive);
-	mainTab->enableEdition();
-}
-
 void MainWindow::on_actionExport_triggered()
 {
 	DiveLogExportDialog diveLogExport;

desktop-widgets/mainwindow.h

@@ -54,7 +54,6 @@ public:
 
 	enum class ApplicationState {
 		Default,
-		EditDive,
 		PlanDive,
 		EditPlannedDive,
 		EditDiveSite,
@@ -155,7 +154,6 @@ slots:
 	void refreshDisplay();
 	void showProfile();
 	void refreshProfile();
-	void editCurrentDive();
 	void planCanceled();
 	void planCreated();
 	// Some shortcuts like "change DC" or "copy/paste dive components"

desktop-widgets/profilewidget.cpp

@@ -2,9 +2,11 @@
 
 #include "profilewidget.h"
 #include "profile-widget/profilewidget2.h"
+#include "commands/command.h"
 #include "core/color.h"
 #include "core/settings/qPrefTechnicalDetails.h"
 #include "core/settings/qPrefPartialPressureGas.h"
+#include "core/subsurface-string.h"
 #include "qt-models/diveplannermodel.h"
 
 #include <QToolBar>
@@ -49,7 +51,7 @@ void EmptyView::resizeEvent(QResizeEvent *)
 	update();
 }
 
-ProfileWidget::ProfileWidget()
+ProfileWidget::ProfileWidget() : originalDive(nullptr)
 {
 	ui.setupUi(this);
 
@@ -116,6 +118,10 @@ ProfileWidget::ProfileWidget()
 	connect(ui.profPO2, &QAction::triggered, pp_gas, &qPrefPartialPressureGas::set_po2);
 
 	connect(&diveListNotifier, &DiveListNotifier::settingsChanged, view.get(), &ProfileWidget2::settingsChanged);
+	connect(view.get(), &ProfileWidget2::editCurrentDive, this, &ProfileWidget::editDive);
+	connect(view.get(), &ProfileWidget2::stopAdded, this, &ProfileWidget::stopAdded);
+	connect(view.get(), &ProfileWidget2::stopRemoved, this, &ProfileWidget::stopRemoved);
+	connect(view.get(), &ProfileWidget2::stopMoved, this, &ProfileWidget::stopMoved);
 
 	ui.profCalcAllTissues->setChecked(qPrefTechnicalDetails::calcalltissues());
 	ui.profCalcCeiling->setChecked(qPrefTechnicalDetails::calcceiling());
@@ -149,6 +155,7 @@ void ProfileWidget::setEnabledToolbar(bool enabled)
 
 void ProfileWidget::setDive(const struct dive *d)
 {
+	// If the user was currently editing a dive, exit edit mode.
 	stack->setCurrentIndex(1); // show profile
 
 	bool freeDiveMode = d->dc.divemode == FREEDIVE;
@@ -176,8 +183,14 @@ void ProfileWidget::setDive(const struct dive *d)
 
 void ProfileWidget::plotCurrentDive()
 {
+	// Exit edit mode if the dive changed
+	if (editedDive && originalDive != current_dive)
+		exitEditMode();
+
 	setEnabledToolbar(current_dive != nullptr);
-	if (current_dive) {
+	if (editedDive) {
+		view->plotDive(editedDive.get(), editedDc);
+	} else if (current_dive) {
 		setDive(current_dive);
 		view->setProfileState(current_dive, dc_number);
 		view->resetZoom(); // when switching dive, reset the zoomLevel
@@ -190,14 +203,9 @@ void ProfileWidget::plotCurrentDive()
 
 void ProfileWidget::setPlanState(const struct dive *d, int dc)
 {
-	setDive(d); // show subsurface logo
-	view->setPlanState(d, dc);
-}
-
-void ProfileWidget::setEditState(const struct dive *d, int dc)
-{
+	exitEditMode();
 	setDive(d);
-	view->setEditState(d, dc);
+	view->setPlanState(d, dc);
 }
 
 void ProfileWidget::unsetProfHR()
@@ -211,3 +219,63 @@ void ProfileWidget::unsetProfTissues()
 	ui.profTissues->setChecked(false);
 	qPrefTechnicalDetails::set_percentagegraph(false);
 }
+
+void ProfileWidget::editDive()
+{
+	// We only allow editing of the profile for manually added dives
+	// and when no other editing is in progress.
+	if (!current_dive ||
+	   (!same_string(current_dive->dc.model, "manually added dive") && current_dive->dc.samples) ||
+	   (DivePlannerPointsModel::instance()->currentMode() != DivePlannerPointsModel::NOTHING) ||
+	   editedDive)
+		return;
+
+	editedDive.reset(alloc_dive());
+	editedDc = dc_number;
+	copy_dive(current_dive, editedDive.get()); // Work on a copy of the dive
+	originalDive = current_dive;
+	DivePlannerPointsModel::instance()->setPlanMode(DivePlannerPointsModel::ADD);
+	DivePlannerPointsModel::instance()->loadFromDive(editedDive.get());
+	view->setEditState(editedDive.get(), 0);
+}
+
+void ProfileWidget::exitEditMode()
+{
+	if (!editedDive)
+		return;
+	DivePlannerPointsModel::instance()->setPlanMode(DivePlannerPointsModel::NOTHING);
+	view->setProfileState(current_dive, dc_number); // switch back to original dive before erasing the copy.
+	editedDive.reset();
+	originalDive = nullptr;
+}
+
+// Update depths of edited dive
+static void calcDepth(dive &d, int dcNr)
+{
+	d.maxdepth.mm = get_dive_dc(&d, dcNr)->maxdepth.mm = 0;
+	fixup_dive(&d);
+}
+
+void ProfileWidget::stopAdded()
+{
+	if (!editedDive)
+		return;
+	calcDepth(*editedDive, editedDc);
+	Command::editProfile(editedDive.get(), Command::EditProfileType::ADD, 0);
+}
+
+void ProfileWidget::stopRemoved(int count)
+{
+	if (!editedDive)
+		return;
+	calcDepth(*editedDive, editedDc);
+	Command::editProfile(editedDive.get(), Command::EditProfileType::REMOVE, count);
+}
+
+void ProfileWidget::stopMoved(int count)
+{
+	if (!editedDive)
+		return;
+	calcDepth(*editedDive, editedDc);
+	Command::editProfile(editedDive.get(), Command::EditProfileType::MOVE, count);
+}

desktop-widgets/profilewidget.h

@@ -9,10 +9,13 @@
 #include <vector>
 #include <memory>
 
+struct dive;
 class ProfileWidget2;
 class EmptyView;
 class QStackedWidget;
 
+extern "C" void free_dive(struct dive *);
+
 class ProfileWidget : public QWidget {
 	Q_OBJECT
 public:
@@ -21,18 +24,30 @@ public:
 	std::unique_ptr<ProfileWidget2> view;
 	void plotCurrentDive();
 	void setPlanState(const struct dive *d, int dc);
-	void setEditState(const struct dive *d, int dc);
 	void setEnabledToolbar(bool enabled);
 private
 slots:
 	void unsetProfHR();
 	void unsetProfTissues();
+	void stopAdded();
+	void stopRemoved(int count);
+	void stopMoved(int count);
 private:
+	// The same code is in command/command_base.h. Should we make that a global feature?
+	struct DiveDeleter {
+		void operator()(dive *d) { free_dive(d); }
+	};
+
 	std::unique_ptr<EmptyView> emptyView;
 	std::vector<QAction *> toolbarActions;
 	Ui::ProfileWidget ui;
 	QStackedWidget *stack;
 	void setDive(const struct dive *d);
+	void editDive();
+	void exitEditMode();
+	std::unique_ptr<dive, DiveDeleter> editedDive;
+	int editedDc;
+	dive *originalDive;
 };
 
 #endif // PROFILEWIDGET_H

desktop-widgets/tab-widgets/maintab.cpp

@@ -221,21 +221,6 @@ void MainTab::displayMessage(QString str)
 	ui.diveNotesMessage->animatedShow();
 }
 
-void MainTab::enableEdition()
-{
-	if (current_dive == NULL || editMode)
-		return;
-
-	ui.editDiveSiteButton->setEnabled(false);
-	MainWindow::instance()->diveList->setEnabled(false);
-	MainWindow::instance()->setEnabledToolbar(false);
-
-	ui.dateEdit->setEnabled(true);
-	displayMessage(tr("This dive is being edited."));
-
-	editMode = true;
-}
-
 // This function gets called if a field gets updated by an undo command.
 // Refresh the corresponding UI field.
 void MainTab::divesChanged(const QVector<dive *> &dives, DiveField field)

desktop-widgets/tab-widgets/maintab.h

@@ -57,7 +57,6 @@ slots:
 	void closeMessage();
 	void closeWarning();
 	void displayMessage(QString str);
-	void enableEdition();
 	void escDetected(void);
 	void updateDateTimeFields();
 	void colorsChanged();

profile-widget/profilewidget2.cpp

@@ -283,6 +283,8 @@ void ProfileWidget2::divePlannerHandlerReleased()
 {
 	if (zoomLevel)
 		return;
+	if (currentState == EDIT)
+		emit stopMoved(1);
 	shouldCalculateMax = true;
 	replot();
 }
@@ -334,6 +336,8 @@ void ProfileWidget2::mouseDoubleClickEvent(QMouseEvent *event)
 		int minutes = lrint(profileScene->timeAxis->valueAt(mappedPos) / 60);
 		int milimeters = lrint(profileScene->profileYAxis->valueAt(mappedPos) / M_OR_FT(1, 1)) * M_OR_FT(1, 1);
 		plannerModel->addStop(milimeters, minutes * 60);
+		if (currentState == EDIT)
+			emit stopAdded();
 	}
 }
 
@@ -919,20 +923,31 @@ void ProfileWidget2::divePlannerHandlerMoved()
 	plannerModel->editStop(index, data);
 }
 
+std::vector<int> ProfileWidget2::selectedDiveHandleIndices() const
+{
+	std::vector<int> res;
+	res.reserve(scene()->selectedItems().size());
+	for (QGraphicsItem *item: scene()->selectedItems()) {
+		if (DiveHandler *handler = qgraphicsitem_cast<DiveHandler *>(item))
+			res.push_back(handleIndex(handler));
+	}
+	return res;
+}
+
 void ProfileWidget2::keyDownAction()
 {
 	if ((currentState != EDIT && currentState != PLAN) || !plannerModel)
 		return;
 
-	Q_FOREACH (QGraphicsItem *i, scene()->selectedItems()) {
-		if (DiveHandler *handler = qgraphicsitem_cast<DiveHandler *>(i)) {
-			int row = handleIndex(handler);
-			divedatapoint dp = plannerModel->at(row);
+	std::vector<int> handleIndices = selectedDiveHandleIndices();
+	for (int row: handleIndices) {
+		divedatapoint dp = plannerModel->at(row);
 
-			dp.depth.mm += M_OR_FT(1, 5);
-			plannerModel->editStop(row, dp);
-		}
+		dp.depth.mm += M_OR_FT(1, 5);
+		plannerModel->editStop(row, dp);
 	}
+	if (currentState == EDIT && !handleIndices.empty())
+		emit stopMoved(handleIndices.size()); // TODO: Accumulate key moves
 }
 
 void ProfileWidget2::keyUpAction()
@@ -940,18 +955,18 @@ void ProfileWidget2::keyUpAction()
 	if ((currentState != EDIT && currentState != PLAN) || !plannerModel)
 		return;
 
-	Q_FOREACH (QGraphicsItem *i, scene()->selectedItems()) {
-		if (DiveHandler *handler = qgraphicsitem_cast<DiveHandler *>(i)) {
-			int row = handleIndex(handler);
-			divedatapoint dp = plannerModel->at(row);
+	std::vector<int> handleIndices = selectedDiveHandleIndices();
+	for (int row: handleIndices) {
+		divedatapoint dp = plannerModel->at(row);
 
-			if (dp.depth.mm <= 0)
-				continue;
+		if (dp.depth.mm <= 0)
+			continue;
 
-			dp.depth.mm -= M_OR_FT(1, 5);
-			plannerModel->editStop(row, dp);
-		}
+		dp.depth.mm -= M_OR_FT(1, 5);
+		plannerModel->editStop(row, dp);
 	}
+	if (currentState == EDIT && !handleIndices.empty())
+		emit stopMoved(handleIndices.size()); // TODO: Accumulate key moves
 }
 
 void ProfileWidget2::keyLeftAction()
@@ -959,18 +974,18 @@ void ProfileWidget2::keyLeftAction()
 	if ((currentState != EDIT && currentState != PLAN) || !plannerModel)
 		return;
 
-	Q_FOREACH (QGraphicsItem *i, scene()->selectedItems()) {
-		if (DiveHandler *handler = qgraphicsitem_cast<DiveHandler *>(i)) {
-			int row = handleIndex(handler);
-			divedatapoint dp = plannerModel->at(row);
+	std::vector<int> handleIndices = selectedDiveHandleIndices();
+	for (int row: handleIndices) {
+		divedatapoint dp = plannerModel->at(row);
 
-			if (dp.time / 60 <= 0)
-				continue;
+		if (dp.time / 60 <= 0)
+			continue;
 
-			dp.time -= 60;
-			plannerModel->editStop(row, dp);
-		}
+		dp.time -= 60;
+		plannerModel->editStop(row, dp);
 	}
+	if (currentState == EDIT && !handleIndices.empty())
+		emit stopMoved(handleIndices.size()); // TODO: Accumulate key moves
 }
 
 void ProfileWidget2::keyRightAction()
@@ -978,15 +993,15 @@ void ProfileWidget2::keyRightAction()
 	if ((currentState != EDIT && currentState != PLAN) || !plannerModel)
 		return;
 
-	Q_FOREACH (QGraphicsItem *i, scene()->selectedItems()) {
-		if (DiveHandler *handler = qgraphicsitem_cast<DiveHandler *>(i)) {
-			int row = handleIndex(handler);
-			divedatapoint dp = plannerModel->at(row);
+	std::vector<int> handleIndices = selectedDiveHandleIndices();
+	for (int row: handleIndices) {
+		divedatapoint dp = plannerModel->at(row);
 
-			dp.time += 60;
-			plannerModel->editStop(row, dp);
-		}
+		dp.time += 60;
+		plannerModel->editStop(row, dp);
 	}
+	if (currentState == EDIT && !handleIndices.empty())
+		emit stopMoved(handleIndices.size()); // TODO: Accumulate key moves
 }
 
 void ProfileWidget2::keyDeleteAction()
@@ -994,15 +1009,15 @@ void ProfileWidget2::keyDeleteAction()
 	if ((currentState != EDIT && currentState != PLAN) || !plannerModel)
 		return;
 
-	QVector<int> selectedIndices;
-	Q_FOREACH (QGraphicsItem *i, scene()->selectedItems()) {
-		if (DiveHandler *handler = qgraphicsitem_cast<DiveHandler *>(i)) {
-			selectedIndices.push_back(handleIndex(handler));
-			handler->hide();
-		}
+	std::vector<int> handleIndices = selectedDiveHandleIndices();
+	// For now, we have to convert to QVector.
+	for (int index: handleIndices)
+		handles[index]->hide();
+	if (!handleIndices.empty()) {
+		plannerModel->removeSelectedPoints(handleIndices);
+		if (currentState == EDIT)
+			emit stopRemoved(handleIndices.size());
 	}
-	if (!selectedIndices.isEmpty())
-		plannerModel->removeSelectedPoints(selectedIndices);
 }
 
 void ProfileWidget2::clearPictures()

profile-widget/profilewidget2.h

@@ -66,6 +66,9 @@ public:
 
 signals:
 	void editCurrentDive();
+	void stopAdded(); // only emitted in edit mode
+	void stopRemoved(int count); // only emitted in edit mode
+	void stopMoved(int count); // only emitted in edit mode
 
 public
 slots: // Necessary to call from QAction's signals.
@@ -186,6 +189,7 @@ private:
 #endif
 	friend class DiveHandler;
 	bool shouldCalculateMax; // Calculate maximum time and depth (default). False when dragging handles.
+	std::vector<int> selectedDiveHandleIndices() const;
 
 	// We store a const pointer to the shown dive. However, the undo commands want
 	// (understandably) a non-const pointer. Since the profile has a context-menu

qt-models/diveplannermodel.cpp

@@ -28,21 +28,21 @@ CylindersModel *DivePlannerPointsModel::cylindersModel()
 	return &cylinders;
 }
 
-void DivePlannerPointsModel::removePoints(const QVector<int> &rows)
+void DivePlannerPointsModel::removePoints(const std::vector<int> &rows)
 {
-	if (!rows.count())
+	if (rows.empty())
 		return;
-	QVector<int> v2 = rows;
+	std::vector<int> v2 = rows;
 	std::sort(v2.begin(), v2.end());
 
-	for (int i = v2.count() - 1; i >= 0; i--) {
+	for (int i = (int)v2.size() - 1; i >= 0; i--) {
 		beginRemoveRows(QModelIndex(), v2[i], v2[i]);
 		divepoints.erase(divepoints.begin() + v2[i]);
 		endRemoveRows();
 	}
 }
 
-void DivePlannerPointsModel::removeSelectedPoints(const QVector<int> &rows)
+void DivePlannerPointsModel::removeSelectedPoints(const std::vector<int> &rows)
 {
 	removePoints(rows);
 
@@ -230,7 +230,7 @@ bool DivePlannerPointsModel::updateMaxDepth()
 
 void DivePlannerPointsModel::removeDeco()
 {
-	QVector<int> computedPoints;
+	std::vector<int> computedPoints;
 	for (int i = 0; i < rowCount(); i++) {
 		if (!at(i).entered)
 			computedPoints.push_back(i);

qt-models/diveplannermodel.h

@@ -4,6 +4,7 @@
 
 #include <QAbstractTableModel>
 #include <QDateTime>
+#include <vector>
 
 #include "core/deco.h"
 #include "core/planner.h"
@@ -36,7 +37,7 @@ public:
 	Qt::ItemFlags flags(const QModelIndex &index) const override;
 	void gasChange(const QModelIndex &index, int newcylinderid);
 	void cylinderRenumber(int mapping[]);
-	void removeSelectedPoints(const QVector<int> &rows);
+	void removeSelectedPoints(const std::vector<int> &rows);
 	void setPlanMode(Mode mode);
 	bool isPlanner() const;
 	void createSimpleDive(struct dive *d);
@@ -116,7 +117,7 @@ private:
 	explicit DivePlannerPointsModel(QObject *parent = 0);
 	void clear();
 	int addStop(int millimeters, int seconds, int cylinderid_in, int ccpoint, bool entered, enum divemode_t);
-	void removePoints(const QVector<int> &rows);
+	void removePoints(const std::vector<int> &rows);
 	void setupStartTime();
 	void setupCylinders();
 	int lastEnteredPoint() const;