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

Doc: inline rule changes: block vs. single-line comment - when? #6335

Closed
tshinnic opened this issue Jun 7, 2016 · 2 comments
Closed

Doc: inline rule changes: block vs. single-line comment - when? #6335

tshinnic opened this issue Jun 7, 2016 · 2 comments
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion documentation Relates to ESLint's documentation

Comments

@tshinnic
Copy link

tshinnic commented Jun 7, 2016

What version are you using?
Saw this oddity with 2.9.0, updated to 2.11.1 with no improvement

What did you do?
Attempted to turn off a rule in a small section of code using a single-line comment format

What happened?
Needing to turn off a rule in a small section of code, and since I've used single-line comments before with ESLint, I added

// eslint-disable prefer-template

to the source. The error "Unexpected string concatenation" still appeared. Experimenting I altered this to

/* eslint-disable prefer-template */

and this worked - the error messages no longer appeared for the rest of the source file.

What did you expect to happen?
There seems to be a gap in either the documentation or the ESLint inline comment parser. I'll assume it is a documentation problem.

There is a need to be clear when inline rule changes require the block comment format vs. when the single line comment format will be acceptable. I cannot find a clear statement one way or the other.

There are examples of both formats in the documentation, with newer change variations shown as single line format, e.g.

// eslint-disable-next-line no-alert

and older more complex variations illustrated using block comments, e.g.

/* eslint quotes: ["error", "double"], curly: 2 */

Since the single-line format does work for something like "eslint-disable-next-line" (and thank you again for that capability!) it would seem that this format should be generally applicable. And yet the initial problem example seems to demonstrate single-line is only sometimes usable.

Oh, I see, there is a discontinuity... lib/eslint.js#L364-L372

If I'm reading that correctly, "eslint-disable-line" and "eslint-disable-next-line" can only be used from within single-line comments, and the other changes can only be used from within block comments. Which then agrees with my experiences above.

At the very least, update the doc user-guide/configuring to say near start of Configuring Rules ...

"To configure rules inside of a file using configuration comments, use a block comment in the following format:"

Or perhaps that should be near Disabling Rules with Inline Comments. Then perhaps change the much later line "To disable all rules on a specific line:" to

"To disable all rules on a specific line**, use a single-line comment**:"

or something more smoothly read? 😃

@eslintbot eslintbot added the triage An ESLint team member will look at this issue soon label Jun 7, 2016
@platinumazure platinumazure added documentation Relates to ESLint's documentation core Relates to ESLint's core APIs and features evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels Jun 7, 2016
@platinumazure
Copy link
Member

Yes, we very intentionally allow certain inline config comments to be line-only, and others are block-only. We could probably improve the documentation on that front.

@kaicataldo
Copy link
Member

Agreed that this can be made clearer (I also think the extra comments in the example are really confusing). Went ahead and opened a PR.

@kaicataldo kaicataldo added accepted There is consensus among the team that this change meets the criteria for inclusion and removed core Relates to ESLint's core APIs and features evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion labels Jun 9, 2016
@nzakas nzakas closed this as completed in dcd4ad7 Jun 10, 2016
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Feb 6, 2018
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Feb 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion documentation Relates to ESLint's documentation
Projects
None yet
Development

No branches or pull requests

4 participants