sm00th / bitlbee-discord

Bitlbee plugin for Discord (http://discordapp.com)
GNU General Public License v2.0
291 stars 27 forks source link

Enforce space after emote URLs #149

Closed Alcaro closed 6 years ago

Alcaro commented 6 years ago

Using the official client, say :thonk::thonk: in any channel.

Result: \ :thonk: https://cdn.discordapp.com/emojis/379295935689392148.png:thonk: https://cdn.discordapp.com/emojis/379295935689392148.png, where the :thonk is considered part of the URL, which makes it leads nowhere (some clients consider the final : part of the URL too, but that doesn't work either).

The simplest fix is this tweak to the emote regexes; regexes are greedy, so this won't emit duplicate spaces. It does yield an extra space at the end if a message ends with an emote, which can be avoided by doubling the number of regexes, but that's a bit of extra code for questionable gains. Your choice if it's worth it.

dgw commented 6 years ago

Doubling the number of regexes? Why not just put the space match in a capture group? Instead of ?, use ( ?) in the matching regexes, and insert \\3 instead of just a space in the replacements.

dequis commented 6 years ago

and insert \\3 instead of just a space in the replacements

That inserts nothing if there was no space

sm00th commented 6 years ago

Thanks for the patch. I am wondering whether bitlbee-discord should add spaces that weren't there to begin with. Maybe we can get away with just enclosing the url in brackets? Then :thonk::thonk: will become :thonk:[https://cdn.discordapp.com/emojis/379295935689392148.png]:thonk:[https://cdn.discordapp.com/emojis/379295935689392148.png]. This should be parsable by most clients and we kinda sorta relay the message untouched. Will this work?

dequis commented 6 years ago

YMMV, depends on the url matching regexp each client uses. Space is the safest option.

sm00th commented 6 years ago

Space is the safest option.

Yeah, but it is a bit awkward. I'd really prefer the brackets, they also help visually highlight the fact the url is autogenerated.

dgw commented 6 years ago

Brackets are a good idea, though I'd go with <> instead of [] (among other uses, plain-text email often displays URLs inline using <>, so it's an established convention).

That inserts nothing if there was no space

Right, no idea what I was thinking. Do these regexes support conditionals? Maybe check if the emoji is at the end of the line ($) and insert a space after it if not. (Note that I've never actually used regex conditionals, I just know that some implementations have them.)

Alcaro commented 6 years ago

Possible, yes, but it would surprise me if that works for everyone. IIRC, the original IRC specification considers [\]{|} to be letters (and case variations of each other), so it wouldn't surprise me if some clients follow that. Even Discord itself renders it as :thonk:[[https://cdn.discordapp.com/emojis/379295935689392148.png\]:thonk:\[https://cdn.discordapp.com/emojis/379295935689392148.png](https://cdn.discordapp.com/emojis/379295935689392148.png\]:thonk:\[https://cdn.discordapp.com/emojis/379295935689392148.png)].

In my client, [http://example.com/] and {http://example.com/} are not acknowledged as links. However, (http://example.com/) and <http://example.com/> work.

As do the formatting opcodes, for example [\x02\x02http://example.com\x02\x02]. Though I've heard of a few IRC clients that don't understand the IRC formatting opcodes, and render them as glitched boxes instead...

Personally I see no reason to highlight the URLs as autogenerated, it's not like humans post like that. Everyone using bitlbee-discord knows that it's not the primary client, and that compromises have been made.

I prefer space, but <> would be fine as well.

e: github, I don't recall telling you to remove my <>s

sm00th commented 6 years ago

I prefer space, but <> would be fine as well.

<> are fine, I used [] as an example. Let's stick with <> for now and if it rises any issues for people - we'll switch to spaces.

Alcaro commented 6 years ago

Okay, you're the boss.

Another option would be appending a #, which would make the links lead to https://cdn.discordapp.com/emojis/379295935689392148.png#:thonk: which works. But that's kinda dumb too, <> is better.

sm00th commented 6 years ago

Great, thanks.