Translation key migration #367

LouisDelbosc · 2022-12-09T14:12:12Z

Related issue #336

I did not add the script I used. If you wish for me to commit it, please tell me where should I put it.

yohanboniface · 2022-12-09T14:37:08Z

Just to make it clear, this commit is only about switching from arbitrary keys to using back the English strings as keys, right ?

Next to that, we want a script that will generate (and keep up to date) the various i18n json files, and this script should be commited and integrated to the i18n workflow, correct @LouisDelbosc ?

LouisDelbosc · 2022-12-09T15:49:08Z

The current PR is about migration from one translation key standard to a new one, one where we can extract translation automatically.

I don't know what strategy for automation translation we adopt, however it'll be easily automate-able (❓)

LouisDelbosc · 2022-12-09T16:36:05Z

Hmm test are breaking because I have an issue with i18next.
It seems it has trouble looking at some keys that are using specials characters such as :.
Do you have any idea how to fix it ?

Otherwise I'll look at it next Tuesday.

berhalak · 2022-12-09T17:15:30Z

Hmm test are breaking because I have an issue with i18next. It seems it has trouble looking at some keys that are using specials characters such as :. Do you have any idea how to fix it ?

Otherwise I'll look at it next Tuesday.

The : character is interpreted as a namespace selector. I'm checking how we can disable it. I'm afraid we will have the same problem with ..

berhalak · 2022-12-09T18:37:24Z

I have a fix for this, just waiting for tests and will push a PR.

berhalak · 2022-12-09T19:25:15Z

This PR hopefully should address the issue you are facing.

berhalak · 2022-12-12T17:31:33Z

@LouisDelbosc the fix has been merged, it should now be possible to use natural keys with : or . characters.

LouisDelbosc · 2022-12-13T10:16:35Z

Thank you @berhalak. Do you have an idea where the should I put the script to generate the translations keys ? Do you have a folder where you put this kind of code ?

berhalak · 2022-12-13T13:21:22Z

Yes, most scripts are in /buildtools folder.

berhalak · 2022-12-14T09:58:52Z

What is a plan for the French translation file?

LouisDelbosc · 2022-12-14T10:27:19Z

We'll do it later. Tomorrow I'll put the script in /buildtools and a command line in the package.json file.
@yohanboniface correct me if I'm wrong but I think french translations will be done in another PR ?

yohanboniface · 2022-12-15T16:34:19Z

@yohanboniface correct me if I'm wrong but I think french translations will be done in another PR ?

We'll finish the translations in another PR yes, but we'll need the French json keys to be updated accordingly, is that in your radar ?

buildtools/generate_translation_keys.js

package.json

static/locales/en.client.json

buildtools/generate_translation_keys.js

app/client/models/AppModel.ts

berhalak · 2022-12-16T13:29:13Z

static/locales/en.client.json

+    "Clear cell": "Clear cell",
+    "Clear values": "Clear values",
+    "Copy anchor link": "Copy anchor link",
+    "Delete {{count}} columns_one": "Delete {{count}} columns",


Something is off with the script, every resource that needs pluralization is wrong. For example here, for one column it should say "Delete column".

As I stated in the issue #336, we cannot automatically determine what would be the plural or singular form only by looking at the keys/texte.
You might ask "why bother with plurals and singular" ? Depending on the language, we could have multiples plurals variables, for one, for two maybe for three, some language may have edge case.

Since we cannot determine the value from the key, we decide to put in key the plural form and correct by hands the value for singular or other case.

I'll do the correction by hands on the singular keys which are off

yohanboniface · 2022-12-16T17:10:35Z

Since we cannot determine the value from the key,

Given we are using our own helper (makeT), why not dealing with both key in it, and then generating both singular and plural automatically ?

I'm thinking about something like this:

makeT("my singular text", count={myvar}, plural_form="my {{myvar}} plural form")

LouisDelbosc · 2022-12-19T09:34:03Z

Since we cannot determine the value from the key,

Given we are using our own helper (makeT), why not dealing with both key in it, and then generating both singular and plural automatically ?

I'm thinking about something like this:
makeT("my singular text", count={myvar}, plural_form="my {{myvar}} plural form")

It would be possible but since there is multiples plural_form possible, the interface should be more like:

makeT("my singular text",
  {
    count,
    two: "my plural for {{count}}",
    three: "my {{count}} form",
    other: "{{count}} form",
  })

with one, two three and others the part after they key for i18next to know the plural form : https://www.i18next.com/translation-function/plurals

yohanboniface · 2022-12-19T10:41:41Z

It would be possible but since there is multiples plural_form possible, the interface should be more like

What about just supporting one and many for a first step ?

berhalak

Found a couple of keys that are still using old schema. Regarding the build error, you can rebase on main branch, it should be fixed there.

berhalak · 2022-12-19T17:57:51Z

static/locales/en.client.json

