GNOME.org
 

[gnome-system-tools] Improve error reporting by using a wrapper around oobs_object_commit()

commit 3af979fbb76eb5459ba5e68a23a51fb18a4fc1dd
Author: Milan Bouchet-Valat <nalimilan club fr>
Date:   Wed Aug 19 13:02:11 2009 +0200

    Improve error reporting by using a wrapper around oobs_object_commit()
    
    Never use oobs_object_commit() directly, but call gst_tool_commit(), which will take care of showing an appropriate error if needed. This does not fix the case where shown data is inconsistent with system's, but at least the user is properly warn.
    Also improve already present error reporting in gst-tool.c with a new function show_oobs_error_dialog() which handles all kind of errors (except platform).

 src/common/gst-tool.c         |   74 ++++++++++++++++++++++++++++++----------
 src/common/gst-tool.h         |    3 ++
 src/network/callbacks.c       |    6 ++--
 src/network/hosts.c           |    2 +-
 src/network/locations-combo.c |    2 +-
 src/network/network-tool.c    |    4 +-
 src/services/callbacks.c      |    4 +-
 src/shares/callbacks.c        |   10 +++---
 src/shares/share-settings.c   |    8 ++--
 src/time/time-tool.c          |    4 +-
 src/users/callbacks.c         |   18 +++++-----
 src/users/user-settings.c     |    2 +-
 12 files changed, 88 insertions(+), 49 deletions(-)
---
diff --git a/src/common/gst-tool.c b/src/common/gst-tool.c
index f778b01..77404bb 100644
--- a/src/common/gst-tool.c
+++ b/src/common/gst-tool.c
@@ -194,6 +194,42 @@ show_access_denied_dialog (GstTool *tool,
 	gtk_widget_destroy (dialog);
 }
 
+/* Handle all oobs errors but OOBS_RESULT_NO_PLATFORM, which should happen in specific cases */
+static void
+show_oobs_error_dialog (GstTool   *tool,
+			int        operation,
+			OobsResult result)
+{
+	GtkWidget *dialog;
+	const gchar *primary_text, *secondary_text;
+
+	/* Complex case to treat separately */
+	if (result == OOBS_RESULT_ACCESS_DENIED) {
+		show_access_denied_dialog (tool, operation);
+		return;
+	}
+
+	if (operation == OPERATION_UPDATE)
+		primary_text = N_("The configuration could not be loaded");
+	else
+		primary_text = N_("The configuration could not be saved");
+
+	if (result == OOBS_RESULT_MALFORMED_DATA)
+		secondary_text = N_("Invalid data was found.");
+	else /* OOBS_RESULT_ERROR */
+		secondary_text = N_("An unknown error occurred.");
+
+	dialog = gtk_message_dialog_new (GTK_WINDOW (tool->main_dialog),
+					 GTK_DIALOG_MODAL,
+					 GTK_MESSAGE_ERROR,
+					 GTK_BUTTONS_CLOSE,
+					 "%s", _(primary_text));
+	gtk_message_dialog_format_secondary_text (GTK_MESSAGE_DIALOG (dialog),
+						  "%s", _(secondary_text));
+	gtk_dialog_run (GTK_DIALOG (dialog));
+	gtk_widget_destroy (dialog);
+}
+
 static GObject*
 gst_tool_constructor (GType                  type,
 		      guint                  n_construct_properties,
@@ -226,20 +262,14 @@ gst_tool_constructor (GType                  type,
 	}
 
 	result = oobs_session_get_platform (tool->session, NULL);
-
-	switch (result) {
-	case OOBS_RESULT_NO_PLATFORM:
+	if (result == OOBS_RESULT_NO_PLATFORM) {
 		dialog = gst_platform_dialog_new (tool->session);
 		gtk_dialog_run (GTK_DIALOG (dialog));
 		gtk_widget_destroy (dialog);
-		break;
-	case OOBS_RESULT_ACCESS_DENIED:
-	case OOBS_RESULT_ERROR:
-		show_access_denied_dialog (tool, OPERATION_UPDATE);
+	}
+	else if (result != OOBS_RESULT_OK) {
+		show_oobs_error_dialog (tool, OPERATION_UPDATE, result);
 		exit (-1);
-		break;
-	default:
-		break;
 	}
 
 	gst_tool_update_async (tool);
@@ -472,6 +502,19 @@ gst_tool_hide_report_window (GstTool *tool)
 	gtk_widget_hide (tool->report_window);
 }
 
+/* Simple wrapper around oobs_object_commit(), that shows an error if needed */
+OobsResult
+gst_tool_commit (GstTool    *tool,
+		 OobsObject *object)
+{
+	OobsResult result = oobs_object_commit (object);
+
+	if (result != OOBS_RESULT_OK)
+		show_oobs_error_dialog (tool, OPERATION_COMMIT, result);
+
+	return result;
+}
+
 static void
 on_commit_finalized (OobsObject *object,
 		     OobsResult  result,
@@ -481,15 +524,8 @@ on_commit_finalized (OobsObject *object,
 
 	gst_tool_hide_report_window (user_data->tool);
 
-	switch (result) {
-	case OOBS_RESULT_ACCESS_DENIED:
-		show_access_denied_dialog (user_data->tool, OPERATION_COMMIT);
-		break;
-	case OOBS_RESULT_OK:
-	default:
-		/* FIXME: handle other errors */
-		break;
-	}
+	if (result != OOBS_RESULT_OK)
+		show_oobs_error_dialog (user_data->tool, OPERATION_COMMIT, result);
 
 	if (user_data->func)
 		(* user_data->func) (object, result, user_data->data);
diff --git a/src/common/gst-tool.h b/src/common/gst-tool.h
index 13271ca..1f1f7f2 100644
--- a/src/common/gst-tool.h
+++ b/src/common/gst-tool.h
@@ -97,6 +97,9 @@ GtkIconTheme *gst_tool_get_icon_theme     (GstTool *tool);
 
 void         gst_tool_show_help       (GstTool*, gchar*);
 
+OobsResult   gst_tool_commit          (GstTool *tool,
+				       OobsObject *object);
+
 void         gst_tool_commit_async    (GstTool             *tool,
 				       OobsObject          *object,
 				       const gchar         *message,
diff --git a/src/network/callbacks.c b/src/network/callbacks.c
index a84f6e4..b5552c6 100644
--- a/src/network/callbacks.c
+++ b/src/network/callbacks.c
@@ -426,7 +426,7 @@ on_host_aliases_delete_clicked (GtkWidget *widget, gpointer data)
       gtk_list_store_remove (GTK_LIST_STORE (model), &iter);
 
       oobs_list_iter_free (list_iter);
-      oobs_object_commit (OOBS_OBJECT (GST_NETWORK_TOOL (tool)->hosts_config));
+      gst_tool_commit (tool, OOBS_OBJECT (GST_NETWORK_TOOL (tool)->hosts_config));
     }
 }
 
@@ -538,7 +538,7 @@ on_hostname_focus_out (GtkWidget *widget, GdkEventFocus *event, gpointer data)
       if (res == GTK_RESPONSE_ACCEPT)
 	{
 	  oobs_hosts_config_set_hostname (tool->hosts_config, hostname);
-	  oobs_object_commit (OOBS_OBJECT (tool->hosts_config));
+	  gst_tool_commit (GST_TOOL (tool), OOBS_OBJECT (tool->hosts_config));
 	}
       else
 	{
@@ -566,7 +566,7 @@ on_domain_focus_out (GtkWidget *widget, GdkEventFocus *event, gpointer data)
   if (changed)
     {
       oobs_hosts_config_set_domainname (tool->hosts_config, domain);
-      oobs_object_commit (OOBS_OBJECT (tool->hosts_config));
+      gst_tool_commit (GST_TOOL (tool), OOBS_OBJECT (tool->hosts_config));
     }
 
   g_object_set_data (G_OBJECT (widget), "content-changed", GINT_TO_POINTER (FALSE));
diff --git a/src/network/hosts.c b/src/network/hosts.c
index 21f4318..2a0efe4 100644
--- a/src/network/hosts.c
+++ b/src/network/hosts.c
@@ -342,7 +342,7 @@ host_aliases_dialog_save (GtkTreeIter *iter)
       host_aliases_add (host, &list_iter);
     }
 
-  oobs_object_commit (OOBS_OBJECT (GST_NETWORK_TOOL (tool)->hosts_config));
+    gst_tool_commit (tool, OOBS_OBJECT (GST_NETWORK_TOOL (tool)->hosts_config));
 }
 
 void
diff --git a/src/network/locations-combo.c b/src/network/locations-combo.c
index c708160..4defbac 100644
--- a/src/network/locations-combo.c
+++ b/src/network/locations-combo.c
@@ -216,7 +216,7 @@ on_combo_changed (GtkWidget *widget, gpointer data)
       gst_network_locations_set_location (locations, str);
       gst_tool_update_gui (priv->tool);
 
-      oobs_object_commit (locations->hosts_config);
+      gst_tool_commit (priv->tool, locations->hosts_config);
       gst_tool_commit_async (priv->tool, locations->ifaces_config,
 			     _("Changing network location"), NULL, NULL);
       g_free (str);
diff --git a/src/network/network-tool.c b/src/network/network-tool.c
index 82d6fc4..5b332ae 100644
--- a/src/network/network-tool.c
+++ b/src/network/network-tool.c
@@ -90,7 +90,7 @@ save_dns (GList *list, gpointer data)
   GstNetworkTool *tool = (GstNetworkTool *) data;
 
   oobs_hosts_config_set_dns_servers (tool->hosts_config, list);
-  oobs_object_commit (OOBS_OBJECT (tool->hosts_config));
+  gst_tool_commit (GST_TOOL (tool), OOBS_OBJECT (tool->hosts_config));
 }
 
 static void
@@ -99,7 +99,7 @@ save_search_domains (GList *list, gpointer data)
   GstNetworkTool *tool = (GstNetworkTool *) data;
 
   oobs_hosts_config_set_search_domains (tool->hosts_config, list);
-  oobs_object_commit (OOBS_OBJECT (tool->hosts_config));
+  gst_tool_commit (GST_TOOL (tool), OOBS_OBJECT (tool->hosts_config));
 }
 
 static GObject*
diff --git a/src/services/callbacks.c b/src/services/callbacks.c
index 47a3179..d92b53e 100644
--- a/src/services/callbacks.c
+++ b/src/services/callbacks.c
@@ -75,7 +75,7 @@ show_settings (void)
 
 	if (response != GTK_RESPONSE_NONE) {
 		service_settings_table_save (service);
-		oobs_object_commit (GST_SERVICES_TOOL (tool)->services_config);
+		gst_tool_commit (tool, GST_SERVICES_TOOL (tool)->services_config);
 	}
 
 	g_object_unref (service);
@@ -162,7 +162,7 @@ on_service_toggled (GtkCellRenderer *renderer, gchar *path_str, gpointer data)
 							 (new_value) ? OOBS_SERVICE_START : OOBS_SERVICE_IGNORE,
 							 /* FIXME: hardcoded value... */
 							 50);
-		oobs_object_commit (GST_SERVICES_TOOL (tool)->services_config);
+		gst_tool_commit (tool, GST_SERVICES_TOOL (tool)->services_config);
 
 		gtk_list_store_set (GTK_LIST_STORE (model),
 				    &iter,
diff --git a/src/shares/callbacks.c b/src/shares/callbacks.c
index 22d22e9..258c00d 100644
--- a/src/shares/callbacks.c
+++ b/src/shares/callbacks.c
@@ -217,11 +217,11 @@ on_delete_share_clicked (GtkWidget *widget, gpointer data)
 		if (OOBS_IS_SHARE_NFS (share)) {
 			list = oobs_nfs_config_get_shares OOBS_NFS_CONFIG ((GST_SHARES_TOOL (tool)->nfs_config));
 			oobs_list_remove (list, list_iter);
-			oobs_object_commit (GST_SHARES_TOOL (tool)->nfs_config);
+			gst_tool_commit (tool, GST_SHARES_TOOL (tool)->nfs_config);
 		} else {
 			list = oobs_smb_config_get_shares OOBS_SMB_CONFIG ((GST_SHARES_TOOL (tool)->smb_config));
 			oobs_list_remove (list, list_iter);
-			oobs_object_commit (GST_SHARES_TOOL (tool)->smb_config);
+			gst_tool_commit (tool, GST_SHARES_TOOL (tool)->smb_config);
 		}
 
 		gtk_list_store_remove (GTK_LIST_STORE (model), &iter);
@@ -363,7 +363,7 @@ on_is_wins_toggled (GtkWidget *widget, gpointer data)
 	gtk_widget_set_sensitive (entry, !is_wins_server);
 
 	oobs_smb_config_set_is_wins_server (OOBS_SMB_CONFIG (GST_SHARES_TOOL (tool)->smb_config), is_wins_server);
-	oobs_object_commit (GST_SHARES_TOOL (tool)->smb_config);
+	gst_tool_commit (tool, GST_SHARES_TOOL (tool)->smb_config);
 }
 
 gboolean
@@ -376,7 +376,7 @@ on_workgroup_focus_out (GtkWidget *widget, GdkEvent *event, gpointer data)
 
 	if (workgroup && *workgroup) {
 		oobs_smb_config_set_workgroup (OOBS_SMB_CONFIG (GST_SHARES_TOOL (tool)->smb_config), workgroup);
-		oobs_object_commit (GST_SHARES_TOOL (tool)->smb_config);
+		gst_tool_commit (tool, GST_SHARES_TOOL (tool)->smb_config);
 	}
 	g_free (workgroup);
 
@@ -392,7 +392,7 @@ on_wins_server_focus_out (GtkWidget *widget, GdkEvent *event, gpointer data)
 	wins_server = g_strdup (gtk_entry_get_text (GTK_ENTRY (widget)));
 
 	oobs_smb_config_set_wins_server (OOBS_SMB_CONFIG (GST_SHARES_TOOL (tool)->smb_config), wins_server);
-	oobs_object_commit (GST_SHARES_TOOL (tool)->smb_config);
+	gst_tool_commit (tool, GST_SHARES_TOOL (tool)->smb_config);
 	g_free (wins_server);
 
 	return FALSE;
diff --git a/src/shares/share-settings.c b/src/shares/share-settings.c
index bcb8d32..94d94ae 100644
--- a/src/shares/share-settings.c
+++ b/src/shares/share-settings.c
@@ -377,10 +377,10 @@ modify_share (OobsShare *new_share, OobsShare *old_share, OobsListIter *list_ite
 
 	table_modify_share_at_iter (iter, new_share, list_iter);
 
-	oobs_object_commit (new_config);
+	gst_tool_commit (tool, new_config);
 
 	if (old_config != new_config)
-		oobs_object_commit (old_config);
+		gst_tool_commit (tool, old_config);
 }
 
 static void
@@ -404,7 +404,7 @@ add_new_share (OobsShare *share)
 
 	table_add_share (share, &iter);
 
-	oobs_object_commit (config);
+	gst_tool_commit (tool, config);
 }
 
 static void
@@ -424,7 +424,7 @@ delete_share (GtkTreeIter *iter, OobsShare *share, OobsListIter *list_iter)
 	oobs_list_remove (list, list_iter);
 	table_delete_share_at_iter (iter);
 
-	oobs_object_commit (config);
+	gst_tool_commit (tool, config);
 }
 
 void
diff --git a/src/time/time-tool.c b/src/time/time-tool.c
index af6fc0d..7456a2e 100644
--- a/src/time/time-tool.c
+++ b/src/time/time-tool.c
@@ -209,7 +209,7 @@ on_apply_timeout (GstTimeTool *tool)
 				   (gint) year, (gint) month, (gint) day,
 				   (gint) hour, (gint) minute, (gint)second);
 
-	oobs_object_commit (tool->time_config);
+	gst_tool_commit (GST_TOOL (tool), tool->time_config);
 	gst_time_tool_start_clock (tool);
 
 	inhibit_screensaver (tool, FALSE);
@@ -770,7 +770,7 @@ gst_time_tool_run_timezone_dialog (GstTimeTool *time_tool)
 
 	if (!timezone || strcmp (tz_name, timezone) != 0) {
 		oobs_time_config_set_timezone (OOBS_TIME_CONFIG (time_tool->time_config), tz_name);
-		oobs_object_commit (time_tool->time_config);
+		gst_tool_commit (GST_TOOL (time_tool), time_tool->time_config);
 		gtk_label_set_text (GTK_LABEL (label), tz_name);
 	}
 }
