Allow comment of a "else[ if]" block to be dedented

[clementprevot]

Description

When writing if...else block with 1TBS style, it's hard to comment the else (or else if) comment as the comment for the else block is indented at the same level than the if block content.

Input

The code looked like this before beautification:

// In this case
if (foo) {
    // Do something

// Else do something else
} else {
    // Do something else
}

Expected Output

The code should have looked like this after beautification:

if (foo) {
    // Do something

// Else do something else
} else {
    // Do something else
}

Actual Output

The code actually looked like this after beautification:

if (foo) {
    // Do something

    // Else do something else
} else {
    // Do something else
}

Environment

OS: Linux Mint 17

Settings

{
    "indent_size": 4,
    "indent_char": " ",
    "eol": "\n",
    "indent_level": 0,
    "indent_with_tabs": false,
    "preserve_newlines": true,
    "max_preserve_newlines": 5,
    "jslint_happy": false,
    "space_after_anon_function": false,
    "brace_style": "collapse-preserve-inline",
    "keep_array_indentation": true,
    "keep_function_indentation": true,
    "space_before_conditional": true,
    "break_chained_methods": false,
    "eval_code": false,
    "unescape_strings": false,
    "wrap_line_length": 130,
    "wrap_attributes": "auto",
    "wrap_attributes_indent_size": 4,
    "end_with_newline": true,
    "operator_position": "before-newline"
}

[bitwiseman]

Because of #200, the // Else do something else comment is printed before the beautifier knows about the close brace and unindent. We could solve this one case by look ahead, but it would leave so many others - big bug farm.

There might be a more general hack/rule, where we special case any group of comments that occur on the lines directly before an element.

/* block comment */
if (foo) {
    do_something();
// FIXED? - These comments currently stay indented
// this comment set has a blank line after it
// it might be reasonable for it to stay indented
// That is harder to implement, maybe a separate change
/* block comments follow the same behavior */

// FIXED - These comments currently stay indented
// this set, has no blank lines after it so it definitely should stick to the element below
} else if (bar) {
    // this set, already behaves correctly
    do_something_else();  /* Mutliple */ /* inline */ // and same line comments stays here
// FIXED - These comments currently stay indented
// this set, has no blank lines after it so it sticks to the element below
/* block comments
 * are not special
 */
} else if (baz) {
    /* still correct */
    do_something_else2(); // and same line comments stays here

    // These comments stay indented
    // The blank line after between them and the next element means they stay here

} else {
    /* still correct */
    do_something_else3(); // and same line comments stays here

// FIXED - These comments currently stay indented
// This is odd, but is what the rule would make happen.  
// Of course, this is also an odd place to put comments
}
next_step();

/* let's talk about unbraced */
if (foo)
    do_something();
// without a brace, this unindents because the if statement ended at the semicolon.

// Else do something else
// this set, has no blank lines but is already unindented
else if (bar)
    // FIXED - These comments currently unindent
    // would stick to the element below
    do_something_else();  /* Mutliple */ /* inline */ // and same line comments stays here
// without a brace, this unindents because the if statement ended at the semicolon.
// this set, has no blank lines after but is already unindented
// but it leaves the inline comments where they are
/* block comments
 * are not special
 */
else if (baz)
    /* FIXED - These comments currently unindent, now stick */
    do_something_else2(); // and same line comments stays here

// without a brace, this unindents because the if statement ended at the semicolon.
// The blank line after between them and the next element makes no difference

else
    /* FIXED - These comments currently unindent 
     * would stick to the element below
     */
    do_something_else3();  // same line comment stays here

// This no longer looks odd. And is a reasonable place for a comment. 
next_step();

/* let's talk about unbraced inline */
if (foo) do_something();
// without a brace, this unindents because the if statement ended at the semicolon.

// Else do something else
// this set, has no blank lines after it so it sticks to the element below
else if (bar)  /* inline is fine */ do_something_else();  /* Mutliple */ /* inline */ // and same line comments stays here
// without a brace, this unindents because the if statement ended at the semicolon.
// this set, has no blank lines after but is already unindented
// but it leaves the inline comments where they are
/* block comments
 * are not special
 */