+    "Trash is empty.": "Trash is empty.",
+    "Unpin Document": "Unpin Document",
+    "Workspace not found": "Workspace not found",
+    "You are on the {{siteName}} site. You also have access to the following sites:": "You are on the {{siteName}} site. You also have access to the following sites:",


Those two should be merged (version for the personal site and for the team site).

How should it be merged ? We no longer using context and as you can see in the code below, both are still used.

Ahhh, so the condition is wrong. Those two should be swapped.

berhalak · 2022-12-19T21:20:29Z

static/locales/en.client.json

-      "FullCopies": "Permission to access the document in full when needed",
-      "AttributeNamePlaceholder": "Attribute name",
-      "Everyone": "Everyone",
-      "EveryoneElse": "Everyone Else",


This key is still used in ACL page.

They are not really removed either. Here the new AccessRules translations keys:

"AccessRules": { "Add Column Rule": "Add Column Rule", "Add Default Rule": "Add Default Rule", "Add Table Rules": "Add Table Rules", "Add User Attributes": "Add User Attributes", "Allow everyone to copy the entire document, or view it in full in fiddle mode.\nUseful for examples and templates, but not for sensitive data.": "Allow everyone to copy the entire document, or view it in full in fiddle mode.\nUseful for examples and templates, but not for sensitive data.", "Allow everyone to view Access Rules.": "Allow everyone to view Access Rules.", "Attribute name": "Attribute name", "Attribute to Look Up": "Attribute to Look Up", "Checking...": "Checking...", "Condition": "Condition", "Default Rules": "Default Rules", "Delete Table Rules": "Delete Table Rules", "Enter Condition": "Enter Condition", "Everyone": "Everyone", "Everyone Else": "Everyone Else", "Invalid": "Invalid", "Lookup Column": "Lookup Column", "Lookup Table": "Lookup Table", "Permission to access the document in full when needed": "Permission to access the document in full when needed", "Permission to view Access Rules": "Permission to view Access Rules", "Permissions": "Permissions", "Remove column {{- colId }} from {{- tableId }} rules": "Remove column {{- colId }} from {{- tableId }} rules", "Remove {{- tableId }} rules": "Remove {{- tableId }} rules", "Remove {{- name }} user attribute": "Remove {{- name }} user attribute", "Reset": "Reset", "Rules for table ": "Rules for table ", "Save": "Save", "Saved": "Saved", "Special Rules": "Special Rules", "Type a message...": "Type a message...", "User Attributes": "User Attributes", "Users": "Users" },

I count 32 keys for both the new AccessRules and the old translations keys.

berhalak · 2022-12-19T21:23:29Z

static/locales/en.client.json

-      "UpdatingRecordLayout":"Updating record layout."
-    },
-    "RecordLayoutEditor": {
-      "AddField":"Add Field",


Those keys as still used.

They are not really removed. I removed the folder key (in this case components) to have a flatter file. The new RecordLayoutEditor translations look like this :

"RecordLayoutEditor": { "Add Field": "Add Field", "Create New Field": "Create New Field", "Show field {{- label}}": "Show field {{- label}}", "Save Layout": "Save Layout", "Cancel": "Cancel" }

Of course the file has been updated and there the key named AddField is replaced by Add Field

LouisDelbosc · 2022-12-20T16:03:41Z

@berhalak I don't have the errors there is on the CI locally. Was I just lucky ?

berhalak · 2022-12-20T16:41:54Z

@berhalak I don't have the errors there is on the CI locally. Was I just lucky ?

I'm restarting the last build, can't reproduce the error locally.

berhalak

Thanks, @LouisDelbosc, it looks great. I will merge this PR after those conflicts are resolved. Sorry for the delay, but this is a big PR, and we need to synchronize it with the release schedule.

paulfitz · 2022-12-29T17:04:08Z

Just to be clear, is the next step for this PR for @LouisDelbosc to resolve the merge conflicts?

Someone recently volunteered to do a Portuguese translation and is already testing a weblate project https://hosted.weblate.org/projects/grist/client/. I don't want to slow them down, so I'm willing to fix up anything that needs fixing to adapt any work they do. It would be better to get this PR through sooner rather than later though, both for this reason and because it touches a lot of files (so merge conflicts will continue to accumulate).

That said it is a holiday period, Happy New Year everyone, looking forward to 2023 bringing a very multilingual Grist :-)

yohanboniface · 2022-12-29T17:26:23Z

@paulfitz hey :)

Just to be clear, is the next step for this PR for @LouisDelbosc to resolve the merge conflicts?

I'd say so!

@LouisDelbosc is indeed in holiday ;)
I've had a look yesterday to see if I could resolve the merge conflicts so to make this PR land and not to have more conflicts while the main branch continues, but the conflicts were too nested in the code for me :/

berhalak · 2023-01-03T11:20:21Z

Thanks @LouisDelbosc !

LouisDelbosc · 2023-01-03T15:03:18Z

There was new conflicts I fixed. I'd like this PR to be merged so I don't have to fix conflict as a job 😅

