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

[RFC] Translations and string parameters #131

Closed
melon3r opened this issue Aug 24, 2014 · 15 comments
Closed

[RFC] Translations and string parameters #131

melon3r opened this issue Aug 24, 2014 · 15 comments

Comments

@melon3r
Copy link
Member

melon3r commented Aug 24, 2014

Right now tomb translations DON'T work as I intended when I pushed #123. Back then I didn't realize that parameter expansion would mess with gettext, and thus, I took for granted that it would work on every string... I was wrong.

To make translations work as intended I propose the following: take $variables out of the strings and use ::placeholders instead, which can be substituted after going through gettext. For example:

# Before
_failure "No tomb with name $tombname found!"

#After
_failure "No tomb with name ::1 found!" $tombname

That means that translators would face ::n placeholders, instead of variables with sensible names. To help them we could add comments (the name of the variable, or even more sensible sentences) to the placeholders:

_failure "No tomb with name ::1(the name of a tomb) found!" $tombname

Let me know what you think please. I want everybody's thumbs-up before I proceed, because although the zsh code is pretty simple, there's some work to be done on the .pot and .po files, plus in all the tomb strings, to adapt to this new model.

Suggestions for other symbols or formats for the placeholders and comments are welcome, this is not definitive. For example, I like:

_failure "Hey ::1 the name of the user::, do you have the key to ::2 the name of the tomb::?" $username $tombname
@hellekin
Copy link
Contributor

I don't understand the problem related to using the actual variable name. That seems to be adding complication (not complexity) for the translators. What exactly is the problem you want to solve?

In the linked patch, on line 416, I would use: local msg="$(gettext -s '$2')" so that variable names in the string passed to gettext are not interpreted by the shell.

Are you saying that gettext would turn $tomb into $tombe for French?

@melon3r
Copy link
Member Author

melon3r commented Aug 24, 2014

If you pass '$2' it will try to find the translation of literally $2. But even if that wasn't the case, the variables get expanded before getting to gettext.

I think I understand you though. Do you mean that you don't understand why use the placeholders? Well, we could use the variables themselves as placeholders (using single-quotes ' everywhere), but we'd have to pass the variables separately anyway, and using something different to the $ sign would ensure we are not expanding the variables inadvertently and also denote that visually.

It'd be enough to use +tombname instead of $tombname, but to be honest we don't even need names on the placeholders, as they'd be positional, meaning that the first variable would go in the first place, then the second, etc. But it's always pleasing (for me) to see separate words when translating, as in ::tomb size:: vs $tombsize, as I can read the whole sentence more naturally.

Edit: Added flow chart
tomb_output

2nd Edit:
Ooops, I didn't read your last question! $tomb wouldn't reach gettext, but secret.tomb would. And if we used single quotes ', although we'd translate the right thing, we'd have no access to the variables containing the values in the msg() function.

@hellekin
Copy link
Contributor

Hmmm, correct me if I'm wrong. Considering the string Now, the tomb $tombname is open.

Passing msg="$(gettext -s '$2')" would expand to: msg="$(gettext -s 'Now, the tomb $tombname is open.')", which would return $msg # => "Ahora la tumba $tombname esta abierta."

Or should we escape $tombname into \$tombname?

P.S.: depending on the language, the position of multiple variables is likely to change.

@hellekin
Copy link
Contributor

I guess I'm wrong. msg="$(gettext -s "'"${2}"'")" should be correct though.

@melon3r
Copy link
Member Author

melon3r commented Aug 24, 2014

Yeah, I think the last one is correct. The problem is that we don't have the value for $tombname after gettext, so we can't expand it.

@melon3r
Copy link
Member Author

melon3r commented Aug 24, 2014

Oh, wait. It depends on how you're storing the first string. It could be:
'Now, the tomb $tombname is open.'
or
"Now, the tomb $tombname is open."

In the second case, $tombname would be expanded before getting to the gettext line, so we'd have msg="$(gettext -s 'Now, the tomb secret.tomb is open.')"

Oh, that's the same as escaping the $ with \.

I understand what you mean by changing the position of the variables. In that case this still holds:

_failure "Now the tomb ::1 tomb name:: is open at ::2 mount location::."

We can have placeholders named ::1, ::2, ::3, etc. and comments for the translators to know what each one is. Then we can substitute by number, instead of by position as I said previously. It's still easy to do.

