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

Make SlashCommand#getHelp() return translated text when available. #90

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Andre601
Copy link

@Andre601 Andre601 commented Jul 25, 2024

Pull Request

Pull Request Checklist

Please follow the following steps before opening this PR.

PRs that do not complete the checklist will be subject to denial for
missing information.

Pull Request Information

Check and fill in the blanks for all that apply:

  • My PR fixes a bug, error, or other issue with the library's codebase.
  • My PR is for the commands module of the JDA-Utilities library.
  • My PR creates a new module for the JDA-Utilities library: ______.

Description

This overrides the getHelp() method to return a translated description using the default locale through getDescriptionLocalization().get(TranslateUtil.getDefaultLocale)
This should allow a user to set the description within the localization files without needing to set the help text itself.
Should there be no localized description for the default locale (Be null or empty) will it default to using this.help

This also adds getDefaultLocale() in the TranslateUtil to retrieve the default locale that was set.

I didn't add the same functionality to the name, as names in SlashCommands need to be alphanummeric when creating, which with this aproach would be problematic.

Given that this overrides a method for SlashCommand, should it automatically also be applied to Subcommands too.

Closes #89

helpMessage = getDescriptionLocalization().get(TranslateUtil.getDefaultLocale());
}

return (helpMessage == null || helpMessage.isEmpty()) ? this.help : helpMessage;
Copy link
Owner

Choose a reason for hiding this comment

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

We should always return this.help if it is specified, and only do this logic if it's not

Copy link
Author

Choose a reason for hiding this comment

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

Sorry for a late reply... Honestly forgot this.

The main issue is, that help is always there. By default is it no help available, so if we should have this logic will this require modifications to the default help message (i.e. make it null and have getHelp() return the default text in case of null), which I feel like a breaking change here.

Maybe another aproach could be a dedicated option and getter for the command description for slash commands that defaults to returning getHelp() if nothing is set?

Copy link
Author

Choose a reason for hiding this comment

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

Any input on this?

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.

Translations from property file not applied?
2 participants