Additional I18n APIs. #12

prokopyl · 2016-07-18T06:56:14Z

The goal here is to separate translation data from runtime parameters, allowing gettext to detect tranlatable strings more reliably.

This is done by adding a new class, I18nText (name can be subject to change), holding only the information gettext needs, marking it with the I.i() method before passing it to I.t(). The other I.t() sets of methods will not be deprecated.

This also allows to defer the I.t() method call from the initial I.i() marking, allowing other APIs (such as GUI, Commands, ItemStackBuilder, RawTextBuilder …) to take I18nText objects instead of regular strings, allowing them to automatically select the appropriate locale for the targeted player, for instance.

The basic implementation is here, but the integration with the other APIs still needs to be done. Extra utility methods may also be added.

* NEW: I18nText is a new structure holding translation data, separating runtime arguments from the rest and making it easier for gettext to detect. * NEW: I.i() is a new set of method allowing to easily generate I18nText objects. * NEW: The I.t() set of methods can now use I18nText objects. * NEW: The I18n.translate() set of methods can now use I18nText objects.

AmauryCarrade · 2016-07-18T09:43:47Z

src/main/java/fr/zcraft/zlib/components/i18n/I18n.java

@@ -647,6 +647,11 @@ public static String translate(String context, String messageId, String messageI
 return translate(null, context, messageId, messageIdPlural, count, parameters);
 }

