Should indentation always stack?

[lucaswerkmeister]

I just ran across the following issue:

void testFunctionArguments() {
    "abC".any((Character c) => c.integerValue > 10);
    variable Integer i = 1;
    "1 2 fizz 4 buzz fizz 7 8 fizz buzz 11 fizz 13 14 fizzbuzz".split().every((String s) {
        value j = i++;
        if (j % 15 == 0) {
            return s == "fizzbuzz";
        }
        if (j % 5 == 0) {
            return s == "buzz";
        }
        if (j % 3 == 0) {
            return s == "fizz";
        }
        return s == j.string;
    });
}

Looks good, right? The formatter begs to differ, and instead outputs:

void testFunctionArguments() {
    "abC".any((Character c) => c.integerValue > 10);
    variable Integer i = 1;
    "1 2 fizz 4 buzz fizz 7 8 fizz buzz 11 fizz 13 14 fizzbuzz".split().every((String s) {
            value j = i++;
            if (j % 15 == 0) {
                return s == "fizzbuzz";
            }
            if (j % 5 == 0) {
                return s == "buzz";
            }
            if (j % 3 == 0) {
                return s == "fizz";
            }
            return s == j.string;
        });
}

It looks almost the same, but the anonymous function (the fizzbuzz checker) is indented one more level. Why? Because both the ( of every and the { of the function open a formatting context, each with one level of indentation.

The question is now: Is that okay?

Arguments for fixing it: Well, if it looks broke, fix it! Duh. Arguments against fixing it: It would completely break my indentation model, and I don’t want to descend into the FormattingWriter yet again, and I hate it, and why can’t it be nice, and whine :-(

I can’t decide this issue myself, since I can’t possibly have a neutral point of view on this: I want nothing more than to say “WONTFIX”, and never have to change the architecture again, but if others think that this isn’t acceptable, I might have to fix it.

Therefore: request for comments! This has never worked before, but if you’re reading this, please please PLEASE leave a comment. Just tell me if the second code looks acceptable or not. Please?

[lucaswerkmeister]

Some more examples. How should this be indented?

void foo() {
fun((String s) {
return s + "hi";
},
"hi");
fun2(
"hi",
"hi");
}

Please take a few seconds to make up your mind. Then, look at these results:

• Ceylon IDE (Eclipse), Indent (Ctrl-I)

void foo() {
    fun((String s) {
        return s + "hi";
    },
    "hi");
    fun2(
        "hi",
        "hi");
}

• ceylon.formatter, default settings

void foo() {
    fun((String s) {
            return s + "hi";
        },
        "hi");
    fun2(
        "hi",
        "hi");
}

[lucaswerkmeister]

For the record, I really think that the formatter’s way is more regular – if you look at the example from my previous comment, the second argument to fun lost its indentation for no obvious reason.

[lucaswerkmeister]

Removed milestone. I would be very happy to keep the current behaviour indefinitely; if you disagree, speak up :) (again, request for comments)

[gavinking]

FTR I think the IDE handles this "better". I think it should have indent level 1. However, there is another closely-related case where the IDE is just broken. Consider:

    "A nonempty sequence containing the results of applying 
     the given mapping to the elements of this sequence."
    shared default actual [Result+] collect<Result>(
        "The transformation applied to the elements."
        Result collecting(Element element))
            => ArraySequence(populateArray(size,
            (Integer index) {
                value element = getFromFirst(index);
                if (exists element) { 
                    return collecting(element);
                }
                else {
                    assert (is Element null);
                    return collecting(null); 
                }
            }));

    "Return a nonempty sequence containing the given 
     [[elements]], followed by the elements of this 
     sequence."
    shared actual default [Other,Element+]
    withLeading<Other>(Other element)
            => [element, *this];

Correct Indentation completely breaks this.

[lucaswerkmeister]

But isn’t the IDE way irregular? It produces this:

value v = foo([[[[[a,
    b],
    c],
    d],
    e],
    f],
    g);

which completely hides that a, …, g are all on different nesting levels. When I start more complicated nesting,

value v = foo([[[[[a,
    b],
    c], [
    x,
    y,
    z
    ],
    d],
    e],
    [
    g,
    [
    h,
    i
    ],
    j
    ],
    f],
    g);

it’s incomprehensible in the IDE. The formatter instead produces

value v = foo([[[[[a,
                        b],
                    c], [
                    x,
                    y,
                    z
                ],
                d],
            e],
        [
            g,
            [
                h,
                i
            ],
            j
        ],
        f],
    g);

where the nesting levels are immediately visible.

(Of course, this example is very contrived, but I don’t think the general idea of it is too far-fetched.)

[lucaswerkmeister]

Note: with the resolution of #105, indentation in either direction can now stack only if it’s directly applied. This means that FormattingWriter now supports the IDE’s indentation mode, we’d just need to change FormattingVisitor to specify ifApplied instead of always for some indentations.