implement full path parsing, including element accessors & literals

implement full path parsing, including element accessors & literals

R=arv
BUG=

Committed: https://github.com/Polymer/observe-js/commit/d99eff1

[rafaelw, 2014-05-23 04:25:43]

Arv,

Here's the beginnings of this patch. It's mostly done, but I'm not ready for a
full review yet -- I need to add more tests and modify
NodeBind/TemplateBinding/PolymerExpressions to follow.

Are you Ok with this approach? I was trying to strike a balance between perf,
codesize and clarity.

[arv, 2014-05-23 17:13:30]

I'm not sure a state machine is the simplest and most efficient solution. I
agree that your code is pretty easy to follow though.

Another thing that might be good to do is to not append at every step but use
something like eatWhile(isIdentChar) and then you have the start and end index
of your ident, so you only need to create the final string.

I'll see if I can find some time to show you what I mean in code instead of
trying to explain this in a CL comment.

https://codereview.appspot.com/98450048/diff/1/src/observe.js
File src/observe.js (right):

https://codereview.appspot.com/98450048/diff/1/src/observe.js#newcode123
src/observe.js:123: case undefined:
Does this non string cause a deopt?

https://codereview.appspot.com/98450048/diff/1/src/observe.js#newcode141
src/observe.js:141: if (char.trim() == '')
Just list the whitespace characters and include it in the switch.

  switch (code) {
    case 0xA0:  // No-break space
    case 0xFEFF:  // Byte Order Mark
    case 0x2028:  // Line Separator
    case 0x2029:  // Paragraph Separator
      return true;
  }

(In Traceur we use the character code because it is faster.)

https://codereview.appspot.com/98450048/diff/1/src/observe.js#newcode236
src/observe.js:236: key = undefined;
move into if above

https://codereview.appspot.com/98450048/diff/1/src/observe.js#newcode237
src/observe.js:237: } else if (action == 'append') {
maybe switch?

[rafaelw, 2014-05-23 23:29:28]

Added tests, quote escaping and serialization

https://codereview.appspot.com/98450048/diff/1/src/observe.js
File src/observe.js (right):

https://codereview.appspot.com/98450048/diff/1/src/observe.js#newcode123
src/observe.js:123: case undefined:
Not sure, but moved above.

On 2014/05/23 17:13:30, arv wrote:
> Does this non string cause a deopt?

https://codereview.appspot.com/98450048/diff/1/src/observe.js#newcode141
src/observe.js:141: if (char.trim() == '')
done.

On 2014/05/23 17:13:30, arv wrote:
> Just list the whitespace characters and include it in the switch.
> 
>   switch (code) {
>     case 0xA0:  // No-break space
>     case 0xFEFF:  // Byte Order Mark
>     case 0x2028:  // Line Separator
>     case 0x2029:  // Paragraph Separator
>       return true;
>   }
> 
> (In Traceur we use the character code because it is faster.)

https://codereview.appspot.com/98450048/diff/1/src/observe.js#newcode236
src/observe.js:236: key = undefined;
not sure what you mean.

On 2014/05/23 17:13:29, arv wrote:
> move into if above

https://codereview.appspot.com/98450048/diff/1/src/observe.js#newcode237
src/observe.js:237: } else if (action == 'append') {
On 2014/05/23 17:13:29, arv wrote:
> maybe switch?

Done.

[rafaelw, 2014-05-23 23:30:19]

Also, this change polymer-expressions will fix the breakage that this causes:

https://codereview.appspot.com/95550049

[arv, 2014-05-23 23:37:00]

https://codereview.appspot.com/98450048/diff/20002/src/observe.js
File src/observe.js (right):

https://codereview.appspot.com/98450048/diff/20002/src/observe.js#newcode125
src/observe.js:125: var code = char.charCodeAt(0);
Why not pass in the char code directly? No need for the string object here.

https://codereview.appspot.com/98450048/diff/20002/src/observe.js#newcode242
src/observe.js:242: key = undefined;
I meant:

if (key !== undefined) {
  keys.push(key);
  key = undefined;
}

since we know whether it is undefined already

https://codereview.appspot.com/98450048/diff/20002/src/observe.js#newcode249
src/observe.js:249: key = key + newChar;
+=

https://codereview.appspot.com/98450048/diff/20002/src/observe.js#newcode255
src/observe.js:255: if (nextChar === undefined)
A length check is cleaner

[rafaelw, 2014-05-23 23:50:19]

https://codereview.appspot.com/98450048/diff/20002/src/observe.js
File src/observe.js (right):

https://codereview.appspot.com/98450048/diff/20002/src/observe.js#newcode125
src/observe.js:125: var code = char.charCodeAt(0);
Mostly because it's convenient to return char back out again below at line 134.

On 2014/05/23 23:36:59, arv wrote:
> Why not pass in the char code directly? No need for the string object here.

https://codereview.appspot.com/98450048/diff/20002/src/observe.js#newcode242
src/observe.js:242: key = undefined;
done.

On 2014/05/23 23:36:59, arv wrote:
> I meant:
> 
> if (key !== undefined) {
>   keys.push(key);
>   key = undefined;
> }
> 
> since we know whether it is undefined already

https://codereview.appspot.com/98450048/diff/20002/src/observe.js#newcode249
src/observe.js:249: key = key + newChar;
On 2014/05/23 23:36:59, arv wrote:
> +=

Done.

https://codereview.appspot.com/98450048/diff/20002/src/observe.js#newcode255
src/observe.js:255: if (nextChar === undefined)
On 2014/05/23 23:36:59, arv wrote:
> A length check is cleaner

Done.

[rafaelw, 2014-05-23 23:51:10]

Committed patchset #10 manually as rd99eff1 (presubmit successful).

[arv, 2014-05-23 23:52:04]

LGTM