-
Notifications
You must be signed in to change notification settings - Fork 238
Conversation
} | ||
] | ||
} | ||
{ | ||
'begin': '([a-zA-Z_?$][\\w?$]*)\\s*(=)\\s*(?:(async)(?:\\s+))?(function\\*?)\\s*(\\*?)\\s*([a-zA-Z_?$][\\w?$]*)?\\s*(\\()' | ||
# foo: function() |
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 don't think this line [function name] is correct, as object-literal functions have to be anonymous.
¯\_(ツ)_/¯
var obj = {foo: function(){}}
console.log(obj.foo); //=> function(){}
obj = {foo: function abc(){}}
console.log(obj.foo); //=> function abc(){}
I rather keep it as is
@MaximSokolov Do you want this to be reviewed alongside #307? |
@50Wliu It's not necessary |
Recognize function bodies. Tokenize rest parameter Tokenize arrow functions with one parameter Don't scope not built-in objects as 'support'
New approach of functions recognition: {
'begin': '(?=...)'
'end': '...'
'name': '3'
'patterns': [
{
'begin': '\\G'
'end': '...'
'name': '1'
'patterns': [ ... ]
}
{
'include': '#function_body' # 2
}
]
}
All these regions can be scoped. Not sure what profit of nested function bodies' blocks to autocompletion, syntax highlighting; so I'll keep |
'name': 'meta.function.json.js' | ||
# [param|(params)] => [expression|{statements}] | ||
'begin': '(?=(?<![A-Za-z0-9])((\\(([^\\(\\)]*)?\\))|[\\w$]+)\\s*=>)' | ||
'end': '(?!(\\s*{|\\G\\(|\\G[\\w$]+|/\\*))((?=\\s*\\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.
'name': 'meta.function.prototype.js' | ||
# [async] function [name](params) | ||
# function* name(params) – generator function declaration | ||
'begin': '(?=(?:\\basync\\b\\s*)?\\bfunction\\b)' |
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.
No need for ?:
non-capturing group because we're not capturing anything here anyway.
'name': 'punctuation.definition.parameters.end.js' | ||
'name': 'meta.function.js' | ||
# "foo": function... | ||
'begin': '(?=((\'[^\']*?\')|("[^"]*?"))\\s*:\\s*(\\basync\\b\\s*)?\\bfunction\\b)' |
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.
So from some testing with other grammars I think the following will also work: ((\'.*?\')|(".*?"))
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.
Specs fail on
a.push("x" + y + ":function()");
a.push('x' + y + ':function()');
# Conflicts: # spec/javascript-spec.coffee
@@ -281,240 +281,283 @@ | |||
'name': 'variable.language.js' | |||
} | |||
{ | |||
'captures': |
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.
Why remove this capture?
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.
support.class
-> variable.other.object
for Sound
as it isn't a built-in class
|
'patterns': [ | ||
{ | ||
'include': '#function-params' | ||
'match': '(\\.\\.\\.)([a-zA-Z_$][a-zA-Z_$0-9]*)' |
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.
Second square-brackets match can be shortened to [\\w$]*
|
Regression time: function test()
{ //punctuation.definition.function.body.end.bracket.curly.js
if(something)
{ // meta.brace.curly.js
} // punctuation.definition.function.body.end.bracket.curly.js
} // meta.brace.curly.js We need to wrap curly bracket matching in a begin/end instead of a Also, in the case of |
Here's another problem I found: typing the following code (same as the above example) incorrectly marks the function test()
{
if(something) // entity.name.function.js!!!
{
}
} |
Numbers cannot be matched as illegal because of default parameters: function multiply(a, b = 1){} |
'patterns': [ | ||
{ | ||
'include': '#function-params' | ||
'begin': '\\G' | ||
'end': '(?<=\\))' |
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.
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.
Looking into it...what's the highlighted scope that's flickering?
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.
Never mind, got it: meta.function.js
. Not sure why though. It only seems to happen when there's an odd number of spaces.
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.
Moving the function_body include above this match appears to fix it. For safety I would arrange the patterns like this:
- comments
- function_body
- This one
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.
fixed
Added support for: {
foo: p => {}
"foo": p => {}
'foo': p => {}
} |
Looks good to me - can you just address the two style comments above? I wasn't able to find any glaring bugs through some testing, so I think this is ready to 🚢 and get some more extensive testing on Atom master :). |
Before:
obj
–support.class
*
–storage.type
foo
–entity.name.function.js
After:
obj
–variable.other.object
*
–storage.modifier.generator
foo
–variable.other.property
Code for testing
Fixes #303
Fixes #309
Fixes #193
Fixes #223
Fixes #322
Closes#197Replaces and closes #224