Better IRC --> Discord nick matching by checking available users

[redfellow]

Just a suggestion:

If Discord server contains users Red, John and Mack, you'd normally highlight them via @Red etc. This works from IRC too, except that IRC etiquette for highlight differs somewhat, on IRC it's customary to write Red: message or Red, message.

Would be pretty cool if these could be detected and replaced on the fly as @Nick.

Matching without the colon or comma wouldn't be a good thing however, as that would probably lead to multiple false matches.

[Throne3d]

I think this should be configurable if added. It might be slow on very large guilds, and also seems likely to cause a lot of false positives even if we do constrain it to situations with commas or colons after (if there's a user named red, not Red, it'd get eg I like the color red, but I'm not sure it looks good on me).

[redfellow]

To be more specific, the IRC etiquette mostly has lines starting with the mention, so I think a helper function like this would be relatively safe and wouldn't require that much cpu time:

const processPossibleMentions = function(str) {
	const nicks = getDiscordNicks();

	for (let index = 0; index < nicks.length; index++) {
		const nick = nicks[index];
		const startRegex = new RegExp("^" + nick + "+[,|:]+.*$");
		const endRegex = new RegExp("^.*,[ ]?" + nick + "+(?|!|.)?$");
		const startMention = startRegex.test(str);
		const endMention= endRegex.test(str);

		if (startMention || endMention) {
			return str.replace(nick, "@" + nick);
		}
	}
	
	return str;
};

const getDiscordNicks = function() {
	return ["Red", "Jack", "John"];
};

console.log(processPossibleMentions("I like the color red, but I'm not sure it looks good on me"));
console.log(processPossibleMentions("Jack: how you doing man?"));
console.log(processPossibleMentions("Darude: Sandstorm is a great song!"));
console.log(processPossibleMentions("John, what's up?"));
console.log(processPossibleMentions("Where are you, Jack?"));

/*
Outputs: 
  I like the color red, but I'm not sure it looks good on me
  @Jack: how you doing man?
  Darude: Sandstorm is a great song!
  @John, what's up?
  Where are you, @Jack?
*/

[Mikaela]

I often see lines ending to , nick and think it should also be matched.

[redfellow]

@Mikaela updated my suggestion to account for that.

[Throne3d]

I think you've confused (x|y) with [x|y] – your current code seems to match extra | symbols – and I'd also probably want to use the regex as part of the replacement, not part of the check for presence.

Examples of why:

processPossibleMentions('Jack|, what?'); // "@Jack|, what?"
// with getDiscordNicks also including 'red':
processPossibleMentions("I like the color red. Don't you think, red?"); // "I like the color @red. Don't you think, red?"

(Admittedly the second still pings the right person, but it's in an inconsistent location.)

Nicknames should also be escaped, if only to prevent false positives on the regex matching if not when it later replaces things.

With regards to large guilds, the issue is that trying to load all the usernames of all the people in a guild of, say, 2000 people, takes a very long time (or might in fact be prevented); constructing a regex for each of these nicknames upon each message and then testing each message against it would be – well, maybe not slow, but – suboptimal. The following code, for example, takes a noticeable amount of time in the Chrome console:

const str = "1234567890";
for (let i = 0; i < 1000; i++) {
  const regex = new RegExp("^.*" + i + ".*$");
  console.log("matches?", regex.test(str));
}

I'm also not sure if we should match by just nickname, or also username, and then also how we deal with case sensitivity issues. I think we've tried to figure this out elsewhere in the code but I'm not sure we standardized on anything useful. (At least for my use case, it'd be good if it did pay attention to case, since some of my users go by capitalized common words, but I might just disable this feature, anyway.)

[redfellow]

Yeah, I made a mistake using [] instead of (), I rarely venture into Regex territory, sorry. :) I know the str.replace should be using the Regex with capture groups thrown in the mix to avoid the 2nd example problem, but I wasn't trying to impose a ready solution upon the project (that I'd do via a pull request after extensively testing solutions). This was just a dirty example that could be built on.

As a Discord user says something in Discord and it's relayed to IRC, the IRC user only knows the handle shown there, it would be logical the matching is done just by what's shown to people over IRC, so actual Discord xxx#0000 usernames would be overcomplicating what's needed imo.

All I'd want is these common cases to turn into mentions over at Discords side, within the constraints of what's feasible and easy to detect - lack of this is what's keeping some people in the community (at least in my case) from migrating away from IRC (they don't want to lose highlights from nick: and nick,.

edit: The console.log is what's making your test costly, that loop for 20000 users would take around 200ms actually. Or ~20ms for 1000. You could skip regexping altogether in the case of if (msg.indexOf(nick) === -1) {, which would make it really light.

console.time("Loop");
var str = "1234567890";
for (var i = 0; i < 10000; i++) {
	const regex = new RegExp("^.*" + i + ".*$");
	if (regex.test(str)) {
		// code
	}
}
console.timeEnd("Loop");

[Throne3d]

It looks like we support all of the prefix ones ("Jack, test" and "Jack: test") as of #470. I'll close this for now, since matching on the end of a message seems more speculative, but if people really want that then we can look into adding it?