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 use Korean to slash_command() #210

Merged
merged 4 commits into from Dec 21, 2021
Merged

Conversation

happy-jin1234
Copy link
Contributor

@happy-jin1234 happy-jin1234 commented Dec 17, 2021

Summary

Fixed an issue where the Korean slash_command name is supported in Discord but not in this module.

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 black .
  • 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, ...)

@vcokltfre
Copy link
Member

That regex should in fact be ^[\w-]{1,32}$ to be in line with Discord's API docs. If we do it this way then we end up with a huge amount of characters and scripts in that regex and it becomes unreadable and less efficient.

@vcokltfre
Copy link
Member

see Application Command Naming

@thesadru
Copy link
Collaborator

thesadru commented Dec 17, 2021

We shouldn't handle the characters of any specific language but getting rid of the islower check is a good idea

What about

name = name.lower()
assert re.fullmatch(r"[\w-]{1,32}", name), "..."

@EQUENOS
Copy link
Member

EQUENOS commented Dec 17, 2021

Are you sure that Korean slash command names are supported? Looks like they're not (according to Application Command Naming)

@shiftinv shiftinv added the bug Something isn't working label Dec 17, 2021
@happy-jin1234
Copy link
Contributor Author

Are you sure that Korean slash command names are supported? Looks like they're not (according to Application Command Naming)

image
image

It seems supported

@thesadru
Copy link
Collaborator

They must be supported. Localization wouldn't be possible if they weren't.

@EQUENOS EQUENOS merged commit 132a4d7 into DisnakeDev:master Dec 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants