JS and PHP pass all 47 tests. In JS: - formatters always with val,context,args - ...
16 years, 6 months ago
(2009-10-03 02:12:20 UTC)
#1
JS and PHP pass all 47 tests.
In JS:
- formatters always with val,context,args
- predicates always with val,context,args
- predicates as .section but using .if notation
and they have their own target, for example:
{.if color is "blue"}
- more_formatters can optionally be an array
instead of a function (this was already in PHP)
In PHP:
- Even though PHP, like JS, can have extra params,
I tried a different approach as a test for
other languages (and maybe making JS do it)
- more_formatters could always be an array
- more_formatters3 is an array of formatters that
get three arguments, where the old array
has formatters that only get one
- predicates using .if syntax
- more_predicates and more_predicates3
- more_predicates3 and more_formatters3 get
context and args in addition to value
Tests
- activated some tests for JS and PHP
- need to add some for predicates and things that
use arguments.
http://codereview.appspot.com/124104/diff/1/2
File javascript/json-template.js (right):
http://codereview.appspot.com/124104/diff/1/2#newcode48
Line 48: (META_ESCAPE[meta_right]||meta_right) +
TODO: more safely
http://codereview.appspot.com/124104/diff/1/2#newcode126
Line 126: stacker:stack,
Oops.. not needed, was there for firebug at one point...
http://codereview.appspot.com/124104/diff/1/2#newcode130
Line 130: new_context= {};
Arg, this should be null instead -- minor thing though.
http://codereview.appspot.com/124104/diff/1/2#newcode183
Line 183: return frame.index;
$index starting from 1, not zero. Can imagine if the W3C suggested that for
<ol><li> items...
OK, great that you got this all working. I'm going to be more picky about ...
16 years, 6 months ago
(2009-10-03 05:16:02 UTC)
#2
OK, great that you got this all working.
I'm going to be more picky about the JS code -- you can have more leeway on the
PHP code, as long as it passes the tests. Although we should strive to make the
APIs as consistent as possible, allowing for language differences.
I still have to figure out exactly what I want in predicates. I don't think
these tests can pass, because you have .if in the code, but no .if in the tests.
I will work on the details of predicates... if there is soemthing you want to
see other than what we've discussed let me know.
http://codereview.appspot.com/124104/diff/1/2
File javascript/json-template.js (right):
http://codereview.appspot.com/124104/diff/1/2#newcode48
Line 48: (META_ESCAPE[meta_right]||meta_right) +
Please hold off on this until it does what we discussed -- also a unit test
would be nice. It's possible to have JS-specific unit tests in browser_test.py
although it's set up a little awkwardly.
http://codereview.appspot.com/124104/diff/1/2#newcode83
Line 83: return context.Lookup('base-url')+'/'+val;
I would like if this behaves like Python and is smart WRT leading/trailing
slashes, but this could bea TODO
http://codereview.appspot.com/124104/diff/1/2#newcode97
Line 97: function JsonTemplateException(message) {
I'd rather not have this here yet, let's talk about it separately from the other
issues.
http://codereview.appspot.com/124104/diff/1/2#newcode126
Line 126: stacker:stack,
On 2009/10/03 02:12:20, sroussey wrote:
> Oops.. not needed, was there for firebug at one point...
So you should upload another snapshot with this removed rather than replying to
yourself :)
http://codereview.appspot.com/124104/diff/1/2#newcode183
Line 183: return frame.index;
On 2009/10/03 02:12:20, sroussey wrote:
> $index starting from 1, not zero. Can imagine if the W3C suggested that for
> <ol><li> items...
OK. But you must have broken Python too then.
http://codereview.appspot.com/124104/diff/1/2#newcode228
Line 228: if (matches && (predicate = DEFAULT_PREDICATES[matches[1]])) {
I will have to think about this stuff... I suggested a more general API for
registering formatters/predicates in one of the bugs. This doesn't match what's
in Python now.
http://codereview.appspot.com/124104/diff/1/2#newcode246
Line 246: if (predicate_target==null || predicate_target==undefined)
Please keep the style consistent and use spaces around ==, here and throughout.
http://codereview.appspot.com/124104/diff/1/2#newcode253
Line 253: predicate_target: predicate_target, // public attribute
I think that sections blocks have names and no predicates, while "if" blocks
have predicates but no names.
If that's the case then we should create a new "class" -- in Python I created
_PredicateSection and _Section, which derive from _AbstractSection.
http://codereview.appspot.com/124104/diff/1/2#newcode372
Line 372: var _SECTION_RE =
/^(?:(repeated)\s+)?section\s+(@|[A-Za-z0-9$_-]+)\s*$/;
I don't see why these changed.
http://codereview.appspot.com/124104/diff/1/2#newcode374
Line 374: var _CALL_RE = /^([A-Za-z0-9$_-]+)(?:\b\s*(.*?))?\s*$/;
As discussed I don't want to parse any arguments directly in the template
language. This can be left for user code. Perhaps I can come up with some more
examples in Python.
http://codereview.appspot.com/124104/diff/1/3
File jsontemplate_test.py (right):
http://codereview.appspot.com/124104/diff/1/3#newcode856
Line 856: class PredicatesTest(testy.Test):
Change description says you have .if, but I don't see any .if here.
Issue 124104: Update to JS and PHP for args, predicates (with .if), context in formatter, predicates...
Created 16 years, 6 months ago by sroussey
Modified 10 years, 4 months ago
Reviewers: andychu
Base URL:
Comments: 15