GNOME Bugzilla – Bug 546135
GIcon and implementations improvements
Last modified: 2008-08-09 15:10:15 UTC
GIcon and friends need some improvements. 1. Make more Pythonic: add meaningful __repr__, support __hash__ and __eq__ in meaningful way as already provided by C-level GIO (this is _important_ to be able to use icons as dict keys). 2. GThemedIcon constructor should accept a string or a sequence of strings (see also this: Could not write function themed_icon_new_from_names: No ArgType for char**). 3. There are three functions in GLoadableIcon for which codegen cannot generate wrappers. 4. I think it would be better to have GFileIcon() constructor instead of GFile.icon_new() method. The latter looks to me like polluting GFile with hardly important (for GFile, I mean) method. But see also bug 546132. 5. What about GEmblemedIcon? I'm lost in all these different library versions, is it new (SVN only) or should it be wrapped now?
Created attachment 115790 [details] [review] patch for point 1 above Changes: * all icon implementations now support __eq__ and __hash__; * meaningful __repr__ added for gio.FileIcon (<gio.FileIcon at 0x###: file:///home/paul/test.png>) and gio.ThemedIcon (<gio.ThemedIcon at 0x###: bla-bla-bla>); * gio.FileIcon is now explicitely wrapped (needed because we attach the slots I mentioned above).
Comment on attachment 115790 [details] [review] patch for point 1 above Its looks good. I'd just refactor the richcompare to a separate function to avoid duplication.
Agreed. I broke out pygio_do_icon_richcompare() and those 3 functions just call the helper now. Sending ChangeLog Sending gio/gio-types.defs Sending gio/gio.override Transmitting file data ... Committed revision 924. 2008-08-05 Paul Pogonyshev <pogonyshev@gmx.net> Bug 546135 – GIcon and implementations improvements * gio/gio-types.defs (FileIcon): New 'define-object'. * gio/gio.override (pygio_do_icon_richcompare) (_wrap_g_icon_tp_richcompare, _wrap_g_icon_tp_hash) (_wrap_g_file_icon_tp_richcompare, _wrap_g_file_icon_tp_hash) (_wrap_g_file_icon_tp_repr, _wrap_g_themed_icon_tp_richcompare) (_wrap_g_themed_icon_tp_hash, _wrap_g_themed_icon_tp_repr): New functions.
Created attachment 116092 [details] [review] patch for points 2 and 3 above + unit tests This implements what I outlined in points 2 and 3 above. Note that I broke overrides out to 'gicon.override' as they became quite many already. I don't understand how 'test_gio.py' is invoked to begin with, so I added gio.Icon and related tests there instead of breaking out 'test_gicon.py'. According to point 4: since we now have gio.FileIcon type, we actually have to define a constructor, because default (zero argument) constructor yields an unusable object with 'file' unset. Should the constructor replace gio.File.icon_new() completely (as we have not had gio in a stable release, it is possible to remove the method now) or co-exist in parallel?
Created attachment 116225 [details] [review] patch for points 2 and 3 above + unit tests, second revision Changes: newly wrapped gio.LoadableIcon methods are documented; icon tests are in their own file. Already committed as we discussed on IRC. Sending ChangeLog Sending gio/Makefile.am Adding gio/gicon.override Sending gio/gio.defs Sending gio/gio.override Sending tests/Makefile.am Adding tests/test_gicon.py Sending tests/test_gio.py Transmitting file data ........ Committed revision 931. 2008-08-09 Paul Pogonyshev <pogonyshev@gmx.net> Bug 546135 – GIcon and implementations improvements * gio/gio.defs (gio.LoadableIcon.load) (gio.LoadableIcon.load_async, gio.LoadableIcon.load_finish): Document. * gio/Makefile.am: * gio/gicon.override: New file: parts of 'gio.override', three methods of gio.LoadableIcon and gio.ThemedIcon constructor. * gio/gio.override: Move over all icon-related overrides to 'gicon.override'. * tests/Makefile.am: * tests/test_gicon.py: New file: parts of 'test_gio.py' and several new gio.Icon tests. * tests/test_gio.py (TestThemedIcon): Move over to 'test_gicon.py'.
Created attachment 116228 [details] [review] patch for point 4 above + one test Here is the last patch. It changes g_file_icon_new wrapper from method of gio.File to constructor of gio.FileIcon, which it logically is, as already discussed on IRC. I also updated test file and added one test for the new constructor.
Sending ChangeLog Sending gio/gio.defs Sending tests/test_gicon.py Transmitting file data ... Committed revision 933. 2008-08-09 Paul Pogonyshev <pogonyshev@gmx.net> Bug 546135 – GIcon and implementations improvements * gio/gio.defs (g_file_icon_new): Change from method of gio.File to constructor of gio.FileIcon. * tests/test_gicon.py (TestIcon.test_eq, Test_Eq.test_hash) (TestLoadableIcon.setUp): Adapt accordingly. (TestFileIcon): New test case.