Extract text from CLI to enable internationalisation by CarlBeek · Pull Request #182 · ethereum/staking-deposit-cli

CarlBeek · 2021-02-12T16:59:25Z

In order to support internationalisation, the text needs to be removed from the CLI and broken out into separate content files (.json).

This PR implements this via a function that determines the context in which it was called (from the call stack) and uses this information to determine the correct files to load in.

samajammin · 2021-02-16T21:10:36Z

@CarlBeek nice!

Just to confirm the file structure, to translate eth2deposit/intl/en/cli/existing_mnemonic.json, we'd upload the translated file e.g. for Spanish as eth2deposit/intl/es/cli/existing_mnemonic.json or e.g. for German eth2deposit/intl/de/cli/existing_mnemonic.json.

samajammin · 2021-02-16T23:16:56Z

@wackerow I've tested out uploading nested JSON into Crowdin & can confirm that this format is supported 👍

File uploaded (you'll need to accept my manager invite in order to view):
https://crowdin.com/project/eth2-launchpad-test/settings#files

File translation:
https://crowdin.com/translate/eth2-launchpad-test/4/en-es?filter=basic&value=0

wackerow · 2021-02-17T02:56:52Z

@CarlBeek This looks great! Thanks so much for tackling this.

I took a look through the codebase to find what's remaining to refactor, and will list it here to track:

cli/new_mnemonic.py
X[ ] key_handling/key_derivation/mnemonic.py
X[ ] settings.py
deposit.py
X[ ] credentials.py
X[ ] key_handling/key_derivation/path.py?
X[ ] key_handling/key_derivation/tree.py?
cli/existing_mnemonic.py
cli/generate_keys.py
utils/validation.py

Happy to give a hand refactoring the remaining pages, just let me know 👍😀

CarlBeek · 2021-02-22T17:28:47Z

@wackerow, Thanks for the list. I've tackled the remaining files in 5fceda2.
Regarding:

key_handling/key_derivation/mnemonic.py
credentials.py
key_handling/key_derivation/path.py
key_handling/key_derivation/tree.py
The messages and errors should never be shown to the user. They are there for internal testing & sanity checks. If a user ever sees one of those messages, it means something has gone catastrophically wrong.

settings.py only lists the names of the testnets & mainnet, are we planning on translating these?

wackerow · 2021-02-23T19:50:04Z

@CarlBeek Updated the list above, and looks like that testing bug was fixed? I think the only thing left is a language flag, and an initial prompt if this flag isn't provided (before selecting mnemonic language).

Anything else I'm missing before this is ready?

hwwhww

I got errors while executing the macOS binary:

➜ ./deposit --new-mnemonics
Traceback (most recent call last):
  File "eth2deposit/deposit.py", line 4, in <module>
  File "<frozen importlib._bootstrap>", line 983, in _find_and_load
  File "<frozen importlib._bootstrap>", line 967, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 677, in _load_unlocked
  File "/usr/local/lib/python3.7/site-packages/PyInstaller/loader/pyimod03_importers.py", line 623, in exec_module
  File "eth2deposit/cli/existing_mnemonic.py", line 28, in <module>
  File "eth2deposit/utils/intl.py", line 40, in load_text
FileNotFoundError: [Errno 2] No such file or directory: 'eth2deposit/intl/en/cli/existing_mnemonic.json'
[51580] Failed to execute script deposit

I guess the build.spec should include the new json files to datas.

https://github.com/ethereum/eth2.0-deposit-cli/blob/b7f4188633ee89013eb789b6468ef2534019ae54/build_configs/macos/build.spec#L8

wackerow · 2021-02-26T19:05:13Z

@@ -0,0 +1,35 @@
+{
+    "validate_password": {
+        "msg_password_prompt": "Type the password that secures your validator keystore(s)",


Forwarding on a comment by @samajammin on initial intl audit (#180) that I agree with... consider making this clearer that the user is about to create a new password, vs typing an existing password, eg:
Create a password that will secure your validator keystore(s)
"Type the password that secures" kinda implies there is already password currently securing it.

samajammin · 2021-03-02T00:45:50Z

@@ -0,0 +1,14 @@
+{
+    "from_mnemonic": {
+        "msg_key_creation": "Creating your keys:\t\t"


Is it possible for us to avoid including the tabs (\t) in the JSON files?

We might be able to ignore it in Crowdin but it'd be ideal to remove these to not confuse translators.

I will look into extraction this from the json and having the tabs encoded in separate print statements. 👍

Co-authored-by: Hsiao-Wei Wang <[email protected]>

Arabic, Romanian, Portuguese (Brazilian), Indonesian, Greek, French, Chinese (simplified), Korean

wackerow · 2021-03-22T18:22:23Z

@CarlBeek Hey Carl! Wanted to circle back to this and try to organize final steps. I think the only thing left before was:

Language flag / initial prompt

The translations from Crowdin are starting to come in, and the Launchpad side of things is tee'd up for at least 8 of these languages, but one final step is to make sure the CLI commands shown on the website correctly match this repo.

Trying to get an estimate on how much is left here. Should we just temporarily default the CLI commands shown on the Launchpad to English while we finalize things here? Added the languages that are translated so far to a branch off this one, and put it in PR #191.

I also see the Prater testnet was just added... Can't think of how this would delay/change anything, just wanted to make sure.

Almost there!

Italian, Japanese

hwwhww

Found some issues during QA. It looks good to me once these issues are fixed.
Awesome work! 🎉

* cli-shell-debug-0: intl start_index confirmation

CarlBeek · 2021-05-04T14:11:46Z

@hwwhww, I've changed quite a bit since you last looked. As far as I can see everything is working perfectly from a translation standpoint. Please can you take one, hopefully final, look 🙏

hwwhww

The description of the folder argument + the translations have to be updated; otherwise, LGTM! 👍 👍

hwwhww · 2021-05-05T11:11:57Z

+def validate_int_range(num: Any, low: int, high: int) -> int:
+    '''
+    Verifies that `num` is an `int` andlow <= num < high
+    '''
+    try:
+        num_int = int(num)  # Try cast to int
+        assert num_int == float(num)  # Check num is not float
+        assert low <= num_int < high  # Check num in range
+        return num_int
+    except (ValueError, AssertionError):
+        raise ValidationError(load_text(['err_not_positive_integer']))


It's unclear why it includes low but excludes high. What do you think about including the given upper bound and adjusting the argument:

Suggested change

def validate_int_range(num: Any, low: int, high: int) -> int:

'''

Verifies that `num` is an `int` andlow <= num < high

'''

try:

num_int = int(num) # Try cast to int

assert num_int == float(num) # Check num is not float

assert low <= num_int < high # Check num in range

return num_int

except (ValueError, AssertionError):

raise ValidationError(load_text(['err_not_positive_integer']))

def validate_int_range(num: Any, lower_bound: int, upper_bound: int) -> int::

'''

Verifies that `num` is an `int` and `lower_bound <= num <= upper_bound`

'''

try:

num_int = int(num) # Try cast to int

assert num_int == float(num) # Check num is not float

assert lower_bound <= num_int <= upper_bound # Check num in range

return num_int

except (ValueError, AssertionError):

raise ValidationError(load_text(['err_not_positive_integer']))

and call validate_int_range(num, 0, 2**32 - 1).

Two reasons:

It follows python's range/indexing convention (eg x in range(0,8), list[0,8])

validate_int_range(num, 0, 2**32) is neater than validate_int_range(num, 0, 2**32 - 1)

hwwhww · 2021-05-05T11:19:00Z

+    '--non_interactive',
+    default=False,
+    is_flag=True,
+    help='Disables interactive prompts.',


Unimportant, just FYI it didn't show when I execute python eth2deposit/deposit.py --help. I guess it's because deposit CLI mixes click.option and custom jit_option?

It is because I purposefully hid this option, I intended only to use it for testing. (See L47 for hidden argument.)

Are you of the opinion that we should be opening this up to users? (It may be useful to those integrating this into other tools)

CarlBeek · 2021-05-05T13:24:10Z

Thanks for your feedback, @hwwhww. I'll merge this now and we can open separate PRs to change validate_int_range() and --non_interactive if we decide this is necessary in the future.

Internationalisation!

CarlBeek added 4 commits February 12, 2021 16:12

Adds abilility to swap out text for internationalisation reasons

1413081

Remove auto function detector

d828bb5

Adds option auto function & file detection

6e3a0a3

adds existing_mnemonic

3924b16

CarlBeek added enhancement New feature or request do not merge labels Feb 12, 2021

wackerow reviewed Feb 17, 2021

View reviewed changes

Comment thread eth2deposit/utils/constants.py Outdated

CarlBeek added 3 commits February 22, 2021 18:11

Move remainder of messages into json files

5fceda2

Fix typo

b8c94a8

Moves intl/utils.py to utils/intl.py

1f9626a

CarlBeek commented Feb 22, 2021

View reviewed changes

Comment thread eth2deposit/cli/new_mnemonic.py Outdated

Add msg_mnemonic_presentation back

4f10175

Use load_text in test_new_mnemonic.py

b7f4188

hwwhww reviewed Feb 25, 2021

View reviewed changes

Comment thread eth2deposit/intl/en/cli/existing_mnemonic.json Outdated

Comment thread eth2deposit/intl/en/cli/generate_keys.json Outdated

Comment thread eth2deposit/intl/en/cli/new_mnemonic.json Outdated

wackerow reviewed Feb 26, 2021

View reviewed changes

wackerow mentioned this pull request Feb 26, 2021

Translations (do-not-merge) #180

Closed

samajammin reviewed Mar 2, 2021

View reviewed changes

CarlBeek and others added 2 commits March 8, 2021 11:30

@hwwhww's content nitpicks

d38323e

Co-authored-by: Hsiao-Wei Wang <[email protected]>

imports 8 languages from crowdin

2b222cd

Arabic, Romanian, Portuguese (Brazilian), Indonesian, Greek, French, Chinese (simplified), Korean

wackerow and others added 5 commits March 22, 2021 11:27

imports 2 languages from Crowdin

61e0c15

Italian, Japanese

Enables intl language selection for existing_mnemonic options

b288277

enables load_text() to have a lang parameter

907bb2e

Convert remaining click.Options to JITOptions

e439443

load_test bug fixes

621ee4f

Extract new eth1 withdrawl params into JSON files

da97e6f

CarlBeek requested a review from hwwhww April 15, 2021 08:28

hwwhww approved these changes Apr 19, 2021

View reviewed changes

Comment thread eth2deposit/cli/generate_keys.py Outdated

Comment thread eth2deposit/utils/intl.py

Comment thread eth2deposit/deposit.py

Comment thread eth2deposit/cli/generate_keys.py Outdated

CarlBeek added 16 commits April 19, 2021 11:46

Move eth1 withdrawal warnings into intl json files

06d7920

Update README with intl language options

e15806b

Move int-range checking from click to custom

b31f814

Move choice checking from click to custom

68bb20d

Intl password confirmations

ff34c4b

Small param typo

06470b0

clarify en password confirmation prompts

902c279

confirmation refactor

b46e702

intl start_index confirmation

bd99461

intl start_index confirmation

126ebe2

Fix tests by adding --non_interactive flag.

64faf7e

* cli-shell-debug-0: intl start_index confirmation

adds confirm to start index for non-english languages

261fdd9

disallow 0 deposits (must be 1 or greater)

0b8d1c4

Mnemonic password warnign via repeated input

7962d4c

More lambdas for laziness!

d07aea6

Inform user why input validation failed

6f388d8

CarlBeek requested a review from hwwhww May 4, 2021 14:11

hwwhww reviewed May 5, 2021

View reviewed changes

CarlBeek added 2 commits May 5, 2021 13:53

Standardise argument headers

12a6615

Fixes/removes incorrect help text about folders

afad2b6

CarlBeek merged commit a678da8 into dev May 5, 2021

CarlBeek mentioned this pull request Jun 24, 2021

Internationalisation release #206

Merged

CarlBeek deleted the translation branch March 16, 2022 11:19

sangheraio pushed a commit to earthwallet/earth-wallet-cli that referenced this pull request Aug 3, 2023

Merge pull request ethereum#182 from ethereum/translation

b7fbfd7

Internationalisation!

valefar-on-discord mentioned this pull request Aug 27, 2024

Partial deposit ethstaker/ethstaker-deposit-cli#93

Closed

Conversation

CarlBeek commented Feb 12, 2021

Uh oh!

samajammin commented Feb 16, 2021

Uh oh!

samajammin commented Feb 16, 2021

Uh oh!

Uh oh!

wackerow commented Feb 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CarlBeek commented Feb 22, 2021

Uh oh!

Uh oh!

wackerow commented Feb 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hwwhww left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wackerow commented Mar 22, 2021

Uh oh!

hwwhww left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CarlBeek commented May 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hwwhww left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CarlBeek commented May 5, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

wackerow commented Feb 17, 2021 •

edited

Loading

wackerow commented Feb 23, 2021 •

edited

Loading

CarlBeek commented May 4, 2021 •

edited

Loading