GNOME Bugzilla – Bug 660580
Move shell contacts search closer to gnome-contacts
Last modified: 2011-10-05 19:08:26 UTC
This is more in line with what gnome-contacts does. I do get more results with this, but unfortunately, the display side is lacking, most contacts show up as 'Unknown / offline / X', even though gnome-contacts manages to display meaningful information for the same contacts.
Created attachment 197893 [details] [review] Move shell contacts search closer to gnome-contacts
*** Bug 660446 has been marked as a duplicate of this bug. ***
Created attachment 197925 [details] [review] improve contacts display The second patch is by Florian, and makes the visual appearance of eds contacts in the shell a lot nicer.
Review of attachment 197893 [details] [review]: ::: src/shell-contact-system.c @@ +132,3 @@ gboolean have_im_substring = FALSE; + Useless whitespace. @@ +151,3 @@ + p = strstr (name, term); + if (p == name) + have_alias_prefix = TRUE; The "alias" in there does no longer match what it is used for (nick and name) ... can't think of a better name though.
Review of attachment 197925 [details] [review]: Looks good.
Created attachment 198235 [details] [review] Move shell contacts search closer to gnome-contacts Match folks' name and nick fields, in addition to alias, and look at email addresses in addition to im addresses. This is more in line with what gnome-contacts does. To match this new usage, rename the ALIAS_..._WEIGHT and IM_..._WEIGHT constants to NAME_ and ADDR_, respectively.
Review of attachment 198235 [details] [review]: Looks mostly good. I *think* folks_email_details_get_email_addresses() doesn't ref the return value, so the code should be fine - I have trouble to get my head around folks/gee memory management though, so take this with a grain of salt :-) ::: src/shell-contact-system.c @@ +119,3 @@ char *alias = shell_util_normalize_and_casefold (folks_alias_details_get_alias (FOLKS_ALIAS_DETAILS (individual))); + char *name = shell_util_normalize_and_casefold (folks_name_details_get_full_name (FOLKS_NAME_DETAILS (individual))); + char *nick = shell_util_normalize_and_casefold (folks_name_details_get_nickname (FOLKS_NAME_DETAILS (individual))); Leaking name and nick
Comment on attachment 197925 [details] [review] improve contacts display The contacts-display patch needs improving - when we only match an email address, it still shows up as "Unknown" (and folks/gee is awesome enough to not allow us to get that from JS :/)
Created attachment 198348 [details] [review] Move shell contacts search closer to gnome-contacts Fixed leaking name/nick. I've confirmed with some folks hackers that memory is handled correctly for get_im_addresses()/get_email_addresses().
Created attachment 198361 [details] [review] contact-system: Add helper method to get a (display) email Folks uses collection/set objects from libgee to store email addresses associated with an individual. Unfortunately to extract addresses, parts of libgee which are unusable from (introspected) bindings have to be used[0], so add a helper method. Note that no effort is made to ensure that the returned mail actually corresponds to the one we matched - neat as this would be, it'd require much more intrusive changes than just making sure that we display something better than "Unknown" in search results ... [0] in particular gee_iterator_get(), which is annotated as "return: (transfer full): gpointer"
Created attachment 198362 [details] [review] contact-display: Try harder to display a meaningful name We now match individuals on other properties than alias, so take this into account when representing a contact in search results to avoid having them show up as "Unknown". Updated the previous patch to also try an email before falling back to "Unknown".
Review of attachment 198348 [details] [review]: Looks good.
Review of attachment 198361 [details] [review]: Overall looks good, one leak needs to be fixed though. ::: src/shell-contact-system.c @@ +362,3 @@ + } + + g_object_unref (addrs_iter); Should unref email_addrs as well.
Review of attachment 198362 [details] [review]: Looks good.
Attachment 198348 [details] pushed as 4ec5e55 - Move shell contacts search closer to gnome-contacts Attachment 198361 [details] pushed as 503508a - contact-system: Add helper method to get a (display) email Attachment 198362 [details] pushed as 0d5618f - contact-display: Try harder to display a meaningful name (In reply to comment #13) > Overall looks good, one leak needs to be fixed though. > > ::: src/shell-contact-system.c > @@ +362,3 @@ > + } > + > + g_object_unref (addrs_iter); > > Should unref email_addrs as well. Actually, no. folks_email_detail_get_email_addresses() returns a (transfer none) GeeSet, which is email_addrs. folks_im_detail_get_im_addresses() returns a (transfer none) GeeMultiMap, from which we get a (transfer full) GeeCollection, which is im_addrs. Yay for API consistency \o/