eslint: Support block-comment version of eslint-disable-line
Tell us about your environment
- ESLint Version: master
- Node Version: any
- npm Version: any
What parser (default, Babel-ESLint, etc.) are you using? default
Please show your full configuration:
{ "rules": { "no-alert": [1] } }
What did you do? Please include the actual source code causing the issue.
/* eslint-disable-line */ alert(1);
What did you expect to happen?
It should work like in JSCS:
/* jscs:ignore */ alert(1)
What actually happened? Please include the actual, raw output from ESLint.
Error reported.
About this issue
- Original URL
- State: closed
- Created 7 years ago
- Reactions: 14
- Comments: 47 (31 by maintainers)
Commits related to this issue
- Update: support block comment ersion of eslint-disable-line fixes #8781 — committed to erindepew/eslint by deleted user 7 years ago
- Update: support block comment ersion of eslint-disable-line fixes #8781 — committed to erindepew/eslint by deleted user 7 years ago
- Update: support eslint-disable-* block comments (fixes #8781) — committed to erindepew/eslint by deleted user 7 years ago
- Update: support eslint-disable-* block comments (fixes #8781) — committed to erindepew/eslint by deleted user 7 years ago
I think it would be best to report a problem at that location, and ignore the comment. However, it might be a breaking change to start reporting a problem for those cases, since we previously always ignored them.
We could ignore them for now, then start reporting a problem for them in the next major release.
I think it’s fine to just ignore disable the entire line in that case. We already do that with
// eslint-disable-lineat the end of a line, so in my opinion it’s not too confusing to to also ignore the entire line if there is a block comment in the middle.I think this should disable both rules, by just processing the comments separately.
We already do the same thing in a similar case:
We already handle similar cases:
With the current behavior, reporting is disabled for
foo, but enabled forbar.Right now, we preprocess all
disable-lineanddisable-next-linedirectives into regulardisableandenabledirectives:So I think we could just do the same thing here regardless of whether the
disable-linecomment is a block.Would it make sense to also make
eslint-disable-next-linealso available in block comments to fix this issue?I’d be happy to take this on if no one else wants it!
@platinumazure Mind explaining what it is that you don’t like about this solution? It’s clear that you’re not satisfied with this solution, but I don’t know what about it you’re against.
My 2c: The most compelling reason for me to support this is the JSX use case (which only allows block comments), and I would also argue that it would be even more confusing to only allow those style comments in JSX but not allow them elsewhere. So it seems to me like supporting this style comment everywhere makes the most sense.
The multiline comment is an interesting case. Seems like it should treat lines that contain the beginning or end line of the comment as “the line” in question.
I agree that a line comment would probably be clearer most of the time, but this feels like a case where we can decide on behavior, document it, and trust our users rather than limit them. And for what it’s worth, this seems like a pretty reasonable style for a
eslint-disable-next-linecomment:@jcrben FYI you can use
/* eslint-disable */and/* eslint-enable */to do the same thing with ESLint, and you can even pass a rule or list of rules to enable/disable.@not-an-aardvark’s comment here I believe covers everything we discussed during the meeting.
To simplify, rules for
eslint-disable-lineandeslint-disable-next-line(both line and block) comments should be:@erindepew Please let us know if anything is unclear!
We should make a decision about this. I’m still 👍 for the JSX use case. It looks like we have some team members for it and some against. Is it maybe time to bring this to the TSC? @eslint/eslint-tsc
I think that we have to clarify the specification of this feature before the decision. I think this proposal still has much ambiguity.
eslint-enableandeslint-disable-lineIn the following case, I’m not sure how it should behave for errors at thefoo. I guess it should be disabled. If it’s/* eslint-disable */ var /* eslint-enable */ foo, the errors at thefooare reported because thefoois after/* eslint-enable */. Above sample is similar to this. If it’svar /* eslint-enable */ foo //eslint-disable-line, the errors at thefooare not reported because thefoois on the line which has//eslint-disable-line. I think that//eslint-disable-lineand/* eslint-disable/enable */are different type of comments to help avoiding confusion.eslintandeslint-envcomments which are in line comments? Currently, we support those only on block comments. If we need to handle line comments and block comments consistently, we should consider this. I think that we should keep about this behavior as is.In line comment case, we can write only one comment on a line, so it fits as the directive to control the state of a line. We can write block comments with many variations. It can bring ambiguity and complexity to users and developers.
We can disable reports with line comments in most cases already. We can disable reports with a
/* eslint-disable/enable */comment pair in remaining cases. So I’m not sure that this feature has more value to introduce the ambiguity and complexity.Just found some relative:
And that is music for me:
Same was for JSCS. Was.
I’m not about writing code with business logic. Just about thinking how it should work.
After this little research I’m even more sure we should support both versions for ALL directives. People keeps creating issues because they think it’s more native. And there are no real reasons to not change anything — We should move forward.
Thank you for contribution and I don’t know what I can do here but please mention me if I needed. p.s. Please, feel free to close it if you decide that it shouldn’t be done.
@platinumazure Well
Yes, you can’t put it into the template literal, but you dont need it, because you can place block-comment BEFORE it.
And you can’t do this with line comment:
I’d like to just throw out there that I think that both line and block comments should be allowed for all configuration comments, in regards to @platinumazure’s comment. I remember being confused why I had to have specific types for certain configuration options, and in the case of JSX, requiring line comments are very limiting and make for messy code, which is why we have this issue. That’s my vote.