Blocks, arrays, and expressions over indented

[maksimr]

Problem with object indentation. Based on #268

Expected:

console.log({
  foo: 'foo'
});

Output:

console.log({
        foo: 'foo'
    });

[bitwiseman]

Excellent thanks! This is a different scenario from #268.

This is a good example of inconsistency caused by #200. In #256, the request is for the reverse of this, and we have trouble doing it. This issue is due to the fix I chose for #262.

Though I'm inclined to leave this as it is, I'm not opposed to changing the behavior to match what you expect here - I just want to make sure it is clear and predictable.

[downquark]

Asynchronous Module Definition (AMD) API example.
Expected:

define(["dojo/_base/declare", "my/Employee", "dijit/form/Button",
        "dojo/_base/lang", "dojo/Deferred"
], function (declare, Employee, Button, lang, Deferred) {
    return declare(Employee, {
        constructor: function () {
            new Button({
                onClick: lang.hitch(this, function () {
                    new Deferred().then(lang.hitch(this, function () {
                        this.salary * 0.25;
                    }));
                })
            });
        }
    });
});

Output:

define(["dojo/_base/declare", "my/Employee", "dijit/form/Button",
        "dojo/_base/lang", "dojo/Deferred"
    ], function (declare, Employee, Button, lang, Deferred) {
        return declare(Employee, {
                constructor: function () {
                    new Button({
                            onClick: lang.hitch(this, function () {
                                new Deferred().then(lang.hitch(this, function () {
                                            this.salary * 0.25;
                                        }));
                            })
                        });
                }
            });
    });

The extra indentation is partially caused by the fixes for #268 and #275, correct?

[bitwiseman]

No, the direct cause of this was #259. The fix for 241 was what got me looking at it. #268 and #275 address the (function (...) { ... }); scenario.

I can roll back #259, but I think it is generally valid - by default lines inside () should be indented. At the same time I understand the reason for the formatting you'd like to see, from the structure of the code I can totally see the appeal. I just want to make sure the deindented scenarios are clearly stated and well understood exceptions to the rule.

We've already gotten bugs regarding the indenting being wrong. Take a minute to look at the examples in #256. As you can see there, the code for var x = lines is a different code path which still tries to do what you're asking for above, but then tries to fall back to indenting when it encounters the second variable.

How I think I'd describe what you're requesting is that the beautifier minimally indent blocks, arrays, and expressions. We were under indenting them (some people were upset), now we're over indenting (some people are upset). What we really need is for the beautifier to start with a fully indented structure and then remove any extra indents.

Some more examples:

opts.preserve_newlines=true
opts.indent_size=4

// minimal indenting understands there is no potential for confusion
// due to under indenting "function() {...}" in this case 
new Deferred().then(lang.hitch(this, function () {
    this.salary * 0.25;
}));

// minimal indenting treats "{ ... }" as thought it were all on one line.
// So, the sibling otherParam does not need indenting 
// due to under indenting "function() {...}" in this case 
new Deferred().then(lang.hitch(this, function () {
    this.salary * 0.25;
}, otherParam));

// same thing here
new Deferred().then(lang.hitch(this, function () {
    this.salary * 0.25;
}), otherParam);

// same thing here, ordering doesn't change things
new Deferred().then(otherParam, lang.hitch(this, function () {
    this.salary * 0.25;
}));

// minimal indenting sees that "otherParam" is a on a new line 
//   (either by perserved newline or line wrap)
// it indents "function() {...}" to match this sibling parameter
new Deferred().then(lang.hitch(this, 
    otherParam, function () {
        this.salary * 0.25;
    }));

// like previous only more so.  
// otherParam is not a sibling of "function() {...}"
// since otherParam is on a new line, indenting must clearly shows this.
new Deferred().then(lang.hitch(this, 
        function () {
            this.salary * 0.25;
        }),
   otherParam);

Does this look right to you all?

[downquark]

What would this class definition look like with minimal indenting?

define(["dojo/_base/declare", "my/Foo", "dijit/form/Button", "dojo/_base/lang", "dojo/Deferred"
], function (declare, Foo, Button, lang, Deferred) {
    return declare(null, {
        obj: new Foo()
    });
});

[bitwiseman]

// assume 70 character word wrap and preserve newlines
// input - no newlines so defaults are added
define(["dojo/_base/declare", "my/Foo", "dijit/form/Button", "dojo/_base/lang", "dojo/Deferred"], function (declare, Foo, Button, lang, Deferred) { return declare(null, { obj: new Foo()});});

// output
define(["dojo/_base/declare", "my/Foo", "dijit/form/Button",
    "dojo/_base/lang", "dojo/Deferred"
], function (declare, Foo, Button, lang, Deferred) {
    return declare(null, {
        obj: new Foo()
    });
});

// input - new line at "function" keyword (not the start of a new block/array/object literal) would cause indent
define(["dojo/_base/declare", "my/Foo", "dijit/form/Button", "dojo/_base/lang", "dojo/Deferred"], 
function (declare, Foo, Button, lang, Deferred) { return declare(null, { obj: new Foo()});});

// output
define(["dojo/_base/declare", "my/Foo", "dijit/form/Button",
        "dojo/_base/lang", "dojo/Deferred"
    ], 
    function (declare, Foo, Button, lang, Deferred) {
        return declare(null, {
            obj: new Foo()
        });
    });

The basic rule would be when inside a block and a line starts with something other than a {}[]) then all sibling of that item (and transitively the children of those siblings) must indent.

[maksimr]

👍

Big thanks! It work for me.