uber / standard

JavaScript Standard Style — One Style to Rule Them All
MIT License
70 stars 8 forks source link

change indent to 2 #27

Closed chrisirhc closed 9 years ago

chrisirhc commented 9 years ago

It looks like this is the typical configuration. Users can then copy this to .eslintrc without worrying about having conflicting configurations.

I was having an issue where uber-standard was picking up my local ~/.eslintrc which was taken from this repository, however, the indent is always overridden to [2,2] in the index.js.

I'm not even sure [2, 4] is a sane configuration. It requires 4-space indent in functions but 2-space indents otherwise.

Raynos commented 9 years ago

:-1:

The typical configuration is 4.

chrisirhc commented 9 years ago

Is that right? Running uber-standard gives a bunch of warnings if I indent 4 spaces within a function.

Raynos commented 9 years ago

that's weird. It should not do so. For my v3.7.1 respects 4 spaces for everything and works fine.

Note that I do not have an .editorconfig. @malandrew I actually dislike uber-standards "feature" where it non-deterministically reads your editorconfig and has different behaviour.

@chrisirhc try deleting the editorconfig

chrisirhc commented 9 years ago

Without a ~/.eslintrc file, I ran uber-standard with the a file with the following contents and the indentation is valid:

'use strict';

var x = {
  test2: function test2() {
      return 1;
  }
};

function myApp() {
    // test
    return x;
}

module.exports = myApp;

Are we going with a 4 space indentation? I see https://github.com/uber/standard/blob/master/index.js going with 2 space indentation, as well as many other components.

From what I heard that's also what the linters configured with Phab are checking for, 2-space indentations.

Raynos commented 9 years ago

@chrisirhc it's a bug that having a two space indent for an object is valid; that's really weird.

uber-standard is going with 4 spaces; what other people do is not relevant.

andrewdeandrade commented 9 years ago

Indentation is 4 spaces by default. When lint-trap (uber-standard precursor) was written, the overwhelming majority of lines of javascript at the company was 4-space indented. @raynos I too dislike the .editorconfig feature. At the time when lint-trap was written, I wanted to have all the code everywhere use one indentation size, 2 or 4, so long as it was the same everywhere. However, that was one holy-war I had no desire to participate in so we chose a default and gave people a way to override the default. Trying to enforce one single indentation size would have jeopardized getting lint-trap adopted at all. We lost that battle to win the war. Because lint-trap used eslint, jshint and jscs, we didn't go with .eslintrc files since that would only satisfy one linter.

You have two ways of overriding the indentation:

.eslintrc file:

{
    "rules": {
        "indent": [
            2,
            2
        ],
        "max-len": [
            2,
            80,
            2
        ],
    }
}

or .editorconfig file:

# EditorConfig: http://editorconfig.org

root = true

[*]
indent_style = space
indent_size = 2
end_of_line = lf
charset = utf-8
trim_trailing_whitespace = true
insert_final_newline = true

The advantage of the .editorconfig file is that you can install the .editorconfig plugin for your text editor and have some lint issues fixed for you automatically.

@chrisirhc A good question is why are you using an .eslintrc file copied from this repository. You shouldn't have an eslintrc file at all unless you have a good reason to overwrite 1-2 rules.

This line https://github.com/uber/standard/blob/master/index.js#L38 should only overwrite the indentation for the baseConfig that standard loads. The eslintrc file from your project should overwrite the default indentation. Are you certain that you're loading eslintrc file correctly? Is this happening with npm run lint or does this only happen with arc lint and arc diff?

Regarding object literal indentation, that is a known bug: https://github.com/eslint/eslint/issues/2278

We might look into using the indent rule from the nodeca plugin to get around it. https://github.com/nodeca/eslint-plugin-nodeca/blob/master/lib/indent.js