Implemented the trans and blocktrans tags #1
Conversation
Notes: - Implemented unit tests for both tags which show how they work in normal situations, and how they throw exceptions in the right situations - Added Loader.findResourcesDir() so that callers can find the first resources directory along the same path where the loaders load the templates. This way, the translations and the templates can go together, where-ever they are located. - Modified the environment to call findResourcesDir once. - This code depends on ilib for translation. Debating on whether or not this should be the case. On the one hand, more i18n should happen in the filters, as the support is lacking there. Ilib can help with that. On the other, it would be cleaner if reinhardt could stand alone...
|
This is the first commit. Not ready to pull it into your repo yet. Still need to update the documentation and decide about the ilib issue. (Should reinhardt depend on it or not?) |
|
thanks for putting the code online. my thoughs:
|
| var x = env.getTemplate('page12.html').render(c); | ||
| assert.equal(true, false); | ||
| } catch (e) { | ||
| // success because it is supposed to throw an exception |
There was a problem hiding this comment.
looks like you want assert.throws() here http://ringojs.org/api/master/assert/index.html#throws
- use 3 spaces for indent - use assert.throws for the tests - move trans and blocktrans tag implementations to their own i18n.js file
| var {Environment} = require('../lib/environment'); | ||
| var {Template} = require("../lib/template"); | ||
| var {TemplateSyntaxError} = require('../lib/errors'); | ||
| var ilib = require("../../ilib"); |
There was a problem hiding this comment.
This path works for tests and in ringo on a web server, but only when ilib is checked out right next to reinhardt. In ringo running on jetty, you can use simply require("ilib") and it will find it properly in the packages dir, but that doesn't work for the tests. Is there some way to add to the path where the test framework looks for packages?
There was a problem hiding this comment.
Is there some way to add to the path where the test framework looks for packages?
yeah, there's a command line arg:
ringo --modules /path/to/packages/
or you can install the package globally with rp install --global
If you create a Resources class with a translate method, the code will use that to do translation. Also, you can pass a resBundleFactory method to the environment as you construct it to create new bundles as necessary. Of course, all of it can be implemented using ilib, but it is up to the app to choose which translation solution they prefer as long as they wrap it in the interface mentioned above.
Updated the documentation to talk about the new trans and blocktrans tags, how to use them, and a few pitfalls to avoid.
|
Hi @oberhamsi, I think this PR is ready for review now. Please take a look. Please read the updated doc careful to make sure I got it all correct. I'm going to put a few questions in line comments there for you. If it is okay, then I will work with it for a bit from my own master branch while developing my own site. I'll probably find a few bugs or upgrades here and there which I can fix. Once I'm comfortable that it is all stable, we can merge my master branch back to the main repo again. |
tab -> 4 spaces, trailing whitespace eliminated.
| the built-in filters. You might look for Ringo packages on RP that implement | ||
| locale-sensitive filter plugins or implement them yourself. | ||
|
|
||
| Just so that you are aware when you use them, the following filters perform |
There was a problem hiding this comment.
Do we want to list these i18n problems here? This is part of the big list of i18n problems I talked about previously.
There was a problem hiding this comment.
Yes, I think we could even call it more explicitly: "Problems with i18n in Reinhard" or so :) Not sure the detailed list of problematic filters makes sense. I created orfon#17 and orfon#16
And IMO this is still so much info that it needs its own i18n.md. Templates.md should just give a short overview - maybe the first paragraph - and then link to i18n.md. Like the other sections above it.
|
You could list |
Changes based on review comments from Simon. * Moved the resource bundle factory call into the trans and blocktrans nodes * Moved the globalization docs into i18n.md and linked it to the other docs * Updated tags.md to add notes about trans and blocktrans
|
@oberhamsi final review? |
|
👍 looks great. seems obvious to say it: but i especially like the docs ;) please open PR on orfon/reinhardt if you want the github karma points - or i can merge from here. |
Notes: