Undo: remove ADD mode from main tab

Instead of letting the user edit the fields before adding a dive,
simply add an empty dive. Thus, the ADD mode of the main tab can
be removed.

Constructing a new dive with default-depth and making sure that
the dive is displayed correctly is very subtle. This all needs
to be detangled in due course.

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

7 files changed, 92 insertions, 168 deletions

diff --git a/desktop-widgets/command.cpp b/desktop-widgets/command.cpp
index 55fbeded9..ccfcbc524 100644
--- a/desktop-widgets/command.cpp
+++ b/desktop-widgets/command.cpp
@@ -9,9 +9,9 @@
 namespace Command {
 
 // Dive-list related commands
-void addDive(dive *d, const QString &newDS, bool autogroup, bool newNumber)
+void addDive(dive *d, bool autogroup, bool newNumber)
 {
-	execute(new AddDive(d, newDS, autogroup, newNumber));
+	execute(new AddDive(d, autogroup, newNumber));
 }
 
 void importDives(struct dive_table *dives, struct trip_table *trips, struct dive_site_table *sites, int flags, const QString &source)
diff --git a/desktop-widgets/command.h b/desktop-widgets/command.h
index 627e5360d..82cb06f31 100644
--- a/desktop-widgets/command.h
+++ b/desktop-widgets/command.h
@@ -20,9 +20,7 @@ QAction *redoAction(QObject *parent);	// Create an redo action.
 // distance are added to a trip. dive d is consumed (the structure is reset)!
 // If newNumber is true, the dive is assigned a new number, depending on the
 // insertion position.
-// Id newDS is not empty, a dive site with that name will be created. d->dive_site
-// should be null in this case.
-void addDive(dive *d, const QString &newDS, bool autogroup, bool newNumber);
+void addDive(dive *d, const bool autogroup, bool newNumber);
 void importDives(struct dive_table *dives, struct trip_table *trips, struct dive_site_table *sites, int flags, const QString &source);
 void deleteDive(const QVector<struct dive*> &divesToDelete);
 void shiftTime(const QVector<dive *> &changedDives, int amount);
diff --git a/desktop-widgets/command_divelist.cpp b/desktop-widgets/command_divelist.cpp
index e7bc31820..4e1359ea7 100644
--- a/desktop-widgets/command_divelist.cpp
+++ b/desktop-widgets/command_divelist.cpp
@@ -359,22 +359,14 @@ void DiveListBase::redo()
 	finishWork();
 }
 