diff --git a/src/users/callbacks.c b/src/users/callbacks.c
index fe5d47d..5ee2d3c 100644
--- a/src/users/callbacks.c
+++ b/src/users/callbacks.c
@@ -286,8 +286,8 @@ on_user_new_clicked (GtkButton *button, gpointer user_data)
 		oobs_list_set (users_list, &list_iter, user);
 
 		users_table_add_user (user, &list_iter);
-		oobs_object_commit (GST_USERS_TOOL (tool)->users_config);
-		oobs_object_commit (GST_USERS_TOOL (tool)->groups_config);
+		gst_tool_commit (tool, GST_USERS_TOOL (tool)->users_config);
+		gst_tool_commit (tool, GST_USERS_TOOL (tool)->groups_config);
 	}
 }
 
@@ -329,8 +329,8 @@ on_user_settings_clicked (GtkButton *button, gpointer user_data)
 
 		if (gst_dialog_is_authenticated (tool->main_dialog)) {
 			/* change users/groups configuration */
-			oobs_object_commit (GST_USERS_TOOL (tool)->users_config);
-			oobs_object_commit (GST_USERS_TOOL (tool)->groups_config);
+			gst_tool_commit (tool, GST_USERS_TOOL (tool)->users_config);
+			gst_tool_commit (tool, GST_USERS_TOOL (tool)->groups_config);
 #ifdef HAVE_POLKIT
 		/* With PolicyKit1, we don't have to check for authorizations: just try to commit,
 		 * the backend will trigger authentication if possible, or fail */
@@ -339,7 +339,7 @@ on_user_settings_clicked (GtkButton *button, gpointer user_data)
 
 			/* change self, only if it is the modified user */
 			if (user == oobs_self_config_get_user (OOBS_SELF_CONFIG (object))) {
-				oobs_object_commit (GST_USERS_TOOL (tool)->self_config);
+				gst_tool_commit (tool, GST_USERS_TOOL (tool)->self_config);
 			}
 #endif
 		}
@@ -369,7 +369,7 @@ on_user_delete_clicked (GtkButton *button, gpointer user_data)
 	g_list_foreach (list, (GFunc) gtk_tree_row_reference_free, NULL);
 	g_list_free (list);
 
-	oobs_object_commit (GST_USERS_TOOL (tool)->users_config);
+	gst_tool_commit (tool, GST_USERS_TOOL (tool)->users_config);
 }
 
 void
@@ -412,7 +412,7 @@ on_group_new_clicked (GtkButton *button, gpointer user_data)
 		oobs_list_set (groups_list, &list_iter, group);
 
 		groups_table_add_group (group, &list_iter);
-		oobs_object_commit (GST_USERS_TOOL (tool)->groups_config);
+		gst_tool_commit (tool, GST_USERS_TOOL (tool)->groups_config);
 	}
 }
 
@@ -452,7 +452,7 @@ on_group_settings_clicked (GtkButton *button, gpointer user_data)
 								  &filter_iter, &iter);
 		group_settings_dialog_get_data (group);
 		groups_table_set_group (group, list_iter, &filter_iter);
-		oobs_object_commit (GST_USERS_TOOL (tool)->groups_config);
+		gst_tool_commit (tool, GST_USERS_TOOL (tool)->groups_config);
 	}
 
 	g_object_unref (group);
@@ -480,7 +480,7 @@ on_group_delete_clicked (GtkButton *button, gpointer user_data)
 	g_list_foreach (list, (GFunc) gtk_tree_row_reference_free, NULL);
 	g_list_free (list);
 
-	oobs_object_commit (GST_USERS_TOOL (tool)->groups_config);
+	gst_tool_commit (tool, GST_USERS_TOOL (tool)->groups_config);
 }
 
 /* User settings callbacks */
diff --git a/src/users/user-settings.c b/src/users/user-settings.c
index 2a72cee..ba03eaf 100644
--- a/src/users/user-settings.c
+++ b/src/users/user-settings.c
@@ -222,7 +222,7 @@ get_main_group (const gchar *name)
 		oobs_list_set (groups_list, &list_iter, group);
 
 		groups_table_add_group (group, &list_iter);
-		oobs_object_commit (OOBS_OBJECT (config));
+		gst_tool_commit (tool, OOBS_OBJECT (config));
 
 		return group;
 	}
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]