GNOME Bugzilla – Bug 571631
Implement whois
Last modified: 2011-05-16 13:18:14 UTC
The following branch (of Pierre-Luc) adds support for command /whois (jid/ircnick) in chat windows, it will open the Contact information dialog. http://git.collabora.co.uk/?p=user/pierlux/empathy.git;a=shortlog;h=refs/heads/whois
Created attachment 128641 [details] [review] [PATCH] Implement /whois libempathy-gtk/empathy-chat.c | 48 ++++++++- libempathy/empathy-chatroom.c | 201 ++++++++++++++++++++++++++++++++- libempathy/empathy-chatroom.h | 12 ++ libempathy/empathy-tp-roomlist.c | 28 +++++ src/empathy-chat-window.c | 15 +++ src/empathy-main-window.glade | 2 +- src/empathy-new-chatroom-dialog.c | 182 ++++++++++++++++++++++-------- src/empathy-new-chatroom-dialog.glade | 160 +++++++++++++------------- 8 files changed, 518 insertions(+), 130 deletions(-)
- Seems this branch is based on an old version of the 'room-ui' branch. Please rebase on top of master with just 4cdfd3dde157d9ad574db3464f0a2542a5296ba6 (I only reviewed this commit). - We could save a variable by using directly: trimmed_msg = g_strchug (g_strdup (msg)); - Actually I'm not sure if we should remove trailing spaces. Disabling command if prefixed by a space could make sense. For example: Alice: How can I clear my chat window? Bob: /clear - With current code commands like /clearer or /whoishe would be supported. They shouldn't. - gchar * nickname >> gchar *nickname - A EmpathyContactFactory is leaked. It should be unreffed. - The EmpathyContact is leaked too. - Shouldn't we wait/check that the contact is ready or something? - I'd add the name of the command in the 2 error messages. - Maybe "Unknown command foo" would be better than "Unsupported command foo" ? - I'm not convince by this "whois-command" signal. IMHO, we should have a generic "command" signal with the command name as argument. But then the arguments of the command should be parsed by the signal catcher. - // Don't send unsupported commands >> don't use C++ style comments
I accidentally re-implemented this on my branch for bug 592795.
Created attachment 187704 [details] [review] Add a /whois command. Fixes: <https://bugzilla.gnome.org/show_bug.cgi?id=571631>
Are you sure you got the dependency right? I suspect you meant bug 592795 ...
Review of attachment 187704 [details] [review]: looks good ::: libempathy-gtk/empathy-chat.c @@ +1003,3 @@ + conn = empathy_tp_chat_get_connection (priv->tp_chat); + tp_connection_get_contacts_by_id (conn, + gchar *event = g_strdup_printf ( This command sounds weird to me, but you're the native speaker here. :)
Merged with a tweaked version of that comment, which also sounded weird to me. http://git.gnome.org/browse/empathy/commit/?id=9c42c7ae5d46bf3bf2aa4aab8e4d615876982eca