After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 608166 - Add contacts live search bar
Add contacts live search bar
Status: RESOLVED FIXED
Product: empathy
Classification: Core
Component: Contact List
2.29.x
Other Linux
: Normal enhancement
: ---
Assigned To: empathy-maint
Depends on:
Blocks: 619873
 
 
Reported: 2010-01-26 15:27 UTC by PabloEstefo
Modified: 2010-06-10 15:20 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description PabloEstefo 2010-01-26 15:27:53 UTC
The whole description is here: http://live.gnome.org/FrequentRelatedContactsEmpathy
Comment 1 Guillaume Desmottes 2010-02-08 16:16:21 UTC
Hi Pablo. Thanks a lot of your suggestion, there are really good ideas here!

We tend to use one bug report per bug/feature. If I understand your suggestions correctly I see 3 ideas here:
A) Display more frequent contacts
B) Display contacts related to our current work
C) Improve contacts search

I suggest to focus on C) in this bug report. A) is pretty close to bug #571668 and B) is a nice idea but will need some Zeitgeist integration.

So, I do agree that having a nice live search bar would be great. I think this bar should be hidden by default and displayed when you start typing in the contacts list or hitting Ctrl+F. The contacts list would be filtered to display only contacts matching (using their ID, alias or real name) the search pattern.
The bar would have a close button (as the chat view search bar) and would disappear when hitting "Escape".

What do you think about this approach? Is that something you'd be interested to work on?
Comment 2 Praveen Thirukonda 2010-03-26 09:14:18 UTC
 >I think this bar should be hidden by default 

if it's hidden by default then, only a small number of people will use it.

gmail's chat widget provides a search box at all times and is very sueful. since empathy allows many accounts to be used together,s o number of contacts will be generally high and a search box displayed at all times makes sense IMHO.
Comment 3 Patryk Zawadzki 2010-03-26 09:26:14 UTC
(In reply to comment #1)
> So, I do agree that having a nice live search bar would be great. I think this
> bar should be hidden by default and displayed when you start typing in the
> contacts list or hitting Ctrl+F. The contacts list would be filtered to display
> only contacts matching (using their ID, alias or real name) the search pattern.
> The bar would have a close button (as the chat view search bar) and would
> disappear when hitting "Escape".

Search bars are better suited for documents when you always show the whole file but let the user skip to the next/previous match. What you describe here is a search bar.

I think we need a filter bar instead. So it should:

1) be placed above the treeview (so I guess that would mean moving the status to the bottom),
2) stay visible at all times and
3) act as a treeview model filter and actually hide all the contacts that do not match the entered string.

Maybe the usability team has something to add here?
Comment 4 Hylke Bons 2010-03-26 14:37:32 UTC
(In reply to comment #3)
> I think we need a filter bar instead. So it should:
> 
> 1) be placed above the treeview (so I guess that would mean moving the status
> to the bottom),

Status is best kept  at the top.

> 2) stay visible at all times and

Not so sure about that, it's kind of ugly and confusing with two text entries (one for status). Like said, we can have an accelerator and also a menu entry should be enough. Also it should appear when you start typing.

> 3) act as a treeview model filter and actually hide all the contacts that do
> not match the entered string.

Definitely. Match substrings too.

> 
> Maybe the usability team has something to add here?
Comment 5 Xavier Claessens 2010-03-26 15:15:50 UTC
Juste a note for live search: I did a big part of HildonLiveSearch and it is LGPL, that could be useful as a base for EmpathyLiveSearch or even GtkLiveSearch.
Comment 7 Felix Kaser 2010-04-18 17:26:22 UTC
I've updated the branch and for now there remain only two problems:

- all groups should be expanded during the search
- hitting backspace during the search sometimes results in collapsed groups which should be expanded. hitting backspace once again the group expands (I guess this has something to do with the order the filter checks the contacts if they should be visible or not)

I will take care of those last parts as soon as I can find some time. If someone could review the branch already would be great.

