JavaScript callbacks (or similar) don't respect Allman style indentation

[mattbell87]

Currently when you write a callback or anything similar in CodeMirror it indents to the wrong place if you're using Allman style indentation. I've tested this with the latest development snapshot of Brackets and the demo editor on https://codemirror.net/.

Here's how to reproduce this issue. If you type the following JavaScript line:

$('element').click(function()

Then press enter and type a { bracket, it will indent like this:

$('element').click(function()
                   {

This also happens outside of callbacks in some cases, for example when I create a single instance object:

var obj = new (Class.extend
               ({

The desired outcome would be for the indentation of the bracket to match the previous line, for example:

$('element').click(function()
{
    ...
});

var obj = new (Class.extend
({
    ...
}));

JS Bin

http://jsbin.com/sojiqo/edit?html,js,output

[mattbell87]

I've also noticed a few other things about this bug:

• The bug specifically happens when you press the enter key (see the issue), it has already indented the line to the wrong spot before you start to type anything else
• In Brackets, if the file is saved as plain text (ie. no js extension) it works as it should, the bug doesn't occur
• It also happens when coding in most other programming languages

[baconbrad]

Just so you know there are complications to using Allman style curly braces with JavaScript. JavaScript engines have ASI (automatic semicolon insertion).

So

return
{
    a: "A",
    b: "B"
};

Gets interpreted as

return

[mattbell87]

I've personally never encountered that issue as I have never returned an object literal that way. There are workarounds and alternate coding methods if you needed to do something like that. But hey if you're working on a project with others the general rule is to respect the code formatting that the rest of the project uses, whether Allman or K&R (or other styles).

This isn't just JavaScript though. I tried coding a delegate in a C# file and it appears to happen in C# as well:

void Test()
{
    Action x = new Action(delegate()
                          {}
}

[mattbell87]

Looking at the source code it appears to be a problem with how it handles the Lexical smart indentation. When you perform the steps it returns at Line 680 on javascript.js, but I'm not sure if it should be returning there. I might be wrong but I think it should be treating it as though it were a plain text line so that it sets the column at the right place.

I commented out line 680 and it's closer to working (it would now be returning at 681). I performed the same steps as in the original post, but it's still off a bit. (Instead of indenting to the " ( " it now indents to the first " ' ").

  $('test').click(function()
    {

  });

Obviously commenting out code is not the way to fix this and I'd need a much clearer understanding of the code, but hopefully this makes it easier for someone to find where the problem might be.

[baconbrad]

Yup, you can return it using:

var obj =
{
    a: "A",
    b: "B"
};

return obj;

Just figured I would let you know. ASI is typically why Allman is not used in a lot of JavaScript projects. I am unsure if that factors into why CodeMirror handles it this way though.

[foo123]

No this is how the default js-mode in codemiror handles indentation, it uses the unix-convention hard-coded. Personaly i use Allman-style but for inline contexts (as in the return example above) i use unix-style. Which i think is best, since allman-style has several advantages, like makes code easier to read, delimits blocks very easily, can comment-out parts very easily and so on..

[marijnh]

I assume you don't want every line starting with a '(' or '{' to be deindented. The auto-indentation will need some way to figure out when to indent normally and when to dedent. Or, if you don't like auto-indentation (which you seem to repeatedly refer to as a 'bug'), just turn it off.

[mattbell87]

Auto/smart indentation is definitely not a bug, I actually really like it and I would like to leave it on. This only happens when coding anonymous function parameters. I don't see this action as a desired effect, thats why I'm calling this particular action a bug. :)

If you'd like, you could reproduce the steps in my original post on CodeMirror, then reproduce those same steps on ACE (which also auto-indents) and see the difference.

[mattbell87]

I added a JS Bin to simulate this. It could potentially be turned in to a unit test if desired. http://jsbin.com/sojiqo/edit?html,js,output

[marijnh]

I can see the issue. I am just not sure what behavior to encode in the mode to fix it. Ace's rules seem very simplistic, and will also de-indent this code:

foo(bar,
{baz: 10})

I'd say that's not an improvement.

[mattbell87]

Hey @marijnh,

If we are to say the formats for their respective styles are:

Allman:

foo(bar,
{
    baz: 10
});

1TBS / K&R:

foo(bar, {
    baz: 10
}

I think what should happen in the mode should be:

1. If you've just typed anything other than an opening bracket ( { [ and you hit enter, then type an opening bracket we assume Allman style and match the indentation from the previous line.
2. If you've just typed an opening bracket ( { [ and you hit enter, then we assume 1TBS style and we match the indentation from the previous line plus one extra tab (which is how it behaves already).

Also for a more compact style like the one you used:

foo(bar,
{baz: 10})

You could say: if all the brackets were closed on the second line, then indent one. So when you type the last ) it becomes:

foo(bar,
   {baz: 10})

I assume you'd want it to look like that.

I added a fourth test to the JS Bin based on this code :).

[marijnh]

if all the brackets were closed on the second line,

You'll usually want to make a decision about indentation before the line was entirely typed.

I can add a configuration parameter that enables this behavior for all braces and parens that start a line, if you want to. But if I were you I'd just start doing what the rest of the JavaScript community is doing instead.

[mattbell87]

I admit I prefer the look of Allman style braces because they look more clear to me, but I accept that 1TBS is used more than Allman in the JavaScript community and I'm fine with using it in other projects. I accept both styles have advantages and disadvantages and I always use the style that the project I'm coding on uses.

But this is not a JavaScript issue only, it does affect other languages as I demonstrated in C# in Brackets. Allman is more popular in the C# and Java communities. Because CodeMirror is being used in editors such as Brackets and potentially many more IDE's in the future, you'd want the project to support all different styles of coding to cater to different communities and personal preferences.

I think adding a configuration parameter is a great step :)

[marijnh]

Attached patch adds an allmanIndentation option to the clike mode. Note that to enable it you have to do gymnastics like mode: Object.create(CodeMirror.mimeModes["text/x-c"], {allmanStyle: {value: true}}) (or equivalent pre-ES5 junk) -- get the configuration object for a MIME type, and create a new object that has its properties but also the allmanStyle property set.