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

fix: properly parse aliases for base commands #1728

Merged
merged 2 commits into from
Aug 26, 2024

Conversation

AstreaTSS
Copy link
Member

Pull Request Type

  • Feature addition
  • Bugfix
  • Documentation update
  • Code refactor
  • Tests improvement
  • CI/CD pipeline enhancement
  • Other: [Replace with a description]

Description

Previously, aliases like the below wouldn't be properly registered for the prefixed command variant of hybrid commands:

message = HybridSlashCommand(
    name="message",
    description="Hosts public-facing messaging commands.",
    dm_permission=False,
    aliases=["msg"],
)

In this case, trying to use <prefix>msg would not make the bot respond. The hybrid manager was just ignoring any commands without a callback or that were only meant as a jumping point for subcommands, meaning it would never read the aliases from them.

For the first part, this PR properly parses these types of commands (and makes it so it doesn't add improper aliases for base commands). However, this introduced a new problem - the command hooks were run after an empty callback check, which means that the above example would still not properly get parsed. That was fixed by moving the hook above the callback check.

Changes

  • Parse and use aliases from "empty" base commands.
  • Don't add subcommand aliases as aliases to base command.
  • Move add command hook logic above the callback check.

Related Issues

Test Scenarios

See example above.

Python Compatibility

  • I've ensured my code works on Python 3.10.x
  • I've ensured my code works on Python 3.11.x

Checklist

  • I've run the pre-commit code linter over all edited files
  • I've tested my changes on supported Python versions
  • I've added tests for my code, if applicable
  • I've updated / added documentation, where applicable

@AstreaTSS AstreaTSS mentioned this pull request Aug 12, 2024
Copy link
Contributor

@LawMixer LawMixer left a comment

Choose a reason for hiding this comment

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

Looks & feels good to my dev discord bot!

@AstreaTSS AstreaTSS merged commit e566d1e into interactions-py:unstable Aug 26, 2024
2 checks passed
@AstreaTSS AstreaTSS deleted the hybrid-aliases-fix branch August 26, 2024 12:45
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.

3 participants