-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Enforce indentation in member expressions #628
Comments
Smallish ecosystem impact:
Also, automatically fixable. |
Rule looks good, with the exception of the concern below: Not totally keen on forcing chaining to be indented e.g. stream
.pipe(through(() => {
})) // having closing braces indented like this looks broken IMO
another.line() // especially as there's no closing brace on this line Having parens/braces dangling out there looks like there's something unclosed. Would prefer having consistently open indentation closed by a paren or brace, as it allows you to quickly find indentation/brace issues. Currently it's immediately obvious when something may be amiss with your code if there's a closing paren and the wrong amount of indentation around it, even if there's no syntax error. })
}
}
// ahhh. rest easy now son, we've closed all the braces Not strongly against this though since it could just be because I wasn't using any editor-integrated linter until recently, so there was much more reliance on easily parsable visual clues in the code. Perhaps now I can relax this and rely more on |
@timoxley that is a great point, and is very disconcerting/confusing when reading code with that disjoined indentation due to this chaining indentation. |
Please, set it to some value! Different code editors have different default settings for chain indentation when you run their code reformat tool. Lines jump left-right without any rule, and git history gets messed (like in my current project 😢 ). |
A rule to this effect is coming in JavaScript Standard Style: standard/standard#628
@feross I think it's time to bring this up again since the eslint default in standard v11 seems to have swung to requiring member expression indentation and it's not clear a consensus was reached for standard going either way regarding this. |
I stand by my earlier argument that indenting members creates a weird closing paren/curly indentation unlike any other syntax and thus is undesirable. i.e. changing whether something is chained or not shouldn't need to change the entire indentation of all arguments and where the closing paren is: Consider wanting to break this chain onto multiple lines: a.pipe().pipe().then(() => {
// function body
})
b() Without member indentation, closing curly stays in the same position, as does the function body: a
.pipe()
.pipe()
.then(() => {
// function body
})
b() With member indentation, everything chained now needs to be indented for some reason: a
.pipe()
.pipe()
.then(() => {
// function body also needs indentation now?
}) // closing paren suddenly indented?
b() // no other rule results in a closing curly/paren + newline changing -2 levels of indentation |
No indentations does seem a tad more pragmatic |
I see the arguments for both ways of doing it. My thoughts are that whenever something that is conceptually one single unit goes onto multiple lines, you want some way of distinguishing the part that has extended onto later lines, so you can recognize it, even without the context of the former lines. Indentation is a good way to distinguish these lines, IMO. I do something similar for argument lists: function foo (
argument1, argument2, argument3
) {
// function body...
} And for really long console.log(
variable1,
variable2,
variable3,
variable4
) And for math: const x = (variable1 * variable2 * variable3) + variable4 -
(variable5 / variable6) And for ternaries: const bool = condition
? variable1
: variable2 And in JSX: const App = () => {
return (
<div
class='my-cool-class-name'
style='some: cool; styles: here;'
aria-label='accessibility stuff'
>
Text goes here
</div>
)
} I think all these cases are more confusing without the extra indent: function foo (
argument1, argument2, argument3
) {
// function body...
}
console.log(
variable1,
variable2,
variable3,
variable4
)
const x = (variable1 * variable2 * variable3) + variable4 -
(variable5 / variable6)
const bool = condition
? variable1
: variable2
const App = () => {
return (
<div
class='my-cool-class-name'
style='some: cool; styles: here;'
aria-label='accessibility stuff'
>
Text goes here
</div>
)
} Given these cases (which if they're not enforced yet, I'd like to eventually enforce) I think it's more consistent to indent in this case as well. |
🗣 Standard 11 is released! Run Changelog: https://github.com/standard/standard/blob/master/CHANGELOG.md#1100---2018-02-18 |
I propose adding
{ "MemberExpression": 1 }
to theindent
option.Example of incorrect code:
Examples of correct code:
A small change, but adds more consistency.
Rule: http://eslint.org/docs/rules/indent#memberexpression
The text was updated successfully, but these errors were encountered: