GNOME Bugzilla – Bug 639877
Retrieve all the past history when scrolling back on a chat window
Last modified: 2013-01-31 15:53:29 UTC
Right now when you open a chat window, we ask the logger for the last bits of the history. It would be nice if scrolling up would cause all the history to be retrieved.
s/all/progressively more, as you keep scrolling up/ - infinite history style
Yeah, I really like that on the N900.
*** Bug 606754 has been marked as a duplicate of this bug. ***
*** Bug 658889 has been marked as a duplicate of this bug. ***
This is blocked by https://bugs.freedesktop.org/show_bug.cgi?id=41772
(In reply to comment #1) > s/all/progressively more, as you keep scrolling up/ - infinite history style Or you could use chunking (see how GNOME Documents handles scrolling). One thing to note - it might be necessary to allow viewing the history for multi-party chats (ie. with more than one participant).
Err, make that more than two participants. :)
We should also support search through logs from the conversation window. So, if I search for "Badger", I'm jumped to the most recent log containing "Badger" and can go back in time to see older occurences.
I have started working on this: http://git.gnome.org/browse/empathy/log/?h=wip/infinite-history
I moved the branch to: http://cgit.freedesktop.org/~debarshir/empathy/log/?h=infinite-history
This is now ready to be reviewed.
Ping? Would be cool to have it for 3.5.4.
Rebased the branch on top of current Git master.
I tested the branch with a not patched theme (the ubuntu theme), the scrollbar goes top down when trying to scroll up. Is that something we could avoid ? We don't usually split function call on one line per argument. We just wrap if the call is longer than 80 chars. theme_adium_add_message() is getting a bit hard to understand. Would be good to document its arguments; especially @add_html_func and @js_funcs. I'd use #define rather than magic numbers when indexing js_funcs. theme_adium_prepend_html: could you please document a bit the part of the code loading and executing a js script?
empathy-chat --------------------- I guess the 2 last commits can be squashed together. + gboolean watch_scroll; + guint max_page_size; + guint scroll_offset; Please document the semantic of those variables The if/else block you added to got_filtered_messages_cb() isn't that clear to me. Some more comments about the logic would be good. chat_scrollable_connect(): connecting signals in an idle callback looks weird to me. Can't we connect it right away and just ignore it when we don't need it? Maybe rename chat_add_logs() to chat_fetch_more_logs() or something? chat_add_logs: I'm not a fan of creating the walker there. Can't we create it earlier? It will probably be used anyway so there is no harm creating it sooner in the object.
(In reply to comment #14) > I tested the branch with a not patched theme (the ubuntu theme), the scrollbar > goes top down when trying to scroll up. Is that something we could avoid ? > We don't usually split function call on one line per argument. We just wrap if > the call is longer than 80 chars. Fixed. > theme_adium_add_message() is getting a bit hard to understand. Would be good to > document its arguments; especially @add_html_func and @js_funcs. I added some documentation. I replaced theme_adium_append_html with theme_adium_prepend_html and called it theme_adium_add_html. This simplifies things a bit. > I'd use #define rather than magic numbers when indexing js_funcs. I used enums to preserve the symbol names.
(In reply to comment #15) > + gboolean watch_scroll; > + guint max_page_size; > + guint scroll_offset; > Please document the semantic of those variables Done. > The if/else block you added to got_filtered_messages_cb() isn't that clear to > me. Some more comments about the logic would be good. Done. > chat_scrollable_connect(): connecting signals in an idle callback looks weird > to me. Can't we connect it right away and just ignore it when we don't need it? I explained the reasoning behind the current code in a comment. > Maybe rename chat_add_logs() to chat_fetch_more_logs() or something? > chat_add_logs: I'm not a fan of creating the walker there. Can't we create it > earlier? It will probably be used anyway so there is no harm creating it > sooner in the object. Ok, I moved it to the constructed method. Please note that currently I can not use Empathy because it is refusing to log into password based accounts even though the password exists in the keyring, nor does it actually prompt with a dialog to enter it. So I am unable to make bigger changes as I won't be able to test them.
Thanks Rishi. Your branch looks good to me. It's now only blocked by a tp-logger release and the approval of the release team if we want to get this in 3.6 as we are post feature freeze already.
Rebased the branch against master.
No telepathy-logger release for 3.5.91 so this will be for 3.7.
Rebased the branch against master. This depends on https://bugs.freedesktop.org/show_bug.cgi?id=54270 being merged and released.
The logger bits have now been merged.
Now that telepathy-logger 0.8 has been released, I have merged and pushed this to master. Thanks a lot for the reviews!
*** Bug 658888 has been marked as a duplicate of this bug. ***