http://git.collabora.co.uk/?p=user/kaserf/empathy.git;a=shortlog;h=refs/heads/live-search-608166
Comment 8 Guillaume Desmottes 2010-05-06 09:09:06 UTC
Comments after some testing (I'll let Xavier do the code review as he know this thing much better than I do):
- Typing the first key of the search is really slow. The ones after seems ok
- Maybe make the search entry a bit higher? It looks a bit small here
- Would be good if hitting escape when we are in the bar makes it disappear and reset the search (like with the search bar in chats)
Comment 9 Felix Kaser 2010-05-06 09:37:42 UTC
irc snipplet of some discussion about this:


<Zdra> one reason of slowness: https://bugzilla.gnome.org/show_bug.cgi?id=616871
...
<Zdra> why is contact_list_store_contact_update() called on every contact when taping a letter in the live search?
<Zdra> I think the live search should be inside a GtkToolbar or something
<kaserf> cassidy: the escape key works fine here...except its slow because show offline is toggled again...
<Zdra> more like the search bar in epiphany
...
<Zdra> ah, ok, starting live search toggles show-offline...
<cassidy> wjt, hey, I heard you're dreaming to have live search support in Empathy right? :) If you are interested in testing kaserf's branch (from a feature pov) feel free to do and comment on https://bugzilla.gnome.org/show_bug.cgi?id=608166
<Zdra> I think show-offline should be using a GtkTreeModelFilter instead of adding/removing form the EmpathyContactListStore
<kaserf> Zdra: agreed
<kaserf> I thought about that too while implementing the search
<Zdra> so the EmpathyContactListStore should always have offline inside, and can drop its "show-offline" property
<Zdra> that's a view property, not store/model
<Zdra> features are really badly spread over store/view :(
<cassidy> kaserf, maybe hitting escape in the contact list should hide the search entry as well?
<kaserf> it does ^^
<kaserf> (should :D)
<Zdra> kaserf, so the filter should be made private inside EmpathyContactListView, and it has a visible_func that returns TRUE if: (match live-search string || contact_online || show_offline)
<cassidy> nope, it closes the window here
<cassidy> it reset it only if the entry is focused
<kaserf> cassidy: thats true
<cassidy> I think it would be more convenient
<kaserf> and intended...if you want to close the window with the actual search
<Zdra> kaserf, also deleting last letter in the livesearch entry should hide it
<kaserf> hmm...I wasnt sure about it so I did the first that came in mind
<kaserf> Zdra: I thought about that too...but isnt it odd that the entry disappears when its empty?
<Zdra> tbh, do exactly how it's done in hildon, I totally love it :)
<kaserf> Zdra: you're biased ;)
* cassidy really like the hildon approach as well
Comment 10 Xavier Claessens 2010-05-06 09:48:12 UTC
Some early comments:

1) I think the GtkTreeModelFilter should be kept as private implementation detail inside EmpathyContactListView. The EmpathyLiveSearch can be hooked inside EmpathyContactListView, like that any places where we use a contact list will gain live search for free (chatroom members list).

2) The show-offline property should be moved from EmpathyContactListStore to EmpathyContactListView. The store would always contain all contacts, and the filter would be used to hide offline contacts if needed (remember point1 where I said the filter should be inside the view). Like that, the visible_func would be something like ((live search not empthy && contact match live search text) || (live search empty && (contact online || 'show-offline' property is true on the view)))

3) Ideally it would be awesome if we could move EmpathyListSearch to GTK at some point (as GtkLiveSearch). I think it's a feature that could benefit any GtkTreeView. For that, the live search shouldn't depend on any empathy stuff. So you should have something like empathy_live_search_set_visible_func(), like that the view can set its own visible_func and play with EmpathyContact stuff.

4) Removing last char from the entry should hide the live search IMO.

5) I think the live search could be packed inside a bar, like a GtkToolbar, to look more like the search bar in epiphany/firefox.
Comment 11 Xavier Claessens 2010-05-06 12:33:39 UTC
6) EmpathyLiveSearch should be LGPL, preferably.
Comment 12 Xavier Claessens 2010-05-23 07:28:21 UTC
I took a look at latest code. It is getting better IMO, some observations:

 - Why did you rename to EmpathyContactSearch? The whole point here is it does not depend on contact, so I prefer EmpathyLiveSearch name.

 - You didn't move the "show-offline" property from EmpathyContactListStore to EmpathyContactListView. Now you have the filter inside the view, I think you could use it to show/hide offline contacts, that should be just a couple of lines inside contact_list_view_is_visible_contact(). Note that currently it does not seems to search in offline contacts anymore.

 - When erasing the text in the entry, groups become visible again, but they are collapsed.

 - "started" signal is emitted on each key press

 - I don't really know how, but I got in the state where the entry was hidden, but typing letters was still filtering contacts.

 - You should not show the entry on key press event, some languages like Chinees needs composing the chars with hitting more than one key. In that case you want to show the entry only once the first visible char is composed. See also how we compose ê on French AZERTY keyboard: first we hit '^' key which display nothing (but generate a key-press-event) then hit the 'e' letter and at that moment a 'ê' is composed. I don't think your trick to check if unicode is != 0 is enough, I think you should start the search in contact_search_text_changed() because that's emitted when the composition is done and a visible letter is entered in the entry. Unicode is really complex, let's InputMethod decide for us ;-) For this to work, we have a hack in HildonLiveSearch:

            /* If the entry is realized and has focus, it is enough to catch events.
             * This assumes that the toolbar is a child of the hook widget. */
            gtk_widget_realize (priv->entry);
            if (!GTK_WIDGET_HAS_FOCUS (priv->entry))
                gtk_widget_grab_focus (priv->entry);

We do that just before forwarding the event to the entry.

 - In contact_search_key_press_event_cb(), you should get the return value of gtk_widget_event() and return that value for the key-press-event signal you're handing.
Comment 13 Felix Kaser 2010-05-24 12:23:04 UTC
(In reply to comment #12)
> I took a look at latest code. It is getting better IMO, some observations:
Thanks a lot :) I should have warned you that there are not all the things done yet...

> 
>  - Why did you rename to EmpathyContactSearch? The whole point here is it does
> not depend on contact, so I prefer EmpathyLiveSearch name.
Agreed
> 
>  - You didn't move the "show-offline" property from EmpathyContactListStore to
> EmpathyContactListView. Now you have the filter inside the view, I think you
> could use it to show/hide offline contacts, that should be just a couple of
> lines inside contact_list_view_is_visible_contact(). Note that currently it
> does not seems to search in offline contacts anymore.
IMHO that should be done in a different patch/branch. I would prefer to merge live search once the last problems are solved (without showing offline contacts) and add that feature afterwards (show offline as part of the filter + auto-show-offline in search).
> 
>  - When erasing the text in the entry, groups become visible again, but they
> are collapsed.
That bug is giving me a hard time. I had the same problem with the initial implementation, but there I found a workaround (double refilter when new search string is smaller then the old), which seems to be useless with the new implementation... Don't ask me why...
> 
>  - "started" signal is emitted on each key press
noted :)
> 
>  - I don't really know how, but I got in the state where the entry was hidden,
> but typing letters was still filtering contacts.
fixed that on saturday...is on the repo now.
> 
>  - You should not show the entry on key press event, some languages like
> Chinees needs composing the chars with hitting more than one key. In that case
> you want to show the entry only once the first visible char is composed. See
> also how we compose ê on French AZERTY keyboard: first we hit '^' key which
> display nothing (but generate a key-press-event) then hit the 'e' letter and at
> that moment a 'ê' is composed. I don't think your trick to check if unicode is
> != 0 is enough, I think you should start the search in
> contact_search_text_changed() because that's emitted when the composition is
> done and a visible letter is entered in the entry. Unicode is really complex,
> let's InputMethod decide for us ;-) For this to work, we have a hack in
> HildonLiveSearch:
> 
>             /* If the entry is realized and has focus, it is enough to catch
> events.
>              * This assumes that the toolbar is a child of the hook widget. */
>             gtk_widget_realize (priv->entry);
>             if (!GTK_WIDGET_HAS_FOCUS (priv->entry))
>                 gtk_widget_grab_focus (priv->entry);
> 
> We do that just before forwarding the event to the entry.
uh thats pretty nice and a good point, noted
> 
>  - In contact_search_key_press_event_cb(), you should get the return value of
> gtk_widget_event() and return that value for the key-press-event signal you're
> handing.
noted as well :)

thanks a lot for the review! the thing with collapsed groups really annoys me. I've investigated this a lot during the initial implementation and could not find the reason for this bug. I also called gtk_tree_view_expand_row manually which did not do the trick. Probably the best way to solve this would be expand_all, which is one of the features still missing. If anyone has an idea how to solve it, please go ahead! (I think the why is obvious: the group becomes visible before any child does: top down, and gtk tree view sets expanded = false per default for empty groups...however calling expand_row should expand it...)
Comment 14 Xavier Claessens 2010-05-24 12:29:04 UTC
Note that we have code that store the expanded/collapsed state in a file, and restore that state when starting empathy. Maybe that code is on the way here? I don't remember how it is done exactly, but you could check that.
Comment 15 Patryk Zawadzki 2010-05-24 12:33:43 UTC
Ad comment 13:

Isn't there a way to tell GTK+ you're still updating the model so it shouldn't parse its contents yet?
Comment 16 Felix Kaser 2010-05-24 13:13:34 UTC
(In reply to comment #14)
> Note that we have code that store the expanded/collapsed state in a file, and
> restore that state when starting empathy. Maybe that code is on the way here? I
> don't remember how it is done exactly, but you could check that.
I've already seen it. Shouldnt be too hard to do this, maybe thats why it was at the bottom of my todo list ;)

(In reply to comment #15)
> Ad comment 13:
> 
> Isn't there a way to tell GTK+ you're still updating the model so it shouldn't
> parse its contents yet?
uh that sounds interesting! I couldnt find anything in the doc... :(
Comment 17 Patryk Zawadzki 2010-05-24 13:22:54 UTC
Felix:

Ah, I thought there was a separate API. You can try this however: 1) set model to null, 2) update the tree, 3) re-attach the model.

Not sure if this will solve the problem.
Comment 18 Felix Kaser 2010-05-24 13:59:17 UTC
(In reply to comment #17)
> Felix:
> 
> Ah, I thought there was a separate API. You can try this however: 1) set model
> to null, 2) update the tree, 3) re-attach the model.
> 
> Not sure if this will solve the problem.

did not work :( I guess the way to go is to expand all during the search and restore the expanded setting with "empathy_contact_group_get_expanded" when the search is over.
Comment 19 Felix Kaser 2010-05-27 15:44:01 UTC
I've updated the branch: http://git.collabora.co.uk/?p=user/kaserf/empathy.git;a=shortlog;h=refs/heads/live-search-608166

the only thing missing right now is to automatically toggle show-offline. I would like to do this in a different branch, because I think it is quite some refactoring of the current code.

could you give it a look please?
Comment 20 Guillaume Desmottes 2010-05-28 06:55:41 UTC
*** Bug 619873 has been marked as a duplicate of this bug. ***
Comment 21 Xavier Claessens 2010-06-04 13:30:16 UTC
I reviewed empathy-utf8 and empathy-live-search. Didn't look deeply inside the treeview code yet.

Sorry about being pedentic, but I really would like to get that right, so we can propose to get included inside GTK itself, when it proved to be working correctly.

Thanks for your patches, I think you are doing a great job, and we are really getting close to something mergeable :D

1) I know UTF-8 is the future, but empathy-utf8.ch is copyrighted 20010... Also I don't think you are allowed to set Collabora on the copyright since that code is stolen from Hildon, and they got it from EDS. Could you please re-check correctly the licence (LGPLv2.1 or 2.1+ ?), the authors and the copyright holder?

Of course if you made modifications yourself, it's fine to add collabora copyright in addition to the eds/hildon one.

2) Could you please set LGPLv2.1+ on EmpathyLiveSearch? It seems to be GPLv2+ atm.

3) EmpathyLiveSearch could have a "hook-widget" read/write property together with getter/setter methods empathy_live_search_get/set_hook_widget().

I think the constructor function should take the hook widget in args since the live search is unusable without it. Maybe accept hook_widget to be NULL in case the user want to set it later:

GtkWidget *empathy_live_search_new(GtkWidget *hook_widget);

4) empathy_live_search_stop_search() could be removed, just call gtk_widget_hide(live_search). Same for empathy_live_search_start_search() that could be replaced by gtk_widget_show(live_search). Inside EmpathyLiveSearch you could connect "show" signal (to grab focus) and "hide" signal (to refilter with empty text). Instead of connecting those signals, you can implement GtkWidgetClass->show and GtkWidgetClass->hide in empathy_live_search_class_init(), make your implementation chainup to the parent class.

Also note that empathy_live_search_stop_search() can be simplified by simply set the text on the entry to "" and let live_search_text_changed() do the rest.

5) live_search_text_changed(): Since all the work will be done in hide/show callbacks, that function can be simplified to something like this:

{
  text = gtk_entry_get_text (entry);
  if (g_utf8_len (text) < 1)
    text = NULL;
  if (text == NULL)
    gtk_widget_hide (self);
  else
    gtk_widget_show (self);

  g_signal_emit (self, "refilter", text);
}

I'm not completely sure why g_utf8_len() is needed, but HildonLiveSearch does it and I remember that was useful :)

6) "started" and "stopped" signals could be removed, they are identical to "show" and "hide" that does already exist on GtkWidget base class.

7) old_entry_text in EmpathyLiveSearchPriv is not used.

8) You should connect "destroy" signal on the hook widget, if it is emitted, drop your ref, set priv->hook_widget to NULL, and hide the livesearch.

9) Instead of a "refilter" signal, I like the idea HildonLiveSearch got recently: add a read/write "text" property with getter/setter methods. Like that users can connect "notify::text" to know when to refilter. Also could be useful to set that property so I can define a text to search intially for example.

10) I think we can make the usage of EmpathyLiveSearch much easier by providing a method like this one:

gboolean empathy_live_search_matches(EmpathyLiveSearch *self, gchar *str);

That function can do all the utf8 work.

To be clearer, here is what happens:
  - The entry emits "changed" signal. The livesearch gets the text and keeps the stripped text in private struct. Then call g_object_notify (self, "text");
 - The treeview gets "notify::text" signal and call empathy_live_search_matches() for each row.
 - empathy_live_search_matches() says if the text matches, using empathy_utf8_strstrcasedecomp_needle_stripped(), giving the pre-computed stripped text.
Comment 22 Xavier Claessens 2010-06-04 13:38:03 UTC
Oh I found why it is slow when stopping the live search. You can remove main_window_search_stopped_cb(), we decided to handle offline contacts later.
Comment 23 Xavier Claessens 2010-06-04 13:57:03 UTC
I've found a bug when playing with the branch:

1) Search for some contacts, say "Marco"
2) click on the treeview, either select a contact, or just click in white area
3) type some more letters

=> The live search is reset and filter only the letters typed in step 3.
Comment 24 Felix Kaser 2010-06-04 15:16:17 UTC
thanks a lot for the review!

(In reply to comment #21)
> I reviewed empathy-utf8 and empathy-live-search. Didn't look deeply inside the
> treeview code yet.
> 
> Sorry about being pedentic, but I really would like to get that right, so we
> can propose to get included inside GTK itself, when it proved to be working
> correctly.
no worries about that. how can I get better if nobody tells me what I did wrong? :)
> 
> Thanks for your patches, I think you are doing a great job, and we are really
> getting close to something mergeable :D
> 
> 1) I know UTF-8 is the future, but empathy-utf8.ch is copyrighted 20010... Also
> I don't think you are allowed to set Collabora on the copyright since that code
> is stolen from Hildon, and they got it from EDS. Could you please re-check
> correctly the licence (LGPLv2.1 or 2.1+ ?), the authors and the copyright
> holder?
> 
> Of course if you made modifications yourself, it's fine to add collabora
> copyright in addition to the eds/hildon one.
> 
> 2) Could you please set LGPLv2.1+ on EmpathyLiveSearch? It seems to be GPLv2+
> atm.

thanks about all the license stuff, I'm not very experienced with that.
> 
[...]
> 
> 
> 10) I think we can make the usage of EmpathyLiveSearch much easier by providing
> a method like this one:
> 
> gboolean empathy_live_search_matches(EmpathyLiveSearch *self, gchar *str);
> 
> That function can do all the utf8 work.
> 
> To be clearer, here is what happens:
>   - The entry emits "changed" signal. The livesearch gets the text and keeps
> the stripped text in private struct. Then call g_object_notify (self, "text");
>  - The treeview gets "notify::text" signal and call
> empathy_live_search_matches() for each row.
>  - empathy_live_search_matches() says if the text matches, using
> empathy_utf8_strstrcasedecomp_needle_stripped(), giving the pre-computed
> stripped text.
If I understand correctly, this method does only the pure string matching. nothing else? If I would like to search for the user name and id I would have to call this twice for each contact right? (except the magic which is needed with groups)


