Skip to content
This repository has been archived by the owner on Mar 13, 2018. It is now read-only.

fix one time mustache parsing #201

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jakemac53
Copy link
Contributor

This fixes one time bindings parsing when using list or map accessors so a trailing space is no longer required:

[[foo['bar']]] now works where you used to have to do [[foo['bar'] ]]

@justinfagnani
Copy link
Contributor

I commented on the Dart version of this CL:

I wonder if we should track balancing of the terminators. A case like [[ foo[bar[0]] ]] looks like it'll terminate early. Same with {{ foo({'x': {'y': 0}}) }}. The requirement of binging delegate would be that 1) {} and [] must
be balanced, and probably also 2) Non-significant terminators chars (like in
strings) are escaped with \

@justinfagnani
Copy link
Contributor

@rafaelw

@jmesserly
Copy link
Contributor

Hmmm. Personally not really a fan of trying to track "balanced" terminators, since we know nothing about the grammar of the expression. The workaround seems fine to me -- just add a space. Not sure this is worth the code size tradeoff. (edit: where "this" means "tracking balanced terminators", not this pull req, which I'm ambivalent about.)

@justinfagnani
Copy link
Contributor

Without tracking balanced tokens I think you have to give up on this PR, since it'll terminate valid expressions early.

@jakemac53
Copy link
Contributor Author

The previous logic also was terminating valid expressions early, this just handles a common edge case that the previous logic didn't cover with very little extra overhead.

In terms of balancing the terminators I have an implementation on the dart side now that assumes escaping of terminators inside of strings, and otherwise allows you to escape terminators outside of strings (forcing escaping inside of strings seems extreme, and I think its a fair assumption that quoted things are strings). That adds around 50 relatively short lines right now. We could remove the escaping terminators logic and assume a sane language with balanced brackets to get rid of around 15-20 lines of that code.

If we do go with the balancing terminators route, its definitely a breaking change as well so that should be considered.

For reference the current dart implementation is here: https://codereview.chromium.org/712333006/patch/20001/30002

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants