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

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

davfsa
Copy link

@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
@@ -3589,31 +3590,20 @@ def remove_listener(self, event: type[_EventT], listener: ListenerCallbackSig[_E
The component to enable chained calls.
"""

@abc.abstractmethod
def with_listener(
Copy link
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 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
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

```py
client = tanjun.Client.from_rest_bot(bot)

@client.with_client_callback("closed")
Copy link
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 :)

return lambda callback: ClientCallback(callback, name=name)


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

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).

component.add_listener(event_type, self._callback)


@typing.runtime_checkable
Copy link
Owner

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


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

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]

@@ -715,9 +717,25 @@ def remove_client_callback(self, name: str, callback: tanjun.MetaEventSig, /) ->
if self._client:
self._client.remove_client_callback(name, callback)

@typing.overload
def with_client_callback(
Copy link
Owner

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