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

Add user param to menu() #4913

Merged
merged 17 commits into from
Mar 18, 2024

Conversation

laggron42
Copy link
Contributor

@laggron42 laggron42 commented Mar 23, 2021

Type

  • Bugfix
  • Enhancement
  • New feature

Description of the changes

A user can be provided when using menus instead of having to rely on the hardcoded ctx.author for defining who should be allowed to interact. If nothing is provided, falls back to that default ctx.author

@github-actions github-actions bot added the Category: Core - API - Utils Package This is related to stuff in `redbot.core.utils` label Mar 23, 2021
@Jackenmen
Copy link
Member

The way this is currently done is breaking as arguments to menu() can be passed positionally

@Jackenmen Jackenmen added the Type: Enhancement Something meant to enhance existing Red features. label Apr 1, 2021
@Jackenmen Jackenmen added this to the 3.4.9 milestone Apr 1, 2021
@Jackenmen Jackenmen added the Blocked By: Damage Control Blocked in the interest of preventing damage. label Apr 1, 2021
@Drapersniper
Copy link
Contributor

The way this is currently done is breaking as arguments to menu() can be passed positionally

A simple fix would be adding the user argument as the last argument in the fingerprint which should make it non breaking in positional usages

@laggron42
Copy link
Contributor Author

Actually that's a pretty annoying problem which I didn't think of. Yeah, passing the user as last argument wouldn't break anything, but that would then make the docs invalid

Additionally, they must take all of the parameters of this function, in addition to a string representing the emoji reacted with. This parameter should be the last one, and none of the parameters in the handling functions are optional

Is this worth a breaking change? Can't think of a way to follow that without breaking other installations

@Jackenmen Jackenmen removed the Blocked By: Damage Control Blocked in the interest of preventing damage. label Apr 9, 2021
@laggron42
Copy link
Contributor Author

Based on Jack's comments, I modified the way it's done. user is a keyword-only argument with a default set to None. We check if user is None to determine whether the parameter should be passed to the controls or not. That way, this implementation is not a breaking change.

I'm aware of the new d.py menus function, didn't really look into it when first writing this PR. If you prefer to close this in favor of their menus, that's ok, I'm probably going to switch to their menus too anyway. Just wanted to make that PR working at least.

@github-actions github-actions bot added the Category: Core - Help This is related to our help command and/or HelpFormatter API. label Mar 17, 2024
@Jackenmen Jackenmen force-pushed the menus-change-author branch from 2cf8a22 to 621d678 Compare March 17, 2024 02:16
@Jackenmen Jackenmen force-pushed the menus-change-author branch from 621d678 to d13b17e Compare March 17, 2024 02:23
@Jackenmen Jackenmen changed the title [utils.menus] Add user param Add user param to menu() Mar 18, 2024
Copy link
Member

@Jackenmen Jackenmen left a comment

Choose a reason for hiding this comment

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

I think this is a useful change that shouldn't affect anything badly with the current implementation where we only call controls differently if the argument is passed.

Since the menu() API is ancient (aside from the recently added set usebuttons functionality and related fixes) I added provisional notes expiring in 2 months, let's see if we run into any issues here. Thanks for making the PR!

@Jackenmen Jackenmen merged commit 8c29765 into Cog-Creators:V3/develop Mar 18, 2024
17 checks passed
@red-githubbot red-githubbot bot added the Changelog Entry: Pending Changelog entry for this PR hasn't been added by repo maintainers yet. label Mar 18, 2024
@Jackenmen Jackenmen added Changelog Entry: Added Changelog entry for this PR has already been added to changelog PR. and removed Changelog Entry: Pending Changelog entry for this PR hasn't been added by repo maintainers yet. labels Mar 22, 2024
Dav-Git pushed a commit to Dav-Git/Red-DiscordBot that referenced this pull request Sep 8, 2024
Co-authored-by: jack1142 <[email protected]>
Co-authored-by: Jakub Kuczys <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Core - API - Utils Package This is related to stuff in `redbot.core.utils` Category: Core - Help This is related to our help command and/or HelpFormatter API. Changelog Entry: Added Changelog entry for this PR has already been added to changelog PR. Type: Enhancement Something meant to enhance existing Red features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants