Merge pull request #726 from Subsurface-divelog/rewriteErrorHandling

Rewrite error handling

core/CMakeLists.txt

@@ -40,6 +40,7 @@ set(SUBSURFACE_CORE_LIB_SRCS
 	divesite.cpp
 	divelist.c
 	equipment.c
+	errorhelper.c
 	file.c
 	gas-model.c
 	git-access.c

core/cloudstorage.cpp

@@ -61,6 +61,7 @@ void CloudStorageAuthenticate::uploadFinished()
 		myLastError.clear();
 	} else if (cloudAuthReply == QLatin1String("[VERIFY]")) {
 		csSettings.setVerificationStatus(CS_NEED_TO_VERIFY);
+		report_error(qPrintable(tr("Cloud account verification required, enter PIN in preferences")));
 	} else if (cloudAuthReply == QLatin1String("[PASSWDCHANGED]")) {
 		free(prefs.cloud_storage_password);
 		prefs.cloud_storage_password = prefs.cloud_storage_newpassword;

core/dive.h

@@ -698,6 +698,7 @@ extern "C" {
 extern int report_error(const char *fmt, ...);
 extern void report_message(const char *msg);
 extern const char *get_error_string(void);
+extern void set_error_cb(void(*cb)(void));
 
 extern struct dive *find_dive_including(timestamp_t when);
 extern bool dive_within_time_range(struct dive *dive, timestamp_t when, timestamp_t offset);

core/errorhelper.c

@@ -0,0 +1,55 @@
+// SPDX-License-Identifier: GPL-2.0
+#ifdef __clang__
+// Clang has a bug on zero-initialization of C structs.
+#pragma clang diagnostic ignored "-Wmissing-field-initializers"
+#endif
+#include "dive.h"
+#include "membuffer.h"
+
+#define VA_BUF(b, fmt) do { va_list args; va_start(args, fmt); put_vformat(b, fmt, args); va_end(args); } while (0)
+
+static struct membuffer error_string_buffer = { 0 };
+static void (*error_cb)(void) = NULL;
+/*
+ * Note that the act of "getting" the error string
+ * buffer doesn't de-allocate the buffer, but it does
+ * set the buffer length to zero, so that any future
+ * error reports will overwrite the string rather than
+ * append to it.
+ */
+const char *get_error_string(void)
+{
+	const char *str;
+
+	if (!error_string_buffer.len)
+		return "";
+	str = mb_cstring(&error_string_buffer);
+	error_string_buffer.len = 0;
+	return str;
+}
+
+int report_error(const char *fmt, ...)
+{
+	struct membuffer *buf = &error_string_buffer;
+
+	/* Previous unprinted errors? Add a newline in between */
+	if (buf->len)
+		put_bytes(buf, "\n", 1);
+	VA_BUF(buf, fmt);
+	mb_cstring(buf);
+
+	/* if an error callback is registered, call it */
+	if (error_cb)
+		error_cb();
+
+	return -1;
+}
+
+void report_message(const char *msg)
+{
+	(void)report_error("%s", msg);
+}
+
+void set_error_cb(void(*cb)(void)) {
+	error_cb = cb;
+}

core/save-git.c

@@ -440,42 +440,6 @@ static void create_dive_buffer(struct dive *dive, struct membuffer *b)
 	save_dive_temperature(b, dive);
 }
 
-static struct membuffer error_string_buffer = { 0 };
-
-/*
- * Note that the act of "getting" the error string
- * buffer doesn't de-allocate the buffer, but it does
- * set the buffer length to zero, so that any future
- * error reports will overwrite the string rather than
- * append to it.
- */
-const char *get_error_string(void)
-{
-	const char *str;
-
-	if (!error_string_buffer.len)
-		return "";
-	str = mb_cstring(&error_string_buffer);
-	error_string_buffer.len = 0;
-	return str;
-}
-
-int report_error(const char *fmt, ...)
-{
-	struct membuffer *buf = &error_string_buffer;
-
-	/* Previous unprinted errors? Add a newline in between */
-	if (buf->len)
-		put_bytes(buf, "\n", 1);
-	VA_BUF(buf, fmt);
-	mb_cstring(buf);
-	return -1;
-}
-
-void report_message(const char *msg)
-{
-	(void)report_error("%s", msg);
-}
 
 /*
  * libgit2 has a "git_treebuilder" concept, but it's broken, and can not

core/subsurfacestartup.c

@@ -163,20 +163,24 @@ void print_files()
 	const char *remote = 0;
 	const char *filename, *local_git;
 
-	filename = cloud_url();
-
-	is_git_repository(filename, &branch, &remote, true);
 	printf("\nFile locations:\n\n");
+	if (!same_string(prefs.cloud_storage_email, "") && !same_string(prefs.cloud_storage_password, "")) {
+		filename = cloud_url();
+
+		is_git_repository(filename, &branch, &remote, true);
+	} else {
+		/* strdup so the free below works in either case */
+		filename = strdup("No valid cloud credentials set.\n");
+	}
 	if (branch && remote) {
 		local_git = get_local_dir(remote, branch);
 		printf("Local git storage: %s\n", local_git);
 	} else {
 		printf("Unable to get local git directory\n");
 	}
-	char *tmp = cloud_url();
-	printf("Cloud URL: %s\n", tmp);
-	free(tmp);
-	tmp = hashfile_name_string();
+	printf("Cloud URL: %s\n", filename);
+	free((void *)filename);
+	char *tmp = hashfile_name_string();
 	printf("Image hashes: %s\n", tmp);
 	free(tmp);
 	tmp = picturedir_string();

desktop-widgets/divelistview.cpp

@@ -983,7 +983,7 @@ void DiveListView::loadImageFromURL(QUrl url)
 		if (image.isNull()) {
 			// If this is not an image, maybe it's an html file and Miika can provide some xslr magic to extract images.
 			// In this case we would call the function recursively on the list of image source urls;
-			MainWindow::instance()->getNotificationWidget()->showNotification(tr("%1 does not appear to be an image").arg(url.toString()), KMessageWidget::Error);
+			report_error(qPrintable(tr("%1 does not appear to be an image").arg(url.toString())));
 			return;
 		}
 

desktop-widgets/mainwindow.cpp

@@ -86,6 +86,15 @@ extern "C" int updateProgress(const char *text)
 
 MainWindow *MainWindow::m_Instance = NULL;
 
+extern "C" void showErrorFromC()
+{
+	MainWindow *mainwindow = MainWindow::instance();
+	if (mainwindow) {
+		mainwindow->getNotificationWidget()->showNotification(get_error_string(), KMessageWidget::Error);
+	}
+}
+
+
 MainWindow::MainWindow() : QMainWindow(),
 	actionNextDive(0),
 	actionPreviousDive(0),
@@ -244,6 +253,7 @@ MainWindow::MainWindow() : QMainWindow(),
 
 	setupSocialNetworkMenu();
 	set_git_update_cb(&updateProgress);
+	set_error_cb(&showErrorFromC);
 
 	// Toolbar Connections related to the Profile Update
 	SettingsObjectWrapper *sWrapper = SettingsObjectWrapper::instance();
@@ -287,7 +297,6 @@ MainWindow::MainWindow() : QMainWindow(),
 
 	// now let's set up some connections
 	connect(graphics(), &ProfileWidget2::enableToolbar ,this, &MainWindow::setEnabledToolbar);
-	connect(graphics(), &ProfileWidget2::showError, this, &MainWindow::showError);
 	connect(graphics(), &ProfileWidget2::disableShortcuts, this, &MainWindow::disableShortcuts);
 	connect(graphics(), &ProfileWidget2::enableShortcuts, this, &MainWindow::enableShortcuts);
 	connect(graphics(), &ProfileWidget2::refreshDisplay, this, &MainWindow::refreshDisplay);
