955355 - Sort channel list in IRC tooltips

Status: RESOLVED FIXED

(Chat Core :: IRC, defect)

Description

[Benedikt Pfeifer]

*** Original post on bio 1918 at 2013-04-04 10:22:00 UTC ***

*** Due to BzAPI limitations, the initial description is in comment 1 ***

Comment 1

[Benedikt Pfeifer]

Attachment: Patch

*** Original post on bio 1918 as attmnt 2319 at 2013-04-04 10:22:00 UTC ***

The list of channels in IRC participants'/contacts' tooltips should be sorted to make it easier to look for a particular channel.

The attached patch sorts the list, ignoring the prefixes of the channel and user.

Comment 2

[Patrick Cloke [:clokep]]

Comment on attachment 8354085 [details] [diff] [review]
Patch

*** Original change on bio 1918 attmnt 2319 at 2013-04-04 10:36:28 UTC ***

This is a good idea. :)

>diff --git a/chat/protocols/irc/irc.js b/chat/protocols/irc/irc.js
>+    // Sort the list of channels, ignoring the prefixes of channel and user.
>+    // "self" is favored over bind(this) here because of the nested functions.
>+    let self = this;
>+    let sortChannels = function(aChannelStr) {
>+      let prefixes = self.userPrefixes.concat(self.channelPrefixes);
The list of prefixes doesn't change, we can define it outside of the function, which will then use less "selfs" and probably be easier to figure out.

>+      let noPrefixSort = function (a, b) {
>+        a = self.normalize(a, prefixes);
>+        b = self.normalize(b, prefixes);
>+        return a < b ? -1 : a > b ? 1 : 0;
>+      };
>+      return aChannelStr.split(" ").filter(function(name) name != "")
>+                        .sort(noPrefixSort).join(" ");
Does aChannelStr.trim().split(" ").sort(noPrefixSort).join(" "); do the same thing?

>     const kFields = {
[...]
>       away: null,
Somewhere down here it should say "channels: null", you need to set it to use the sortChannels function.

Comment 3

[Benedikt Pfeifer]

Attachment: Patch v2

*** Original post on bio 1918 as attmnt 2321 at 2013-04-04 18:22:00 UTC ***

Using trim and regex-splitting on the string now instead of filtering the array, as suggested. The nested functions are gone as suggested on IRC, too.

Comment 4

[Benedikt Pfeifer]

Comment on attachment 8354085 [details] [diff] [review]
Patch

*** Original change on bio 1918 attmnt 2319 at 2013-04-04 18:22:07 UTC was without comment, so any subsequent comment numbers will be shifted ***

Comment 5

[Patrick Cloke [:clokep]]

Comment on attachment 8354087 [details] [diff] [review]
Patch v2

*** Original change on bio 1918 attmnt 2321 at 2013-04-04 18:37:59 UTC ***

This looks fine and is a good improvement, I think.

Comment 6

[Patrick Cloke [:clokep]]

*** Original post on bio 1918 at 2013-04-04 23:05:32 UTC ***

http://hg.instantbird.org/instantbird/rev/d460a4aafe68

Thanks!