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

Indent comment indentation: Comments can align with previous/next lines of code even if whitespace in between #9733

Closed
platinumazure opened this issue Dec 18, 2017 · 6 comments
Assignees
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 bug ESLint is working incorrectly indent Relates to the `indent` rule rule Relates to ESLint's core rules

Comments

@platinumazure
Copy link
Member

Tell us about your environment

  • ESLint Version: 4.13.1 (via online demo)
  • Node Version: N/A
  • npm Version: N/A

What parser (default, Babel-ESLint, etc.) are you using?

Default parser

Please show your full configuration:

Configuration
{
    "rules": {
        "indent": ["error", 4]
    }
}

What did you do? Please include the actual source code causing the issue, as well as the command that you used to run ESLint.

if (foo) {

// blah
    statement();

}

What did you expect to happen?

3:1 - Expected indentation of 4 spaces but found 0. (indent)

What actually happened? Please include the actual, raw output from ESLint.

No lint error.


The bug (as I see it) is that the indent rule has a tolerance for comments to match the indentation of the previous line, that of the next line, or the "actual correct location". However, I think this should only apply if there actually is code on the immediately previous or next lines, rather than code that is separated by one or more lines of whitespace.

@platinumazure platinumazure added bug ESLint is working incorrectly evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion indent Relates to the `indent` rule rule Relates to ESLint's core rules labels Dec 18, 2017
@platinumazure platinumazure changed the title Indent comment indentation: Indent comment indentation: Comments can align with previous/next lines of code with whitespace in between Dec 18, 2017
@platinumazure platinumazure changed the title Indent comment indentation: Comments can align with previous/next lines of code with whitespace in between Indent comment indentation: Comments can align with previous/next lines of code even if whitespace in between Dec 18, 2017
@platinumazure
Copy link
Member Author

@not-an-aardvark: Is this a bug, or working as designed? Thanks!

@not-an-aardvark
Copy link
Member

I think this is technically working as designed, but I agree with your analysis that the design could be improved here by making it more strict. I'm fine with calling it a bug.

@platinumazure platinumazure added accepted There is consensus among the team that this change meets the criteria for inclusion and removed evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion labels Mar 21, 2018
@platinumazure
Copy link
Member Author

I'll work on this later this week.

@platinumazure platinumazure self-assigned this Mar 23, 2018
@sstern6
Copy link
Contributor

sstern6 commented May 20, 2018

@platinumazure was wondering if you were still looking into this one? Wouldnt mind taking a crack at it. If youre still looking into it though no worries.

@platinumazure
Copy link
Member Author

platinumazure commented May 20, 2018 via email

@platinumazure
Copy link
Member Author

Working on this.

platinumazure added a commit that referenced this issue Jul 21, 2018
Also needed to ensure that comments between element lists have desired offsets set if first element is on same line as beginning of element list
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Jan 24, 2019
@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 Jan 24, 2019
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 bug ESLint is working incorrectly indent Relates to the `indent` rule rule Relates to ESLint's core rules
Projects
None yet
Development

No branches or pull requests

3 participants