bitjson / s18n

Semantic localization for html.
MIT License
23 stars 10 forks source link

Using original strings instead of Native locale translation #35

Closed dhbahr closed 7 years ago

dhbahr commented 7 years ago

Hi.

I'm trying to use this module on a project but I've come across 1 deal breaker for me.

In https://github.com/bitjson/s18n/blob/master/lib/localize.js#L46 you use the translated value for the native locale instead of the original string

I would propose to change nativeLocale[i].string for nativeLocale[i].hash this way we can use coded strings on the templates ;)

Best regards

bitjson commented 7 years ago

Hey @dhbahr I'd love to review a pull request! Ideally, we should avoid a breaking change, but we can definitely add functionality with additional settings.

dhbahr commented 7 years ago

@bitjson to what branch should I make the PR ? And how long would it take to make it to the NPM package after approval?

bitjson commented 7 years ago

Hey @dhbahr – PRs should go to the master branch – #36 looks good. The issue right now is that #36 breaks quite a few tests, and doesn't seem to work.

dhbahr commented 7 years ago

I saw the test report, but I have no idea what could cause them, considering the contents of the PR, which led me to believe maybe i should have made it to a different branch. Can you point me in the direction to fix it? Do you have access to Travis output or would that be helpful? I really have no idea what else I would have to provide.

bitjson commented 7 years ago

Sure – you can see full details from Travis by clicking the "Details" link at the bottom of the PR. Here's the failing build: https://travis-ci.org/bitjson/s18n/builds/221775529

You can also clone the repo and run the tests yourself via npm test.

dhbahr commented 7 years ago

Ok, now I see the issue, and my change will never make it through the tests. The thing is your locales use a seemingly random hash as key thus you need to look for the native locale values, whilst the behavior I'm looking for relies on the key to actually be the original string Thanks for your time, I think you can close this one as I don't see our interests aligning

bitjson commented 7 years ago

Thanks for the follow up – yes, this module is somewhat different from a traditional i18n library – please see the explanation in the readme.

It seems like you're looking for a more traditional, manual system like https://github.com/mashpie/i18n-node or https://github.com/jeresig/i18n-node-2

dhbahr commented 7 years ago

Well yeah, the thing is I'm actually using gulp-l10n which relies on your s18n .. i just made a local copy on my tasks .. not super clean, but then again it works for me.

Thanks for everything

bitjson commented 7 years ago

Just curious, is there are reason you're not using the locale extraction system?

If you're doing everything manually anyways, you may be better off with a different gulp plugin, like https://github.com/filaraujo/gulp-i18n-localize

dhbahr commented 7 years ago

I'm using 'gulp-jade-l10n-extractor' (https://github.com/vhpoet/gulp-jade-l10n-extractor) extracting directly from the jade templates and creating a messages.pot I can push to a PhraseApp project. But even if I would use gulp-l10n's I'm not quite sure it would solve my issue since the idea is to be able to translate even the native locale, so that I can use coded strings in the templates if you know what I mean.

Bottom line my exact requirement was to be able to replace original strings as opposed to native locale translation