pangloss / vim-javascript

Vastly improved Javascript indentation and syntax support in Vim.
http://www.vim.org/scripts/script.php?script_id=4452
3.8k stars 358 forks source link

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

Open BrandonMathis opened 3 years ago

BrandonMathis commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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
amadeus commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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