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

allow experimental object rest/spread #4

Closed
wants to merge 4 commits into from

Conversation

helloyou2012
Copy link
Contributor

@millermedeiros
Copy link
Owner

millermedeiros commented Jul 7, 2016

can you add a test to avoid regressions? https://github.com/millermedeiros/esformatter-semicolon-first/tree/master/test/compare - just need to include an example on the input and output files and run npm test to check it:

cd esformatter-semicolon-first
npm i
npm test

the best fix would be to replace espree with esformatter-parser, but this also works.

@helloyou2012
Copy link
Contributor Author

Ok, I will add test case.

// comment
;[7, 8].forEach(doStuff)

var y = 8
;(function() {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uhm.. weird that this behavior changed. I would not expect the code to be formatted this way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before that, this behavior have changed. Maybe espree have changed. I'm checking.

@helloyou2012
Copy link
Contributor Author

helloyou2012 commented Jul 7, 2016

Source :

var x = 2
[1,2,3].map(function() {})

esformatter-semicolon-first plugin run stringBefore :

var x = 2
;[1,2,3].map(function() {})

After VariableDeclaration transform to :

var x = 2;
[1, 2, 3].map(function() {})

esformatter-semicolon-first plugin run tokenAfter :

var x = 2;[1, 2, 3].map(function() {})

PS: tokenAfter should not be necessary.

@millermedeiros
Copy link
Owner

uhm, I'll check this later. seems really weird. I'm sure behavior was fine previously. maybe esformatter is doing more stuff than before... or maybe I should just add the ; on stringAfter instead.

millermedeiros added a commit that referenced this pull request Aug 7, 2016
@millermedeiros
Copy link
Owner

@helloyou2012 I ended up updating the parser and using your test cases (did some aggressive rebase but kept you as the commit author)

thanks a lot for bringing this into my attention. I'm not using this plugin on my own projects (I like semicolons) so I would not notice it was broken. Cheers!

@millermedeiros
Copy link
Owner

just released v1.2.0 with the fix!

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

Successfully merging this pull request may close these issues.

2 participants