-
Notifications
You must be signed in to change notification settings - Fork 236
Fix function/method calls being superseded #321
Conversation
Builtins tests added for function calls to builtin methods of `support.function.js` and `support.function.dom.js`.
Make old function tests pass
Syntax fix
Syntax refix
Trailing whitespace
grammars/javascript.cson
Outdated
| '2': | ||
| 'name': 'support.function.js.console' | ||
| # console.log(arg1, "arg2", [...]) | ||
| 'begin': '\\bconsole' |
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.
What if we have consolea.log()?
grammars/javascript.cson
Outdated
| 'name': 'support.function.dom.js' | ||
| } | ||
| { | ||
| 'match': ".+" |
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.
This seems dangerous...we should make sure the function name is valid.
grammars/javascript.cson
Outdated
| 'end': '(?!\\G)' | ||
| 'patterns': [ | ||
| { | ||
| 'begin': '\\b(\\.)(assert|clear|debug|error|info|log|profile|profileEnd|time|timeEnd|warn)\\s*(?=\\()' |
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.
\\s*(\\.)\\s*
Have to test for spaces here as well. I don't think we need the \\b anymore if we test for spaces.
|
Overall looks pretty good. Left a few small comments. |
|
Also, it would be great if we could tokenize this properly: console
.
log(); // <-- meta.function-call, no support.function.console.jsor console.
log();etc. Not just limited to something
.
hi(); |
|
@MaximSokolov Is this ready for merge? It looks good to me. |
|
🚢 |
Fix function/method calls being superseded
Continuation of #295
Fixes #294
Fixes #320
Replaces and closes #295