GNOME Bugzilla – Bug 432318
[Python bindings] RBEntryView column_custom methods
Last modified: 2009-03-06 09:15:01 UTC
The {append,insert}_column_custom methods of RBEntryView are not bound to Python. Build errors are: Could not write method RBEntryView.append_column_custom: No ArgType for GCompareDataFunc Could not write method RBEntryView.insert_column_custom: No ArgType for GCompareDataFunc
Created attachment 86791 [details] [review] Add a GDestroyNotify argument to each C function. It is possible to bind these to Python, but it is nice to avoid a memory leak by offering a way to clean up the callback afterwards. The downside is that this breaks the API of RBEntryView.
Thanks for catching this, stopping leaks in the bindings is alawys good. I've committed it to svn trunk. > The downside is that this breaks the API of RBEntryView. It only breaks the C API, and you can't build C plugins outside the source tree, so all it could break is a few patches (which are easily updatable).
Created attachment 90247 [details] [review] Actually bind the append/insert methods. Reverts one line of the previous patch, which was incorrect. Moves the python method calling stuff used by rhythmdb as well into override_common, which seemed the logical place for it.
Created attachment 90265 [details] [review] Clean up previous patch. Check the call to PyEval_CallObject, to help plugin developers. Tidy up use of XINCREF vs. INCREF. Fix a small memory leak.
Created attachment 91059 [details] [review] Update; try using existing method names to make the patch simpler. Also fix calling ``Py_DECREF (args)'' too early, which led to crashes.
What is the current status of this patch? Is there anything I can do to help expedite its inclusion into svn?
The patch was fairly well-tested, but I don't know whether it will still apply to SVN trunk. It should be easy to clean up, but iirc I wanted some feedback on whether it was simpler to use the existing _rhythmdb_query_model_sort_func in _wrap_rb_entry_view_append_column_custom and so on, and whether it was okay to add new functions to override_common.c.
This mostly looks OK (and still applies cleanly); columns don't actually need to have sort functions, though, so both the sort function and data args should be optional. It seems a bit silly to have append_column_custom and insert_column_custom in the python API, since append_column_custom is just insert_column_custom(..., -1).
This seems to apply successfully to both 0.11.6 and the trunk, and seems to work okay using 0.11.6. Any chance of getting it applied? I'm writing a plugin that needs a working rb.EntryView.insert_column_custom().
I'd be OK with applying an updated patch that made the sort function and data arguments optional. I'm less concerned about having both append_column_custom() and insert_column_custom().
Created attachment 129937 [details] [review] patch updated to svn trunk, make sort parameters optional Updated the previous version of the patch to apply cleanly to the trunk. Make sort params (sort_func, data) optional for the python bindings. However, if the caller does specify NULL sort params, the sort_data assert in rb_entry_view_resort_model() is triggered later on. This function assumes there is sort data present for the current sorting column. Should rb_entry_view_resort_model() be fixed to not care if sort_data is null or should the lack of sort func/data make the column somehow not sortable? Is being able to specify NULL sort func/data really that useful?
When a column with no sort function is added, the column header is not made clickable, so the user can't try to sort on that column. There are a couple of non-sortable columns already: the playing status column, the podcast feed status column, and the title/artist/album column in the play queue sidebar, so python code should have the ability to add more.
2009-03-06 Jonathan Matthew <jonathan@d14n.org> patch by: Tim Retout <tim@retout.co.uk>, Mike Gratton <mike@vee.net> * bindings/python/override_common.c: (_rhythmdb_query_model_sort_data_free), (_rhythmdb_query_model_sort_func): * bindings/python/override_common.h: * bindings/python/rb.defs: * bindings/python/rb.override: * bindings/python/rhythmdb.override: * widgets/rb-entry-view.c: (rb_entry_view_resort_model): Add bindings for the methods used to add custom columns to entry views. Fixes #432318.