Merge lp:~cjcurran/indicator-sound/iconthemepath into lp:~indicator-applet-developers/indicator-sound/trunk_3

Proposed by Conor Curran
Status: Merged
Merged at revision: 218
Proposed branch: lp:~cjcurran/indicator-sound/iconthemepath
Merge into: lp:~indicator-applet-developers/indicator-sound/trunk_3
Diff against target: 0 lines
To merge this branch: bzr merge lp:~cjcurran/indicator-sound/iconthemepath
Reviewer Review Type Date Requested Status
Mikkel Kamstrup Erlandsen (community) Approve
Review via email: mp+53005@code.launchpad.net

Description of the change

Uses the new iconthemepath setter in dbusmenu to allow for populating the icon on a menuitem from a non standard iconthemepath.

To post a comment you must log in.
Revision history for this message
Mikkel Kamstrup Erlandsen (kamstrup) wrote :

Looks good - a few style comments - do with them what you will :-)

40 + private string? parse_icon_path (string path)
41 + {
42 + if (path == "")return null;
43 + var icon_file = File.new_for_path (path);
44 + if (icon_file.get_path() == null)return null;
45 + return icon_file.get_basename().split(".")[0];
46 + }
47 +

You can simplify this by just using Path.get_basename(path) instead of creating a GFile. I would just write:

private string? parse_icon_path (string path)
{
  return ( path == "" ? null : Path.get_basename(path).split(".")[0] );
}

71 + gchar* paths[] = {"/usr/share/banshee-1/icons", NULL};

Just for pedantry - can you make this a const? Ie 'const gchar* paths[]'.

review: Approve
222. By Conor Curran on 2011-03-11

applied Mikkel's suggestions

Preview Diff

Empty

Subscribers

People subscribed via source and target branches