Implement maxLineWidth

[lucaswerkmeister]

Okay, so here's the plan:

Obviously, we can't have the visitX methods write directly to the writer. Instead, there needs to be a method that takes a String (or maybe a String|Token) and buffers these tokens until it can write some of them out to the writer to finish a line. As a second parameter, this method should take the indentLevel that this token should get if it would be the first of its line. The decision where to break the line would also be based on this. And there would be two more parameters: named wantsSpaceBefore and wantsSpaceAfter or something like that, both of type Boolean?, where wantsSpaceAfter takes precedence over wantsSpaceAfter if both are specified for any gap.

[lucaswerkmeister]

Changes;

• The caller of the write method can't know which absolute indentLevel he needs to give when he has no idea what the current indentLevel is! Instead, the parameter needs to be relative.
• Also, it should really be two parameters: preIndent and postIndent. For example, methodName( has a preIndent of 0 and a postIndent of 1.
• Also, they should be optional, to indicate a token that must not be broken into a new line (;, (, ...).

[lucaswerkmeister]

We might also need a better priority model than "spaceAfter always overrides spaceBefore"... this probably means making them Integers instead of Booleans, but that sounds like the corner cases of unforeseen collisions will haunt me.

[lucaswerkmeister]

Wait, that’s still not enough. Consider this:

shared actual void visitPositionalArgumentList(PositionalArgumentList that) {
    writeToken(that.mainToken, null, 1, false, false); // "("
    variable Boolean hasFirst = false;
    for (PositionalArgument argument in CeylonIterable(that.positionalArguments)) {
        if (hasFirst) {
            writer.write(", ");
        }
        argument.visit(this);
        hasFirst = true;
    }
    writeToken(that.mainEndToken, null, ???, false, true); // ")"
}

What’s the postIndent of the right parenthesis? It depends: If there was a line break after the left parenthesis and its postIndent of 1 was introduced, the right parenthesis needs to undo that with a postIndent of 1 – but if there wasn’t, the right parenthesis needs a postIndent of 0 instead.

This will probably result in writeToken returning some indentationContext that can later be close()d.

[lucaswerkmeister]

And with this we’re probably at the point where the visitor and the writer should be two separate classes.

[lucaswerkmeister]

Actually, I don’t think the indentationContext should have a close() method; instead, writeToken should take it as an optional argument (defaulting to null):

value context = writeOut(that.mainToken, ...);
...
writeOut(that.mainEndToken, ..., context);

[HenningB]

Just to add a small personal preference: I set my line lengths to infinity for Java formatters, as I feel arbitrary newlines in my code just messes up the whole readability. I consider a newline at 80, 120, 160 arbitrary, if only my code needs 89, 135, 168 chars at this line.

I know, we're might get into religious fights here. Just to consider this in an upcoming formatter, as screen sizes are becoming bigger, and no sane person is using vi to write Ceylon code (well, okay, as long Ceylon isn't scriptable).

Ceylon probably gets very long lines with its higher order functions, so some (arbitrary) newlines might be required :( . Or something half-dynamic with some logic, that puts all arguments in the next line if it crosses the 80-char boundary?

Ich weiß, Wunschkonzert.

[lucaswerkmeister]

I set my line lengths to infinity for Java formatters

Already specified :) Not implemented yet, but specified.

EDIT: removed something that made no sense at all

[lucaswerkmeister]

About the contexts, I think we also need a way to get a context without a token:

shared actual void visitStatement(Statement that) {
    value context = fWriter.acquireContext();
    that.visitChildren(this);
    fWriter.writeToken(that.mainEndToken, ..., context); // ";"
}

Of course, this can be achieved by writing an empty token (value context = fWriter.writeToken("", null, null, 0, 0)), but it would be much cleaner to have a distinct method for that.

[lucaswerkmeister]

Also, it should really be two parameters: preIndent and postIndent

Do we even need preIndent (or indentBefore, as it’s called now in the code)? I can’t think of an example where you’d need it, and without an example I can’t figure out how it should interact with closing the context of a preIndented token…

[lucaswerkmeister]

WOO! The FormattingWriter is almost finished; the only thing missing is, well, maxLineLength :D

I hope I’ve done a decent enough job at documenting the code, but here’s a quick and incomplete overview:

• the FormattingWriter has a tokenQueue, a tokenStack and an indentStack.
  • tokenQueue: all tokens that haven’t yet been written
  • also contains NoTokens (for acquireContext) and LineBreaks (for nextLine)
  • tokenStack: all the tokens that have been written, but not closed
  • indentStack: those tokens of tokenStack that stand at the end of a line (i. e., those whose postIndent is effective)
• tokens are written with the writeToken method; this will fast-forward the tokenStream until that token (enqueueing comments), enqueue that token, and then attempt to write out lines.
• currently, lines are written where there are LineBreaks in the tokenQueue (decided in tryNextLine); this is where the implementation of maxLineWidth will happen.
• when writing a line (writeLine method), the indentation is written (sum of all postIndents on the indentStack), and then each token is taken from the tokenQueue, written to the writer, and put onto the tokenStack, unless it closes a FormattingContext, in which case the context it closes and all its successors are popped from the two stacks.

[lucaswerkmeister]

crud. I think I’ve overlooked something. For example:

invokeThisMethod(firstArgument,
    secondArgument,
    thirdArgument);

Where does the indentation for secondArgument and thirdArgument come from? It can’t be the ,s, because then thirdArgument should be indented twice. No, it has to come from the ( – which means that having a separate indentStack is rubbish, the indentation comes from all tokens on the tokenStack.

[lucaswerkmeister]

also, I currently can’t close contexts that are still in the queue, which becomes visible as soon as all enqueued tokens influence indentation.

[lucaswerkmeister]

okay, here’s the changed plan: an internal version of closeContext takes a ClosingElement&QueueElement instead of a FormattingContext, and in addition to clearing the context from the tokenStack if present, it also goes through the tokenQueue and transforms all elements up to the argument by removing empty openings and changing OpeningTokens to NoContextTokens (name may vary).

[lucaswerkmeister]

okay, I implemented all that in 39b63b5. The context system is starting to grow complicated... I hope it really works this time. (NoContextTokens are simply called Tokens, because, well, why use an overly long name and introduce an extra class when you don’t need to?)

[lucaswerkmeister]

Sweet Jesus. It is done.

I still need a better LineBreakStrategy, but that’s a different issue.