Catch-all for unhandled events #186
Conversation
Ignoring pong as it is actually handled but the event is still passed along. Fixes jaraco#185
irc/client.py
Outdated
| @@ -918,6 +918,8 @@ def _handle_event(self, connection, event): | |||
| matching_handlers = sorted( | |||
| self.handlers.get("all_events", []) + self.handlers.get(event.type, []) | |||
| ) | |||
| if len(matching_handlers) == 0 and event.type != "all_raw_messages" and event.type != "pong": | |||
There was a problem hiding this comment.
It's starting to look like 'pong' and 'all_raw_messages' aren't special conditions. I wonder if there's a way that 'pong' could not be treated as a special condition (possibly by having a handler added by default). Can you add a comment explaining why "all_raw_messages" should be ignored?
There was a problem hiding this comment.
I originally used this downstream to only handle "really" unhandled events and all_raw_messages created duplicate events of things that were already handled as their non-raw versions.
Technically I suppose this could be moved to implementation of the handler to filter out events it doesn't want to handle so it's up to the user.
Pong is special because it is handled already so making the auto-pong an actual handler instead of doing it internally would probably be the right way.
|
Have you considered adding tests for this behavior? This library has historically been pretty bereft of tests, but it would be nice to work toward more test coverage. Otherwise, these lines could be removed and tests would still pass. |
|
I can write tests if they work in the first place. 😄 |
|
Yes, it seems the tests started failing spontaneously, since the last commit before your contrib passed tests just fine. I've fixed the tests failing at main, so I'll re-open this PR to re-run the tests. |
|
It looks like the remaining test failures are nitpicky linters. I can fix that by running 'black' on the codebase. |
|
Right, sorry, I need to come around back to this when I continue refactoring again. |
|
Feel free to re-engage on this issue at your leisure. In the meantime, I'll close the PR. |
Ignoring pong as it is actually handled but the event is still
passed along.
Fixes #185