Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Insert zero-width space in Telegram usernames sent to IRC #112

Closed
jwflory opened this issue Jan 7, 2019 · 12 comments
Closed

Insert zero-width space in Telegram usernames sent to IRC #112

jwflory opened this issue Jan 7, 2019 · 12 comments
Assignees
Labels
new change Adds new capabilities or functionality
Milestone

Comments

@jwflory
Copy link
Member

jwflory commented Jan 7, 2019

Inspired by 42wim/matterbridge#175.


Summary

Insert zero-width space in Telegram usernames sent to IRC to prevent pinging IRC usernames with the same Telegram nick

Background

If someone is on both sides of a bridged Telegram group and IRC channel, and has the same username, they will get pinged on IRC whenever they send a message from Telegram. This can be annoying for IRC users when chatting from Telegram.

One solution is to insert a zero-width character into the Telegram username field sent to IRC. This prevents pinging IRC users when they chat from Telegram.

Details

I think the space needs to be inserted after the first character or before the last. Maybe the easiest solution is to insert it after the first character of the Telegram username.

This space should only be inserted in a username relayed from Telegram to IRC.

Outcome

Better user experience for people chatting on both sides of a bridge when their Telegram username and IRC nick are the same

@jwflory jwflory added help wanted Anyone is welcome to help us with this! good first issue Good for newcomers priority:med new change Adds new capabilities or functionality labels Jan 7, 2019
@jwflory jwflory added this to the v1.3 milestone Jan 7, 2019
@xforever1313
Copy link
Member

We might want to add a way to disable this if a user doesn't want it. There is no globally accepted standard for the character encoding according to Wikipedia. Zero-width space appears to be unicode, but there may be IRC servers out there that only support ASCII.

I can't imagine there are many modern IRC servers out there that don't support Unicode (FreeNode appears to since it can send Emojis), but you never know I suppose...

@nic-hartley
Copy link

Using square brackets around usernames instead of angle brackets should fix this -- []{}|\ are all considered part of the nickname, so [some-nick] shouldn't ping someone nick'd some-nick.

@jwflory jwflory added priority:crit and removed good first issue Good for newcomers help wanted Anyone is welcome to help us with this! priority:med labels Feb 2, 2019
@jwflory
Copy link
Member Author

jwflory commented Feb 2, 2019

Discussed in 2019-02-02 RITlug developers' meeting. This issue is targeted for the v1.3 milestone, estimated to release on March 2nd, 2019.


@Tjzabel volunteered to take this one on for the v1.3 development sprint. It may happen later in the sprint cycle (after #115?). We'll follow up at the next developers' meeting on Saturday, Feb. 9th.

@Tjzabel
Copy link
Member

Tjzabel commented Feb 22, 2019

Update

I went ahead and implemented a ZWP in the middle of nicks. It looks like this:

from.username = from.username.substr(0,1) + "\u200B" + from.username.substr(1);

Further considerations

Adding a ZWP essentially breaks all our unit tests that include a username. This will take some time to look through. This raises further questions:

  1. Do we want this toggleable?
  2. Will the implementation of "\u200B" as the ZWP break any IRC servers/handlers?

@Tjzabel
Copy link
Member

Tjzabel commented Feb 23, 2019

After further discussion, we are making the ZWSP a default behavior. We don't see much of a blocker or issue with having a ZWSP inside of a nick in every day communications.

With that being said, this has been implemented, and works as planned. The only issue at the moment is this breaks all our current Telegram unit tests, which have "expected usernames" without ZWSP inserted. In order to remedy this, I am going to see if I can make a default username a global var in these unit tests, and see if that can fix the issue.

@Tjzabel
Copy link
Member

Tjzabel commented Mar 2, 2019

Update

This issue is essentially complete. A simple override to the ResolveUserName method was all that was necessary in order to fix all the Telegram unit tests.

Next step is to write additional tests to make sure the ZWP works as intended. Mileage may vary at this stage, since I overrode the method.... puts on thinking cap

@jwflory
Copy link
Member Author

jwflory commented Mar 2, 2019

@Tjzabel Do you think this change will make the v1.3 window?

@Tjzabel
Copy link
Member

Tjzabel commented Mar 2, 2019

@jwflory not by today's meeting, but possibly by the end of today.

@jwflory
Copy link
Member Author

jwflory commented Mar 3, 2019

@Tjzabel got this in commit 1207f99 / PR #128. Closing as complete. 🎬

@jwflory jwflory closed this as completed Mar 3, 2019
@tml1024
Copy link

tml1024 commented Nov 2, 2020

I think this is a somewhat weird feature. It breaks grepping for nicknames in one's IRC logs, for instance. Why not instead add a marker at the end of the nick, like the Matrix bridge apparently does? It adds [m]. Teleirc could add [t].

@Tjzabel
Copy link
Member

Tjzabel commented Nov 2, 2020

@tml1024 hello, we have a toggle to turn this feature off. If you wish to not make use of this feature, add IRC_SHOW_ZWSP=false to your .env file.

@tml1024
Copy link

tml1024 commented Nov 2, 2020

Thanks. Will suggest that we do that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new change Adds new capabilities or functionality
Projects
None yet
Development

No branches or pull requests

5 participants