google / eslint-config-google

ESLint shareable config for the Google JavaScript style guide
Apache License 2.0
1.74k stars 140 forks source link

Fix: Update the indent rule #41

Closed stramel closed 7 years ago

stramel commented 7 years ago

Pulled from lighthouse eslintrc file.

philipwalton commented 7 years ago

What's missing from this is the Google style suggestion to indent 4 spaces for line continuations. AFAIK there's no config that supports this currently in this rule, and I wouldn't want to turn on something that reports an error for code that is valid according to the Google style guide.

For example, this is valid:

methodWithAReallyReallyLongNameThatTakesMultipleParams('foo', 'bar',
    function() { // indent 4 spaces to show that this is a continuation.
  doSomething(); // indend 2 spaces as normal within a block.
});
stramel commented 7 years ago

@philipwalton Is that the suggested spacing on a function inlined into the call expression? Or should it be like this?

methodWithAReallyReallyLongNameThatTakesMultipleParams('foo', 'bar',
    function() { // indent 4 spaces to show that this is a continuation.
      doSomething(); // indend 2 spaces as normal within a block. (6 total spaces)
    }); // indent 4 spaces 

I'm wondering if this would get us closer to the rules for indent.

'indent': [2, 2, {
  SwitchCase: 1,
  VariableDeclarator: 2,
  CallExpression: { arguments: 2 },
  FunctionExpression: { body: 1, parameters: 2 },
}],
philipwalton commented 7 years ago

Both your example and my example are correct, and I don't think there's a config that supports both.

I'm not able to test it right now, but if there's a config that doesn't report errors or warnings for these, then I think it's ok. If it does report warnings then I wouldn't want to merge.

stramel commented 7 years ago

Alright, well, I believe this will throw warnings. I will close until I can find a solution that better fits all cases. Thanks for your help!