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

Vietnamese Translation #2931

Merged
merged 3 commits into from
Mar 30, 2020
Merged

Vietnamese Translation #2931

merged 3 commits into from
Mar 30, 2020

Conversation

qunm00
Copy link
Contributor

@qunm00 qunm00 commented Mar 29, 2020

No description provided.

Comment on lines 47 to 64
#: /Users/tessus/data/work/joplin/Tools/../CliClient/app/app.js:136
#: /Users/tessus/data/work/joplin/Tools/../CliClient/app/cli-utils.js:160
msgid "Y"
msgstr "Có"

#: /Users/tessus/data/work/joplin/Tools/../CliClient/app/app.js:136
#: /Users/tessus/data/work/joplin/Tools/../CliClient/app/cli-utils.js:160
msgid "n"
msgstr "không"

#: /Users/tessus/data/work/joplin/Tools/../CliClient/app/app.js:136
msgid "N"
msgstr "Không"

#: /Users/tessus/data/work/joplin/Tools/../CliClient/app/app.js:136
msgid "y"
msgstr "có"

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please do not translate these, otherwise the terminal app won't work, since it expects the English letters as answer.

(I'm not sure why those are in the translation file. I will have to talk to Laurent about this.)

Copy link
Owner

Choose a reason for hiding this comment

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

The answers are translated too so for example in French you can type "O" for "oui" or "N" for "non". But I believe it will only work if it's a one-letter answer so I guess it should have been "c" and "k" in this case.

Copy link
Collaborator

Choose a reason for hiding this comment

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

In that case we should add a translation description in the .pot file. Or use a sub string of the translation.

Copy link
Owner

Choose a reason for hiding this comment

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

We can't use a sub-string of the translation because we don't know how yes/no is abbreviated in each language. As for a comment, maybe, but I don't know how since all this is generated dynamically.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We can't use a sub-string of the translation because we don't know how yes/no is abbreviated in each language.

I meant just the first letter. So even, if someone translates Y as XYZ, X would be accepted by the cli app. Currently it would only work, if someone actually used a translation that said X. The same is true for the output of (y/n) or (Y/N).

@tessus
Copy link
Collaborator

tessus commented Mar 29, 2020

Awesome job. Thank you. There's one small issue, which I have mentioned as an inline comment.

@tessus tessus added the translation i18n and l10n label Mar 29, 2020
@tessus
Copy link
Collaborator

tessus commented Mar 30, 2020

You should just leave the message strings empty, not remove the entries completely.

@qunm00
Copy link
Contributor Author

qunm00 commented Mar 30, 2020

You should just leave the message strings empty, not remove the entries completely.

Sure, I'll make the changes.

@tessus
Copy link
Collaborator

tessus commented Mar 30, 2020

Great, thanks for the translation.

@tessus tessus merged commit 336cadb into laurent22:master Mar 30, 2020
@laurent22
Copy link
Owner

@tessus, by the way for translations please could you also use the format "All: ...."? The "All" keyword means it will get picked up by the changelog script.

@tessus
Copy link
Collaborator

tessus commented Mar 30, 2020

Yes, will do from now on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
translation i18n and l10n
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants