Indentation after =

[lucaswerkmeister]

Currently, the formatter turns

value v =
    something;

into

value v =
something;

which is obviously wrong. However, if I simply give the = an indentAfter of 1, that will stack everywhere, which is wrong as well. Potential solutions:

1. disallow line breaks after =

Cheap solution, and I suppose I could live with it. The correct way to format the above would then be

value v
    = something;

2. indentAfter = 1, save context in a global variable and close it with next token

Ugly because I have to remember to close that context in a thousand places, and dangerous if I forget it in one place. Still, simple solution that requires no changes to the architecture.

3. add notion of “volatile” indentation to FormattingWriter

Basically the same thing as 2, except built into the FormattingWriter: a context marked volatile = true isn’t returned, but instead automatically closed on next writeToken. Problem: I no longer have the guarantee that there’s no other context to be closed, so Tokens probably have a set of contexts they close instead of one.

4. completely decouple context stacking and direction of indentation

Currently, indentBefore never stacks, indentAfter always does. I could add two flags stackIndentBefore, -After, and make false and true the defaults. = would then have stackIndentAfter = false. 3 is basically one half of this.

I’m leaning towards solution 3.

[lucaswerkmeister]

3.5 alternative implementation: add to indentBefore for next token

An alternative way to implement 3 would be to add a nextIndentBefore argument which is added to the indentBefore of the next token. Advantage: Works even if the next token closes a context, and the concept might be easier to explain / understand as it’s nearer to the original indentation (what you mean really is an indentBefore of the following token).

[lucaswerkmeister]

Problem: I no longer have the guarantee that there’s no other context to be closed, so Tokens probably have a set of contexts they close instead of one.

Actually, that’s not a problem at all, as closing a context also pops / closes all contexts on top of it – therefore, if the token has a context it closes and there is a “volatile” context to close as well, the context of the Token in the queue is the “older” of the two contexts. This means that 3.5’s

Advantage: Works even if the next token closes a context

isn’t valid.

[lucaswerkmeister]

I’m leaning towards 3.5 because it has the advantages of 3 – simple usage, only requires FormattingVisitor changes in one place – but seems a bit more familiar with the general architecture.

[gavinking]

@lucaswerkmeister have you reviewed how Correct Indentation handles this? It treats a line that starts with an = or =>, or follows a line which ends in an = or => as a "continuation" line, giving it a double indent. i.e. it does the same thing with = and => that it does with + and friends.

[lucaswerkmeister]

@gavinking This isn’t about lines that start with = – I have regular indentBefore for these (although it might be just 1 for =, I’ll have to check that). The issue is that indentation after a = normally stacks, so in the current architecture, I have to choose between

value v = foo(
        bar);
// two levels – `=` (unnecessary here) and `(`

or (current behavior)

value v =
foo(bar);
// no levels, because to avoid the above, `=` doesn’t add any indentation

while I really want

value v =
    foo(bar);
// one level

IIRC, the IDE currently produces

value v =
foo(bar);

[lucaswerkmeister]

Actually, writing these code samples got me thinking again. What about these:

value v
= foo(
bar);
value v =
foo(
bar);

With the solutions I proposed above, this would be formatted as

value v
        = foo(
    bar);
value v =
    foo(
    bar);

Is this right? Or should it be

value v
        = foo(
            bar);
value v =
    foo(
        bar);

which would mean that the indentation introduced by the = token – in one case before it, in the other case after it – does stack. But still,

value v = foo(
    bar);
// NOT
value v = foo(
        bar);

which would mean that the indentation after the = stacks iff there’s a line break after the token, and the indentation before the = stacks iff there’s a line breaks before the token. (Currently, indentation after a token always stacks, indentation before a token never stacks.)

[lucaswerkmeister]

FTR, the IDE currently yields

value v
        = foo(
    bar);
value v =
        foo(
    bar);

[gavinking]

I think the IDE's behavior is correct and desirable. I personally really like stuff like:

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

[gavinking]

Consider the alternatives:

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

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

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

I don't find any of these better, though, in fairness, the third would be OK.

[lucaswerkmeister]

I would have thought that the last alternative would be okay, but you’re right, in your code sample it makes a lot more sense to not stack. That’s the current behavior, so yay, no change necessary :D

This still leaves the question of what to do with

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

I think this should be

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

which would mean that the =>’s indentAfter should stack normally, but only if the => is the last token of its line.

(Note that breaking the line after the => only happens if it’s that way in the user code; if the original code doesn’t contain any line breaks, or if you use the formatter without a token stream, you get

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

instead.)

[lucaswerkmeister]

Hm, the IDE proposes

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

instead. @gavinking what do you think?

[lucaswerkmeister]

I’ve pushed the following solution to the issue-37-solution-5 branch:

5. indentAfter only stacks if the token is the last of its line

Adds a parameter to FormattingWriter.writeToken to only use the token’s indentAfter if the token is the last of its line. That is, in

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

the => opens a context, but in

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

it doesn’t.

@gavinking if you don’t object, I’ll merge this into master tomorrow-ish.

[gavinking]

That's fine, I guess, but it's going to be annoying that the formatter can't be made to produce the same indentation that Correct Indentation produces...

[lucaswerkmeister]

Okay, so option then. Names: indentationAfterSpecifierExpressionStart in { stack, addPreIndent } or something like that? I’ll add that tomorrow.