+ public static String translate(Locale locale, I18nText text, Object... parameters)
+ {
+ return translate(null, text.getContext(), text.getMessageId(), text.getMessageId(), text.getCount(), parameters);


return translate(null, text.getContext(), text.getMessageId(), /* → */ text.getPluralMessageId(), /* ← */ text.getCount(), parameters);

?

* NEW: ActionGui now sets the locale for the ItemStackBuilders it returns. * NEW: ItemStackBuilder now handles locale and I18nTexts. * NEW: GuiBase: sendMessage() is a new set of method to send (translated) messages to the user of the GUI. * NEW: InventoryGui: setTitle() now handles I18nTexts. * BUG: I18n: fixed I18nText translation handling. * BUG: I18n: added some missing documentation.

* NEW: RawText and RawTextPart now handles locale and I18nTexts. * NEW: Command now sets the locale for the RawText it takes. * NEW: ItemStackBuilder now sets the locale for the RawText it takes. * NEW: ListHeaderFooter now sets the locale for the RawText it takes. * NEW: MessageSender now sets the locale for the RawText it takes. * NEW: RawMessage now sets the locale for the RawText it takes. * NEW: Titles now sets the locale for the RawText it takes.

* NEW: Command methods now handles I18nTexts. * NEW: RawTextPart hover() can now take an ItemStackBuilder, and sets its locale. * NEW: ItemStackBuilder.getLocale() is a new method to retreive the locale currently assigned to the ItemStack.

…-i18n

AmauryCarrade

Revue préliminaire — je regarde plus en détail demain.

AmauryCarrade · 2018-03-07T01:03:58Z

src/main/java/fr/zcraft/zlib/components/i18n/I.java

@@ -305,4 +315,18 @@ public static void sendTcn(Player player, String context, String singular, Strin
 player.sendMessage(I18n.translate(I18n.getPlayerLocale(player), context, singular, plural, count, parameters));
 }

+ public static LazyTranslation i(String messageId)


Changer en l pour lazy ?

Manque la doc :P

AmauryCarrade

Tout plein de remarques, certaines demandant des modifications (celles liées à gettext notamment), d'autres des idées, questions ou suggestions.

Pour les I.truc il faudrait aussi absoluement renommer tous les I.t*(Locale, …) en, par exemple, I.tl*(Locale, …) car actuellement les signatures sont dupliquées pour un même nom de méthode et gettext n'aime pas du tout, ce qui fait que ces chaînes ne sont pas extractibles.
Il faudrait le faire uniquement dans la version 0.100.0 par contre, étant donné que ça casserait la compatibilité.

Sinon, certaines autres classes pourraient être mises à jour pour appeler d'ores et déjà locale sur les RawText et autres ItemStackBuilder, je pense notamment à PaginatedTextView et les commandes d'aide (cela dit, pour ces dernières, tu es en train de les refaire donc je pense qu'on peut passer outre).

Aussi certaines méthdes prenant des String ont été oubliées :

ItemUtils.setDisplayName ;
ItemUtils.hasDisplayName ;
TextualBanners.getStringBanners (à voir, vu que les caractères sont restreins l'intégrer au système de traductions n'est peut-être pas une bonne idée) ;
plein de méthodes dans ActionBar ;
les méthodes prenant un String de ListHeaderFooter ;
les méthodes prenant un String de MessageSender ;
les méthodes prenant un String de RawMessage, ainsi que sa méthode broadcast (incluant celle prenant un RawMessage qui n'a pas sa locale mise à jour — certes c'est diffusé à tous mais on pourrait faire une boucle manuelle et l'envoyer avec la bonne locale à chaque joueur ?) ;
les méthodes prenant un String de Titles.

C'est tout, je crois !

AmauryCarrade · 2018-03-07T15:02:41Z

src/main/java/fr/zcraft/zlib/components/commands/Command.java

 parseArgs(args);

 try
 {
 if (canExecute(sender, args))
 result = complete();
 }
- catch (CommandException ignored) {}
+ catch (CommandException ignored){}


L'autre qui retire des espaces

AmauryCarrade · 2018-03-07T15:11:55Z

src/main/java/fr/zcraft/zlib/components/gui/GuiBase.java

+ * @param message The message to send to the player.
+ * @param parameters Parameters for the translatable format text, if any.
+ */
+ protected void sendMessage(LazyTranslation message, Object... parameters)


Sans demander un retrait, quelle utilité à sendMessage dans le cadre d'un GUI, qui recouvre quasi systématiquement le chat (bien que l'implémentation soit générique et permette d'avoir des GUI en texte, certes… ça reste très marginal) ?

AmauryCarrade · 2018-03-07T15:12:58Z

src/main/java/fr/zcraft/zlib/components/gui/GuiBase.java

+ */
+ protected void sendMessage(LazyTranslation message, Object... parameters)
+ {
+ sendMessage(I.t(message, parameters));


Pas de traduction automatique ici vu que la locale du joueur, pourtant accessible, n'est pas passée ?

AmauryCarrade · 2018-03-07T15:18:16Z

src/main/java/fr/zcraft/zlib/components/i18n/I.java

@@ -87,6 +87,16 @@ public static String t(String text, Object... parameters)
 {
 return I18n.translate(null, null, text, null, null, parameters);
 }
+
+ public static String t(Locale locale, LazyTranslation text, Object... parameters)


Il serait préférable d'avoir un autre nom de méthode, afin que gettext ne s'emmêle pas (cherchant un paramètre là où il n'y en a pas, considérant que la signature est la même que la première méthode t.

Par exemple tl (pour translate lazily).

AmauryCarrade · 2018-03-07T15:19:30Z

src/main/java/fr/zcraft/zlib/components/i18n/I.java

+ return I18n.translate(locale, text, parameters);
+ }
+
+ public static String t(LazyTranslation text, Object... parameters)


Même remarque que pour l'autre pour le nom. Le nom peut également être tl vu que gettext ne va pas analyser ces méthodes du tout pour extraîre les chaînes (uniquement les I.l en plus de ce qu'il y a avant cette PR).

AmauryCarrade · 2018-03-07T15:38:42Z

src/main/java/fr/zcraft/zlib/tools/items/ItemStackBuilder.java

+ *
+ * @return The current ItemStackBuilder instance, for methods chaining.
+ */
+ public ItemStackBuilder lore(LazyTranslation text, Object ...parameters)


lore prend une collection de lignes plus haut, pourquoi pas ici ?

AmauryCarrade · 2018-03-07T15:40:36Z

src/main/java/fr/zcraft/zlib/tools/items/ItemStackBuilder.java

+ *
+ * @return The current ItemStackBuilder instance, for methods chaining.
+ */
+ public ItemStackBuilder longLore(int lineLength, LazyTranslation text, Object ...parameters)


Ordre des paramètres pas très cohérent, bien que je comprenne l'envie de garder les paramètres ensemble… (un autre avantage d'avoir les paramètres dans LazyTranslation d'ailleurs).

De plus on perd les versions avec ChatColor, voulu ?

AmauryCarrade · 2018-03-07T15:42:08Z

src/main/java/fr/zcraft/zlib/components/rawtext/RawTextPart.java

@@ -185,6 +228,20 @@ public T translate(ItemStack item)
 return translate(trName);
 }

+ public T locale(Locale locale)


Peut-être une version locale(Player) qui va récupérer direct la langue du joueur donné ?

AmauryCarrade · 2018-03-07T15:43:40Z

src/main/java/fr/zcraft/zlib/tools/items/ItemStackBuilder.java

+ * @param locale The locale to use.
+ * @return The current ItemStackBuilder instance, for methods chaining.
+ */
+ public ItemStackBuilder locale(Locale locale)


Peut-être une version locale(Player) qui va récupérer direct la langue du joueur donné ?

AmauryCarrade · 2018-03-07T15:44:32Z

src/main/java/fr/zcraft/zlib/tools/items/ItemStackBuilder.java

+ * Retreives the locale assigned to this ItemStackBuilder
+ * @return the locale assigned to this ItemStackBuilder, or null if not defined.
+ */
+ public Locale getLocale()


Une méthode hasLocale pourrait simplifier la lecture du code en plusieurs endroits, si t'as la foi. Ou même localeIfMissing(Locale). (Pareil pour RawText.)

En effet le schéma

if(truc.getLocale() == null) truc.locale(I18n.getPlayerLocale(zeplayer));

est super courant dans le code, donc le remplacer par truc.localeIfMissing(zeplayer) pourrait être plus lisible.

AmauryCarrade · 2018-03-07T18:59:01Z

J'oubliais mais il faudra aussi mettre à jour le format d'extraction de gettext dans la javadoc de la classe I. Je pourrai le faire si tu ne connais pas le format.

AmauryCarrade reviewed Jul 18, 2016
View reviewed changes

prokopyl added 5 commits July 18, 2016 15:07

Merge branch 'master' into feature-new-i18n

53cafc1

* BUG: RawTextPart: hover() now correctly handles Locales.

d3da46c

AmauryCarrade added C ⋅ API Component – API-related (generic) T ⋅ Feature Type – Feature Request labels Sep 18, 2016

AmauryCarrade and others added 2 commits January 27, 2018 13:43

Merge branch 'master' into feature-new-i18n

ad1fab8

Merge branch 'master' of github.com:zDevelopers/zLib into feature-new…

b7b0a28

…-i18n

prokopyl self-assigned this Mar 7, 2018

prokopyl requested a review from AmauryCarrade March 7, 2018 00:26

Renamed class I18nText to LazyTranslation

5d1e035

AmauryCarrade requested changes Mar 7, 2018

View reviewed changes

Renamed method I.t to I.l to match LazyTranslation

27e29b3

AmauryCarrade requested changes Mar 7, 2018

View reviewed changes

Various minor changes

0b8ae14

prokopyl marked this pull request as draft November 12, 2020 17:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Additional I18n APIs. #12

Additional I18n APIs. #12

prokopyl commented Jul 18, 2016

AmauryCarrade Jul 18, 2016

AmauryCarrade left a comment

AmauryCarrade Mar 7, 2018

AmauryCarrade Mar 7, 2018

AmauryCarrade left a comment

AmauryCarrade Mar 7, 2018

AmauryCarrade Mar 7, 2018

AmauryCarrade Mar 7, 2018

AmauryCarrade Mar 7, 2018

AmauryCarrade Mar 7, 2018

AmauryCarrade Mar 7, 2018

AmauryCarrade Mar 7, 2018

AmauryCarrade Mar 7, 2018

AmauryCarrade Mar 7, 2018

AmauryCarrade Mar 7, 2018

AmauryCarrade commented Mar 7, 2018

Additional I18n APIs. #12

Are you sure you want to change the base?

Additional I18n APIs. #12

Conversation

prokopyl commented Jul 18, 2016

Choose a reason for hiding this comment

AmauryCarrade left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AmauryCarrade left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AmauryCarrade commented Mar 7, 2018