refactor($parse): separate tokenizing vs parsing more #9431
Conversation
| var ch = this.text.charAt(this.index); | ||
| if (ch === '"' || ch === "'") { | ||
| this.readString(ch); | ||
| } else if (this.isNumber(ch) || (this.index < length-1) && ch === '.' && this.isNumber(this.peek())) { |
There was a problem hiding this comment.
why is (this.index < length-1) needed?
There was a problem hiding this comment.
Because isNumber(false) is true when there is nothing after the '.'. Could also add a is-string check to isNumber which might be better then this length check?
There was a problem hiding this comment.
well, false is not a number, so isNumber should be fixed :)
|
otherwise, lgtm |
jbedard
force-pushed
the
parse-token-vs-parse
branch
3 times, most recently
from
0305fac to
21843de
Compare
|
Updated. I also changed one more |
jbedard
force-pushed
the
parse-token-vs-parse
branch
from
21843de to
906f5a2
Compare
jeffbcross
force-pushed
the
master
branch
3 times, most recently
from
8292c21 to
7b9fddf
Compare
|
looks good, provided there are no perf implications. |
|
Moving this to backlog to review/merge after 1.3.0 GA since the diff is so substantial. |
petebacondarwin
force-pushed
the
master
branch
from
4dd5a20 to
998c61c
Compare
Fixes `a.b` and `a .b` generating different getterFns
Fixes `{'': ...}` turning into `{"''": ...}`
Fixes `{.: ...}` parsing as `{'.': ...}` instead of throwing
Fixes angular#9131
jbedard
force-pushed
the
parse-token-vs-parse
branch
from
906f5a2 to
16c3a9a
Compare
|
I've rebased this... |
|
This PR also fixes #9893 |
|
@petebacondarwin we should get this in in 1.3.4 or 1.3.3 before it gets bit rotten. @lgalfaso or @rodyhaddad could help with the review. |
|
I'll take care of the review |
|
landed with fbad280 |
Fixes
a.banda .bgenerating different getterFns (which was mentioned a while back in #8901 (comment))Fixes #9893 (
{'': ...}being converted to{"''": ...})Fixes
{.: ...}parsing as{'.': ...}instead of throwingFixes #9131 (similar to how #9229 fixes it)
Also simplifies (imo) a few areas in parse.js. For example...
fn(which had 2 meanings before)textis always the token text (previously it was the raw value for strings/numbers).consume()is used in a few more places instead of.expect()and will output a "Unexpected end of expression" instead of.expect()returning false and failing later