(In reply to comment #23)
> I've found a bug when playing with the branch:
> 
> 1) Search for some contacts, say "Marco"
> 2) click on the treeview, either select a contact, or just click in white area
> 3) type some more letters
> 
> => The live search is reset and filter only the letters typed in step 3.
what happens exactly? is the search still visible or not? I thought I already had this one fixed...see following commit: http://git.collabora.co.uk/?p=user/kaserf/empathy.git;a=commit;h=80489f376e40b906f38fe9f88fe086c72ac9848f

what I missed from your review is what we began to discuss in IRC some days (maybe a week?) ago, which was about what to set as hook widget (treeview or scrolled window) and that treeview should be focused by default on startup...Somehow we did not finish the discussion there so I'm not sure which way to go now...
Comment 25 Xavier Claessens 2010-06-04 15:35:34 UTC
> > To be clearer, here is what happens:
> >   - The entry emits "changed" signal. The livesearch gets the text and keeps
> > the stripped text in private struct. Then call g_object_notify (self, "text");
> >  - The treeview gets "notify::text" signal and call
> > empathy_live_search_matches() for each row.
> >  - empathy_live_search_matches() says if the text matches, using
> > empathy_utf8_strstrcasedecomp_needle_stripped(), giving the pre-computed
> > stripped text.
> If I understand correctly, this method does only the pure string matching.
> nothing else? If I would like to search for the user name and id I would have
> to call this twice for each contact right? (except the magic which is needed
> with groups)

Right.

> 
> (In reply to comment #23)
> > I've found a bug when playing with the branch:
> > 
> > 1) Search for some contacts, say "Marco"
> > 2) click on the treeview, either select a contact, or just click in white area
> > 3) type some more letters
> > 
> > => The live search is reset and filter only the letters typed in step 3.
> what happens exactly? is the search still visible or not? I thought I already
> had this one fixed...see following commit:
> http://git.collabora.co.uk/?p=user/kaserf/empathy.git;a=commit;h=80489f376e40b906f38fe9f88fe086c72ac9848f

The live search is not hidden in my steps, it's just that before step 3 the live search contains "marco" and lost the focus. Typing a new letter (step 3) replaces the text in the entry by the new letters I'm typing.

> what I missed from your review is what we began to discuss in IRC some days
> (maybe a week?) ago, which was about what to set as hook widget (treeview or
> scrolled window) and that treeview should be focused by default on
> startup...Somehow we did not finish the discussion there so I'm not sure which
> way to go now...

Didn't though about that yet, I reviewed only EmpathyLiveSearch so far, I'll review its usage in main-window/treeview later :)
Comment 26 Xavier Claessens 2010-06-04 20:11:21 UTC
I did some changes:

1) I pushed in empathy master some coding style changes I saw in your branch (aligning functions args). You should not do things like that in an unrelated branch because it make it harder to read and has more risks of conflicting. Better to make a separate trivial branch I can merge directly ;-)

2) I squashed all your patches because we won't really need that long history and it's not possible to follow patch by patch.

3) I reverted the changes for showing offline contacts that was left in your branch, as said on comment #22

4) I removed "search_vbox" from the glade, we can pack the live search directly in main_vbox, no need to make a vbox inside a vbox ;-)

You can find my cleaned branch there: http://git.collabora.co.uk/?p=user/xclaesse/empathy.git;a=shortlog;h=refs/heads/live-search
Comment 27 Felix Kaser 2010-06-04 22:26:24 UTC
(In reply to comment #21)
> 
> 1) I know UTF-8 is the future, but empathy-utf8.ch is copyrighted 20010... Also
> I don't think you are allowed to set Collabora on the copyright since that code
> is stolen from Hildon, and they got it from EDS. Could you please re-check
> correctly the licence (LGPLv2.1 or 2.1+ ?), the authors and the copyright
> holder?
> 
> Of course if you made modifications yourself, it's fine to add collabora
> copyright in addition to the eds/hildon one.
> 
> 

I have some questions about this...:

Should I simply copy the copyright header from hildon-helper? That one contains the line 

> Copyright (C) 2005, 2006 Nokia Corporation, all rights reserved.

which doesnt seem really up to date.

Does the "version 2.1 of the License, or (at your option) any later version." mean LGPL v2.1+?

Also, the license in hildon-helper does not say anything about EDS...

Should I at least add my name as a contact, since I've added it to empathy...?

Sorry for all those questions, I'm not really familiar / experienced with all the license stuff, but I hope to get it as soon as possible.
Comment 28 Xavier Claessens 2010-06-04 23:09:30 UTC
Yes, the '+' in version number means the licence has the ", or (at your option) any later version.".

The licence/copyright in hildon-helper.c is wrong :(

The licence of the original code in EDS is LGPLv2 (note: it is version 2 only, not later version), and Copyright (C) 2003 Ximian, Inc.

This is a bit annoying tbh... I'll check with hildon guys what we can do for this.
Comment 29 Xavier Claessens 2010-06-06 09:05:54 UTC
fyi: I cleaned, simplified and almost rewrote the UTF8 functions. I moved them directly inside empathy-live-search.c and delete empathy-utf8.c. Looking closer to those functions and now I think they are really not generic UTF-8 functions, their behaviour is specific to the live search. It does not do strstr-like match anymore since it match only start of words.

This is fixing my comment 10 as well :-)

Also, I saw "contact" could be leaked in various cases in contact_list_view_filter_visible_func(). I fixed that.

You can cherry-pick the commit from my live-search branch: http://git.collabora.co.uk/?p=user/xclaesse/empathy.git;a=shortlog;h=refs/heads/live-search,﷒0﷓.

Please read the changes I made and tell me if you see anything wrong in it ;-)
Comment 30 Felix Kaser 2010-06-08 08:08:20 UTC
I have picked your commits and realized the changes from your review.

Can you give it another look? Thanks for the good review, I really like it more this way (without all the signals firing and that stuff).

you can find the actual branch at http://git.collabora.co.uk/?p=user/kaserf/empathy.git;a=shortlog;h=refs/heads/live-search-v2
Comment 31 Xavier Claessens 2010-06-09 11:46:39 UTC
I did a final cleanup myself, and merged everything \o/

Known issues:

 - If the live search has text "xav" and then a contact named "Xavier" becomes online, it won't be displayed in the search results, until the live search is closed. This is a GtkTreeModelFilter bug #621076. Felix made an ugly workaround for that in empathy code but I droped that code, since I now have confirmation that it is a GTK issue (made GTK unit test to demonstrate) and the issue is not that bad tbh.

 - Sometimes the live search does not get the focus and typing on keyboard does nothing. Notably it happens when empathy is first started. The main window is presented, so the keyboard focus should be somewhere inside it... but not on the treeview... Don't know exactly what happens... bug #621089

 - Offline contacts should be shown when livesearch is started, but that needs a bit of refactoring in the internal code. Will be done in separate branch/bug. Bug #621090

Closing this bug, future improvement will be done in separate bugs/branches.
Comment 32 Felix Kaser 2010-06-09 12:01:06 UTC
(In reply to comment #31)
> I did a final cleanup myself, and merged everything \o/
thanks \o/

> 
> Known issues:
> 
>  - If the live search has text "xav" and then a contact named "Xavier" becomes
> online, it won't be displayed in the search results, until the live search is
> closed. This is a GtkTreeModelFilter bug #621076. Felix made an ugly workaround
> for that in empathy code but I droped that code, since I now have confirmation
> that it is a GTK issue (made GTK unit test to demonstrate) and the issue is not
> that bad tbh.

good to know! :)
Comment 33 John Stowers 2010-06-10 13:48:39 UTC
Instead of the 'X' on the right to clear, could it instead use the standard (aka used in evolution, etc) GtkEntry with the embedded clear icon on the right hand side?
Comment 34 Felix Kaser 2010-06-10 13:52:27 UTC
(In reply to comment #33)
> Instead of the 'X' on the right to clear, could it instead use the standard
> (aka used in evolution, etc) GtkEntry with the embedded clear icon on the right
> hand side?

I had that at an earlier stage. But since the button closes the search I liked the X more.
Comment 35 Xavier Claessens 2010-06-10 15:20:39 UTC
I agree with Felix, that button is not the clear the text in entry, but to close it completely.

Also, please report as new bug if you have problems with the live search, this bug is about initial implementation, and is RESOLVED :)