@@ -429,7 +438,6 @@ MainWindow *MainWindow::instance()
 // this gets called after we download dives from a divecomputer
 void MainWindow::refreshDisplay(bool doRecreateDiveList)
 {
-	getNotificationWidget()->showNotification(get_error_string(), KMessageWidget::Error);
 	information()->reload();
 	TankInfoModel::instance()->update();
 	MapWidget::instance()->reload();
@@ -562,10 +570,9 @@ void MainWindow::on_actionCloudstorageopen_triggered()
 		return;
 
 	QString filename;
-	if (getCloudURL(filename)) {
-		getNotificationWidget()->showNotification(get_error_string(), KMessageWidget::Error);
+	if (getCloudURL(filename))
 		return;
-	}
+
 	if (verbose)
 		qDebug() << "Opening cloud storage from:" << filename;
 
@@ -591,13 +598,12 @@ void MainWindow::on_actionCloudstoragesave_triggered()
 {
 	QString filename;
 	if (!dive_table.nr) {
-		getNotificationWidget()->showNotification(tr("Don't save an empty log to the cloud"), KMessageWidget::Error);
+		report_error(qPrintable(tr("Don't save an empty log to the cloud")));
 		return;
 	}
-	if (getCloudURL(filename)) {
-		getNotificationWidget()->showNotification(get_error_string(), KMessageWidget::Error);
+	if (getCloudURL(filename))
 		return;
-	}
+
 	if (verbose)
 		qDebug() << "Saving cloud storage to:" << filename;
 	if (information()->isEditing())
@@ -605,14 +611,11 @@ void MainWindow::on_actionCloudstoragesave_triggered()
 
 	showProgressBar();
 
-	if (save_dives(filename.toUtf8().data())) {
-		getNotificationWidget()->showNotification(get_error_string(), KMessageWidget::Error);
+	if (save_dives(filename.toUtf8().data()))
 		return;
-	}
 
 	hideProgressBar();
 
-	getNotificationWidget()->showNotification(get_error_string(), KMessageWidget::Error);
 	set_filename(filename.toUtf8().data(), true);
 	setTitle(MWTF_FILENAME);
 	mark_divelist_changed(false);
@@ -1646,12 +1649,9 @@ int MainWindow::file_save_as(void)
 	if (information()->isEditing())
 		information()->acceptChanges();
 
-	if (save_dives(filename.toUtf8().data())) {
-		getNotificationWidget()->showNotification(get_error_string(), KMessageWidget::Error);
+	if (save_dives(filename.toUtf8().data()))
 		return -1;
-	}
 
-	getNotificationWidget()->showNotification(get_error_string(), KMessageWidget::Error);
 	set_filename(filename.toUtf8().data(), true);
 	setTitle(MWTF_FILENAME);
 	mark_divelist_changed(false);
@@ -1683,14 +1683,12 @@ int MainWindow::file_save(void)
 	if (is_cloud)
 		showProgressBar();
 	if (save_dives(existing_filename)) {
-		getNotificationWidget()->showNotification(get_error_string(), KMessageWidget::Error);
 		if (is_cloud)
 			hideProgressBar();
 		return -1;
 	}
 	if (is_cloud)
 		hideProgressBar();
-	getNotificationWidget()->showNotification(get_error_string(), KMessageWidget::Error);
 	mark_divelist_changed(false);
 	addRecentFile(QStringList() << QString(existing_filename));
 	return 0;
@@ -1701,11 +1699,6 @@ NotificationWidget *MainWindow::getNotificationWidget()
 	return ui.mainErrorMessage;
 }
 
-void MainWindow::showError()
-{
-	getNotificationWidget()->showNotification(get_error_string(), KMessageWidget::Error);
-}
-
 QString MainWindow::displayedFilename(QString fullFilename)
 {
 	QFile f(fullFilename);
@@ -1795,7 +1788,6 @@ void MainWindow::importTxtFiles(const QStringList fileNames)
 
 void MainWindow::loadFiles(const QStringList fileNames)
 {
-	bool showWarning = false;
 	if (fileNames.isEmpty()) {
 		refreshDisplay();
 		return;
@@ -1812,19 +1804,11 @@ void MainWindow::loadFiles(const QStringList fileNames)
 		if (!error) {
 			set_filename(fileNamePtr.data(), true);
 			setTitle(MWTF_FILENAME);
-			// if there were any messages, show them
-			QString warning = get_error_string();
-			if (!warning.isEmpty()) {
-				showWarning = true;
-				getNotificationWidget()->showNotification(warning , KMessageWidget::Information);
-			}
 		} else {
 			failedParses.append(fileNames.at(i));
 		}
 	}
 	hideProgressBar();
-	if (!showWarning)
-		getNotificationWidget()->hideNotification();
 	process_dives(false, false);
 	addRecentFile(fileNames);
 	removeRecentFile(failedParses);

desktop-widgets/mainwindow.h

@@ -87,7 +87,6 @@ public:
 	QUndoStack *undoStack;
 	NotificationWidget *getNotificationWidget();
 	void enableDisableCloudActions();
-	void showError();
 
 private
 slots:

desktop-widgets/subsurfacewebservices.cpp

@@ -735,7 +735,6 @@ void DivelogsDeWebServices::prepareDivesForUpload(bool selected)
 	} else {
 		report_error("Failed to create upload file %s\n", qPrintable(filename));
 	}
-	MainWindow::instance()->getNotificationWidget()->showNotification(get_error_string(), KMessageWidget::Error);
 }
 
 void DivelogsDeWebServices::uploadDives(QIODevice *dldContent)

profile-widget/profilewidget2.cpp

@@ -709,7 +709,7 @@ void ProfileWidget2::plotDive(struct dive *d, bool force)
 
 #ifdef SUBSURFACE_MOBILE
 	if (currentdc->divemode == CCR) {
-		gasYAxis->setPos(itemPos.partialPressure.pos.on);	
+		gasYAxis->setPos(itemPos.partialPressure.pos.on);
 		gasYAxis->setLine(itemPos.partialPressure.expanded);
 
 		tankItem->setVisible(false);
@@ -802,7 +802,6 @@ void ProfileWidget2::plotDive(struct dive *d, bool force)
 		report_error(qPrintable(tr("Show NDL / TTS was disabled because of excessive processing time")));
 	}
 #endif
-	emit showError();
 }
 
 void ProfileWidget2::recalcCeiling()

profile-widget/profilewidget2.h

@@ -90,7 +90,6 @@ public:
 signals:
 	void fontPrintScaleChanged(double scale);
 	void enableToolbar(bool enable);
-	void showError();
 	void enableShortcuts();
 	void disableShortcuts(bool paste);
 	void refreshDisplay(bool recreateDivelist);

subsurface-desktop-main.cpp

@@ -104,8 +104,6 @@ int main(int argc, char **argv)
 	filesOnCommandLine = !files.isEmpty() || !importedFiles.isEmpty();
 	m->loadFiles(files);
 	m->importFiles(importedFiles);
-	// in case something has gone wrong make sure we show the error message
-	m->showError();
 
 	if (verbose > 0) {
 		print_files();