[Merge] lp:~ubuntu-mate-dev/indicator-session/mate-integration into lp:indicator-session

Iain Lane iain at orangesquash.org.uk
Fri Jun 16 11:40:52 UTC 2017


Review: Needs Fixing

Some small refactoring then looks good.

Diff comments:

> 
> === modified file 'src/backend-dbus/actions.c'
> --- src/backend-dbus/actions.c	2017-06-05 13:54:46 +0000
> +++ src/backend-dbus/actions.c	2017-06-14 10:44:37 +0000
> @@ -828,11 +828,45 @@
>    log_and_clear_error (&err, G_STRLOC, G_STRFUNC);
>  }
>  
> +static gboolean
> +have_mate_program (const gchar *program)
> +{
> +  gchar *path;
> +  const gchar *xdg_current_desktop;
> +  gchar **desktop_names;

You can do g_auto(GStrv) desktop_names = NULL; to get it freed automatically.

> +  gboolean have_program;
> +  gboolean is_mate;
> +  int i;
> +
> +  is_mate = FALSE;
> +  xdg_current_desktop = g_getenv ("XDG_CURRENT_DESKTOP");
> +  if (xdg_current_desktop != NULL) {
> +    desktop_names = g_strsplit (xdg_current_desktop, ":", 0);
> +    for (i = 0; desktop_names[i]; ++i) {
> +      if (!g_strcmp0 (desktop_names[i], "MATE")) {

This is g_strv_contains - in pseudocode if you do something like this you can get rid of have_program, is_mate, i (and path from the top scope):

if (g_strv_contains (desktop_names, "MATE")) {
   g_autofree gchar *path = g_find_program_in_path (program);
   return path != NULL;
}

return FALSE;

> +        is_mate = TRUE;
> +      }
> +    }
> +    g_strfreev (desktop_names);
> +  }
> +
> +  if (!is_mate)
> +    return FALSE;
> +
> +  path = g_find_program_in_path (program);
> +  have_program = path != NULL;
> +  g_free (path);
> +
> +  return have_program;
> +}
> +
>  static void
>  my_help (IndicatorSessionActions * self G_GNUC_UNUSED)
>  {
>    if (g_getenv ("MIR_SOCKET") != NULL)
>      url_dispatch_send("http://www.askubuntu.com", NULL, NULL);
> +  else if (have_mate_program ("yelp"))
> +    run_outside_app ("yelp help:mate-user-guide");
>    else
>      run_outside_app ("yelp");
>  }


-- 
https://code.launchpad.net/~ubuntu-mate-dev/indicator-session/mate-integration/+merge/325600
Your team Ubuntu Sponsors Team is requested to review the proposed merge of lp:~ubuntu-mate-dev/indicator-session/mate-integration into lp:indicator-session.



More information about the Ubuntu-sponsors mailing list