@hellekin
Copy link
Contributor

But we want to expand it after gettext translation, so that it's expanded as "$msg" (the translated string)

@melon3r
Copy link
Member Author

melon3r commented Aug 24, 2014

That's what I want to do! 😉

This is how it goes now:

_success "Now, the tomb $tombname is open."
# expands to:
_success "Now, the tomb secret.tomb is open."
# ...
msg="$(gettext -s "Now, the tomb secret.tomb is open.")"
# expands to:
msg="Now, the tomb secret.tomb is open."

This is what you are suggesting (as far as I understand):

_success 'Now, the tomb $tombname is open.' # doesn't expand
# ...
msg="$(gettext -s "'"Now, the tomb $tombname is open."'")"
# expands to:
msg="$(gettext -s 'Now, the tomb  is open.')" # note the double space -- $tombname has no value here
# which expands to:
msg="Now, the tomb  is open."

This is what I suggest:

_success "Now, the ::1 tomb name:: is open." $tombname # doesn't expand
# ...
msg_tmp="$(gettext -s "Now, the ::1 tomb name:: is open.")"
# which expands to:
msg_tmp="Ahora la tumba ::1 tomb name:: está abierta."
# ...
msg="$(ninja_zsh_substitution "Ahora la tumba ::1 tomb name:: está abierta.")"
# which expands to:
msg="Ahora la tumba secret.tomb está abierta."

@jaromil
Copy link
Member

jaromil commented Aug 24, 2014

Interesting analysis. I remember being puzzled by these issues in the past and giving up.

I like gdrooid solution and I am in favour of it.

Only question is why delimiters of 2 chars? :: might be confusing, why not %varname% ?

@hellekin
Copy link
Contributor

What about escaping $ with a backslash? . gettext.sh (cf. /usr/bin/gettext.sh) uses either echo or printf and escapes dollar signs with \ from what I understood.

@melon3r
Copy link
Member Author

melon3r commented Aug 24, 2014

@jaromil I used two chars to differentiate it from the normal use of the char in a string. One might want to use : in a string (we already do), and the same would happen with %. Also, : is easier to type in my keyboard, so I used it for the examples, but we can use anything.

Also note that %varname% by itself wouldn't be possible. We need something like %1%, with a possible comment inside to clarify what's in there: %1 tomb name%. We need to have fixed names for the variables because we are only passing their values, and not their names, to the _msg() function. Could avoid fixed names if we used positional parameters, but as @hellekin pointed out, some languages might need to change the order of the parameters:

# English: A %1 cold% %2 beer%.
# Spanish: Una %2 cerveza% %1 fría%.

@hellekin With backlash we'd get the same effect as with the single quotes, except that we'd have to force it to expand, but again, there'd be no value for $tombname there.

@jaromil
Copy link
Member

jaromil commented Aug 24, 2014

OK. I understood why backslash is not enough. Tricky issue.

I want to go for your solution, but that will be a lot of work since you will have to change also current translations. If you like to make it simplier, I would also go without the "possible comment inside ::" but just a number, since I think the strings are rather clear without the variable name already. But then if you manage to put it in there, is more work, but is nicer.

Either way, when you can do this pull request I will merge it, so please go ahead. I'd rather appoint you in charge for the internationalization of Tomb, since you have shown very good understanding of the problems related.

@melon3r
Copy link
Member Author

melon3r commented Aug 24, 2014

👍 I'll probably make a PR on Wednesday. Tomorrow is my last day in the US and then I have to make a long trip back to Europe, so I'll probably start coding while in flight.

I'll gladly take that charge, but then I'd like to talk on IRC or XMPP (gdrooid@dukgo.com) first if we can agree on a date/time (after I come back to Europe).

@melon3r
Copy link
Member Author

melon3r commented Sep 16, 2014

This is done ^__^

ping @x3nu #132 was merged, there's a lot to translate! :D

@melon3r melon3r closed this as completed Sep 16, 2014
@x3nu
Copy link
Contributor

x3nu commented Sep 16, 2014

on it :D

On 16/09/14 19:13, Dani wrote:

This is done ^__^

ping @x3nu https://github.com/x3nu #132
#132 was merged, there's a lot to
translate! :D


Reply to this email directly or view it on GitHub
#131 (comment).

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

No branches or pull requests

4 participants