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 and brace-style allman #5064

Closed
ChristianAuth opened this issue Jan 25, 2016 · 9 comments

Comments

Projects
None yet
4 participants
@ChristianAuth
Copy link

commented Jan 25, 2016

Hi,

i am trying to use indent with the allman brace style but the results are a bit ugly.

Source:

    /**
     * @inheritDocs
     */
    get help()
    {
        let help =
        { /* Line 54 */
            name: this._name,
            description: 'Provides the development server',
            actions:
            [
                {
                    name: 'start',
                    options: {},
                    description: 'Start the server'
                }
            ]
        };
        return help;
    }

Linting result:

  54:9   error  Expected indentation of 12 space characters but found 8   indent
  55:13  error  Expected indentation of 16 space characters but found 12  indent
  56:13  error  Expected indentation of 16 space characters but found 12  indent
  57:13  error  Expected indentation of 16 space characters but found 12  indent
  58:13  error  Expected indentation of 16 space characters but found 12  indent
  64:13  error  Expected indentation of 16 space characters but found 12  indent
  65:9   error  Expected indentation of 12 space characters but found 8   indent

Eslint --fix source:

    /**
     * @inheritDocs
     */
    get help()
    {
        let help =
            {
                name: this._name,
                description: 'Provides the development server',
                actions:
                [
                {
                    name: 'start',
                    options: {},
                    description: 'Start the server'
                }
                ]
            };
        return help;
    }

Eslint config:

module.exports = {
    "rules": {
        "indent": [
            2,
            4
        ],
        "quotes": [
            2,
            "single"
        ],
        "linebreak-style": [
            2,
            "unix"
        ],
        "semi": [
            2,
            "always"
        ],
        "no-var": 2,
        "brace-style": [
            2,
            "allman"
        ],
        "no-unused-vars": [
            2,
            { "args": "none" }
        ]
    },
    "env": {
        "es6": true,
        "node": true
    },
    "extends": "eslint:recommended"
};

Eslint version: v1.10.3

Is this a expected behaviour or did i misconfigure something?

Cheers
Chris

@eslintbot eslintbot added the triage label Jan 25, 2016

@eslintbot

This comment has been minimized.

Copy link

commented Jan 25, 2016

@ChristianAuth Thanks for the issue! If you're reporting a bug, please be sure to include:

  1. The version of ESLint you are using (run eslint -v)
  2. What you did (the source code and ESLint configuration)
  3. The actual ESLint output complete with numbers
  4. What you expected to happen instead

Requesting a new rule? Please see Proposing a New Rule for instructions.

@ilyavolodin ilyavolodin added bug rule evaluating and removed triage labels Jan 25, 2016

@ilyavolodin

This comment has been minimized.

Copy link
Member

commented Jan 25, 2016

As far as I know, this is expected behavior. Opening and closing braces currently require indentation. @gyandeeps I don't think we have a switch for this behavior, correct?

@gyandeeps

This comment has been minimized.

Copy link
Member

commented Jan 25, 2016

Since there is a variable declaration on line 53 thats why line 54 needs to be indented.
Can you please try this config: "indent": [2, 4, {"VariableDeclarator": 0}]

@ChristianAuth

This comment has been minimized.

Copy link
Author

commented Jan 25, 2016

Sure,

Result:

  59:17  error  Expected indentation of 12 space characters but found 16  indent
  60:21  error  Expected indentation of 16 space characters but found 20  indent
  61:21  error  Expected indentation of 16 space characters but found 20  indent
  66:21  error  Expected indentation of 16 space characters but found 20  indent
  67:17  error  Expected indentation of 12 space characters but found 16  indent
  68:17  error  Expected indentation of 12 space characters but found 16  indent
  69:21  error  Expected indentation of 16 space characters but found 20  indent
  70:21  error  Expected indentation of 16 space characters but found 20  indent
  75:21  error  Expected indentation of 16 space characters but found 20  indent
  76:17  error  Expected indentation of 12 space characters but found 16  indent

Line 59 is the opening bracket of the array

            actions:
            [ /* Line 59 */
                {
@gyandeeps

This comment has been minimized.

Copy link
Member

commented Jan 25, 2016

Can you provide complete code for your example so that I can run it on my side?

@ChristianAuth

This comment has been minimized.

Copy link
Author

commented Jan 25, 2016

Full Source:

'use strict';

/**
 * Requirements
 * @ignore
 */
let BaseCommand = require('./BaseCommand.js').BaseCommand;
let Context = require('../application/Context.js').Context;
let CliWriter = require('../cli/CliWriter.js').CliWriter;
let js = require('../gulp/task/js.js');
let gulp = require('gulp');


/**
 * @memberOf command
 */
class JsCommand extends BaseCommand
{
    /**
     */
    constructor(context, cliWriter)
    {
        super(context, cliWriter);

        // Assign options
        this._name = 'js';
    }


    /**
     * @inheritDoc
     */
    static get injections()
    {
        return { 'parameters': [Context, CliWriter] };
    }


    /**
     * @inheritDocs
     */
    static get className()
    {
        return 'command/JsCommand';
    }


    /**
     * @inheritDocs
     */
    get help()
    {
        let help =
        {
            name: this._name,
            description: 'Compiles and optimizes js files',
            actions:
            [
                {
                    name: 'compile',
                    options:
                    {
                        '--site': 'Restrict processing to a specific site',
                        '--environment': 'Specifies the build environment.'
                    },
                    description: 'Compiles all js files for the given site(s)'
                },
                {
                    name: 'watch',
                    options:
                    {
                        '--site': 'Restrict processing to a specific site',
                        '--environment': 'Specifies the build environment.'
                    },
                    description: 'Watches for changes and compiles all js files for the given site(s) when necessary'
                }
            ]
        };
        return help;
    }


    /**
     * @inheritDocs
     * @returns {Promise<Server>}
     */
    compile(parameters)
    {
        let scope = this;
        return new Promise(function(resolve, reject)
        {
            scope.cliWriter.startSection('Js: compile');
            gulp.start(js.compileTask);
            gulp.on('stop', function()
            {
                scope.cliWriter.endSection();
                resolve();
            });
        });
    }


    /**
     * @inheritDocs
     * @returns {Promise<Server>}
     */
    watch(parameters)
    {
        let scope = this;
        return new Promise(function(resolve, reject)
        {
            scope.cliWriter.startSection('Js: watch');
            gulp.start(js.watchTask);
            resolve();
        });
    }


    /**
     * @inheritDocs
     * @returns {Promise<Server>}
     */
    doExecute(parameters)
    {
        if (parameters.action === 'watch')
        {
            return this.watch(parameters);
        }
        return this.compile(parameters);
    }
}


/**
 * Exports
 */
module.exports.JsCommand = JsCommand;

eslint:

module.exports = {
    "rules": {
        "indent": [2, 4, {"VariableDeclarator": 0}],
        "quotes": [2, "single"],
        "linebreak-style": [2, "unix"],
        "semi": [2, "always"],
        "no-var": 2,
        "brace-style": [2, "allman"],
        "no-unused-vars": [2, { "args": "none" }]
    },
    "env": {
        "es6": true,
        "node": true
    },
    "extends": "eslint:recommended"
};

Thanks for looking into it.

@gyandeeps gyandeeps added accepted and removed evaluating labels Jan 25, 2016

@gyandeeps

This comment has been minimized.

Copy link
Member

commented Jan 25, 2016

Its a bug, We will get it fixed. Thanks for working with us.

image

Its not happy with { when the parent [ is aligned according to allman style and these tokens fall under a variable declarator node.
Config: "indent": [2, 4, {"VariableDeclarator": 0}]

@ChristianAuth

This comment has been minimized.

Copy link
Author

commented Jan 25, 2016

Just tested your commit and it works quite well. Thanks for taking the time.

@gyandeeps

This comment has been minimized.

Copy link
Member

commented Jan 25, 2016

👍

ilyavolodin added a commit that referenced this issue Jan 25, 2016

Merge pull request #5069 from eslint/issue5064
Fix: Indent rule for allman brace style scenario (fixes #5064)

@eslint eslint bot locked and limited conversation to collaborators Feb 6, 2018

@eslint eslint bot added the archived due to age label Feb 6, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.