python bindings for libappindicator don't allow to change icon-theme-path

Bug #527061 reported by Riccardo 'c10ud'
20
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Application Indicators
Fix Released
Undecided
Unassigned
indicator-application (Ubuntu)
Fix Released
Undecided
Unassigned

Bug Description

if you try to set the "icon-theme-path" property, it complains:
TypeError: property 'icon-theme-path' can only be set in constructor

but the constructor only allows 3 arguments

Related branches

Revision history for this message
Sense Egbert Hofstede (sense) wrote :

I can confirm this bug.
Somehow the constructor with four arguments isn't succesfully made available in the Python bindings. Furthermore, the appindicator class does have an icon_path (or something similar) property, but according to the report that isn't accessible, probably because it's never set.

Changed in indicator-application:
status: New → Confirmed
Revision history for this message
Riccardo 'c10ud' (c10ud) wrote :

after googling a bit, i tried modifying the code a bit (i pulled kenvandine's python branch) and came to this conclusion, however i'm unsure of the result since make/make install seem not changing anything (the constructor still requires 3 arguments), i must say i never touched *-bindings before

greets

Revision history for this message
Riccardo 'c10ud' (c10ud) wrote :

whoups, sorry, there's a little add i didn't get rid of after my tests, here's a lighter diff

Revision history for this message
Ted Gould (ted) wrote :

I think that we probably need both functions. So that there is still the basic non-pathed version instead of replacing it.

Revision history for this message
Riccardo 'c10ud' (c10ud) wrote :

so, would something like this do the trick?

greets

Revision history for this message
Riccardo 'c10ud' (c10ud) wrote :

some irc copypasta for reference:

today i managed to correctly build the whole stuff, and seems like the 001 diff works correctly while the new one (002) doesn't. unfortunately there's little (read: no) documentation on bindings creations so i'm walking in the dark (i don't know how duplicate a constructor). any chance some expert can take a look at this? anyway, the 001 seems fine to me, if you wanna test it

i'm using _new_from_model instead of plain _new when creating a new appindicator object, and i think there should be no difference (even if i didn't read the source, i admit)

http://live.gnome.org/PyGTK/WhatsNew28#Hand-written_constructors_need_to_be_updated

Revision history for this message
Johan Dahlin (jdahlin-deactivatedaccount) wrote :

001 seems to be the right solution if call to app_indicator_new_with_path() with the icon_path parameter set to NULL is identical to a call to app_indicator_new().

002 changes the behavior of the python code for instantiating an app indicator wrapper by calling class, eg appinidicator.AppIndicator(). The .defs s-expression format only allows one constructor (is-constructor-of) per wrapped class.
It is possible to provide additional classmethod functions on the class, eg appindicator.AppIndicator.new_with_path().
But that would make sense iff there's a functional difference between creating an app indicator between app_indicator_new/app_indicator_new_with_path.

Revision history for this message
Felix Krull (fkrull) wrote :

001 works fine for me, no segfaults or anything. According to src/libappindicator/app-indicator.c:app_indicator_init, NULL seems to be the default value for the icon_path property. Also, app_indicator_new and new_with_path both use g_object_new, the latter function merely specifies the icon_path property. So, any chance this gets comitted and released sometime soon?

Ted Gould (ted)
Changed in indicator-application:
status: Confirmed → Fix Committed
milestone: none → 0.0.19
Revision history for this message
Ted Gould (ted) wrote : Re: [Bug 527061] Re: python bindings for libappindicator don't allow to change icon-theme-path

Thanks everyone for checking this out, and for the patch. I've
committed it and it should get into this week's release. I've pushed a
package into my bugfix PPA so that people can test it more. The package
is:

   indicator-application_0.0.18-0ubuntu2~ppa1

And the PPA is:

   https://launchpad.net/~ted/+archive/bugfix

Thanks again!

Ted Gould (ted)
Changed in indicator-application:
status: Fix Committed → Fix Released
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package indicator-application - 0.0.19-0ubuntu1

---------------
indicator-application (0.0.19-0ubuntu1) lucid; urgency=low

  * New upstream release.
    * Setup the icons to use the -panel variants even when falling
      back to status icons. (LP: #547072)
    * Change python bindings to use _with_path so that they can set
      the icon path. (LP: #527061)
    * Don't set the fallback timer if we're already in a fallback
      mode to avoid unfalling back excessively. (LP: #529052)
    * Fix distcheck of documentation
 -- Ted Gould <email address hidden> Thu, 01 Apr 2010 15:56:02 -0500

Changed in indicator-application (Ubuntu):
status: New → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.