Skip to content

fix async keyword and comment inside destructuring #602

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

Closed
wants to merge 1 commit into from
Closed

fix async keyword and comment inside destructuring #602

wants to merge 1 commit into from

Conversation

arch-mage
Copy link

@arch-mage arch-mage commented Aug 16, 2016

I think async keyword should be an optional part of
a function. So, I remove async from class and object
method type and move it to be an optional part in
function regex. This is related to #598.

And I add jsComment to jsDestructuringBlock and
jsDestructuringArray.

I think `async` keyword should be an optional part of
a function. So, I remove `async` from class and object
method type and move it to be an optional part in
function regex
@amadeus
Copy link
Collaborator

amadeus commented Aug 17, 2016

First off, thanks for taking the time and opening this PR!

However I think I'd like to rehaul how this is done to be more inline with how the rest of the file works. We tend to abuse the shit out of nextgroup and skipwhite, and skipempty. This means you'd need to probably create a few new specific contained async matches and then nextgroup them. I can work on this over the next few days if what I am saying doesn't make sense.

@arch-mage
Copy link
Author

Sorry If I can't understand what you're saying well (my english is not that good). By the way, what I can understand is that inserting optional async part into jsFuncName regex doesn't match the current project (maybe leading to spaghetti code 😄 ). I agree.
Any way, my Idea is to make async as an optional part of function declaration (it can only appear exactly before func declaration). That way, it won't messed up with async module that already popular. And now, the only way I can think of is to wrap function declaration into a single region. That's only my idea. I don't know if there will be any drawbacks, because I'm not that good at vim scripting.
Maybe this should be closed then?

@amadeus
Copy link
Collaborator

amadeus commented Aug 17, 2016

No worries, I get what you are saying, and you understood generally what I meant! I'll close this and start looking at a fix shortly.

@amadeus amadeus closed this Aug 17, 2016
@arch-mage arch-mage deleted the develop branch August 18, 2016 02:52
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