-AddDive::AddDive(dive *d, const QString &newDS, bool autogroup, bool newNumber)
+AddDive::AddDive(dive *d, bool autogroup, bool newNumber)
 {
 	setText(tr("add dive"));
-	// By convention, d is "displayed dive" and can be overwritten.
+	// By convention, d is a pointer to "displayed dive" or a temporary variable and can be overwritten.
 	d->maxdepth.mm = 0;
 	d->dc.maxdepth.mm = 0;
 	fixup_dive(d);
 
-	// Create new dive site if requested.
-	if (!newDS.isEmpty()) {
-		struct dive_site *ds = alloc_dive_site();
-		ds->name = copy_qstring(newDS);
-		d->dive_site = ds;
-		divesToAdd.sites.emplace_back(ds);
-	}
-
 	// Get an owning pointer to a copied or moved dive
 	// Note: if move is true, this destroys the old dive!
 	OwningDivePtr divePtr(clone_dive(d));
diff --git a/desktop-widgets/command_divelist.h b/desktop-widgets/command_divelist.h
index 841799473..f8d1ba180 100644
--- a/desktop-widgets/command_divelist.h
+++ b/desktop-widgets/command_divelist.h
@@ -91,7 +91,7 @@ private:
 
 class AddDive : public DiveListBase {
 public:
-	AddDive(dive *dive, const QString &newDS, bool autogroup, bool newNumber);
+	AddDive(dive *dive, bool autogroup, bool newNumber);
 private:
 	void undoit() override;
 	void redoit() override;
diff --git a/desktop-widgets/mainwindow.cpp b/desktop-widgets/mainwindow.cpp
index 2a198110e..0e4daf0e2 100644
--- a/desktop-widgets/mainwindow.cpp
+++ b/desktop-widgets/mainwindow.cpp
@@ -177,7 +177,6 @@ MainWindow::MainWindow() : QMainWindow(),
 	disabledList.push_back(disabled);
 
 	registerApplicationState("Default", mainTab, profileContainer, diveList, mapWidget );
-	registerApplicationState("AddDive", mainTab, profileContainer, diveList, mapWidget );
 	registerApplicationState("EditDive", mainTab, profileContainer, diveList, mapWidget );
 	registerApplicationState("PlanDive", divePlannerWidget, profileContainer, divePlannerSettingsWidget, plannerDetails );
 	registerApplicationState("EditPlannedDive", divePlannerWidget, profileContainer, diveList, mapWidget );
@@ -185,7 +184,6 @@ MainWindow::MainWindow() : QMainWindow(),
 	registerApplicationState("FilterDive", mainTab, profileContainer, diveList, &filterWidget2);
 
 	setStateProperties("Default", enabledList, enabledList, enabledList, enabledList);
-	setStateProperties("AddDive", enabledList, enabledList, enabledList, enabledList);
 	setStateProperties("EditDive", enabledList, enabledList, enabledList, enabledList);
 	setStateProperties("PlanDive", enabledList, enabledList, enabledList, enabledList);
 	setStateProperties("EditPlannedDive", enabledList, enabledList, enabledList, enabledList);
@@ -212,7 +210,6 @@ MainWindow::MainWindow() : QMainWindow(),
 		connect(actionsRecent[i], SIGNAL(triggered(bool)), this, SLOT(recentFileTriggered(bool)));
 	}
 	ui.menuFile->insertSeparator(ui.actionQuit);
-	connect(mainTab, SIGNAL(addDiveFinished()), graphics, SLOT(setProfileState()));
 	connect(DivePlannerPointsModel::instance(), SIGNAL(planCreated()), this, SLOT(planCreated()));
 	connect(DivePlannerPointsModel::instance(), SIGNAL(planCanceled()), this, SLOT(planCanceled()));
 	connect(DivePlannerPointsModel::instance(), SIGNAL(variationsComputed(QString)), this, SLOT(updateVariations(QString)));
@@ -897,7 +894,6 @@ void MainWindow::setupForAddAndPlan(const char *model)
 	// setup the dive cylinders
 	DivePlannerPointsModel::instance()->clear();
 	DivePlannerPointsModel::instance()->setupCylinders();
-
 }
 
 void MainWindow::on_actionReplanDive_triggered()
@@ -958,31 +954,27 @@ void MainWindow::on_actionAddDive_triggered()
 	if (!plannerStateClean())
 		return;
 
-	if (diveList->selectedTrips().count() >= 1) {
-		diveList->rememberSelection();
-		diveList->clearSelection();
-	}
-
-	setApplicationState("AddDive");
-	DivePlannerPointsModel::instance()->setPlanMode(DivePlannerPointsModel::ADD);
-
-	// setup things so we can later create our starting dive
-	setupForAddAndPlan("manually added dive"); // don't translate, stored in the XML file
-
-	// now show the mostly empty main tab
-	mainTab->updateDiveInfo();
-
-	mainTab->addDiveStarted();
-
-	graphics->setAddState();
+	// TODO: We (mis)use displayed_dive to construct a default dive using the dive planner.
+	// This means that we have to do all this in a setPlanState()/setProfileState() pair,
+	// to avoid the profile and planner going out of sync (which in turn can lead to crashes).
+	// This should all be simplified. There is no apparent no reason to go via the planner
+	// to create a default profile.
+	clear_dive(&displayed_dive);
+	graphics->setPlanState();
+	DivePlannerPointsModel::instance()->setupStartTime();
 	DivePlannerPointsModel::instance()->createSimpleDive();
-	configureToolbar();
-	graphics->plotDive(nullptr, false, true);
+	displayed_dive.id = dive_getUniqID();
+	displayed_dive.when = QDateTime::currentMSecsSinceEpoch() / 1000L + gettimezoneoffset() + 3600;
+	displayed_dive.dc.model = strdup("manually added dive"); // don't translate! this is stored in the XML file
 	fixup_dc_duration(&displayed_dive.dc);
 	displayed_dive.duration = displayed_dive.dc.duration;
+	graphics->setProfileState();
 
-	// now that we have the correct depth and duration, update the dive info
-	mainTab->updateDepthDuration();
+	Command::addDive(&displayed_dive, autogroup, true);
+
+	// Plot dive actually copies current_dive to displayed_dive and therefore ensures that the
+	// correct data are displayed!
+	graphics->plotDive(nullptr, false, true);
 }
 
 void MainWindow::on_actionRenumber_triggered()
diff --git a/desktop-widgets/tab-widgets/maintab.cpp b/desktop-widgets/tab-widgets/maintab.cpp
index 605a9e5cb..37a42c2d3 100644
--- a/desktop-widgets/tab-widgets/maintab.cpp
+++ b/desktop-widgets/tab-widgets/maintab.cpp
@@ -234,16 +234,6 @@ void MainTab::toggleTriggeredColumn()
 		view->hideColumn(col);
 }
 
-void MainTab::addDiveStarted()
-{
-	ui.tabWidget->setCurrentIndex(0);
-	ui.tabWidget->setTabEnabled(2, false);
-	ui.tabWidget->setTabEnabled(3, false);
-	ui.tabWidget->setTabEnabled(4, false);
-	ui.tabWidget->setTabEnabled(5, false);
-	enableEdition(ADD);
-}
-
 void MainTab::addMessageAction(QAction *action)
 {
 	ui.diveNotesMessage->addAction(action);
@@ -320,9 +310,14 @@ void MainTab::enableEdition(EditMode newEditMode)
 
 static void profileFromDive(struct dive *d)
 {
+	// TODO: We have to put these manipulations into a setPlanState()/setProfileState() pair,
+	// because otherwise the DivePlannerPointsModel and the profile get out of sync.
+	// This can lead to crashes. Let's try to detangle these subtleties.
+	MainWindow::instance()->graphics->setPlanState();
 	DivePlannerPointsModel::instance()->loadFromDive(d);
 	MainWindow::instance()->graphics->setReplot(true);
 	MainWindow::instance()->graphics->plotDive(current_dive, true);
+	MainWindow::instance()->graphics->setProfileState();
 }
 
 // This function gets called if a field gets updated by an undo command.
@@ -434,16 +429,6 @@ bool MainTab::isEditing()
 	return editMode != NONE;
 }
 
-void MainTab::updateDepthDuration()
-{
-	ui.depth->setVisible(true);
-	ui.depthLabel->setVisible(true);
-	ui.duration->setVisible(true);
-	ui.durationLabel->setVisible(true);
-	ui.duration->setText(render_seconds_to_string(displayed_dive.duration.seconds));
-	ui.depth->setText(get_depth_string(displayed_dive.maxdepth, true));
-}
-
 void MainTab::updateNotes(const struct dive *d)
 {
 	QString tmp(d->notes);
@@ -493,7 +478,7 @@ void MainTab::updateDiveInfo(bool clear)
 	ui.location->refreshDiveSiteCache();
 	EditMode rememberEM = editMode;
 	// don't execute this while adding / planning a dive
-	if (editMode == ADD || editMode == MANUALLY_ADDED_DIVE || MainWindow::instance()->graphics->isPlanner())
+	if (editMode == MANUALLY_ADDED_DIVE || MainWindow::instance()->graphics->isPlanner())
 		return;
 	if (!isEnabled() && !clear )
 		setEnabled(true);
@@ -748,105 +733,78 @@ void MainTab::acceptChanges()
 	ui.dateEdit->setEnabled(true);
 	hideMessage();
 	ui.equipmentTab->setEnabled(true);
-	if (lastMode == ADD) {
-		// Handle dive site
-		struct dive_site *pickedDs = ui.location->currDiveSite();
-		QString newDiveSiteName;
-		if (pickedDs == RECENTLY_ADDED_DIVESITE) {
-			newDiveSiteName = ui.location->text();
-			displayed_dive.dive_site = nullptr;
-		} else {
-			displayed_dive.dive_site = pickedDs;
-		}
 
-		copyTagsToDisplayedDive();
-
-		Command::addDive(&displayed_dive, newDiveSiteName, autogroup, true);
+	// Get list of selected dives, but put the current dive last;
+	// this is required in case the invocation wants to compare things
+	// to the original value in current_dive like it should
+	QVector<dive *> selectedDives = getSelectedDivesCurrentLast();
+	if (lastMode == MANUALLY_ADDED_DIVE) {
+		// preserve any changes to the profile
+		free(current_dive->dc.sample);
+		copy_samples(&displayed_dive.dc, &current_dive->dc);
+		addedId = displayed_dive.id;
+	}
+	// now check if something has changed and if yes, edit the selected dives that
+	// were identical with the master dive shown (and mark the divelist as changed)
+	struct dive *cd = current_dive;
 
-		editMode = NONE;
-		MainWindow::instance()->exitEditState();
-		cylindersModel->changed = false;
-		weightModel->changed = false;
-		MainWindow::instance()->setEnabledToolbar(true);
-		ui.editDiveSiteButton->setEnabled(!ui.location->text().isEmpty());
-		emit addDiveFinished();
-		DivePlannerPointsModel::instance()->setPlanMode(DivePlannerPointsModel::NOTHING);
-		MainWindow::instance()->diveList->setFocus();
-		displayed_dive.divetrip = nullptr; // Should not be necessary, just in case!
-		return;
-	} else {
-		// Get list of selected dives, but put the current dive last;
-		// this is required in case the invocation wants to compare things
-		// to the original value in current_dive like it should
-		QVector<dive *> selectedDives = getSelectedDivesCurrentLast();
-		if (lastMode == MANUALLY_ADDED_DIVE) {
-			// preserve any changes to the profile
-			free(current_dive->dc.sample);
-			copy_samples(&displayed_dive.dc, &current_dive->dc);
-			addedId = displayed_dive.id;
-		}
-		// now check if something has changed and if yes, edit the selected dives that
-		// were identical with the master dive shown (and mark the divelist as changed)
-		struct dive *cd = current_dive;
-
-		if (cylindersModel->changed) {
-			mark_divelist_changed(true);
-			MODIFY_DIVES(selectedDives,
-				for (int i = 0; i < MAX_CYLINDERS; i++) {
-					if (mydive != cd) {
-						if (same_string(mydive->cylinder[i].type.description, cd->cylinder[i].type.description)) {
-							// if we started out with the same cylinder description (for multi-edit) or if we do copt & paste
-							// make sure that we have the same cylinder type and copy the gasmix, but DON'T copy the start
-							// and end pressures (those are per dive after all)
-							if (!same_string(mydive->cylinder[i].type.description, displayed_dive.cylinder[i].type.description)) {
-								free((void*)mydive->cylinder[i].type.description);
-								mydive->cylinder[i].type.description = copy_string(displayed_dive.cylinder[i].type.description);
-							}
-							mydive->cylinder[i].type.size = displayed_dive.cylinder[i].type.size;
-							mydive->cylinder[i].type.workingpressure = displayed_dive.cylinder[i].type.workingpressure;
-							mydive->cylinder[i].gasmix = displayed_dive.cylinder[i].gasmix;
-							mydive->cylinder[i].cylinder_use = displayed_dive.cylinder[i].cylinder_use;
-							mydive->cylinder[i].depth = displayed_dive.cylinder[i].depth;
+	if (cylindersModel->changed) {
+		mark_divelist_changed(true);
+		MODIFY_DIVES(selectedDives,
+			for (int i = 0; i < MAX_CYLINDERS; i++) {
+				if (mydive != cd) {
+					if (same_string(mydive->cylinder[i].type.description, cd->cylinder[i].type.description)) {
+						// if we started out with the same cylinder description (for multi-edit) or if we do copt & paste
+						// make sure that we have the same cylinder type and copy the gasmix, but DON'T copy the start
+						// and end pressures (those are per dive after all)
+						if (!same_string(mydive->cylinder[i].type.description, displayed_dive.cylinder[i].type.description)) {
+							free((void*)mydive->cylinder[i].type.description);
+							mydive->cylinder[i].type.description = copy_string(displayed_dive.cylinder[i].type.description);
 						}
+						mydive->cylinder[i].type.size = displayed_dive.cylinder[i].type.size;
+						mydive->cylinder[i].type.workingpressure = displayed_dive.cylinder[i].type.workingpressure;
+						mydive->cylinder[i].gasmix = displayed_dive.cylinder[i].gasmix;
+						mydive->cylinder[i].cylinder_use = displayed_dive.cylinder[i].cylinder_use;
+						mydive->cylinder[i].depth = displayed_dive.cylinder[i].depth;
 					}
 				}
-			);
-			for (int i = 0; i < MAX_CYLINDERS; i++) {
-				// copy the cylinder but make sure we have our own copy of the strings
-				free((void*)cd->cylinder[i].type.description);
-				cd->cylinder[i] = displayed_dive.cylinder[i];
-				cd->cylinder[i].type.description = copy_string(displayed_dive.cylinder[i].type.description);
-			}
-			/* if cylinders changed we may have changed gas change events
-			 * and sensor idx in samples as well
-			 * - so far this is ONLY supported for a single selected dive */
-			struct divecomputer *tdc = &current_dive->dc;
-			struct divecomputer *sdc = &displayed_dive.dc;
-			while(tdc && sdc) {
-				free_events(tdc->events);
-				copy_events(sdc, tdc);
-				free(tdc->sample);
-				copy_samples(sdc, tdc);
-				tdc = tdc->next;
-				sdc = sdc->next;
 			}
-			do_replot = true;
+		);
+		for (int i = 0; i < MAX_CYLINDERS; i++) {
+			// copy the cylinder but make sure we have our own copy of the strings
+			free((void*)cd->cylinder[i].type.description);
+			cd->cylinder[i] = displayed_dive.cylinder[i];
+			cd->cylinder[i].type.description = copy_string(displayed_dive.cylinder[i].type.description);
 		}
+		/* if cylinders changed we may have changed gas change events
+		 * and sensor idx in samples as well
+		 * - so far this is ONLY supported for a single selected dive */
+		struct divecomputer *tdc = &current_dive->dc;
+		struct divecomputer *sdc = &displayed_dive.dc;
+		while(tdc && sdc) {
+			free_events(tdc->events);
+			copy_events(sdc, tdc);
+			free(tdc->sample);
+			copy_samples(sdc, tdc);
+			tdc = tdc->next;
+			sdc = sdc->next;
+		}
+		do_replot = true;
+	}
 
-		if (weightModel->changed) {
-			mark_divelist_changed(true);
-			MODIFY_DIVES(selectedDives,
-				for (int i = 0; i < MAX_WEIGHTSYSTEMS; i++) {
-					if (mydive != cd && (same_string(mydive->weightsystem[i].description, cd->weightsystem[i].description))) {
-						mydive->weightsystem[i] = displayed_dive.weightsystem[i];
-						mydive->weightsystem[i].description = copy_string(displayed_dive.weightsystem[i].description);
-					}
-				}
-			);
+	if (weightModel->changed) {
+		mark_divelist_changed(true);
+		MODIFY_DIVES(selectedDives,
 			for (int i = 0; i < MAX_WEIGHTSYSTEMS; i++) {
-				cd->weightsystem[i] = displayed_dive.weightsystem[i];
-				cd->weightsystem[i].description = copy_string(displayed_dive.weightsystem[i].description);
+				if (mydive != cd && (same_string(mydive->weightsystem[i].description, cd->weightsystem[i].description))) {
+					mydive->weightsystem[i] = displayed_dive.weightsystem[i];
+					mydive->weightsystem[i].description = copy_string(displayed_dive.weightsystem[i].description);
+				}
 			}
+		);
+		for (int i = 0; i < MAX_WEIGHTSYSTEMS; i++) {
+			cd->weightsystem[i] = displayed_dive.weightsystem[i];
+			cd->weightsystem[i].description = copy_string(displayed_dive.weightsystem[i].description);
 		}
 	}
 	if (lastMode == MANUALLY_ADDED_DIVE) {
@@ -904,10 +862,8 @@ void MainTab::rejectChanges()
 	tabBar()->setTabIcon(0, QIcon()); // Notes
 	tabBar()->setTabIcon(1, QIcon()); // Equipment
 	hideMessage();
-	// no harm done to call cancelPlan even if we were not in ADD or PLAN mode...
+	// no harm done to call cancelPlan even if we were not PLAN mode...
 	DivePlannerPointsModel::instance()->cancelPlan();
-	if(lastMode == ADD)
-		MainWindow::instance()->diveList->restoreSelection();
 
 	// now make sure that the correct dive is displayed
 	if (current_dive)
@@ -1027,15 +983,6 @@ void MainTab::on_timeEdit_timeChanged(const QTime &time)
 	shiftTime(dateTime);
 }
 
-void MainTab::copyTagsToDisplayedDive()
-{
-	taglist_free(displayed_dive.tag_list);
-	displayed_dive.tag_list = NULL;
-	Q_FOREACH (const QString &tag, ui.tagWidget->getBlockStringList())
-		taglist_add_tag(&displayed_dive.tag_list, qPrintable(tag));
-	taglist_cleanup(&displayed_dive.tag_list);
-}
-
 void MainTab::on_tagWidget_editingFinished()
 {
 	if (editMode == IGNORE || !current_dive)
diff --git a/desktop-widgets/tab-widgets/maintab.h b/desktop-widgets/tab-widgets/maintab.h
index 2108b7b42..6ae263332 100644
--- a/desktop-widgets/tab-widgets/maintab.h
+++ b/desktop-widgets/tab-widgets/maintab.h
@@ -39,7 +39,6 @@ public:
 	enum EditMode {
 		NONE,
 		DIVE,
-		ADD,
 		MANUALLY_ADDED_DIVE,
 		IGNORE
 	};
@@ -56,7 +55,6 @@ public:
 	void nextInputField(QKeyEvent *event);
 
 signals:
-	void addDiveFinished();
 	void diveSiteChanged();
 public
 slots:
@@ -69,7 +67,6 @@ slots:
 	void updateMode(struct dive *d);
 	void updateDateTime(struct dive *d);
 	void updateDiveSite(struct dive *d);
-	void updateDepthDuration();
 	void acceptChanges();
 	void rejectChanges();
 	void on_location_diveSiteSelected();
@@ -90,7 +87,6 @@ slots:
 	void on_tagWidget_editingFinished();
 	void editCylinderWidget(const QModelIndex &index);
 	void editWeightWidget(const QModelIndex &index);
-	void addDiveStarted();
 	void addMessageAction(QAction *action);
 	void hideMessage();
 	void closeMessage();
@@ -114,7 +110,6 @@ private:
 	bool lastSelectedDive;
 	int lastTabSelectedDive;
 	int lastTabSelectedDiveTrip;
-	void copyTagsToDisplayedDive();
 	dive_trip_t *currentTrip;
 	QList<TabBase*> extraWidgets;
 };