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.

Sign up for GitHub

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

Jump to bottom

Support other languages #4

Merged
merged 4 commits into from
Mar 15, 2022
Merged

Support other languages #4

merged 4 commits into from
Mar 15, 2022

Conversation

jon4hz
Copy link
Contributor

@jon4hz jon4hz commented Mar 14, 2022 •
edited
Loading

Hi, I absolutely love the idea of backup up ssh keys with mnemonic phrases!
This PR should add support for all languages supported by tyler-smith/go-bip39.
The language can be specified with the flag -l or --language. The default is English.

The following languages will be supported:

  • Chinese simplified
  • Chinese traditional
  • Czech
  • English
  • French
  • Italian
  • Japanese
  • Korean
  • Spanish

caarlos0
caarlos0 approved these changes Mar 14, 2022
View reviewed changes
Copy link
Member

@caarlos0 caarlos0 left a comment

Choose a reason for hiding this comment

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

lgtm, couple of pointers only

cmd/melt/main_test.go Outdated Show resolved Hide resolved
cmd/melt/main.go Outdated Show resolved Hide resolved
meowgorithm
meowgorithm reviewed Mar 14, 2022
View reviewed changes
cmd/melt/main.go Outdated Show resolved Hide resolved
meowgorithm
meowgorithm reviewed Mar 14, 2022
View reviewed changes
cmd/melt/main.go Outdated Show resolved Hide resolved
@meowgorithm
Copy link
Member

This is so awesome

example

@meowgorithm
Copy link
Member

Here’s another question. Is German not supported because the seed phrases could potentially be too short and thus too awesome, à la «Zeichenorientierte Benutzerschnittstelle»?

@jon4hz
Copy link
Contributor Author

jon4hz commented Mar 15, 2022

Here’s another question. Is German not supported because the seed phrases could potentially be too short and thus too awesome, à la «Zeichenorientierte Benutzerschnittstelle»?

There is no wordlist for German in the official bip39 wordlists but I'm not sure why. https://github.com/bitcoin/bips/blob/master/bip-0039/bip-0039-wordlists.md

Theoretically support for Portuguese could be added. There's a PR open for quite a while now: tyler-smith/go-bip39#48

@jon4hz
Copy link
Contributor Author

jon4hz commented Mar 15, 2022 •
edited
Loading

Technically any language could be supported though as long as the same wordlist is used for encryption and decryption. The words itselfs aren't that important. The important part is the possition where they are in the list. But I think it would make more sense if you keep this bip39 compatible since this is basically the standard

@jon4hz jon4hz requested a review from meowgorithm March 15, 2022 08:06
@caarlos0
Copy link
Member

amazing work y'all! lets merge this!

@caarlos0 caarlos0 merged commit 7937d4d into charmbracelet:main Mar 15, 2022
aymanbagabas
aymanbagabas reviewed Mar 16, 2022
View reviewed changes
cmd/melt/main.go
Comment on lines 261 to 264
switch strings.ToLower(language) {
case "chinese-simplified":
case "chinese-simplified", "zh", "zh_HANS":
bip39.SetWordList(wordlists.ChineseSimplified)
case "chinese-traditional":
case "chinese-traditional", "zh_HANT":
Copy link
Member

Choose a reason for hiding this comment

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

hmm, I think zh_HANS and zh_HANT wouldn't match here

aymanbagabas added a commit that referenced this pull request Mar 16, 2022
@aymanbagabas
fix: zh_hans & zh_hant matching
dd1bfde 
Fixes: 7937d4d ("feat: support other languages (#4)")
@aymanbagabas aymanbagabas mentioned this pull request Mar 16, 2022
Merged
caarlos0 pushed a commit that referenced this pull request Mar 16, 2022
@aymanbagabas
fix: zh_hans & zh_hant matching (#8)
15ddec6 
Fixes: 7937d4d ("feat: support other languages (#4)")
@caarlos0 caarlos0 mentioned this pull request Mar 16, 2022
Merged
bashbunni pushed a commit that referenced this pull request Mar 16, 2022
@aymanbagabas @bashbunni
fix: zh_hans & zh_hant matching (#8)
e790749 
Fixes: 7937d4d ("feat: support other languages (#4)")
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aymanbagabas aymanbagabas aymanbagabas left review comments

@caarlos0 caarlos0 caarlos0 approved these changes

@meowgorithm meowgorithm meowgorithm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jon4hz @meowgorithm @caarlos0 @aymanbagabas