else if (baz) do_something_else2(); // and same line comments stays here

// without a brace, this unindents because the if statement ended at the semicolon.
// The blank line after between them and the next element makes no difference

else do_something_else3();  // same line comment stays here

// This no longer looks odd. And is a reasonable place for a comment. 
next_step();

This behavior would be straightforward to implement and basically understandable. Might fix a whole class of bugs.

[bitwiseman]

Gah, the problem is achieving this without better parsing. So many places that can go wrong. Wishful thinking.

[gagern]

I disagree with the original request, at least if it is not configurable. I think indented end-of-block comments make a lot of sense in many situations.

if (modulus === 2) {
    // i might be odd here
    i += (i & 1);
    // now i is guaranteed to be even
} else {
    // rounding up using integer arithmetic only
    if (i % modulus)
        i += modulus - (i % modulus);
    // now i is divisible by modulus
}

Here I'm using an end-of-block comment to state a post-condition. The comment doesn't directly describe any particular statement or block, but instead describes the state of some object at a specific point during the execution of the code. As such, it should be indented like a statement, not like the closing parenthesis.

If you can find a way to distinguish between these two use cases, that might be fine. I can imagine a global switch somewhere. Making the distinction based on the presence or absence of empty lines somewhere is bound to confuse people, though, so I'd rather avoid that.

[bitwiseman]

@gagern - Yeah, this is a particularly difficult problem to land universally.

I agree with counter-examples. Those should remain where they are.

I think about it as answering the question "What element is this block of comments attached to?" So, we need to define "block of comments" and then what factors we can use to answer what element each block attaches to. Those factors include, type of previous and following elements, whether a statement/expression ended after the previous or before the following element, and what newlines were present between each block and the previous/following elements.

No matter what answer we choose it will always before wrong in some cases. Indentation of other element should not change based on presence or absence of comments except where newlines at the end of comments change semantics. Beyond that the best we can do is make the behavior predictable and consistent.

So, what do you think of this:

if (modulus === 2) {
    // i might be odd here
    i += (i & 1);
    // now i is guaranteed to be even
    // this block is obviously about the statement above

// this comment is about the block after it.
} else {
    // rounding up using integer arithmetic only
    if (i % modulus)
        i += modulus - (i % modulus);
    // now i is divisible by modulus
    // behavior of comments should be different for single statements vs block statements/expressions
    // without the braces it is hard to decide what to do here, but unindent to block level seems reasonable
}

// Consider the following
if (modulus === 2)
    // i might be odd here
    i += (i & 1);    
    // now i is guaranteed to be even
    // given the newlines structure here, I think we'd keep this indentation.
    // But I'm sure there are counter examples.

// this comment is about the block after it.
else
    // rounding up using integer arithmetic only
    if (i % modulus)
        i += modulus - (i % modulus);
// behavior of comments should be different for single statements vs block statements/expressions
// without the braces it is hard to decide what to do here.
// following the example above this should fully unindent?  Might depend on what follows...

[gagern]

Your example looks good from the point of reading the code, but if code written that way got actually indented to this style, I as a writer of code vould still be very surprised. Personally I'd not expect anything in {…} to get dedented, nor would I expect comments trailing an unbraced statement to maintain the indentation. I guess you'd make the distinctions between these cases based on empty lines, but I consider that again to be a source of a lot of confusion.

If you are looking at empty lines, you are essentially looking at the whitespace before beautification. Can't you look at original indentation, too? Note that if the comment is indented like the closing brace, it should stay that way even if both of them now get indented differently. And if the comment is indented like some of the levels of unbraced control constructs, then it can stay with that level. That way, users could control correct indentation simply by indenting correctly, which is a very intuitive approach I think. And js-beautify would know that in some situations several indentations are possible, so it checks all of them in a relative context but picks a preferred one if none of the alternatives matches the input.

[bitwiseman]

Using indents is an interesting idea. Thanks!