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.
_______________________________________________
desktop-devel-list mailing list
[email protected]
http://mail.gnome.org/mailman/listinfo/desktop-devel-list
