Require block comments to be balanced #572

Closed
feross opened this Issue Jul 22, 2016 · 4 comments

Projects

None yet

3 participants

@feross
Owner
feross commented Jul 22, 2016 edited

http://eslint.org/docs/rules/spaced-comment

This rule (which is already enabled in standard) has a new option to enforce "balanced" spacing in block comments.

For example, this would be an error:

/* This is a block comment*/

It would be need to be rewritten like this:

/* This is a block comment */

As part of this change, I propose that we remove the exceptions that allow the keywords "global", "global", "eslint", and "eslint-disable" to be used without a space:

/*global myVar */

It's odd to enforce a space at the end while making it optional at the beginning for these keywords. With these exceptions removed, this is how these comments would need to be written:

/* global myVar */

Eslint handles space or no-space versions of these directives without issue.

The number of affected repos is moderate (2%):

1..422
# tests 422
# pass  412
# fail  10

Thoughts?

@feross feross added a commit to feross/eslint-config-standard that referenced this issue Jul 23, 2016
@feross Enforce balanced block comments dab976e
@feross feross referenced this issue in feross/eslint-config-standard Jul 23, 2016
Merged

Enforce balanced block comments #41

@feross feross added a commit to feross/eslint-config-standard that referenced this issue Jul 23, 2016
@feross Enforce balanced block comments 4e29de1
@dcousens
Collaborator

LGTM

@feross
Owner
feross commented Jul 25, 2016

This will be included in standard v8

@feross feross closed this Jul 25, 2016
@feross feross referenced this issue Jul 25, 2016
Closed

Release proposal: standard v8 #564

16 of 16 tasks complete
@Kovensky
Kovensky commented Jul 25, 2016 edited

I kinda like being able to write eslint directives in a "style-violating" way (and wish I could do it with more of them, like //eslint-disable-line) because, if the directive has a typo, it will be flagged by the linter instead of just being ignored. OTOH, if you have a directive, it's because it's disabling or configuring specific rules, and if the directive was being ignored, you'd be getting warnings for the things it tries to fix...

@feross
Owner
feross commented Jul 25, 2016

@Kovensky Interesting point. Your last point is correct. If the rule isn't getting picked up (due to a typo) then you'll get a warning because the rule you were trying to disable wasn't actually disabled.

I think this rule change is a good move since there were 2 ways to do something, and now there will be only one.

@saadq saadq referenced this issue in nodejs/nodejs.org Aug 25, 2016
Merged

Update dependencies #873

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