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

[feature] Easier sprintf usage #84

Closed
binarykitchen opened this issue Apr 22, 2013 · 6 comments
Closed

[feature] Easier sprintf usage #84

binarykitchen opened this issue Apr 22, 2013 · 6 comments

Comments

@binarykitchen
Copy link

hello again

i think sprintf is often used and calls to the process should be much easier.

at the moment we call it like that in jade templates, example:

#{t('Video will expire on %s, this means %s', {postProcess: "sprintf", sprintf: [expirationDate, expirationHours]})}

jade templates become ugly and prone to errors if you leave out one } or forgot the proper syntax. i think "usability for programmers" can be improved here. how about:

#{t('Video will expire on %s, this means %s', expirationDate, expirationHours)}

much easier, cleaner and easier to use. what do you reckon?

@jamuhl
Copy link
Member

jamuhl commented Apr 22, 2013

sprintf is not the default for interpolation in i18next. Might check if first or second param is an object holding options, but still i wouldn't choose to do one function doing different stuff depending on call structure in this case.

But you should be able to register a own helper for jade doing this with ease.

@jamuhl jamuhl closed this as completed Apr 22, 2013
@binarykitchen
Copy link
Author

yes i could do my own helper but i think this is something other people probably want too.

most node.js translation modules based on gettext support such a sprintf syntax with unlimited variables. i think that would be cool for your module.

do you think i can try extending your translate() method in file i18next-1.6.1pre by parsing arguments and do a pull request? would you be interested?

@jamuhl jamuhl reopened this Apr 22, 2013
@jamuhl
Copy link
Member

jamuhl commented Apr 22, 2013

sure feel free to add a PR. If nothing brakes chances are big that it get's merged.

@jamuhl
Copy link
Member

jamuhl commented Apr 22, 2013

just make sure the changes are done in the repository for the client https://github.com/jamuhl/i18next as the source from there is only copied and wrapped in the node-module.

@binarykitchen
Copy link
Author

cool, will do in the next days.

@binarykitchen
Copy link
Author

my PR works fine, just tested it on a live website. closing this ticket now.

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

2 participants