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

Enhance IRC join/part message support #1872

Draft
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

Kufat
Copy link
Contributor

@Kufat Kufat commented Aug 6, 2022

A help channel on my IRC network uses matterbridge to link to a help channel on a related Discord. We've noticed a problem where people on the Discord end will respond to help requests from users who've already left. Enabling join/part message support is an option, but it'd be excessively chatty if every join/part was shown.

This adds a new feature that will selectively show join/part/nick change/quit messages if either a user was active recently or the message is inherently important (i.e. kicks and kills.) New config options are ShowActiveUserEvents and ActivityTimeout, explained below in the sample config. It also addresses an issue in the original code where a quit message handler would send an invalid channel in the bridge message because IRC QUIT events aren't populated with a channel name. (Neither are nick changes, which are also supported now.)

This is literally the first thing I've ever written in Go. I attempted to make it consistent with both your code style and language idioms, but please let me know if there's anything that needs to be addressed.

bridge/irc/handlers.go Outdated Show resolved Hide resolved
bridge/irc/handlers.go Outdated Show resolved Hide resolved
@Kufat Kufat marked this pull request as draft August 8, 2022 16:32
@Kufat Kufat marked this pull request as ready for review August 8, 2022 17:01
@Kufat
Copy link
Contributor Author

Kufat commented Aug 8, 2022

Ignoring exhaustruct and snakecase complaints because the Message struct isn't initialized fully anywhere in the IRC code and because snakecase is canonical for IRC numeric enums.

@42wim 42wim added the irc label Aug 13, 2022
Copy link
Owner

@42wim 42wim left a comment

Choose a reason for hiding this comment

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

Sorry for the delay in reviewing.
Very good for your first Go code :)

The getPseudoChannel is an issue, because now you're returning just the first channel that exists, but it's not sure that this specific user is on that channel. Right?.

Maybe you can also keep an extra map which keeps a tab on where the user has activity, so that channel can be used?

bridge/irc/handlers.go Outdated Show resolved Hide resolved
bridge/irc/handlers.go Outdated Show resolved Hide resolved
bridge/discord/helpers.go Outdated Show resolved Hide resolved
bridge/irc/handlers.go Outdated Show resolved Hide resolved
bridge/irc/handlers.go Outdated Show resolved Hide resolved
@Kufat Kufat marked this pull request as draft September 6, 2022 00:36
@codeclimate
Copy link

codeclimate bot commented Sep 6, 2022

Code Climate has analyzed commit 0cd8db8 and detected 0 issues on this pull request.

View more on Code Climate.

@Kufat Kufat marked this pull request as ready for review September 7, 2022 00:02
@Kufat Kufat marked this pull request as draft September 11, 2022 22:28
@Kufat
Copy link
Contributor Author

Kufat commented Sep 11, 2022

This needs a little work for some corner cases involving quits.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants