GNOME Bugzilla – Bug 710731
make it easier to change back to nominal nick
Last modified: 2017-07-25 08:11:35 UTC
When I sign back on to IRC after a network error I often have a fallback nick assigned to me (eg. "mccann_"). After waiting for the "mccann" nick to time out I currently have to click in the nick entry field and type in the full text "mccann" in order to change it back. This is less than ideal for a few reasons. * I have to monitor the channel messages to find out when the previous nick times out * I have to type in the full nick in the entry again * When typing in the nick I lose track of what my nick is currently * Even though we know what the preferred/nominal nick is we don't offer it as a direct option There are a few things we can do better I think. * We could automatically switch back to the nominal nick when it becomes available without any additional work from the user * We could offer a list of most frequently used nicks when editing the nick entry
Created attachment 258026 [details] [review] mainWindow: Add completion to nick entry Compose a list of previously used nicknames and offer it as possible completions for nick changes. Use telepathy-logger for now, because it's there, though we may consider to record nick changes ourselves in the future to avoid scraping through large quantities of messages. (In reply to comment #0) > There are a few things we can do better I think. > * We could automatically switch back to the nominal nick when it becomes > available without any additional work from the user Sounds good, but there are some pitfalls: - we don't actually know when the nick becomes available (though looking for disconnect messages in rooms should work in most cases) - we don't want to change the nick unconditionally, e.g. after changing manually from nick_ to nick-away I have an initial patch, but it'll need a bit more work due to the above, so not attaching it yet ... > * We could offer a list of most frequently used nicks when editing the nick > entry Something like in this patch?
Doesn't seem to apply here on master.
It probably conflicts with the patch in bug 710705.
Created attachment 260452 [details] [review] mainWindow: Add completion to nick entry Rebased patch.
Comment on attachment 260452 [details] [review] mainWindow: Add completion to nick entry Entry completion isn't really a good fit for popover content. Changing back to the nominal nick isn't that hard anymore anyway, as the entry contains the current nick nowadays - for changing to other nickname variants, something like the mockups in bug 768341 seem better. So I'll mark this patch as obsolete and attach patches for the second suggestion, changing back to the nominal nick automatically ...
Created attachment 356098 [details] [review] app: Add setAccountNick() method We currently have no way to differentiate nickname changes from user requests from automatic changes (either us appending underscores on conflicts, or telepathy syncing the various properties on connection). This is something we will need soon, so add a smaller wrapper method for use only with user requested nick changes.
Created attachment 356099 [details] [review] app: Automatically change to nominal nick when it becomes available If a connection fails because the chosen nick is already in use, we automatically retry with an appended underscore. Usually the reason for this error is an unclean disconnect that leaves the nick from the previous connection in zombie state until it times out - to get back to the nominal nick, the user currently has to monitor the chat log for the zombie timing out and manually change the nickname. We can do better than that and use our user tracking to determine when the nominal nick becomes available and change the nickname automatically if there wasn't a user requested change in the meantime.
Review of attachment 356098 [details] [review]: looks good to me, just one question ::: src/application.js @@ +79,3 @@ + setAccountNick: function(account, nick) { + account.set_nickname_async(nick, (a, res) => { + account.set_nickname_finish(res); why is it that you dont use a try, catch clause anymore here, compared to the old code?
Review of attachment 356099 [details] [review]: looks good to me
(In reply to Bastian Ilsø from comment #8) > why is it that you dont use a try, catch clause anymore here, compared to > the old code? I didn't want to do something ugly like passing in an errorCallback, so we already use the "real" error handling in application.js - it's not too big of a deal, we will just revert to the old nick after the timeout instead of immediately on failure. We could still do the ircParser-style error handling, but it's a bit pointless - the default exception handling is essentially: 1. logError() without the optional message parameter 2. abort the current function call 2. doesn't matter in this case, as we're already at the end of the function, so all we really lose is the 'Failed to change nick' message. It's also extremely unlikely to fail - while mission-control will try to sync the change with the server, and the server may reject the change (invalid nick, conflict with an existing nick etc.), none of those failures results in an exception. The only possible failures here are: 1. the nickname is NULL 2. setting the corresponding DBus property failed 1. cannot happen, as the nickname isn't marked as optional, so gjs already throws an exception for the set_nickname_async() call. I don't see how 2. would fail unless either mission-control or dbus itself gets screwed, in which case a failed set_nickname() call is the least of our worries.
Gotcha, thanks for the explanation.
Review of attachment 356098 [details] [review]: lgtm
Review of attachment 356099 [details] [review]: lgtm
Attachment 356098 [details] pushed as f320f96 - app: Add setAccountNick() method Attachment 356099 [details] pushed as 2a9c39d - app: Automatically change to nominal nick when it becomes available