Indentation of `=`/`=>`

[lucaswerkmeister]

The indentation before and after =/=> of a specification, and the stacking of it, presents a problem that I’ve already tried to solve several times, but which keeps popping up – see, for instance, #26, #37, and #104. I think most of the complexity comes from the fact that we’re trying to handle both these cases:

1. The =/=> appears at the end of a line
2. The =/=> appears at the start of the next line

Within FormattingVisitor, when we have to emit the indentation, we can’t distinguish between these cases, since we don’t have any whitespace information.

I suggest that we add a new option to always choose one of these possibilities, and then optimize the indentation for that case.

I’ll probably flesh this out later, but I don’t have the time just right now.

[lucaswerkmeister]

After some more thinking, I think one problem is this code:

Html html
        => Html {
    head = ...;
    body = ...;
}

Observe that the fat arrow is indented, but that this indentation doesn’t stack.

That code and indentation was originally proposed by @gavinking in https://github.com/ceylon/ceylon.formatter/issues/37#issuecomment-37776262, and for some reason I decided to turn it into some kind of dogma that future changes to => indentation had to preserve.

I now think this is a pathological example of a fat arrow preceded by a line break; the much more common case, especially since the introduction of let and if expressions, is functions like this:

shared actual Boolean equals(Object that)
        => if (is This that)
        then this.thing == that.thing
        else false;

shared Float distance(Point p1, Point p2)
        => let (x1 = p1.x, y1 = p1.y, x2 = p2.x, y2 = p2.y)
            let (dx = (x2-x1).magnitude, dy = (y2-y1).magnitude)
                sqrt(dx^2 + dy^2);

function buildParents(String entry) 
        => let(parents = entry.split('/'.equals).filter(not(String.empty)).exceptLast)
            if (exists firstParent = parents.first)
            then
                if (nonempty restOfParents = parents.rest.sequence())
                then
                    restOfParents.scan(firstParent + "/")(
                        (path, nextParent) => "".join { path, nextParent + "/"})
                else { firstParent + "/"}
            else {};

And these functions, as @davidfestal noticed in #104, look hideous if the fat arrow’s indentation doesn’t stack:

shared actual Boolean equals(Object that)
        => if (is This that)
then this.thing == that.thing
else false;

shared Float distance(Point p1, Point p2)
        => let (x1 = p1.x, y1 = p1.y, x2 = p2.x, y2 = p2.y)
    let (dx = (x2-x1).magnitude, dy = (y2-y1).magnitude)
        sqrt(dx^2 + dy^2);

function buildParents(String entry) 
        => let(parents = entry.split('/'.equals).filter(not(String.empty)).exceptLast)
    if (exists firstParent = parents.first)
    then
        if (nonempty restOfParents = parents.rest.sequence())
        then
            restOfParents.scan(firstParent + "/")(
                (path, nextParent) => "".join { path, nextParent + "/"})
        else { firstParent + "/"}
    else {};

The html example above is much better written as:

Html html => Html {
    head = ...;
    body = ...;
}

Therefore: The indentation before the fat arrow stacks iff it was applied, i. e., it is the first token on its line. This will be supported after #103.

[lucaswerkmeister]

For = and => at the end of a line, I think we’ll keep solution 5 from https://github.com/ceylon/ceylon.formatter/issues/37#issuecomment-37866059: The indentation after the fat arrow stacks iff it was applied, resulting in:

Html html =
        Html {
            head = ...;
            body = ...;
        };

Html html = Html {
    head = ...;
    body = ...;
};

shared actual Boolean equals(Object that) =>
        if (is This that)
        then this.thing == that.thing
        else false;

shared Float distance(Point p1, Point p2) =>
        let (x1 = p1.x, y1 = p1.y, x2 = p2.x, y2 = p2.y)
            let (dx = (x2-x1).magnitude, dy = (y2-y1).magnitude)
                sqrt(dx^2 + dy^2);

function buildParents(String entry) =>
        let(parents = entry.split('/'.equals).filter(not(String.empty)).exceptLast)
            if (exists firstParent = parents.first)
            then
                if (nonempty restOfParents = parents.rest.sequence())
                then
                    restOfParents.scan(firstParent + "/")(
                        (path, nextParent) => "".join { path, nextParent + "/"})
                else { firstParent + "/"}
            else {};

I really like the look of these functions; it’s perhaps even better than with the => at the beginning of the line.

[ePaul]

shared actual Boolean equals(Object that)
        => if (is This that)
        then this.thing == that.thing
        else false;

Actually I would write it like this.

shared actual Boolean equals(Object that)
        => if (is This that)
           then this.thing == that.thing
           else false;

I.e. indent everything below the fat arrow the same as the next thing after the arrow, when it is on a new line.

[luolong]

I agree with @ePaul that this formatting makes a whole lot sense:

shared actual Boolean equals(Object that)
        => if (is This that)
           then this.thing == that.thing
           else false;

[lucaswerkmeister]

But that requires alignment (#18). The best I can currently do is this:

shared actual Boolean equals(Object that)
        => if (is This that)
            then this.thing == that.thing
            else false;

but I’m not sure that indentation looks good in other contexts, e. g.

Thing thing = makeThing(
    if (condition)
        then foo
        else bar
);

as opposed to

Thing thing = makeThing(
    if (condition)
    then foo
    else bar
);

On the other hand, switch expression cases already get such indentation, so clearly I didn’t think this through very well. I guess either then/else and case should be indented in the respective expressions, or none of them.

[luolong]

this is better:

Thing thing = makeThing(
    if (condition)
    then foo
    else bar
);

[lucaswerkmeister]

This is nearly done in branch orthogonalizeStacking; FormattingWriter now supports “indentation stacks iff it was applied”, which we need for both directions as discussed in https://github.com/ceylon/ceylon.formatter/issues/105#issuecomment-105999834 and https://github.com/ceylon/ceylon.formatter/issues/105#issuecomment-107502446. The formatter can now produce the following indentation:

Boolean f1() =>
    let (a = 1, b = 2)
        if (a+b == 3)
        then true
        else false;

Boolean f2()
        => let (a = 1, b = 2)
            if (a+b == 3)
            then true
            else false;

Boolean f3() => bool {
    par1 = true;
    par2 = false;
    Boolean par3()
            => true;
    Boolean par4() =>
        false;
};

There’s one snag left though. Currently, there’s the option indentationAfterSpecifierExpressionStart, with the following possible values (code formatting as of ceylon.formatter 1.2.0):

• stack:

  Html html =>
    Html {
        head = nothing;
        body = nothing;
    };
  
  Boolean f1() =>
    let (a = 1, b = 2)
        if (a+b == 3)
        then true
        else false;

• addIndentBefore:

  Html html =>
        Html {
    head = nothing;
    body = nothing;
  };
  
  Boolean f1() =>
        let (a = 1, b = 2)
    if (a+b == 3)
    then true
    else false;

For comparison, the Eclipse IDE’s Correct Indentation action produces:

Html html =>
        Html {
    head = nothing;
    body = nothing;
};

Boolean f1() =>
        let (a = 1, b = 2)
if (a+b == 3)
then true
else false;

which, as you can see, is the same general behavior as --indentationAfterSpecifierExpressionStart=addIndentBefore: the indentation after the => doesn’t stack. This is why addIndentBefore is currently the default for that option.

However, I don’t like that at all. Leaving aside the fact that addIndentBefore is now quite misnamed, I just don’t like the look of it, and don’t think we need to support it. So unless someone objects (@gavinking?), I think I’ll just remove that option entirely and make indentation in both directions stacked iff applied, as discussed in the two comments linked above. (In the orthogonalizeStacking branch, you currently get that behavior with --indentationAfterSpecifierExpressionStart=stack.)

[FroMage]

Wait, can it align parameter declarations too?

void method(A a,
            B b){}

[lucaswerkmeister]

No, that would be another part of #18.

[lucaswerkmeister]

Option removed in 16977ce.

Remaining tiny question: what’s the indentation after a = or =>? One level or two levels?

[lucaswerkmeister]

Leaving it at one level for now. Done!