-
-
Notifications
You must be signed in to change notification settings - Fork 206
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Convert ~ and ! prefix tokens to esprima #276
Conversation
type.isAssign) { | ||
|
||
type.isAssign || | ||
(type.prefix && (token.value === "~" || token.value === "!")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to filter further if type.prefix
is true? Is there ever a situation where a prefix isn't a punctuation? Or are there other punctuator prefixes that this should also cover?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked through the acorn code and I'm pretty sure that currently if type.prefix
is true, the value is either !
or ~
. I only added the value comparison, so that in future if acorn adds another prefix (which might not be a Punctator?) it is not immediately converted to Punctator.
Do you think it is safe to assume that prefix is always a Punctator?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can safely say if the first character of the token is not an identifier start, then it's a punctuator.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just checked again: typeof
, void
and delete
are also prefixes. So a differentiation is necessary.
It seems that my change also makes the testsuite hang on the XMLHttpRequest test. I can't figure out why my change is breaking stuff. Actually it looks like the assert is hanging when comparing the results. |
I finally fixed the tests. They were hanging because i forgot to update the library results and assert can't handle diffs that big. |
This looks great, thanks! I'll do a release later today. |
The two prefixes are now converted to Punctator like esprima is returning them.
fixes #274