-
Notifications
You must be signed in to change notification settings - Fork 178
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
Translations: force json, convert template, default to existing key #2292
base: dev
Are you sure you want to change the base?
Conversation
src/main/kotlin/translations/intentions/ConvertToTranslationIntention.kt
Outdated
Show resolved
Hide resolved
We have already dropped support for new features for MC 1.12, so I don't mind support for lang files being unimplemented. |
…ld assist in implementing automatic yarn/mojmap I18n templating in the future
src/main/kotlin/translations/sorting/TranslationTemplateConfigurable.kt
Outdated
Show resolved
Hide resolved
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.
Looks good other than this one thing, and also formatting needs to be fixed with ktlint
if (translationSettings.isUseCustomConvertToTranslationTemplate) { | ||
translationSettings.convertToTranslationTemplate.replace("\$key", key) | ||
} else { | ||
"net.minecraft.client.resources.I18n.format(\"$key\")" |
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.
Wasn't the point of adding a checkbox for a custom template to use the mappings manager?
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.
My understanding from your previous message is that the mappings API isn't ready yet.
I added the checkbox so that when it's ready it can be integrated without requiring users to tick another box when they update.
Adds 3 new translation features (I can split into 3 PRs if needed)
Add Force JSON Translation
For me (and maybe others), the plugin isn't able to detect the Minecraft version and throws "Cannot determine MC version". This option makes it avoid erroring
Add template for the Convert to Translation feature
net.minecraft.client.resources.I18n isn't always desired, this option allows the template string to be changed
Default to existing key when adding duplicate translation text
Scans the translation file for an existing key that matches the translation text and suggests it inside the Convert to Translation dialog
Feature 3 only supports json translation files, support for lang files can be added if desired.
Apologies if there are any stylistic issues, I am not a Kotlin developer