Re: [PATCH] support for YAST date config in clock applet

[Rodrigo Moya <rodrigo gnome-db org>]

On Tue, 2005-07-19 at 14:41 +0200, Vincent Untz wrote:
> Hi Rodrigo,
> 
> On Tue, July 19, 2005 13:36, Rodrigo Moya said:
> > Hi
> >
> > This patch adds support for SuSE's YAST datetime config tool in the
> > clock applet.
> >
> > Ok to commit?
> 
> Thanks for the patch.
> Some comments (I might be totally wrong since I don't have the whole
> code here):
> 
...

all fixed in the attached patch

> -        if (!config_tool)
> +        if (config_tool) {
> +                bonobo_ui_component_set_prop (popup_component,
> +                                              "/commands/ClockConfig",
> +                                              "hidden", "0",
> +                                              NULL);
> +        } else {
> I don't see the point of this. It was not hidden before, was it?
> 
while working on this patch, I ran it a few times without it finding
correctly the config tool, and it seems it set it up to hidden at that
time. Then, when running the correct code, I had to add this to put it
back to not hidden.

> You also want to change something in config_tool_changed(), I think.
> 
like checking if the command entered by the user is correct? Added to
the patch also.
-- 
Rodrigo Moya <rodrigo gnome-db org>

Index: ChangeLog
===================================================================
RCS file: /cvs/gnome/gnome-panel/applets/clock/ChangeLog,v
retrieving revision 1.592
diff -u -p -r1.592 ChangeLog
--- ChangeLog	18 Jul 2005 12:15:45 -0000	1.592
+++ ChangeLog	19 Jul 2005 14:33:13 -0000
@@ -1,3 +1,14 @@
+2005-07-19  Rodrigo Moya <rodrigo novell com>
+
+	* clock.c: added support for SuSE's YAST datetime config tool.
+	(try_config_tool): support command lines with arguments.
+	(clock_check_config_tool): new function to look for the tools
+	correctly, even if the command line has arguments.
+	(fill_clock_applet): use clock_check_config_tool to look for the
+	configuration tool.
+	(config_tool_changed): check the value entered by the user before
+	using it.
+
 2005-07-15  Rodrigo Moya <rodrigo novell com>
 
 	* clock.c (update_clock): only show the date in the tooltip if the applet
Index: clock.c
===================================================================
RCS file: /cvs/gnome/gnome-panel/applets/clock/clock.c,v
retrieving revision 1.167
diff -u -p -r1.167 clock.c
--- clock.c	18 Jul 2005 12:15:45 -0000	1.167
+++ clock.c	19 Jul 2005 14:33:14 -0000
@@ -82,6 +82,7 @@ static const char* KEY_SHOW_WEEK     = "
 static const char *clock_config_tools [] = {
 	"system-config-date",
 	"redhat-config-date",
+        "/sbin/yast2 timezone",
 	"time-admin",
 };
 
@@ -1784,25 +1785,33 @@ try_config_tool (GdkScreen  *screen,
 {
 	GtkWidget *dialog;
 	GError    *err;
-	char      *argv[2];
-	char      *app;
+	char     **argv;
+	gboolean   app = FALSE;
 
 	g_return_val_if_fail (tool != NULL, FALSE);
 
-	app = g_find_program_in_path (tool);
+        if (tool && g_shell_parse_argv (tool, NULL, &argv, NULL)) {
+                char *path;
+
+                app = TRUE;
+
+                if (!(path = g_find_program_in_path (argv [0])))
+                        app = FALSE;
+
+                g_free (path);
+        }
 
 	if (!app)
 		return FALSE;
-		
-	argv [0] = app;
-	argv [1] = NULL;		
 
 	err = NULL;
 	if (gdk_spawn_on_screen (screen, NULL, argv, NULL, 0, NULL, NULL, NULL, &err)) {
-		g_free (app);
+                g_strfreev (argv);
 		return TRUE;
 	}
-			
+
+        g_strfreev (argv);
+
 	dialog = gtk_message_dialog_new (NULL,
 					 GTK_DIALOG_DESTROY_WITH_PARENT,
 					 GTK_MESSAGE_ERROR,
@@ -1817,8 +1826,6 @@ try_config_tool (GdkScreen  *screen,
 	gtk_window_set_screen (GTK_WINDOW (dialog), screen);
 			
 	gtk_widget_show_all (dialog);			
-
-	g_free (app);
 		
 	return TRUE;
 }
@@ -1956,8 +1963,10 @@ config_tool_changed (GConfClient  *clien
 
 	value = gconf_value_get_string (entry->value);
 
-        g_free (clock->config_tool);
-	clock->config_tool = g_strdup (value);
+        if (clock_check_config_tool (clock, value)) {
+                g_free (clock->config_tool);
+                clock->config_tool = g_strdup (value);
+        }
 }
 
 static void
@@ -2122,6 +2131,30 @@ clock_migrate_to_26 (ClockData *clock)
 				       NULL);
 }
 
+static char *
+clock_check_config_tool (ClockData *cd, const char *config_tool)
+{
+        char **argv = NULL;
+        char *tool = NULL;
+
+        if (config_tool && g_shell_parse_argv (config_tool, NULL, &argv, NULL)) {
+                char *path;
+
+                g_assert (argv != NULL);
+
+                if (!(path = g_find_program_in_path (argv [0])))
+                        tool = NULL;
+                else {
+                        tool = config_tool;
+                        g_free (path);
+                }
+
+                g_strfreev (argv);
+        }
+
+        return tool;
+}
+
 static gboolean
 fill_clock_applet (PanelApplet *applet)
 {
@@ -2227,25 +2260,33 @@ fill_clock_applet (PanelApplet *applet)
 	}
 
         config_tool = NULL;
-	if (cd->config_tool && cd->config_tool [0])
-		config_tool = g_find_program_in_path (cd->config_tool);
+	if (cd->config_tool && cd->config_tool [0]) {
+		config_tool = clock_check_config_tool (cd, cd->config_tool);
+        }
 
         if (!config_tool) {
                 int i;
 
                 for (i = 0; i < G_N_ELEMENTS (clock_config_tools); i++)
-                        if ((config_tool = g_find_program_in_path (clock_config_tools [i])))
+                        if (config_tool = clock_check_config_tool (cd, clock_config_tools [i]))
                                 break;
         }
 
-	if (!config_tool)
+	if (config_tool) {
+                bonobo_ui_component_set_prop (popup_component,
+					      "/commands/ClockConfig",
+					      "hidden", "0",
+					      NULL);
+        } else {
+                g_free (cd->config_tool);
+                cd->config_tool = NULL;
+
 		bonobo_ui_component_set_prop (popup_component,
 					      "/commands/ClockConfig",
 					      "hidden", "1",
 					      NULL);
-        
-        g_free (config_tool);
-	
+        }
+
 	return TRUE;
 }