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

["Vincent Untz" <vuntz gnome org>]

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):

+        if (tool && g_shell_parse_argv (tool, NULL, &argv, NULL))
+        {
should be:
+        if (tool && g_shell_parse_argv (tool, NULL, &argv, NULL)) {
(same comment for some other places)

+        char **argv = NULL;
+        char *tool = NULL;
should be
+        char **argv = NULL;
+        char  *tool = NULL;

I don't think the 'g_assert (argv != NULL);' in clock_check_config_tool()
is okay since g_shell_parse_argv() can fail in some cases (I don't know
when) and config_tool can come from gconf and thus can be set by the user.

+        if (cd->config_tool && cd->config_tool [0]) {
+                config_tool = clock_check_config_tool (cd, cd->config_tool);
+                if (config_tool) {
+                        g_free (cd->config_tool);
+                        cd->config_tool = config_tool;
+                }
+        }
Surely if config_tool is not NULL, then it's cd->config_tool. No need
to changed it. It should probably be:
+        if (cd->config_tool && cd->config_tool [0]) {
+                config_tool = clock_check_config_tool (cd, cd->config_tool);
+                if (!config_tool) {
+                        g_free (cd->config_tool);
+                        cd->config_tool = NULL;
+                }
+        }

-        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?

You also want to change something in config_tool_changed(), I think.

Also, the app variable in try_config_tool() should now really be a
boolean.

Vincent

-- 
Les gens heureux ne sont pas pressés.