Skip to content
This repository has been archived by the owner on Mar 13, 2023. It is now read-only.

feat💥: Switch error and completion function to proper events #641

Merged
merged 7 commits into from
Sep 20, 2022

Conversation

Kigstn
Copy link
Member

@Kigstn Kigstn commented Sep 11, 2022

What type of pull request is this?

  • Non-breaking code change
  • Breaking code change
  • Documentation change/addition
  • Tests change

Description

See the nice bullet list below.

There still is the issue that neither the ComponentError nor the ModalError event will be called if ctx.wait_for_component or ctx.wait_for_modal are used. This was a problem before this however and needs some thinking how best to fix.

Changes

  • add CommandCompletion event
  • add ComponentCompletion event
  • add AutocompleteCompletion event
  • add ModalCompletion event
  • add CommandError event
  • add ComponentError event
  • add AutocompleteError event
  • add ModalError event
  • allow events to be overwritable by setting delete_if_overwritten. This means they will be deleted if the user registeres their own listener for that event
  • (💥) remove ModalResponse event in favor of the new ModalCompletion event
  • (💥) remove on_command calls in favor of the new CommandCompletion event
  • (💥) remove on_component calls in favor of the new ComponentCompletion event
  • (💥) remove on_autocomplete calls in favor of the new AutocompleteCompletion event
  • (💥) remove on_command_error calls in favor of the new CommandError event
  • (💥) remove on_component_error calls in favor of the new ComponentError event
  • (💥) remove on_autocomplete_error calls in favor of the new AutocompleteError event
  • (💥) remove on_error calls in favor of the Error event

Checklist

  • I've formatted my code with Black
  • I've ensured my code works on Python 3.10.x
  • I've tested my code

Copy link
Collaborator

@silasary silasary left a comment

Choose a reason for hiding this comment

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

Please add listeners to naff.ext.sentry for the new error events.

@Kigstn
Copy link
Member Author

Kigstn commented Sep 12, 2022

Please add listeners to naff.ext.sentry for the new error events.

Which ones? Functionality is the same here as on pypi, since the new error events all dispatch Error by default which sentry handles. On pypi, the Error event is also only called if the user has not overwritten it

@silasary
Copy link
Collaborator

Ah, yep. Never mind then. I just misread the code when doing my initial review.

@silasary
Copy link
Collaborator

One thing I'd like to clarify though:

If I override a listener function that has delete_if_overwritten, I can still expect it to work? Because from reading it, it seems closer to delete_if_subscribed than delete_if_overwritten

@Kigstn
Copy link
Member Author

Kigstn commented Sep 13, 2022

One thing I'd like to clarify though:

If I override a listener function that has delete_if_overwritten, I can still expect it to work? Because from reading it, it seems closer to delete_if_subscribed than delete_if_overwritten

Yes. I'm not really happy with the name, happy to take suggestions. If the flag is set, the listener will be deleted if a listener without the flag is added.

The event will still trigger, but only the listener without the flag will be called.

@silasary
Copy link
Collaborator

Let's figure out a better name for it before we merge then.

@LordOfPolls
Copy link
Member

delete_if_overriden would be an apt name, failing that delete_if_subscribed would be a valid name
delete_if_overwritten would be a misnomer

@Kigstn
Copy link
Member Author

Kigstn commented Sep 17, 2022

I'll change it to delete_if_overridden for now then

# Conflicts:
#	naff/api/events/internal.py
#	naff/client/client.py
#	naff/models/naff/listener.py
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants