Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

JetBrains //noinspection whitelisted in comment-format space rule #1524

Merged
merged 3 commits into from
Sep 1, 2016
Merged

JetBrains //noinspection whitelisted in comment-format space rule #1524

merged 3 commits into from
Sep 1, 2016

Conversation

nomaed
Copy link
Contributor

@nomaed nomaed commented Aug 27, 2016

JetBrains IDE’s (IntelliJ IDEA, WebStorm, PhpStorm, etc.) disabling
inspections by adding //noinspection [rulenames] before the
expression. This violates the rule of single-line comments must start
with a space.

JetBrains IDE’s (IntelliJ IDEA, WebStorm, PhpStorm, etc.) disabling
inspections by adding `//noinspection [rulenames]` before the
expression. This violates the rule of single-line comments must start
with a space.
@nomaed nomaed changed the title JetBrains' //noinspection added to comment-format whitelist JetBrains //noinspection added to comment-format whitelist Aug 27, 2016
@nomaed nomaed changed the title JetBrains //noinspection added to comment-format whitelist JetBrains //noinspection whitelisted in comment-format space rule Aug 27, 2016
@jkillian
Copy link
Contributor

Code looks good @nomaed! Mind adding a line to the test here so we can verify that everything works correctly now?

@adidahiya
Copy link
Contributor

I don't think we should encode JetBrains semantics into the rule by default... can we make it a configurable option?

@nomaed
Copy link
Contributor Author

nomaed commented Aug 30, 2016

@adidahiya I wanted to add an option for this, but then I saw that there is already another exception (//#region and //#endregion) so I figured it's pretty much the same thing.
After all, //noinspection ... is not actually a comment but a command, so it makes sense that these things are an exception by default, IMO.

@jkillian
Copy link
Contributor

I'm sort of happy just having it baked-in. Nobody's going to accidentally have //noinspection comments scattered around their codebase unless they're using JetBrains stuff. Having too many small configuration options can be overwhelming, seems like it's best to just "do the right thing" by default when possible.

@adidahiya
Copy link
Contributor

Alright, cool sounds good 👍

@jkillian
Copy link
Contributor

jkillian commented Sep 1, 2016

Thanks @nomaed!

@nomaed nomaed deleted the whitelist-jetbrains-noinspection branch September 1, 2016 05:29
@nomaed
Copy link
Contributor Author

nomaed commented Sep 1, 2016

Thank you all for accepting this addition :)
When is this addition going to be available via npm?

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

Successfully merging this pull request may close these issues.

3 participants