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

Fix `isFirstNested` util to handle shared-line comments #2827

Merged
merged 1 commit into from Sep 2, 2017

Conversation

5 participants
@nwoltman
Contributor

nwoltman commented Aug 22, 2017

Which issue, if any, is this issue related to?

Fixes #2764

Is there anything in the PR that needs further explanation?

This changes the isFirstNested util to ignore comments that are on the same line as the opening brace of a block when checking if the provided node is the first node in the block.

@nwoltman nwoltman force-pushed the nwoltman:fix-2764 branch from 5ec4ed3 to 81ceda0 Aug 22, 2017

@nwoltman nwoltman force-pushed the nwoltman:fix-2764 branch from 81ceda0 to c9c6ef5 Aug 22, 2017

@ntwb

ntwb approved these changes Aug 22, 2017

LGTM, lots of tests ❤️

@evilebottnawi

Good works, thanks!

}
}
/* istanbul ignore next: Should always return in the loop */

This comment has been minimized.

@evilebottnawi

evilebottnawi Aug 22, 2017

Member

Why this line?

This comment has been minimized.

@nwoltman

nwoltman Aug 22, 2017

Contributor

It's impossible for the code execution to reach here. One of the return statements in the for loop will always be executed. This is guaranteed because it returns if it finds the statement node in parent.nodes and that must happen because that node must be in its own parent's list of nodes.

If this is too strange I could change the return false; in the for loop into break; so then this return false; at the end of the function will be executed.

This comment has been minimized.

@evilebottnawi

evilebottnawi Aug 22, 2017

Member

@nwoltman good, just question, thanks!

This comment has been minimized.

@hudochenkov

hudochenkov Aug 22, 2017

Member

Why comment starts with “istanbul ignore next:”?

This comment has been minimized.

@nwoltman

nwoltman Aug 22, 2017

Contributor

Jest uses Istanbul for analyzing code coverage. That comment tells Istanbul not to count that line in the total code coverage since it's impossible to execute (the documentation is in the old Istanbul repo).

@jeddy3

jeddy3 approved these changes Sep 2, 2017

Excellent!

@jeddy3 jeddy3 merged commit cc9ce64 into stylelint:master Sep 2, 2017

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.01%) to 95.649%
Details
@jeddy3

This comment has been minimized.

Member

jeddy3 commented Sep 2, 2017

Changelog:

  • Fixed: *-empty-line-before false positives shared-line comments and "first-nested" option (#2827).

@nwoltman Thanks for this! Sorry about the delay getting it in. A few of us (the stylelint contributors) have been away/relocating.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment