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. http://codereview.appspot.com/124104 --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "JSON Template" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/json-template?hl=en -~----------~----~----~----~------~----~------~--~---
