eclipse-archived / ceylon.formatter

A formatter for the Ceylon programming language, written in Ceylon.
Apache License 2.0
14 stars 11 forks source link

newline before or after operator #66

Closed gavinking closed 10 years ago

gavinking commented 10 years ago

Consider:

        value grownCapacity 
                = (neededCapacity * growthFactor).integer 
                        + 1;

Or

        value grownCapacity 
                = (neededCapacity * growthFactor).integer + 
                        1;

The formatter lines the +1 or 1 up with value:

        value grownCapacity
                = (neededCapacity * growthFactor).integer +
        1;

There should be special handling for lines which end with/start with an operator.

lucaswerkmeister commented 10 years ago

This is because there’s nothing in the first part that introduces any indentation. The = ... is a specifier expressions, whose main tokens (=, =>) have an indentBefore – but these don’t stack.

Worse, the example

value html
        = Html {
    Body {};
}

still applies, so I need to investigate when to indent and when not to. The = should not indent in the general case.

Also, the added part can be a complex, multi-line expression. Does the indentation before/after the + stack to it?

lucaswerkmeister commented 10 years ago

This is what the IDE does:

void run() {
    String s = "a"
            + "b" +
            "c" +
            "d"
            + "e";
}

I suppose that’s how it should look… but I find the operators at the end of the line hideous. Maybe they should just be disallowed?

gavinking commented 10 years ago

Maybe they should just be disallowed?

huh? you're nuts :-)

lucaswerkmeister commented 10 years ago

If I disallow operators at the end of a line, and give operators a regular indentBefore, it could look like this:

void run() {
    String s = "a"
            + "b"
            + "c".pad {
        size = 1;
        character = ' ';
    };
}

The named arguments don’t inherit the +’s indentation… is that okay? Not sure.

gavinking commented 10 years ago

P.S. That code example looks hideous because it's so contrived. Consider:

        value newCapacity 
                = grownCapacity < neededCapacity || 
                  grownCapacity > maxArraySize
                  .... ;

Same thing with || instead of +.

gavinking commented 10 years ago

The named arguments don’t inherit the +’s indentation… is that okay?

Yes, it is. I for one like it that way.

lucaswerkmeister commented 10 years ago

Hm, good point for the ||. I could achieve the following using indentAfterOnlyWhenLineBreak:

void run() {
    String s1 = "a"
            + "b"
            + "c".pad {
        size = 1;
        character = ' ';
    };
    String s2 = "a" +
            "b" +
            "c".pad {
                size = 1;
                character = ' ';
            };
}

Note that in the second example, the +’s indentation is an indentAfter, and these do stack. Feels asymmetrical now :(

lucaswerkmeister commented 10 years ago

Ah no, I had this problem before (in #37), and introduced nextIndentBefore to solve it. So we could have

void run() {
    String s1 = "a"
            + "b"
            + "c".pad {
        size = 1;
        character = ' ';
    };
    String s2 = "a" +
            "b" +
            "c".pad {
        size = 1;
        character = ' ';
    };
}
lucaswerkmeister commented 10 years ago

So, to summarize, the main token of a BinaryOperatorExpression gets an indentBefore and nextIndentBefore of 2 each. I’ll add this tomorrow.

gavinking commented 10 years ago

Thanks :)

luolong commented 10 years ago

My personal preference is to align leading infix operators after a line break with the '=' of the assignment expression if possible. Like this:

value sumOfStuff = calculateSomeStuff(1)
                 + calculateSomeOtherStuff(2);

With trailing operators, the indentation should be aligned with first non-whitespace after the '=' sign.

Like this: value sumOfStuff = calculateSomeStuff(1) + calculateSomeOtherStuff(2);

Roland

So, to summarize, the main token of a BinaryOperatorExpression gets an indentBefore and nextIndentBefore of 2 each. I’ll add this tomorrow.

— Reply to this email directly or view it on GitHub https://github.com/ceylon/ceylon.formatter/issues/66#issuecomment-47287948 .

lucaswerkmeister commented 10 years ago

@luolong: could you please post your examples again as Markdown code segments? In the normal non-monospaced font, I have no idea what your indentation is supposed to be. (Responses via e-mail get no formatting at all, you’ll have to use the web interface for this.)

luolong commented 10 years ago

Sorry, I was on phone. The proper samples are these:

value sumOfStuff = calculateSomeStuff(1)
                 + calculateSomeOtherStuff(2);

and

value sumOfStuff = calculateSomeStuff(1) +
                   calculateSomeOtherStuff(2);
lucaswerkmeister commented 10 years ago

Unfortunately, that’s not possible at the moment, because the FormattingWriter supports only indentation, not alignment (#18).