Config.Client: add notify_remove binding for Python

[Michal Hruby]

Here's a patch which implements also notify_remove python binding.

[Michal Hruby (mhr3)]

This is the correct patch...

[Mark Lee (malept)]

I'm not committing this until bug 411152 is resolved.

[Mark Lee (malept)]

In the meantime, here's a review:

=== modified file 'python/config.override'
--- python/config.override 2009-08-06 06:38:30 +0000
+++ python/config.override 2009-08-09 21:01:40 +0000
@@ -188,6 +188,74 @@
 }
 %%
 override desktop_agnostic_config_client_notify_add kwargs
+
+typedef struct _NotifyData
+{
+ gchar* group;
+ gchar* key;
+ gpointer data_tuple;
+} NotifyData;
+
+static GList *g_lda_notifications = NULL;
+
+void pydesktopagnostic_notifications_append (const gchar* group,
====
Return values need to go on their own lines.
====
+ const gchar* key,
+ gpointer data_tuple)
+{
+ NotifyData *data = g_slice_new0 (NotifyData);
+ data->group = g_strdup (group);
+ data->key = g_strdup (key);
+ data->data_tuple = data_tuple;
+ g_lda_notifications = g_list_append (g_lda_notifications, data);
+}
+
+NotifyData *pydesktopagnostic_notifications_find (const gchar* group,
+ const gchar* key,
+ PyObject *callback,
+ PyObject *extra)
+{
+ GList *it;
+ for (it = g_lda_notifications; it; it = it->next)
+ {
+ NotifyData *data = it->data;
+ PyObject *tuple = data->data_tuple;
+ if (strcmp (group, data->group) || strcmp (key, data->key)) continue;
====
This (and all other instances) need braces.
====
+
+ if (extra)
+ {
+ int comp_result;
+ if (PyTuple_Size (tuple) <= 1) continue;
+ PyObject *t_cb = PyTuple_GetItem (tuple, 0);
+ PyObject *t_extra = PyTuple_GetItem (tuple, 1);
====
Ideally, declarations and code should be separate.
====
+
+ if (PyObject_Cmp (t_cb, callback, &comp_result) == -1) continue;
+ if (comp_result != 0) continue;
+
+ // if we're here the callback is ok
+ if (PyObject_Cmp (t_extra, extra, &comp_result) == -1) continue;
+ if (comp_result == 0)
+ {
+ return data;
+ }
+ }
+ else
+ {
+ // no extra param
+ int comp_result;
+ if (PyTuple_Size (tuple) != 1) continue;
+ PyObject *t_cb = PyTuple_GetItem (tuple, 0);
+
+ if (PyObject_Cmp (t_cb, callback, &comp_result) == -1) continue;
+ if (comp_result == 0)
+ {
+ return data;
+ }
+ }
+ }
+
+ return NULL;
+}
+
====
Is g_list_find_custom not sufficient?
====

[Michal Hruby (mhr3)]

Here's the updated patch.
As for > Is g_list_find_custom not sufficient?
It'd require changing the NotifyData struct, this way it's more compact and it saves a couple of function calls. :)

[Mark Lee (malept)]

Some more nitpicks:

+typedef struct _NotifyData
+{
+ gchar* group;
+ gchar* key;
+ gpointer data_tuple;
+} NotifyData;
+
+static GList *g_lda_notifications = NULL;
+
+NotifyData *
+pydesktopagnostic_notifications_find (const gchar* group,
+ const gchar* key,

the gchar pointers have different formatting than the other pointers.

+ if (comp_result == 0)
+ {
+ return data;
+ }
+ }
+ else
+ {
+ // no extra param
+ PyObject *t_cb;
+ int comp_result;

I'd prefer if the comment is on the same line as the else.

@@ -286,6 +372,64 @@
     return NULL;
   }

+ // to properly support notify_remove we need to do this

This comment should be clearer.