Skip to content

Move event listeners and client callbacks API to use AbstractComponentLoader#794

Open
davfsa wants to merge 3 commits intoCursed-Solutions:masterfrom
davfsa:task/listeners-component-loaders
Open

Move event listeners and client callbacks API to use AbstractComponentLoader#794
davfsa wants to merge 3 commits intoCursed-Solutions:masterfrom
davfsa:task/listeners-component-loaders

Conversation

@davfsa
Copy link
Copy Markdown

@davfsa davfsa commented Jun 25, 2023

Summary

Brings the API for event listeners and client callbacks more in-line with how slash commands and message commands are done.

Checklist

  • I have run nox and all the pipelines have passed.
  • I have made unittests according to the code I have added/modified/deleted.

davfsa added 2 commits June 25, 2023 20:13
Switch to using better method to get python source-code
Comment thread tanjun/abc.py
"""

@abc.abstractmethod
def with_listener(
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Not entirely happy with this change, as it feels a bit of an outliner now

Comment thread tanjun/components.py
Comment on lines +756 to +761
.. deprecated :: 2.15
This argument used to be `name`:

The name this callback is being registered to.

This is case-insensitive.
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Let Let me know if you a different way to document this deprecation

Comment thread tanjun/listeners.py
```py
client = tanjun.Client.from_rest_bot(bot)

@client.with_client_callback("closed")
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Forgot to change the one in client, will do later :)

Comment thread tanjun/listeners.py
return lambda callback: ClientCallback(callback, name=name)


class ClientCallback(typing.Generic[_ClientCallbackSigT], components.AbstractComponentLoader):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

These types shouldn't be publicly exposed or really used anywhere else (and this stuff can all be define in components.py since that's the only place where load_from_scope is relevant).

Comment thread tanjun/listeners.py
component.add_listener(event_type, self._callback)


@typing.runtime_checkable
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

These protocols seems kinda unnecessary, this stuff only needs to work with the standard Component impl

Comment thread tanjun/listeners.py

def as_client_callback(
name: typing.Union[str, tanjun.ClientCallbackNames]
) -> collections.Callable[[_ClientCallbackSigT], ClientCallback[_ClientCallbackSigT]]:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This should just be typed as returning Callable[[_ClientCallbackSigT], _ClientCallbackSigT]

Comment thread tanjun/components.py
self._client.remove_client_callback(name, callback)

@typing.overload
def with_client_callback(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The as_ load_from_scope functions shouldn't replace these decorators, they should just be added alongside this (and in the case of event listeners just directly call the with_listener instead of add_listener to allow for type hint analysis)

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