-
Notifications
You must be signed in to change notification settings - Fork 58
Conversation
Do not include storage modifiers in the variables matches because it's not great to re-use them in code blocks in that case. This also made it possible to put `#variables` as an include in `#code`, which removes the need for having both `#code` and `#variables` in many blocks. Fixes: atom#50
'include': '#code' | ||
} | ||
] | ||
'include': '#anonymous-block-and-instance-initializer' |
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 actual changes, just some splitting up code to make it more readable.
'applyEndPatternLast': 1 | ||
'begin': ''' | ||
(?x:(?= | ||
(?: |
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.
Since we're in a lookahead all the non-capturing groups aren't needed. We're not capturing anything anyway.
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.
Right you are. Copied this from the previous definition, but might as well simplify!
@@ -999,50 +1001,54 @@ | |||
} | |||
] | |||
'variables': | |||
'applyEndPatternLast': 1 |
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 removed this. To be frank, I don't know how It worked with it before, since ;
is matched by #code
which was included before too.
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.
Surprisingly first-mate actually recognizes this. The super-sparse documentation seems to suggest that this only matters when the end patterns of a subpattern and the overarching end
are exactly the same, but if everything already works, then no need to have it!
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.
Cool. In my manual tests it seems to work well, and the specs agrees.
NB. Added this to commit message
|
⛵ |
A multi-array variable still isn't highlighted correctly, eg |
Do not include storage modifiers in the variables matches
because it's not great to re-use them in code blocks in that
case.
This also made it possible to put
#variables
as aninclude in
#code
, which removes the need for havingboth
#code
and#variables
in many blocks.One difference here is that storage modifiers (
private
,final
,static
, etc) will no longer be tagged withmeta.definition.variable.java
,and it shouldn't be IMO. It's not part of the definition, but rather a modification to
a definition. Syntax highlighting remains the same.
This also fixes my comment in #47, but not #47 itself.
Fixes: #50