-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
Ignore comments option for indent rule #9018
Ignore comments option for indent rule #9018
Comments
I think I can see this as a usable option for a small subset of our user base. On the other hand we need to think very very carefully when introducing option in indent rule since its very disruptive and hard to maintain long term. |
I use tabs and start my comments at column 0. This way I can comment out blocks of code without affecting the horizontal placement of the commented code (i.e. indenting with spaces would indent the commented code by two spaces). Personally I find this very readable and useful. The current eslint rules complain about my indent level and I have no way to turn that off short of changing my code style. Ignoring indent levels on comments is exactly what I'm looking for. |
@gyandeeps What you say makes sense. For what it's worth, though, it could also be seen as a backward-compatibility enhancement for users that used to rely on |
Hi, always thanks a lot, ESlint team I met this issue after upgrading Eslint to its latest version. Providing an option to ignoring comments would be greatly appreciated |
I'll champion this. @eslint/eslint-team Anyone want to support this? Or if anyone can convince me of a better way to handle these use cases, I'd love to hear it! Thanks! |
I would like to support this too. And, to add a concrete use-case, sometimes I put JSON object examples in my code to use as a reference, so they are commented out, and I will indent the comments at different levels in order to allow my editor to fold the comments when I'm not looking at them.
The middle lines produce an error in ESLint that I can't disable. |
+1 |
@eslint/eslint-team Following up from my previous comment: I'm championing this because it seems like we should add this feature as a back-compat enhancement for people who were implicitly relying on the old indent rule not enforcing comment indentation. If there's a better way we can handle that scenario in the new indent rule, I'm all for implementing this a different way. But at this point, it feels like the rule is incomplete without some ability to configure comment indent linting (including just ignoring comments in indent checks). Can we get some discussion going on this? I'm happy to close if there's no consensus, but I haven't seen much evidence of discussion outside of this post from gyandeeps. |
@platinumazure I'm not against this option, but now that it has correct autofixing it also seems like it's an easy fix! @servel333 Would the following not work for you? This is what I would expect: // {
// "a": {
// "aa": "01",
// "ab": "02",
// "ac": "03"
// },
// "b": 01,
// } |
@kaicataldo Not always. If the line of comments is short then that works just fine, but if the line of comments is long then it's nice to indent the comments with the JSON so that I can fold the section in my editor. I've used multi line comments ( It's not a critical feature for me, but it's annoying enough that I have to work around it and alter my preferred coding style. |
+1 |
@servel333 Understood - thanks for the explanation. Like I said, I'm not opposed to this in theory. The thing we have to figure out is what this does for autofixing. I think this would mean that autofixing for comments would have to be completely ignored if this option is set. Also, please refrain from |
@kaicataldo Thanks. I totally understand and that makes sense. I don't use autofixing much, but I question what would happen to indented multi line comments when running the autofixer. Does it flatten them or ignore them currently? |
I believe it aligns the opening comment delimiter: https://github.com/eslint/eslint/blob/master/tests/lib/rules/indent.js#L7065-L7084 I don't think it can align the second line because the whitespace might be intentional. |
Support from me for this too. My use case, assuming I'm hitting my max line limit:
Currently I would have to change this to one of the following to still pass my indent rules; ideally I'd like to be able to ignore comment indentation instead:
(Thanks to @kaicataldo's comment above for pointing out the last option). |
+1 to this. I'm getting indentation errors commenting a
(I can't make the |
@elamperti If you're using the Bluebird library, you can put thing.foo()
.then(() => {
doSomething();
})
// .catch((error) => {
// doSomethingElse(error);
// })
.tap(); Though, my personal style is to not indent it these cases, and it would be nice to have better ignore rules, I think this might help. |
+1 as we need to stop those warnings somehow. I may have to try running an older eslint for now as IIRC that did not care about comments. |
@davies147 No need to use an older version. We left the old indent rule in because we knew this was a big change and wanted to make sure it didn't cause too much disruption. Just change |
For all my googling I never found that option mentioned as a workaround for this issue. Many many thanks! |
I can look into this later, but I think this could be handled by the ignoredNodes option. Unfortunately, now that we're not doing comment attachment, that might not currently work. Additionally, I prefer to not think of comments as AST nodes and as tokens instead, so maybe it should require a separate option ( @not-an-aardvark Thoughts? |
Switching to @davies147 I also didn't know such option existed 😕 |
There are so many 👍s on the original comment that I don't know how many team members are supporting this issue. Yowzer. Yes, the fact that comments aren't a node type is why I think an |
I got the list of 👍s from GitHub's API: Results
Right now I think I'm the only team member that has given a 👍 |
If it makes it easier for people to start using the new indent rule I'm for adding this change, since I don't think it adds significant complexity (let me know if I'm wrong!). Thumbed! We just need one more 👍 from the team. |
+1 for this. My usecase has to do with code folding in editors that rely on indentation (e.g Visual studio code). I want this:
folded as:
|
Oh, I didn't see this get accepted. If nobody beats me to it, I'll work on this sometime this week. |
What rule do you want to change?
indent
Does this change cause the rule to produce more or fewer warnings?
Potentially fewer warnings
How will the change be implemented? (New option, new default behavior, etc.)?
New option:
This option will completely disregard lines beginning with comments and allow them to be at any indentation level.
Please provide some example code that this change will affect:
What does the rule currently do for this code?
Reports all comments (except trailing comment on first line)
What will the rule do after it's changed?
Reports none of the comments
The text was updated successfully, but these errors were encountered: