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

Adding 'localizes' to allow some string translations for Snap5 #2341

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jguille2
Copy link
Contributor

Hi!
Adding some "localize"s to allow their string translations.

Copy link
Owner

@jmoenig jmoenig left a comment

Choose a reason for hiding this comment

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

the English translations dictionary has a section for handling long strings. The idea is that we don't directly put long strings into the source code but short ones that we in turn look up in the respective dictionaries. I seem to have not abided by this mechanism myself.

src/gui.js Outdated
'Verification Email..." option in the cloud\n' +
'menu.\n\n' +
'You have ' + response.days_left + ' days left.',
localize(' days left'),
Copy link
Owner

Choose a reason for hiding this comment

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

hmm... for long strings such as the ones in this PR it might make sense to leave it as a single large one, instead of breaking it up into several substrings. I can imagine some languages have a different sentence structure...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jmoenig
I think long strings can be broken into smaller ones. It is good to avoid long strings, both in code and in translations (translations are also code).

But yes, for me the mistake here is to break the message in lines. We can break long messages in sentences. That will be good to have shorter lines and shorter strings... and also good to take the advantage of the work done: some sentences will be repeated more than once, and they will need only one translation (sentences like "Are you sure?").

I added the "localizes" and I didn't want to change the origin code. But if you want, I will update the PR cutting the messages by sentences.

@@ -237,8 +237,8 @@ PaintEditorMorph.prototype.buildEdits = function () {
function () {
if (myself.paper.undoBuffer.length > 0) {
myself.ide.confirm(
'This will erase your current drawing.\n' +
'Are you sure you want to continue?',
localize('This will erase your current drawing.\n') +
Copy link
Owner

Choose a reason for hiding this comment

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

hmmm.... I don't think these strings need to be explicitly localized at all here. When I look at the code for the IDE_Morph >> confirm method it already has a call to localize in it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jmoenig
Yes, but the issue here is the same as the previous comment. I know ide.confirm has already a call to localize, but here we have again message made up with different strings. If we want that the translation dictionary was the same that the code dictionary (the same strings), I have to call a "localize" for each shorter string.

@jguille2
Copy link
Contributor Author

jguille2 commented Mar 8, 2019

Hi @jmoenig,

Is this right for you?

Now, those long strings are still splitted (like the original code was), by splitted in sentences (not by lines).

Other possibility is to split a bit more, using phrases (maybe one sentence has more than one phrase... I'm not sure if these words are in English the best... but I think you know my idea). Then, the strings will be shorter, and I think the translation will not be a problem... but we have those "\n" inside that are strange (Yes translators can move them between strings... but then, the idea of using those strings in different contexts, is not clear).

Ok. If you agree, I will update our (Catalan) language pack with these strings, and also with the newest changes (microphone blocks...)

Thanks

Joan

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

Successfully merging this pull request may close these issues.

3 participants