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.

Sign up for GitHub

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

Jump to bottom

fix(ui): don't require cls argument in select decorators to be positional #1111

Merged
merged 6 commits into from
Oct 29, 2023
Merged

fix(ui): don't require cls argument in select decorators to be positional #1111

merged 6 commits into from
Oct 29, 2023

Conversation

shiftinv
Copy link
Member

Summary

This PR allows the cls parameter in select decorators to be provided as a keyword argument again (@string_select(cls=x, ...)), whereas it previously had to be specified as positional (@string_select(x, ...)).
This regressed in 7467fe8 and made the cls argument not work as expected when provided via keyword.

https://canary.discord.com/channels/808030843078836254/913779868985090089/1149758316126408744 (cc @Enegg)

Checklist

  • If code changes were made, then they have been tested
    • I have updated the documentation to reflect the changes
    • I have formatted the code properly by running pdm lint
    • I have type-checked the code by running pdm pyright
  • This PR fixes an issue
  • This PR adds something new (e.g. new method or parameters)
  • This PR is a breaking change (e.g. methods or parameters removed/renamed)
  • This PR is not a code change (e.g. documentation, README, ...)

@shiftinv shiftinv added s: needs review Issue/PR is awaiting reviews t: bugfix labels Sep 23, 2023
@shiftinv shiftinv added this to the disnake v2.10 milestone Sep 23, 2023
EQUENOS
EQUENOS approved these changes Oct 26, 2023
View reviewed changes
Copy link
Member

@EQUENOS EQUENOS left a comment

Choose a reason for hiding this comment

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

This PR indeed excludes cls from the list of positional-only arguments

@shiftinv shiftinv enabled auto-merge (squash) October 29, 2023 11:55
@shiftinv shiftinv merged commit 038a365 into master Oct 29, 2023
26 checks passed
@shiftinv shiftinv deleted the fix/ui-select-decorator-cls-kwarg branch October 29, 2023 12:00
shiftinv added a commit that referenced this pull request Oct 30, 2023
@shiftinv
fix(ui): don't require cls argument in select decorators to be posi…
ef3a4d3 
…tional (#1111)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@EQUENOS EQUENOS EQUENOS approved these changes

Assignees
No one assigned
Labels
s: needs review Issue/PR is awaiting reviews t: bugfix
Projects
disnake
Status: Done
Milestone
disnake v2.10
Development

Successfully merging this pull request may close these issues.
2 participants
@shiftinv @EQUENOS