-
-
Notifications
You must be signed in to change notification settings - Fork 64
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
refactor(commands): add localization to commands #1148
base: main
Are you sure you want to change the base?
Conversation
@JPBM135 is attempting to deploy a commit to the discordjs Team on Vercel. A member of the Team first needs to authorize it. |
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
locales.reduce<LocalizationMap>((acc, locale) => { | ||
acc[locale] = | ||
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing | ||
i18next.t(resolveKey(options, type), { lng: locale, fallbackLng: "dev", defaultValue: false }) || undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fallbackLng: "dev"
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensures that it will return undefined if not found, if not set, all the lang that doesn't have a translation would be set as English automatically by the English fallback
Ex:
EN => command.ping.description => "check the bot's health"
PT => command.ping.description => undef
i18next.t('command.ping.description', { lng: 'PT', defaultValue: false })
=>check the bot's health
i18next.t('command.ping.description', { lng: 'PT', fallbackLng: 'dev', defaultValue: false })
=>null/undefined
TimeoutCommand, | ||
ClearCommand, | ||
ReportUtilsCommand, | ||
].map((command) => formatCommandToLocalizations("moderation", command as any)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as any
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only way I could found to invalidate the as const
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you know a better way, pls let me know
One thing I can spot right now is:
This way things are nicely separated between "this is part of the framework" and "this is part of yuudachi specific code" |
I agree with this, makes total sense to move the generic stuff to the framework |
I should probably add some comments for things like |
|
How?
commands.json
, which is accessible through thecommands:
prefix under any valid key.commons
object for things likehide
orduration
, the thing is some commands use a slightly different option, e.g: theban
command which has the default days as0
, not consistent with all other properties which have the default to1
. Because of this, the commons have a reasonably specific override property (see below).JSON strucuture
Override system
Using the
ban
command for exemplification:We have the common option for days:
But the ban command uses
0
as default, we can override only the choices portion of the days commons declaring it on the ban command:Resulting in:
The system will get the
name
anddescription
properties from the commons but will use the overriding choices from theban
command