search

kiwiirc / kiwibnc

Apache License 2.0
43 stars 14 forks source link

Support cap-notify #21

Closed DanielOaks closed 5 years ago

DanielOaks commented 5 years ago

This PR implements support for cap-notify including CAP NEW and CAP DEL messages. It also requests new capabilities as necessary, assuming that it always needs to re-authenticate with SASL if it sees the sasl cap come in through CAP NEW.

DanielOaks commented 5 years ago

This should be good to go, I've got it following my current cap clarifications pr advice, which seems sensible I reckon. I've tested out the new hooks and they should allow extensions to do whatever they need to in response to incoming CAP lines.

prawnsalad commented 5 years ago

I've removed the forwardToClient vars from everywhere other than ACK and DEL, the thinking being that if hooks react to any CAP changes they should be doing it on ACK for the most part. If this makes sense to you also then I think this is good to go.

DanielOaks commented 5 years ago

Ah, it should be fine. In the future it may make sense for hooks to only advertise caps like labeled-response when upstream supports them, but honestly for things like that we'll probably need to make some other internal changes as well so it's fine as-is. Thanks for the fixups!

DanielOaks commented 5 years ago

Ah I see, I misread – should be fine yeah. We can adjust as needed later on, for now as it's implemented it looks all good to me.