GNOME Bugzilla – Bug 585168
Scrollback search and jump-to-highlight would be nice.
Last modified: 2010-01-12 15:40:19 UTC
I'm using Empathy for IRC, and am really missing /lastlog and the awaylog from irssi. I'd like to be able to mash ^F and search the scrollback (presumably in reverse order, which might be a tad counterintuitive, but hey); also, I'd like to be able to jump between lines on which I'm highlighted. In both cases, a Firefox-esque "Find:" bar just below the scrollback would probably make sense.
We definitely need this.
EmpathyChatView interface already have APIs to highlight words and find prev/next. This is just a matter of adding a firefox-like search bar in EmpathyChat and bind its buttons to EmpathyChatView methods.
Are you talking about just in the scrollback of the current window, or to also look in the recent logs?
I think it is only for scrollback of the current window. But it could be interesting to have a button "search in older logs" that opens the log window. Or maybe when click "previous" button and you reach the top of the scrollback it could ask "Do you want to search in older logs?" and then open the log window.
I had a first go at this one. You can check it out in the search-bar branch of git://github.com/blackskad/empathy.git Please note that there's no support for older logs yet and an issue with the focus chain, so tabbing between search entry, search buttons and chat entry doesn't really work.
Hi Thomas. Thanks a lot for your branch. Here are some comments. I didn't look at the code yet. - I think we should always highlight the pattern (as Epiphany does). No need to have a button for that. - Maybe add a "case sensitive" checkbox as in Epiphany? - Actually I think we should make it look as much as possible as the Epiphany bar, so the next/previous labels should be renamed too. - Which focus chain issue do you have exactly? Could you please attach a full diff of your branch when submitting for reviewing so we can use the nice "review" feature of the bugzilla. :)
Created attachment 151172 [details] [review] Add a search bar to EmpathyChat The patch adds a basic search bar to EmpathyChat to search the scrollback. Searches are automatically highlighted in the scrollback. I'll create separate bugs for case insensitive search and a button to search in old logs.
Review of attachment 151172 [details] [review]: Looks pretty good to me. I inlined some comments. You should consider to rebase your work-in-progress branches on top of master instead of merging. That helps to keep the historic clean. You can also use git rebase -i and squash patches together when it makes sense. New files should be written using the Telepathy coding style. Sorry for that, I know that style is currently a mess in Empathy. :\ Hopefully search-bar.[ch] are not very big so it should be easy to convert them to the right style. See http://live.gnome.org/Empathy#Hacking ::: libempathy-gtk/empathy-chat.c @@ +1374,3 @@ + /* catch ctrl-f to display the search bar */ + if ((event->state & GDK_CONTROL_MASK) && (event->keyval == GDK_f)) { + empathy_search_bar_show (EMPATHY_SEARCH_BAR(priv->search_bar)); Use "MACRO (badger)". ::: libempathy-gtk/empathy-search-bar.c @@ +16,3 @@ + * License along with this code; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + */ Did you steal code or idea from an existing search bar implementation? If you did that should be mentionned in the header. @@ +46,3 @@ +}; + +static void empathy_search_bar_finalize (GObject *object); Why not reorder those and move them before class_init so we don't need the prototypes? @@ +54,3 @@ +static void empathy_search_bar_previous_cb (GtkButton *button, gpointer user_data); + +// TODO: add button to show the older chat logs Don't use C++ style comments. @@ +124,3 @@ +empathy_search_bar_size_request (GtkWidget *widget, + GtkRequisition *requisition) +{ Is that really needed? GtkContainer doesn't do that for us?
Created attachment 151252 [details] [review] Add a search bar to EmpathyChat New patch containing improvements based on the review. Mostly style related.
Review of attachment 151252 [details] [review]: ::: libempathy-gtk/empathy-search-bar.c @@ +37,3 @@ + EmpathyChatView *chat_view; + + GtkWidget *search_entry; No need to align args. @@ +57,3 @@ + +static void +empathy_search_bar_size_request (GtkWidget *widget, Don't align args. The others args should be aligned with 4 space: static void badger (int mushroom, int snake) @@ +61,3 @@ +{ + GtkBin *bin; + GtkWidget *child; Don't align args. @@ +224,3 @@ + filename = empathy_file_lookup ("empathy-search-bar.ui", "libempathy-gtk"); + gui = empathy_builder_get_file (filename, + "search_widget", &internal, Should be 4-spaces aligned: function (arg1, arg2); @@ +233,3 @@ + g_free (filename); + + /* Add the signals */ You could use empathy_builder_connect to connect those signals.
Created attachment 151256 [details] [review] Add a search bar to EmpathyChat The "one day, i'll get the style right" patch :) It also uses empathy_builder_connect instead of g_signal_connect.
Merged to master! Thanks a lot for this great work. This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.