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

Translate Chess Insights - Preset #8828

Closed
wants to merge 8 commits into from

Conversation

TheYoBots
Copy link
Contributor

Partially completes #3160

@benediktwerner
Copy link
Member

Also, you are missing an import in Presets.scala. You need this at the top next to the other import: import lila.i18n.{ I18nKeys => trans }. Ideally, you should try running it yourself to check that everything looks right. Feel free to ask on Discord if something doesn't work.

And since you mentioned that it partially completes the issue: What's still missing?

@@ -14,42 +15,42 @@ object Preset {

val forMod = List(
Copy link
Member

Choose a reason for hiding this comment

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

Also let mods presets untranslated 👍

@benediktwerner
Copy link
Member

benediktwerner commented May 1, 2021

Hm, still fails to compile.

You are missing the I18nKeys for blurByTimeVariance and howWellDoIMoveEachPieceInTheOpening.

And yeah, you need a Lang in scope when using translations. I think simply changing all the val base = List(...) to def base(implicit lang: Lang) = List(...) and the equivalent for the others should work.

@ornicar
Copy link
Collaborator

ornicar commented May 2, 2021

I don't think insights should be translated.

I'm certain that moderator-only insights presets must not be translated.

@TheYoBots
Copy link
Contributor Author

Would you like me to revert the moderator translations and keep the rest or simply close?

@ornicar
Copy link
Collaborator

ornicar commented May 2, 2021

Although powerful, insights are too raw to be useful to most people. They could be the foundation of something more comprehensive in the feature, and that thing could then be translated. But not the insights as they are now.

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.

4 participants