Repository: reactiflux/discord-irc
Pull request #273 (2017-07-17 16:12:16)

Improve IRC → Discord mentions for non-word characters and partial word matches
Previously, attempting to mention someone with non-`\w` characters in their name would fail. So if you're on a server with a lot of non-latin names, or one where users have mysterious symbols in their names to indicate something (an example could be 'Throne✶', with the '✶' representing that one is an admin) then IRC users would not properly be able to ping them, as it wasn't recognized as being a wordy character. (Fortunately, if the user in question has 'Throne3d' as their Discord username, that worked as a fallback, but it was non-obvious. Typing the symbol would also be hard but plausibly could be copied and pasted.)

This builds on #272 (which standardizes the testing suite in ways that make this easier to test), adds case-insensitive username and nickname matching, matches all strings of characters that don't involve spaces or hash symbols instead of just `\w` ones, adds matching for mentions of the format `@username#discriminator`, and will match partially – e.g., if there's a user "Throne" in the channel, `@Thronetest`, `@Throne's` and `@Throne-won't-see-this` will all have the starting `@Throne` matched and turned into a mention, as Discord does with usernames.

The partial matching prefers longer matches first and foremost, case insensitively, and then prefers matches which have exact case. I couldn't find an easy method to compare case-insensitively across locales (it seems to require you list the locales you want to compare across), and I'm not sure what Discord does internally, so I uppercased them then compared (and have this in a separate function to make it easier to change later, if necessary).

There is, I think, duplicated logic, in that it checks for perfect non-partial matches first, but if that special casing were removed it should produce the same result (except with a bias towards exact case match, instead of what order the Collection uses behind the scenes), but I wasn't sure if there might be some sort of performance problem with large Discord servers so I thought it best to leave that there, at least for now.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/12425176/badge)](https://coveralls.io/builds/12425176)

