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

added notify-away support #175

Merged
merged 12 commits into from
Jul 26, 2023
Merged

added notify-away support #175

merged 12 commits into from
Jul 26, 2023

Conversation

casperstorm
Copy link
Member

This adds support for notify-away extension.
We call WHO when joining a channel to get initial state, and then keeps it synced with AWAY responses.
Away users is appear slightly transparent. This idea is stolen from Textual.

Copy link
Member

@tarkah tarkah left a comment

Choose a reason for hiding this comment

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

Looks great @casperstorm! A couple minor things. Also, we may want to handle RPL_NOWAWAY and RPL_UNAWAY so when we issue an /away command, it'll update our nick in the nicklists. I noticed these are what's sent when we're the ones sending the /away.

data/src/client.rs Outdated Show resolved Hide resolved
data/src/client.rs Outdated Show resolved Hide resolved
@tarkah
Copy link
Member

tarkah commented Jul 23, 2023

I noticed a message from libera about "rate-limiting" so I'm not sure WHO was sent on each of my joined channels. Thoughts around how we should handle this? Do we know what other clients do to circumvent this issue?

Maybe we keep track of a map of channels and the last time they're polled for WHO. If they haven't been polled, we can execute WHO. We check this every X seconds / minutes. We only ensure it's sent once if away-notify is set, otherwise we can use this as a mechanism to poll every X if that feature isn't enabled?

@casperstorm
Copy link
Member Author

I noticed a message from libera about "rate-limiting" so I'm not sure WHO was sent on each of my joined channels. Thoughts around how we should handle this? Do we know what other clients do to circumvent this issue?

Maybe we keep track of a map of channels and the last time they're polled for WHO. If they haven't been polled, we can execute WHO. We check this every X seconds / minutes. We only ensure it's sent once if away-notify is set, otherwise we can use this as a mechanism to poll every X if that feature isn't enabled?

This might be because of the WHO being called for all users joining previously. Can you test again? I don't think we have to make a special case for it - doesn't seem others do that 🤔

@tarkah
Copy link
Member

tarkah commented Jul 24, 2023

I noticed a message from libera about "rate-limiting" so I'm not sure WHO was sent on each of my joined channels. Thoughts around how we should handle this? Do we know what other clients do to circumvent this issue?
Maybe we keep track of a map of channels and the last time they're polled for WHO. If they haven't been polled, we can execute WHO. We check this every X seconds / minutes. We only ensure it's sent once if away-notify is set, otherwise we can use this as a mechanism to poll every X if that feature isn't enabled?

This might be because of the WHO being called for all users joining previously. Can you test again? I don't think we have to make a special case for it - doesn't seem others do that thinking

I'm still getting it:

13:43:29.812:TRACE -- Message received => Message { tags: [Tag { key: "time", value: Some("2023-07-24T20:43:29.722Z") }], source: Some(Server("erbium.libera.chat")), command: Numeric(RPL_TRYAGAIN, ["tarkah", "WHO", "This command could not be completed because it has been used recently, and is rate-limited."]) }

@casperstorm
Copy link
Member Author

Gotcha, yeah let's then add a system which keeps track which we can also use for polling.

Copy link
Member

@tarkah tarkah left a comment

Choose a reason for hiding this comment

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

Awesome feature!

@casperstorm casperstorm merged commit bac2046 into main Jul 26, 2023
1 check passed
@casperstorm casperstorm deleted the feat/away-notify branch July 26, 2023 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants