GNOME Bugzilla – Bug 638766
/nick should be disabled on connection not supporting Renaming
Last modified: 2011-01-13 08:45:58 UTC
I tried to use /nick to change my nick on a room on conference.jabber.org but was unable to accomplish this. /Nick works fine on IRC.
What is the bug / what is the expected outcome?
(In reply to comment #1) > What is the bug / what is the expected outcome? The bug is that /nick should not be shown on the command list on rooms that do not support nick renaming and instead report that the command not supported.
Created attachment 177611 [details] [review] Allow /nick command only in rooms that support nick renaming ...and here comes a patch for it. :)
FYI: you can use tp_proxy_has_interface() on the TpConnection.
Created attachment 177643 [details] [review] Amended patch Thanks Xavier. Patch and branch both updated.
Review of attachment 177643 [details] [review]: Please link your git branch when submiting patches. Note that this doesn't solve the original bug (changing nick in XMPP mucs) but that's a good first step. ::: libempathy-gtk/empathy-chat.c @@ +120,3 @@ * pending messages *before* messages from logs. (#603980) */ gboolean can_show_pending; + gboolean nick_renaming_supported; Storing isn't really useful as tp_proxy_has_interface() is sync. I'd just have an helper renmaing_is_supported() function or do it directly in the nick_is_supported method (see above). @@ +883,3 @@ if (strv[1] == NULL) { for (i = 0; i < G_N_ELEMENTS (commands); i++) { + if (!(!priv->nick_renaming_supported && One cleaner way to do this would be to add a function pointer in ChatCommandItem called is_supported() and returning a gboolean. If this pointer is not NULL, then we use this function to check if the command is supported. @@ +3121,3 @@ connection = empathy_tp_chat_get_connection (priv->tp_chat); + if (tp_proxy_has_interface (connection, + "org.freedesktop.Telepathy.Connection.Interface.Renaming")) { Please #define EMPATHY_IFACE_CONNECTION_INTERFACE_AVATARS at the top of this file with a comment saying that Renaming is an unstable API and so is not generated by tp-glib.
(In reply to comment #6) > Please #define EMPATHY_IFACE_CONNECTION_INTERFACE_AVATARS at the top of this > file with a comment saying that Renaming is an unstable API and so is not > generated by tp-glib. ITYM CONNECTION_INTERFACE_RENAMING, however perhaps it would be better to actually generate the Renaming spec? Actually I'm surprised Empathy isn't already doing that... how do renames work at the moment?
Created attachment 177944 [details] [review] Amended patch, extensions need to be installed Thanks for your suggestions, Guillaume. This patch is now extensible for future command additions too.
(In reply to comment #7) > (In reply to comment #6) > > > Please #define EMPATHY_IFACE_CONNECTION_INTERFACE_AVATARS at the top of this > > file with a comment saying that Renaming is an unstable API and so is not > > generated by tp-glib. > > ITYM CONNECTION_INTERFACE_RENAMING, however perhaps it would be better to > actually generate the Renaming spec? Actually I'm surprised Empathy isn't > already doing that... how do renames work at the moment? Thanks Danielle, I have added Conn.I.Renaming and it will be used for generating the extensions. Pushed the commit to http://gitorious.org/glassrose-gnome/empathy/commits/Allow-nick-command-only-in-supporting-rooms-638766
Review of attachment 177944 [details] [review]: Ideally I would have recorded the extensions/ changes in a separated commit (make things cleaner imho). You should updated extensions/Makefile.am as well. Also, in a next commit it would be nice to stop using SetAlias to change the nick and use Renaming instead. ::: libempathy-gtk/empathy-chat.c @@ +673,3 @@ + connection = empathy_tp_chat_get_connection (priv->tp_chat); + return tp_proxy_has_interface_by_id (connection, + EMP_IFACE_QUARK_CONNECTION_INTERFACE_RENAMING) ? TRUE : FALSE; Why are you using '?' ? You can just return the result of tp_proxy_has_interface_by_id(). @@ +909,3 @@ if (g_ascii_strcasecmp (strv[1], commands[i].prefix) == 0) { + if (commands[i].is_supported != NULL) { + if (!commands[i].is_supported (chat)) { In this case no need to continue iterating, you can break the loop.
I have broken the work into 2 commits and added a new commit for using RequestRename method call instead of SetAliases Public branch: http://gitorious.org/glassrose-gnome/empathy/commits/Allow-nick-command-only-in-supporting-rooms-638766
Looks pretty good. I guess it works fine with tp-idle right? I'd pass a callback to emp_cli_connection_interface_renaming_call_request_rename, if only to display a debug message if it failed.
yep, IRC accounts support /nick as before. public branch updated with last patch amended. http://gitorious.org/glassrose-gnome/empathy/commits/Allow-nick-command-only-in-supporting-rooms-638766
All good thanks, merged to master. I renamed this bug and opened bug #639399 about /nick support in XMPP mucs. 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.