Coverage increased (+0.5%) to 98.958% when pulling **c37384aa79d240de2a12a9eeeb161cad9210ac45 on Throne3d:improve/mentions** into **3a1841c8ca111b899fc3de7c4531f54a788033e8 on reactiflux:master**.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/12425364/badge)](https://coveralls.io/builds/12425364)

Coverage increased (+0.8%) to 99.298% when pulling **7666431512a3da97a833f76a5116eac1cd92e5fd on Throne3d:improve/mentions** into **3a1841c8ca111b899fc3de7c4531f54a788033e8 on reactiflux:master**.

Throne3d: (rebased onto #272)
coveralls: 
[![Coverage Status](https://coveralls.io/builds/12459293/badge)](https://coveralls.io/builds/12459293)

Coverage increased (+0.8%) to 99.298% when pulling **9faa90bf3c61c0bd70e1b2d39dbf79efe129bd63 on Throne3d:improve/mentions** into **2f6a3d5eed9a2bfe22f9aee3dfb208afec8071eb on reactiflux:master**.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/12459293/badge)](https://coveralls.io/builds/12459293)

Coverage increased (+0.8%) to 99.298% when pulling **9faa90bf3c61c0bd70e1b2d39dbf79efe129bd63 on Throne3d:improve/mentions** into **2f6a3d5eed9a2bfe22f9aee3dfb208afec8071eb on reactiflux:master**.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/12599820/badge)](https://coveralls.io/builds/12599820)

Coverage increased (+0.04%) to 99.298% when pulling **84ea64678463a64143e863f6e796e9a33f28daa0 on Throne3d:improve/mentions** into **ddd9c54645dbf6d85b3bb1893cc86786135662cd on reactiflux:master**.

qaisjp: I'm running this on my server. "Partial matching" is not the right description for "match by prefix".

For example, I'm "qaisjp".

- `@qaisjpRubbish` now works
-  `@qaisJP` now works
- partial matching implies that `@qais` would work (it doesn't)
Throne3d: I phrased it that way because `qaisJP` would be a part of the 'word' `qaisjpRubbish` – admittedly it only works for prefix partial matches, so it's not in fact all partial matches, but I'd still consider that to be a partial match (one of the 'word', not the nickname).

Thanks.
qaisjp: Hmm that's true.

In my mind I was thinking that "user input" partially matches "real nickname".

I guess you're saying "real nickname" partially matches "user input"?
Throne3d: Yeah. There's a partial match, in the "haystack" of the user input, of the "needle" of the actual username/nickname.
Throne3d: (Rebased onto master and fixed a merge conflict.)
coveralls: 
[![Coverage Status](https://coveralls.io/builds/13587017/badge)](https://coveralls.io/builds/13587017)

Coverage increased (+0.1%) to 98.328% when pulling **6d2f99bea13896ddf8f55e5590df16d34bf0d32c on Throne3d:improve/mentions** into **b86fd1eaa4dbdedb214bcd5a304ea959b35b8d7a on reactiflux:master**.

Throne3d: @ekmartin Do you have time to review this? I was just thinking it would be nice to cut a new release with the few recent updates, and was going to ask you about it, but since there have been some new features (well, the ignore feature, plus some changes in the upstream `irc-upd` library), it would be nice if we could merge some of the other feature PRs before doing a minor version bump – specifically, this and #230 (which needs a rebase, but I'd be happy to do that, and I think it's addressed your requested changes), though some of the others might be useful too.

I mostly wonder this because, as far as I know, the current release uses an old version of `irc-upd`, which has an issue where the IRC library is permanently in debug mode.

It might also be worth regenerating the `package-lock.json` file, since it looks like it might be out of date. It seems to have old versions of some packages listed, e.g., `irc-upd`. (I'd do that by removing the `node_modules` folder, and the `package-lock.json` file, then re-running `npm install`, since it looks like sometimes Node updates it weirdly? I had trouble running `nyc` just doing a straight `npm update`.)
ekmartin: Yeah - sorry about the delay. I'll set a reminder to get it done sometime this week.
coveralls: 
[![Coverage Status](https://coveralls.io/builds/13833852/badge)](https://coveralls.io/builds/13833852)

Coverage increased (+0.1%) to 98.328% when pulling **8cf07ab4e02f9400228c80054d4a567357060be2 on Throne3d:improve/mentions** into **db39ba237eb68ae521a43d0134c3e33fe5317c82 on reactiflux:master**.


Patch:
diff --git a/lib/bot.js b/lib/bot.js
index 19b7b23..f12b766 100644
--- a/lib/bot.js
+++ b/lib/bot.js
@@ -367,6 +367,18 @@ class Bot {
     return null;
   }
 
+  // compare two strings case-insensitively
+  // for discord mention matching
+  static caseComp(str1, str2) {
+    return str1.toUpperCase() === str2.toUpperCase();
+  }
+
+  // check if the first string starts with the second case-insensitively
+  // for discord mention matching
+  static caseStartsWith(str1, str2) {
+    return str1.toUpperCase().startsWith(str2.toUpperCase());
+  }
+
   sendToDiscord(author, channel, text) {
     const discordChannel = this.findDiscordChannel(channel);
     if (!discordChannel) return;
@@ -400,29 +412,73 @@ class Bot {
     }
 
     const { guild } = discordChannel;
-    const withMentions = withFormat.replace(/@[^\s]+\b/g, (match) => {
-      const search = match.substring(1);
-      const nickUser = guild.members.find('nickname', search);
-      if (nickUser) {
-        return nickUser;
-      }
+    const withMentions = withFormat.replace(/@([^\s#]+)#(\d+)/g, (match, username, discriminator) => {
+      // @username#1234 => mention
+      // skips usernames including spaces for ease (they cannot include hashes)
+      // checks case insensitively as Discord does
+      const user = guild.members.find(x =>
+        Bot.caseComp(x.user.username, username.toUpperCase())
+        && x.user.discriminator === discriminator);
+      if (user) return user;
 
-      const user = this.discord.users.find('username', search);
-      if (user) {
-        return user;
-      }
+      return match;
+    }).replace(/@([^\s]+)/g, (match, reference) => {
+      // this preliminary stuff is ultimately unnecessary
+      // but might save time over later more complicated calculations
+      // @nickname => mention, case insensitively
+      const nickUser = guild.members.find(x =>
+        x.nickname !== null && Bot.caseComp(x.nickname, reference));
+      if (nickUser) return nickUser;
+
+      // @username => mention, case insensitively
+      const user = guild.members.find(x => Bot.caseComp(x.user.username, reference));
+      if (user) return user;
+
+      // @role => mention, case insensitively
+      const role = guild.roles.find(x => x.mentionable && Bot.caseComp(x.name, reference));
+      if (role) return role;
+
+      // No match found checking the whole word. Check for partial matches now instead.
+      // @nameextra => [mention]extra, case insensitively, as Discord does
+      // uses the longest match, and if there are two, whichever is a match by case
+      let matchLength = 0;
+      let bestMatch = null;
+      let caseMatched = false;
+
+      // check if a partial match is found in reference and if so update the match values
+      const checkMatch = function (matchString, matchValue) {
+        // if the matchString is longer than the current best and is a match
+        // or if it's the same length but it matches by case unlike the current match
+        // set the best match to this matchString and matchValue
+        if ((matchString.length > matchLength && Bot.caseStartsWith(reference, matchString))
+          || (matchString.length === matchLength && !caseMatched
+              && reference.startsWith(matchString))) {
+          matchLength = matchString.length;
+          bestMatch = matchValue;
+          caseMatched = reference.startsWith(matchString);
+        }
+      };
 
-      const role = guild.roles.find('name', search);
-      if (role && role.mentionable) {
-        return role;
-      }
+      // check users by username and nickname
+      guild.members.forEach((member) => {
+        checkMatch(member.user.username, member);
+        if (bestMatch === member || member.nickname === null) return;
+        checkMatch(member.nickname, member);
+      });
+      // check mentionable roles by visible name
+      guild.roles.forEach((member) => {
+        if (!member.mentionable) return;
+        checkMatch(member.name, member);
+      });
+
+      // if a partial match was found, return the match and the unmatched trailing characters
+      if (bestMatch) return bestMatch.toString() + reference.substring(matchLength);
 
       return match;
     }).replace(/:(\w+):/g, (match, ident) => {
+      // :emoji: => mention, case sensitively
       const emoji = guild.emojis.find(x => x.name === ident && x.requiresColons);
-      if (emoji) {
-        return `<:${emoji.identifier}>`; // identifier = name + ":" + id
-      }
+      if (emoji) return emoji;
 
       return match;
     });


Test patch:
diff --git a/test/bot.test.js b/test/bot.test.js
index a0ad83d..98e8ad3 100644
--- a/test/bot.test.js
+++ b/test/bot.test.js
@@ -569,7 +569,24 @@ describe('Bot', function () {
 
     const username = 'ircuser';
     const text = 'Hello, @testuser!';
-    const expected = `**<${username}>** Hello, <@${testUser.id}>!`;
+    const expected = `**<${username}>** Hello, ${testUser}!`;
+
+    this.bot.sendToDiscord(username, '#irc', text);
+    this.sendStub.should.have.been.calledWith(expected);
+  });
+
+  it('should convert username-discriminator mentions from IRC properly', function () {
+    const user1 = this.addUser({ username: 'user', id: '123', discriminator: '9876' });
+    const user2 = this.addUser({
+      username: 'user',
+      id: '124',
+      discriminator: '5555',
+      nickname: 'secondUser'
+    });
+
+    const username = 'ircuser';
+    const text = 'hello @user#9876 and @user#5555 and @fakeuser#1234';
+    const expected = `**<${username}>** hello ${user1} and ${user2} and @fakeuser#1234`;
 
     this.bot.sendToDiscord(username, '#irc', text);
     this.sendStub.should.have.been.calledWith(expected);
@@ -637,6 +654,35 @@ describe('Bot', function () {
     this.sendStub.should.have.been.calledWith(expected);
   });
 
+  it('should convert overlapping mentions from IRC properly and case-insensitively', function () {
+    const user = this.addUser({ username: 'user', id: '111' });
+    const nickUser = this.addUser({ username: 'user2', id: '112', nickname: 'userTest' });
+    const nickUserCase = this.addUser({ username: 'user3', id: '113', nickname: 'userTEST' });
+    const role = this.addRole({ name: 'userTestRole', id: '12345', mentionable: true });
+
+    const username = 'ircuser';
+    const text = 'hello @User, @user, @userTest, @userTEST, @userTestRole and @usertestrole';
+    const expected = `**<${username}>** hello ${user}, ${user}, ${nickUser}, ${nickUserCase}, ${role} and ${role}`;
+
+    this.bot.sendToDiscord(username, '#irc', text);
+    this.sendStub.should.have.been.calledWith(expected);
+  });
+
+  it('should convert partial matches from IRC properly', function () {
+    const user = this.addUser({ username: 'user', id: '111' });
+    const longUser = this.addUser({ username: 'user-punc', id: '112' });
+    const nickUser = this.addUser({ username: 'user2', id: '113', nickname: 'nick' });
+    const nickUserCase = this.addUser({ username: 'user3', id: '114', nickname: 'NiCK' });
+    const role = this.addRole({ name: 'role', id: '12345', mentionable: true });
+
+    const username = 'ircuser';
+    const text = '@user-ific @usermore, @user\'s friend @user-punc, @nicks and @NiCKs @roles';
+    const expected = `**<${username}>** ${user}-ific ${user}more, ${user}'s friend ${longUser}, ${nickUser}s and ${nickUserCase}s ${role}s`;
+
+    this.bot.sendToDiscord(username, '#irc', text);
+    this.sendStub.should.have.been.calledWith(expected);
+  });
+
   it('should successfully send messages with default config', function () {
     const bot = new Bot(configMsgFormatDefault);
     bot.connect();