Cropster / ember-l10n

A GNU gettext based localization workflow for Ember
MIT License
15 stars 7 forks source link

Placeholders are translated implicitly #15

Closed LevelbossMike closed 7 years ago

LevelbossMike commented 7 years ago

Hi! Thanks for this great addon. In a project I'm working on we have noticed strange translation behaviour.

When a value passed to a placeholder is included in the translation map the passed value will implicitly be translated. This is pretty counterintuitive and unnecessary as this could also be achieved by using a subexpression in the template layer or using l10n.t explicitly from javascript.

Example:

// with a translation map that maps "world" to "welt" 
{{t "Hello {{name}} name="world"}}
// => "Hello welt"

To work around this we are currently overriding _strfmt and remove this line

I'm not sure when this behaviour would be desired but this should easily be possible to do via a subexpression:

{{t "Hello {{name}} name=(t "world")}}

I'd prefer not overriding _strfmt in the consuming app so maybe this behaviour could be made configurable. Is there a reason why this is included?

Thanks again for this great addon. Happy to submit a PR that changes this behaviour.

arm1n commented 7 years ago

Hey there, thanks for reporting this issue. As there's unfortunately no comment in code and I also cannot exactly remember what was the idea behind these lines of code, we should be able to safely drop it. As you correctly outlined it's easily possible to use a subexpression in the hosting app whenever it's desirable. I will also take the commits from the forks of @HarryLafranc and @tomnez into account and release a new version. The fork of @tomnez targets exactly this issue.