[Bug 32053] Add a TpContactSearch proxy object

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=32053

--- Comment #28 from Guillaume Desmottes <guillaume.desmottes at collabora.co.uk> 2011-01-19 01:11:48 PST ---
+ * Returns: (transfer full): a new contact search channel, or %NULL
It's not a TpChannel either.

+ *  it when you're done with %g_list_free.
Is that properly linked? I'd say it should be "g_list_free()".

I think the long description in SECTION:contact-search should describe the
whole search process (which function use to start a search, how to get the
results and then how to start a new search).

Actually having a simple demo app in examples/client could be useful.

You add your new files to telepathy-glib/introspection.am so they'll be added
to gir.

(In reply to comment #27)
> >       g_cancellable_cancel (self->priv->cancellable);
> > 
> >       while (self->priv->async_res != NULL)
> >         g_main_context_iteration (NULL, TRUE);
> > This is a bit weird; maybe we should request a g_cancellable_cancel_async() in
> > glib?
> > Do we really need to wait btw ?
> 
> As it is now, yes, since we use the same GCancellable, so we need to wait and
> reset it. Maybe we could have a list of GCancellables and just cancel it
> without waiting, or maybe we can cancel it and unref it (if gio cancells it
> before disposing the GCancellable), so we don't need to keep a list. The
> current code works though.

The cancel, unref and create a new one option sounds saner to me. Can you
check the GCancellable implementation and/or test to see if that would work?

> > tp_contact_search_start(): shouldn't we check if another search is already
> > active?
> 
> We do:
> 
>   g_return_if_fail (self->priv->state ==
>       TP_CHANNEL_CONTACT_SEARCH_STATE_NOT_STARTED);

You're right sorry.

> > +  if (with_limit)
> > if (with_limit != NULL)
> > 
> > +      if (!tp_strdiff (chan_type, TP_IFACE_CHANNEL_TYPE_CONTACT_SEARCH))
> > use "continue" to limit nesting.
> 
> I cannot use "continue" there since I have to check both with_limit and
> with_server, and if I continue in the first one, I won't get to check the
> second one. I could do
> 
> if (with_limit != NULL && !tp_strdiff (...))
> 
> to avoid one level. Not sure if it's worth it...

I meant doing:
if (tp_strdiff (chan_type, TP_IFACE_CHANNEL_TYPE_CONTACT_SEARCH))
  continue;

Wouldn't that work?


> > Having test for TpContactSearch would make me an happier badger...
> 
> I tried to look at this, but apparently I need to create some classes for the
> DBus tests and I couldn't really understand how that stuff works.

Yeah you have to write a server side channel object to test your API.
See for example tests/lib/stream-tube-chan.c tested in tests/dbus/stream-tube.c

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.