[Bug 32053] Add a TpContactSearch proxy object

[bugzilla-daemon at freedesktop.org]

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

--- Comment #22 from Emilio Pozuelo Monfort <pochu27 at gmail.com> 2010-12-16 10:21:53 PST ---
Thanks for the thorough review! I'll be more careful next time, specially when
it comes to the documentation...

(In reply to comment #20)
> > +tp_capabilities_supports_contact_search (TpCapabilities *self,
> 
> If there are fixed properties other than the channel type, your channel request
> is going to fail, because you only provide the channel type. So, you should
> skip channel classes that have more than one fixed property, for
> forwards-compat.

Hmm, can you clarify this a bit?

> (I note in passing that the channel type in telepathy-spec should specify a
> typical requestable channel class, and the channel class used in Gabble
> violates "SHOULD always include [...] TargetHandleType". Oh well. We'll just
> have to document that in the spec...)

Just to be sure, there's nothing for me to fix in TpCS, right?

> > + * tp_contact_search_reset_async:
> [...]
> > +  g_assert (self->priv->async_res == NULL);
> 
> This crashes if the caller resets it repeatedly, or resets it while a channel
> is being fetched. At the very least, it should g_return_if_fail, and document
> that it's an error to call this function while async_init or reset_async is in
> progress.
> 
> However, I think it'd be nicer if you could set it up so that resetting while a
> reset is already in progress just makes the first reset fail with
> G_IO_ERROR_CANCELLED, or something.

I have changed the g_assert()s to g_return_if_fail(), and documented that. I'll
look at the GCancellable thing.

> I'm not convinced that tp_contact_search_result_get_field is useful: if you
> want to keep it, you should certainly document its limitations (e.g. if the
> contact has a home and work phone number, it will arbitrarily pick one or the
> other, without telling you which).

Agreed, it's not very useful in its current form... I have changed it to return
a TpContactInfoField, so you don't need to get the list of fields, look for the
one you want, and then free the list.

Everything else should be fixed.

I'll look at adding some tests tomorrow too.

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