Skip to content

Misc Fixes #363

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

Merged
merged 3 commits into from
Apr 6, 2016
Merged

Misc Fixes #363

merged 3 commits into from
Apr 6, 2016

Conversation

amadeus
Copy link
Collaborator

@amadeus amadeus commented Apr 6, 2016

This PR is a few fixes we should get into develop before merging develop into master.

Fixed Comment Regions

  • Fixed comment regions by adding the extend keyword (this means they override containing region endpoints, which they do when it comes to actual code. Before this PR, if you created a block comment inside say an object, with no closing block comment, the comment would appear to end at the end of the object. This is obviously not how block comments work.

oops

Function Argument Declaration Destructuring Improvements

  • Some small improvements to destructuring in function declarations. This is far from complete, but a bit better than it used to be and handles many standard cases (I hope)

yay

Fixed jsBlock vs jsFuncBlock priority in top level code

  • Before this PR, if you wrote an object at the top level of a file (i.e. not contained in a function or object or something), the braces would detect as jsFuncBlock. This bug was introduced from my earlier updates to arrow functions. The simple fix for this was to make jsBlock higher priority. jsFuncBlock continues to work normally in other contexts because specifiers like nextgroup= take priority.

oops2

Note

We still have a major issue with arrow function declarations - allowing the argument parenthesis to span multiple lines. This is actually quite a challenging regex problem, and I don't think I will be able to come up with a fix in a reasonable amount of time and think we should ship the current develop branch with these fixes asap since there's a lot of great stuff in there.

amadeus added 3 commits April 4, 2016 01:56
Our comment detection was a bit buggy because we did not use the extend
keyword.  This meant that if the comment was contained within a region
or a match of sorts, it would get pre-maturely ended by a containing
group.  Using the extend keyword means that it will override any
containing parent matcher and still display a comment, which is how JS
actually interprets the code.
This should vastly improve how destructuring is handled in function
declarations. It is still far from perfect, and a major problem right
now is how to support multiple lines in lambda functions
Since we had to remove the contained argument for jsFuncBlock, it caused
false positives for object definitions at the top level of a file.  I've
made jsBlock a higher priorty than jsFuncBlock by placing jsBlock AFTER
jsFuncBlock.
@davidchambers
Copy link
Collaborator

Fixed comment regions by adding the extend keyword (this means they override containing region endpoints, which they do when it comes to actual code.

This is wonderful! Thanks for the explanation.

@amadeus
Copy link
Collaborator Author

amadeus commented Apr 6, 2016

Any qualms with merging this in? Otherwise I'll go ahead and do just that.

@davidchambers
Copy link
Collaborator

No qualms. I won't claim to understand all the changes, though. ;)

@amadeus amadeus merged commit 2d11ec0 into pangloss:develop Apr 6, 2016
@amadeus
Copy link
Collaborator Author

amadeus commented Apr 6, 2016

Haha, no worries, it's weird to even explain some of them

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

Successfully merging this pull request may close these issues.

2 participants