Skip to content
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

Possible issues with jsFuncBlock detection for arrow functions not wrapped in parens/curly-braces #1231

Open
BrandonMathis opened this issue Mar 24, 2021 · 7 comments

Comments

@BrandonMathis
Copy link

Running this plugin alone with no other plugins. On basic js files it seems that we have having some trouble detecting a jsFuncBlock on arrow functions that are one-line and have no { } or ( ) surrounding the function body.

Bellow is an image the demonstrates this issue. The first two functions have the function's body wrapped in { } or ( ). They highlight correctly but the last function does not.

Screen Shot 2021-03-24 at 2 24 31 PM

@BrandonMathis BrandonMathis changed the title Possible issues with jsFuncBlock detection for arrow functions not wrapped in parans/curly-braces Possible issues with jsFuncBlock detection for arrow functions not wrapped in parens/curly-braces Mar 24, 2021
@BrandonMathis
Copy link
Author

BrandonMathis commented May 28, 2021

Correction, it seems that this is only an issue for arrow functions defined inside a jsClassBlock.

Screen Shot 2021-05-28 at 11 17 55 AM

@amadeus
Copy link
Collaborator

amadeus commented Jun 7, 2021

Yeah, this is definitely a known issue, but I haven't had time to dig into it. It's something to do with syntax priorities I think...

@BrandonMathis
Copy link
Author

BrandonMathis commented Jun 7, 2021

True, I tried my hand @ fixing this a few times & the closest I managed to get is the following modification to the jsClassBlock (I just added @jsExpression to the contains)

-syntax region  jsClassBlock         contained matchgroup=jsClassBraces         start=/{/  end=/}/  contains=jsClassFuncName,jsClassMethodType,jsArrowFunction,jsArrowFuncArgs,jsComment,jsGenerator,jsDecorator,jsClassProperty,jsClassPropertyComputed,jsClassStringKey,jsAsyncKeyword,jsNoise extend fold
+syntax region  jsClassBlock         contained matchgroup=jsClassBraces         start=/{/  end=/}/  contains=@jsExpression,jsClassFuncName,jsClassMethodType,jsArrowFunction,jsArrowFuncArgs,jsComment,jsGenerator,jsDecorator,jsClassProperty,jsClassPropertyComputed,jsClassStringKey,jsAsyncKeyword,jsNoise extend fold

However, I then had to make this modification to the jsClassProperty so that the last line of a function doesn't get highlighted strangely (dropping the semicolon which usually terminates the last line of these js functions 😅)

-syntax match   jsClassProperty          contained /\<\K\k*\ze\s*[=;]/ skipwhite skipempty nextgroup=jsClassValue,jsFlowClassDef
+syntax match   jsClassProperty          contained /\<\K\k*\ze\s*[=]/ skipwhite skipempty nextgroup=jsClassValue,jsFlowClassDef

BrandonMathis added a commit to BrandonMathis/vim-javascript that referenced this issue Jun 7, 2021
@amadeus
Copy link
Collaborator

amadeus commented Jun 7, 2021

It's been a bit since I've work on this, but pretty sure that change will then allow a whole slew of other stuff to exist within jsClassBlock which is not valid syntax

@BrandonMathis
Copy link
Author

BrandonMathis commented Jun 11, 2021

My original solution was to create a new region called jsArrowFuncBlock what contains @jsAll or @jsExpression but I wasn't able to get a regex to properly match for me 😖

syntax region jsArrowFuncBlock contained matchgroup=jsFuncBraces start=/=>/ end=/;/ contains=@jsAll,jsBlock extend fold

Not even sure if the theory is correct either. Seems like I need to match everything from the beginning of a => & then until the ; denoting the end of a line (since you can't have multiple lines without wrapping the function body in { & })

@amadeus
Copy link
Collaborator

amadeus commented Jun 11, 2021

So I am 90% sure the issue is not actually related to any problem with our existing regexes for arrow functions, the issue is simply a priority issue. I think we just have to re-sort a few of the items to fix it

@BrandonMathis
Copy link
Author

Interesting.... I will try some re-ordering on my end and see if I can find the magic order that fixes this 😇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants