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

Add ZWSP between Telegram usernames #322

Merged
merged 3 commits into from
May 24, 2020
Merged

Add ZWSP between Telegram usernames #322

merged 3 commits into from
May 24, 2020

Conversation

Tjzabel
Copy link
Member

@Tjzabel Tjzabel commented May 17, 2020

What This Is

If a user has accounts on both IRC and Telegram with the same username, it is easy to ping oneself across platforms each time a message is sent. A zero-width space (ZWSP) is used to add an invisible differentiation between the usernames on these platforms to prevent cross-platform username pinging.

Developer Details

ResolveUserName is now used instead of user.String() for retrieving username information.
For additional details, see: 42wim/matterbridge#175.

@Tjzabel Tjzabel added new change Adds new capabilities or functionality Telegram Issues relating to Telegram bridge labels May 17, 2020
@Tjzabel Tjzabel added this to the v2.0.0 milestone May 17, 2020
@Tjzabel Tjzabel requested a review from a team May 17, 2020 00:12
@jwflory jwflory added the needs review Pending a peer review before merging or closing label May 17, 2020
@jwflory jwflory added needs changes Needs changes or fixes before closing and removed needs review Pending a peer review before merging or closing labels May 22, 2020
…ross platforms.

ResolveUserName is now used instead of user.String() for retrieving username information.
For additional details, see: 42wim/matterbridge#175.
Helper methods now better reflect their actions.
Copy link
Member

@Zedjones Zedjones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall, I just had one small change but I'm not going to block on it. I didn't test the functionality of this (because I don't actively use both platforms) but it looks good from a code perspective and the tests pass.

internal/handlers/telegram/helpers.go Outdated Show resolved Hide resolved
@jwflory
Copy link
Member

jwflory commented May 24, 2020

Looks good to me. I'm going to merge this in. Thanks for getting this done @Tjzabel @Zedjones!

🎬

@jwflory jwflory merged commit db058f8 into master May 24, 2020
@jwflory jwflory deleted the add/zwp branch May 24, 2020 03:58
@@ -28,6 +28,7 @@ type IRCSettings struct {
Suffix string `env:"IRC_SUFFIX" envDefault:">"`
ShowJoinMessage bool `env:"IRC_SHOW_JOIN_MESSAGE" envDefault:"true"`
ShowLeaveMessage bool `env:"IRC_SHOW_LEAVE_MESSAGE" envDefault:"true"`
ShowZWSP bool `env:"IRC_SHOW_ZWSP" envDefault:"true"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: This needs to go into the env.example and our config glossary too.

@jwflory jwflory linked an issue May 24, 2020 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs changes Needs changes or fixes before closing new change Adds new capabilities or functionality Telegram Issues relating to Telegram bridge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow toggle for ZWP for Telegram usernames sent to IRC
3 participants