I18n #47
I18n #47
Conversation
Not tested yet. Still need to make sure it works before we attempt to merge it anywhere.
If you give it a regular expression, there were various exceptions which are now avoided.
Previously, if you passed a regular expression for the path, it would give an exception.
This is not tested yet. Needs a little more work.
Most of the tests for the locale middleware are written and run correctly without errors, but they do not all pass yet.
More tests pass now, but not all, and the code needs to be cleaned a bit before this can become a PR.
* All unit tests pass, including the existing ones. * Added build.properties in case ringo is not in your path and you want to define where it can be found * Inline documentation is included in the lib/middleware/locale.js file
Debug stuff out!
Converted all tabs to 4 spaces and removed spaces at the end of lines.
These are not related to the locale middleware, so they should not be part of this PR.
Fixed whitespace issues, converting tabs to spaces, and removed debug stuff.
Now catches exceptions and moves on without a locale.
In the session data, if you set an attribute to null or undefined, and then read that variable again, it would return the string "null" or "undefined" instead of the values null or undefined. This fixes part of the problem. If you set the value of an attribute to null or undefined, then it will unset the attribute as per the HttpServlet 3.1 javadocs instead of setting it to a string. This is still not quite working correctly, because if you unset a variable and then test its value, it always returns null instead of undefined.
In the session data, if you set an attribute to null or undefined, and then read that variable again, it would return the string "null" or "undefined" instead of the values null or undefined. This fixes part of the problem. If you set the value of an attribute to null or undefined, then it will unset the attribute as per the HttpServlet 3.1 javadocs instead of setting it to a string. This is still not quite working correctly, because if you unset a variable and then test its value, it always returns null instead of undefined.
|
@oberhamsi and @hns I think this PR is ready to be merged. I have been testing with it for a few weeks now while developing my own site, and it seems to be working fine for me. |
|
@ehoogerbeets sorry about the delay - we've all been very busy the last weeks. I trust your tests but someone of the ringo core team will still have to look through it before merge. |
| @@ -79,7 +79,14 @@ var ServletSession = exports.ServletSession = function(request) { | |||
| // to the attributes in the servlet session object. | |||
| data = new JavaAdapter(org.mozilla.javascript.NativeObject, { | |||
| put: function(name, start, value) { | |||
| getSession().setAttribute(name, value); | |||
| if (typeof(value) === 'undefined' || value === null) { | |||
There was a problem hiding this comment.
This conforms to HttpSession's behavior:
If the value passed in is null, this has the same effect as calling removeAttribute().
There was a problem hiding this comment.
+1 drop it. same with the Object.prototype stuff below (which was already there)
|
I really like the middleware, but we should avoid manipulating the |
Shouldn't be messing around with the Object prototype anyways!
Why does git make me do this? I hate git.
|
Thanks for looking at that Philipp! The code is removed now, and the stick tests still all pass as do the ones for my own web site. So it's ready to have another look. |
|
@grob / @oberhamsi do you see any consequences from the addition of |
| spec.pattern = path instanceof RegExp ? path : normalizePath(path, keys); | ||
| spec.weight = calcWeight(path); | ||
| if (path instanceof RegExp) { | ||
| spec.pattern = path; |
There was a problem hiding this comment.
I think there is a tab instead of spaces. Minor thing, but cosmetics & consistency.
The regexp support will have to be rethought before they can be used.
| ]; | ||
|
|
||
| var Locale = function Locale(spec) { | ||
| var bits = spec.split(/[-_]/g); |
There was a problem hiding this comment.
Later this class should be moved out to the utilities where it can be used more generally...
|
Okay, this PR should contain locale-only changes now. |
|
A very clear merge it from my side 👍 |
|
yey! thanks for your i18n efforts, @ehoogerbeets looking forward to using it myself :) |
|
No probs... more to come on the reinhardt side of course. |
This middleware gleans the locale based on various methods and puts it into the session.