format-file related transcript widget methods return a GError which is used to display meaningful error messages

 * also cleaned up return value confusion: in GLib world, TRUE means successful

4 files changed, 114 insertions, 32 deletions

diff --git a/lib/gtk-experiment-widgets/gtk-experiment-transcript-formats.c b/lib/gtk-experiment-widgets/gtk-experiment-transcript-formats.c
index f2167e2..ba9de6c 100644
--- a/lib/gtk-experiment-widgets/gtk-experiment-transcript-formats.c
+++ b/lib/gtk-experiment-widgets/gtk-experiment-transcript-formats.c
@@ -28,6 +28,7 @@
 #include <stdio.h>
 #include <string.h>
 #include <assert.h>
+#include <errno.h>
 
 #include <glib.h>
 #include <glib/gprintf.h>
@@ -40,18 +41,22 @@
 #define FORMAT_REGEX_COMPILE_FLAGS	(G_REGEX_CASELESS)
 #define FORMAT_REGEX_MATCH_FLAGS	(0)
 
+/** @bug regexp after end of markup is ignored (e.g. "\b<u>ich</u>\b") */
 static gboolean
 gtk_experiment_transcript_parse_format(GtkExperimentTranscriptFormat *fmt,
-				       const gchar *str)
+				       const gchar *str,
+				       GError **error)
 {
 	PangoAttrIterator *iter;
 
 	gchar *pattern, pattern_captures[255], *p;
 	gint capture_count = 0;
 
+	g_return_val_if_fail(error == NULL || *error == NULL, FALSE);
+
 	if (!pango_parse_markup(str, -1, 0, &fmt->attribs, &pattern,
-				NULL, NULL))
-		return TRUE;
+				NULL, error))
+		return FALSE;
 
 	p = pattern_captures;
 	iter = pango_attr_list_get_iterator(fmt->attribs);
@@ -74,17 +79,28 @@ gtk_experiment_transcript_parse_format(GtkExperimentTranscriptFormat *fmt,
 	g_free(pattern);
 
 	fmt->regexp = g_regex_new(pattern_captures,
-				  FORMAT_REGEX_COMPILE_FLAGS, 0, NULL);
-	if (fmt->regexp == NULL ||
-	    g_regex_get_capture_count(fmt->regexp) != capture_count) {
+				  FORMAT_REGEX_COMPILE_FLAGS, 0, error);
+	if (fmt->regexp == NULL) {
+		gtk_experiment_transcript_free_format(fmt);
+		fmt->attribs = NULL;
+
+		return FALSE;
+	}
+
+	if (g_regex_get_capture_count(fmt->regexp) != capture_count) {
+		g_set_error(error,
+			    GTK_EXPERIMENT_TRANSCRIPT_ERROR,
+			    GTK_EXPERIMENT_TRANSCRIPT_ERROR_REGEXCAPTURES,
+			    "Additional regular expression captures not allowed");
+
 		gtk_experiment_transcript_free_format(fmt);
 		fmt->regexp = NULL;
 		fmt->attribs = NULL;
 
-		return TRUE;
+		return FALSE;
 	}
 
-	return FALSE;
+	return TRUE;
 }
 
 /** @private */
@@ -172,31 +188,45 @@ gtk_experiment_transcript_free_formats(GSList *formats)
  * @param trans    Widget instance
  * @param filename File name of format file to load (\c NULL or empty string
  *                 resets any formattings of a previously loaded file)
- * @return \c TRUE on error, else \c FALSE
+ * @param error    GError to set on failure, or \c NULL
+ * @return \c TRUE on success, else \c FALSE
  */
-/** @todo report errors using GError */
 gboolean
 gtk_experiment_transcript_load_formats(GtkExperimentTranscript *trans,
-				       const gchar *filename)
+				       const gchar *filename,
+				       GError **error)
 {
 	FILE *file;
 	gchar buf[255];
-	gboolean res = TRUE;
+	gint cur_line = 0;
+
+	gboolean res = FALSE;
+
+	g_return_val_if_fail(error == NULL || *error == NULL, FALSE);
 
 	gtk_experiment_transcript_free_formats(trans->priv->formats);
 	trans->priv->formats = NULL;
 
 	if (filename == NULL || !*filename) {
-		res = FALSE;
+		res = TRUE;
 		goto redraw;
 	}
 
-	if ((file = g_fopen(filename, "r")) == NULL)
+	if ((file = g_fopen(filename, "r")) == NULL) {
+		g_set_error(error,
+			    GTK_EXPERIMENT_TRANSCRIPT_ERROR,
+			    GTK_EXPERIMENT_TRANSCRIPT_ERROR_FILEOPEN,
+			    "Failed to open format file:\n%s",
+			    g_strerror(errno));
+
 		goto redraw;
+	}
 
-	while (fgets((char *)buf, sizeof(buf)-1, file) != NULL) {
+	/** @bug will fail for lines longer than sizeof(buf)-1 */
+	while (fgets((char *)buf, sizeof(buf), file) != NULL) {
 		GtkExperimentTranscriptFormat *fmt;
 
+		cur_line++;
 		g_strchug(buf);
 
 		switch (*buf) {
@@ -206,10 +236,15 @@ gtk_experiment_transcript_load_formats(GtkExperimentTranscript *trans,
 		case '\0':
 			continue;
 		}
+		/** @todo null-terminate buf at line end to avoid confusing Pango parse errors referring to line 2 */
 
 		fmt = g_new(GtkExperimentTranscriptFormat, 1);
 
-		if (gtk_experiment_transcript_parse_format(fmt, buf)) {
+		if (!gtk_experiment_transcript_parse_format(fmt, buf, error)) {
+			g_prefix_error(error,
+				       "Error parsing \"%s\" on line %d:\n",
+				       filename, cur_line);
+
 			g_free(fmt);
 			fclose(file);
 			gtk_experiment_transcript_free_formats(trans->priv->formats);
@@ -223,7 +258,7 @@ gtk_experiment_transcript_load_formats(GtkExperimentTranscript *trans,
 	trans->priv->formats = g_slist_reverse(trans->priv->formats);
 
 	fclose(file);
-	res = FALSE;
+	res = TRUE;
 
 redraw:
 	gtk_experiment_transcript_text_layer_redraw(trans);
@@ -250,36 +285,42 @@ redraw:
  * @param format_str  Format rule string (with or without markup)
  * @param with_markup Must be \c TRUE if the format_str contains Pango markup,
  *                    else \c FALSE
- * @return \c TRUE on error, else \c FALSE
+ * @param error       GError to set on failure, or \c NULL
+ * @return \c TRUE on success, else \c FALSE
  */
 gboolean
 gtk_experiment_transcript_set_interactive_format(GtkExperimentTranscript *trans,
 						 const gchar *format_str,
-						 gboolean with_markup)
+						 gboolean with_markup,
+						 GError **error)
 {
 	GtkExperimentTranscriptFormat *fmt = &trans->priv->interactive_format;
 
 	gchar *pattern;
 	PangoAttribute *attrib;
 
-	gboolean res = TRUE;
+	gboolean res = FALSE;
+
+	g_return_val_if_fail(error == NULL || *error == NULL, FALSE);
 
 	gtk_experiment_transcript_free_format(fmt);
 	fmt->regexp = NULL;
 	fmt->attribs = NULL;
 
 	if (format_str == NULL || !*format_str) {
-		res = FALSE;
+		res = TRUE;
 		goto redraw;
 	}
 
 	if (with_markup) {
-		res = gtk_experiment_transcript_parse_format(fmt, format_str);
+		res = gtk_experiment_transcript_parse_format(fmt, format_str,
+							     error);
 		goto redraw;
 	}
 	/* else if (!with_markup) */
 
 	fmt->attribs = pango_attr_list_new();
+	g_warn_if_fail(fmt->attribs != NULL);
 	if (fmt->attribs == NULL)
 		goto redraw;
 
@@ -308,17 +349,28 @@ gtk_experiment_transcript_set_interactive_format(GtkExperimentTranscript *trans,
 	}
 
 	pattern = g_strconcat("(", format_str, ")", NULL);
-	fmt->regexp = g_regex_new(pattern, FORMAT_REGEX_COMPILE_FLAGS, 0, NULL);
+	fmt->regexp = g_regex_new(pattern, FORMAT_REGEX_COMPILE_FLAGS, 0, error);
 	g_free(pattern);
-	if (fmt->regexp == NULL ||
-	    g_regex_get_capture_count(fmt->regexp) != 1) {
+	if (fmt->regexp == NULL) {
+		gtk_experiment_transcript_free_format(fmt);
+		fmt->attribs = NULL;
+
+		goto redraw;
+	}
+
+	if (g_regex_get_capture_count(fmt->regexp) != 1) {
+		g_set_error(error,
+			    GTK_EXPERIMENT_TRANSCRIPT_ERROR,
+			    GTK_EXPERIMENT_TRANSCRIPT_ERROR_REGEXCAPTURES,
+			    "Additional regular expression captures not allowed");
+
 		gtk_experiment_transcript_free_format(fmt);
 		fmt->regexp = NULL;
 		fmt->attribs = NULL;
 
 		goto redraw;
 	}
-	res = FALSE;
+	res = TRUE;
 
 redraw:
 	gtk_experiment_transcript_text_layer_redraw(trans);
diff --git a/lib/gtk-experiment-widgets/gtk-experiment-transcript.c b/lib/gtk-experiment-widgets/gtk-experiment-transcript.c
index d5b957c..b014dec 100644
--- a/lib/gtk-experiment-widgets/gtk-experiment-transcript.c
+++ b/lib/gtk-experiment-widgets/gtk-experiment-transcript.c
@@ -66,6 +66,13 @@ static void text_alignment_activated(GtkWidget *widget, gpointer data);
 
 static void reverse_activated(GtkWidget *widget, gpointer data);
 
+/** @private */
+GQuark
+gtk_experiment_transcript_error_quark(void)
+{
+	return g_quark_from_static_string("gtk-experiment-transcript-error-quark");
+}
+
 /**
  * @private
  * Will create \e gtk_experiment_transcript_get_type and set
diff --git a/lib/gtk-experiment-widgets/gtk-experiment-transcript.h b/lib/gtk-experiment-widgets/gtk-experiment-transcript.h
index 9d300b2..1657b0a 100644
--- a/lib/gtk-experiment-widgets/gtk-experiment-transcript.h
+++ b/lib/gtk-experiment-widgets/gtk-experiment-transcript.h
@@ -25,6 +25,7 @@
 #define __GTK_EXPERIMENT_TRANSCRIPT_H
 
 #include <glib-object.h>
+#include <glib.h>
 #include <gdk/gdk.h>
 
 #include <gtk/gtk.h>
@@ -33,6 +34,17 @@
 
 G_BEGIN_DECLS
 
+/** \e GtkExperimentTranscript error domain */
+#define GTK_EXPERIMENT_TRANSCRIPT_ERROR \
+	(gtk_experiment_transcript_error_quark())
+
+/** \e GtkExperimentTranscript error codes */
+typedef enum {
+	GTK_EXPERIMENT_TRANSCRIPT_ERROR_FILEOPEN,	/**< Error opening file */
+	GTK_EXPERIMENT_TRANSCRIPT_ERROR_REGEXCAPTURES	/**< Additional regular expression captures used */
+} GtkExperimentTranscriptError;
+
+/** \e GtkExperimentTranscript type */
 #define GTK_TYPE_EXPERIMENT_TRANSCRIPT \
 	(gtk_experiment_transcript_get_type())
 /**
@@ -87,6 +99,8 @@ typedef struct _GtkExperimentTranscriptClass {
 } GtkExperimentTranscriptClass;
 
 /** @private */
+GQuark gtk_experiment_transcript_error_quark(void);
+/** @private */
 GType gtk_experiment_transcript_get_type(void);
 
 /*
@@ -104,10 +118,12 @@ void gtk_experiment_transcript_set_alignment(GtkExperimentTranscript *trans,
 PangoAlignment gtk_experiment_transcript_get_alignment(GtkExperimentTranscript *trans);
 
 gboolean gtk_experiment_transcript_load_formats(GtkExperimentTranscript *trans,
-						const gchar *filename);
+						const gchar *filename,
+						GError **error);
 gboolean gtk_experiment_transcript_set_interactive_format(GtkExperimentTranscript *trans,
 							  const gchar *format_str,
-							  gboolean with_markup);
+							  gboolean with_markup,
+							  GError **error);
 
 GtkAdjustment *gtk_experiment_transcript_get_time_adjustment(GtkExperimentTranscript *trans);
 void gtk_experiment_transcript_set_time_adjustment(GtkExperimentTranscript *trans,
diff --git a/src/format-selection.c b/src/format-selection.c
index d61b381..bd024e1 100644
--- a/src/format-selection.c
+++ b/src/format-selection.c
@@ -109,6 +109,8 @@ generic_transcript_combo_changed_cb(gpointer user_data, GtkComboBox *combo)
 	GtkTreeIter iter;
 	gchar *filename = NULL;
 
+	GError *error = NULL;
+
 	if (gtk_combo_box_get_active_iter(combo, &iter))
 		gtk_tree_model_get(model, &iter, COL_FILENAME, &filename, -1);
 
@@ -116,7 +118,12 @@ generic_transcript_combo_changed_cb(gpointer user_data, GtkComboBox *combo)
 	 * filename may be empty (null-entry) in which case any active format
 	 * will be reset
 	 */
-	gtk_experiment_transcript_load_formats(trans, filename);
+	if (!gtk_experiment_transcript_load_formats(trans, filename, &error)) {
+		show_message_dialog_gerror(error);
+		g_error_free(error);
+
+		gtk_combo_box_set_active_iter(combo, NULL);
+	}
 	g_free(filename);
 
 #if 0
@@ -142,12 +149,12 @@ generic_transcript_entry_changed_cb(gpointer user_data, GtkEditable *editable)
 	isMarkup = gtk_toggle_button_get_active(toggle);
 
 	res = gtk_experiment_transcript_set_interactive_format(trans, text,
-							       isMarkup);
+							       isMarkup, NULL);
 
 	gtk_entry_set_icon_from_stock(GTK_ENTRY(editable),
 				      GTK_ENTRY_ICON_PRIMARY,
-				      res ? GTK_STOCK_DIALOG_ERROR
-					  : GTK_STOCK_APPLY);
+				      res ? GTK_STOCK_APPLY
+					  : GTK_STOCK_DIALOG_ERROR);
 	gtk_entry_set_icon_sensitive(GTK_ENTRY(editable),
 				     GTK_ENTRY_ICON_PRIMARY,
 				     text != NULL && *text);