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

refactor(commands): add localization to commands #1148

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

JPBM135
Copy link
Contributor

@JPBM135 JPBM135 commented Oct 23, 2022

My pt-BR when? 🥲

How?

  • i18next now supports a new namespace (fancy name for file) called commands.json, which is accessible through the commands: prefix under any valid key.
  • The command file uses an object-like structure (see details below).
  • All options are mapped by name, with the exception of choices which are mapped by value.
  • To take easy on crowding and remove duplicated stuff, the structure contains a commons object for things like hide or duration, the thing is some commands use a slightly different option, e.g: the ban command which has the default days as 0, not consistent with all other properties which have the default to 1. Because of this, the commons have a reasonably specific override property (see below).

This close #1123

JSON strucuture

category -> command.name
sub_command_group.name -> sub_command?.name -> option?.name -> choices?.value
sub_command.name -> option?.name -> choices?.value
option?.name -> choices?.value

Override system

Using the ban command for exemplification:

We have the common option for days:

"days": {
	"name": "days",
	"description": "The amount of days to delete messages for",
	"choices": {
		"0": "0 days",
		"1": "1 day (default)",
		"2": "2 days",
		"3": "3 days",
		"4": "4 days",
		"5": "5 days",
		"6": "6 days",
		"7": "7 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:

"ban": {
	"name": "ban",
	"description": "Ban a member of(f) this guild",
	"options": {
		"days": {
			"choices": {
				"0": "0 days (default)",
				"1": "1 day",
			}
		}
	}
},

Resulting in:

{
    "name": "days",
    "description": "The amount of days to delete messages for",
    "type": 4,
    "choices": [
        {
            "name": "0 days (default)",
            "value": 0,
            "name_localizations": {
                "en-US": "0 days (default)"
            }
        },
        {
            "name": "1 day",
            "value": 1,
            "name_localizations": {
                "en-US": "1 day"
            }
        },
        {
            "name": "2 days",
            "value": 2,
            "name_localizations": {
                "en-US": "2 days"
            }
        },
        {
            "name": "3 days",
            "value": 3,
            "name_localizations": {
                "en-US": "3 days"
            }
        },
        {
            "name": "4 days",
            "value": 4,
            "name_localizations": {
                "en-US": "4 days"
            }
        },
        {
            "name": "5 days",
            "value": 5,
            "name_localizations": {
                "en-US": "5 days"
            }
        },
        {
            "name": "6 days",
            "value": 6,
            "name_localizations": {
                "en-US": "6 days"
            }
        },
        {
            "name": "7 days",
            "value": 7,
            "name_localizations": {
                "en-US": "7 days"
            }
        }
    ]
}

The system will get the name and description properties from the commons but will use the overriding choices from the ban command

@vercel
Copy link

vercel bot commented Oct 23, 2022

@JPBM135 is attempting to deploy a commit to the discordjs Team on Vercel.

A member of the Team first needs to authorize it.

@vercel
Copy link

vercel bot commented Oct 23, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Updated
yuudachi-report ⬜️ Ignored (Inspect) Nov 29, 2022 at 2:30AM (UTC)

@JPBM135 JPBM135 marked this pull request as draft October 23, 2022 15:09
@JPBM135 JPBM135 changed the title refactor(commands): allow commands to localize refactor(commands): add localization to commands Oct 23, 2022
@JPBM135 JPBM135 marked this pull request as ready for review October 23, 2022 21:00
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;
Copy link
Member

Choose a reason for hiding this comment

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

fallbackLng: "dev" ?

Copy link
Contributor Author

@JPBM135 JPBM135 Nov 29, 2022

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));
Copy link
Member

Choose a reason for hiding this comment

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

as any ?

Copy link
Contributor Author

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

Copy link
Contributor Author

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

@iCrawl
Copy link
Member

iCrawl commented Nov 29, 2022

One thing I can spot right now is:

  • Move the types.ts file for localizations to @yuudachi/framework maybe? Seems pretty generic to me and useful for everyone that uses the framework
  • Move the formatting functions maybe to @yuudachi/framework too?

This way things are nicely separated between "this is part of the framework" and "this is part of yuudachi specific code"

@JPBM135
Copy link
Contributor Author

JPBM135 commented Nov 29, 2022

One thing I can spot right now is:

  • Move the types.ts file for localizations to @yuudachi/framework maybe? Seems pretty generic to me and useful for everyone that uses the framework
  • Move the formatting functions maybe to @yuudachi/framework too?

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

@JPBM135
Copy link
Contributor Author

JPBM135 commented Nov 29, 2022

I should probably add some comments for things like fallbackLng: 'dev'

@JPBM135 JPBM135 marked this pull request as draft November 30, 2022 20:02
@JPBM135
Copy link
Contributor Author

JPBM135 commented Nov 30, 2022

  • Add comments to sketchy stuff
  • Move types to the framework
  • Move common functions to the framework

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add localizations to command names, descriptions and options
3 participants