LouisDelbosc force-pushed the translation-key-migration branch from 9bb7021 to 995b9f0 Compare December 9, 2022 16:07

berhalak self-assigned this Dec 9, 2022

LouisDelbosc force-pushed the translation-key-migration branch from 995b9f0 to bf26e66 Compare December 13, 2022 10:11

berhalak self-requested a review December 15, 2022 14:46

berhalak requested changes Dec 16, 2022

View reviewed changes

LouisDelbosc force-pushed the translation-key-migration branch from a966e1e to eacb3f9 Compare December 19, 2022 18:59

LouisDelbosc requested a review from berhalak December 19, 2022 19:00

berhalak requested changes Dec 19, 2022

View reviewed changes

LouisDelbosc force-pushed the translation-key-migration branch from eacb3f9 to f4f0238 Compare December 20, 2022 10:18

LouisDelbosc force-pushed the translation-key-migration branch from f4f0238 to f8fb848 Compare December 21, 2022 11:06

LouisDelbosc requested a review from berhalak December 21, 2022 11:06

berhalak approved these changes Dec 21, 2022

View reviewed changes

LouisDelbosc force-pushed the translation-key-migration branch from f8fb848 to 1a4299b Compare January 3, 2023 09:59

LouisDelbosc added 16 commits January 3, 2023 15:46

Change translation keys for models directory

fb8bd2d

Change translation keys for lib directory

156a471

Change translation keys for aclui directory

32e3d25

Change translation keys for ui2018 directory

7a2a0a7

Change translation keys for components directory

24a6564

Change translation keys for ui directory

b76fe50

Change translation keys for simple context keys

37dc0cc

Change translation keys for complex context keys

e0ae321

Change translation keys for keys with counts

7082e3d

Generated translation keys from code file

1d97137

Remove prefix from translations keys

676c27d

Fix tests

83ea686

Add translation keys generator script

4165c35

Clean generating script and english translation values

c18c6cb

Add fr translation and fix some translation on the go

78a1c2d

Fix new translations

67c09d7

LouisDelbosc force-pushed the translation-key-migration branch from 1a4299b to 67c09d7 Compare January 3, 2023 15:01

berhalak merged commit de1001f into gristlabs:main Jan 3, 2023

vviers added the anct label Jul 11, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Translation key migration #367

Translation key migration #367

LouisDelbosc commented Dec 9, 2022

yohanboniface commented Dec 9, 2022

LouisDelbosc commented Dec 9, 2022

LouisDelbosc commented Dec 9, 2022

berhalak commented Dec 9, 2022

berhalak commented Dec 9, 2022

berhalak commented Dec 9, 2022

berhalak commented Dec 12, 2022

LouisDelbosc commented Dec 13, 2022

berhalak commented Dec 13, 2022

berhalak commented Dec 14, 2022

LouisDelbosc commented Dec 14, 2022

yohanboniface commented Dec 15, 2022

berhalak Dec 16, 2022

LouisDelbosc Dec 16, 2022

yohanboniface commented Dec 16, 2022

LouisDelbosc commented Dec 19, 2022

yohanboniface commented Dec 19, 2022

berhalak left a comment

berhalak Dec 19, 2022

LouisDelbosc Dec 20, 2022

berhalak Dec 20, 2022

berhalak Dec 19, 2022

LouisDelbosc Dec 20, 2022

berhalak Dec 19, 2022

LouisDelbosc Dec 20, 2022

LouisDelbosc commented Dec 20, 2022

berhalak commented Dec 20, 2022

berhalak left a comment •

edited

paulfitz commented Dec 29, 2022

yohanboniface commented Dec 29, 2022

berhalak commented Jan 3, 2023

LouisDelbosc commented Jan 3, 2023

Translation key migration #367

Translation key migration #367

Conversation

LouisDelbosc commented Dec 9, 2022

yohanboniface commented Dec 9, 2022

LouisDelbosc commented Dec 9, 2022

LouisDelbosc commented Dec 9, 2022

berhalak commented Dec 9, 2022

berhalak commented Dec 9, 2022

berhalak commented Dec 9, 2022

berhalak commented Dec 12, 2022

LouisDelbosc commented Dec 13, 2022

berhalak commented Dec 13, 2022

berhalak commented Dec 14, 2022

LouisDelbosc commented Dec 14, 2022

yohanboniface commented Dec 15, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yohanboniface commented Dec 16, 2022

LouisDelbosc commented Dec 19, 2022

yohanboniface commented Dec 19, 2022

berhalak 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

LouisDelbosc commented Dec 20, 2022

berhalak commented Dec 20, 2022

berhalak left a comment • edited

Choose a reason for hiding this comment

paulfitz commented Dec 29, 2022

yohanboniface commented Dec 29, 2022

berhalak commented Jan 3, 2023

LouisDelbosc commented Jan 3, 2023

berhalak left a comment •

edited