preserve location of cons comment

[awalterschulze]

Fixes #278

[michalmuskala]

Wouldn't this be an issue for all operators? What makes | special?

[awalterschulze]

Yeah that is a good point.
We should probably make the same change for all operators.

[awalterschulze]

Well I guess what makes this case special is that it was reported by a whatsapp user.

It was impossible for the user to write:

[
    {an, element}
    % comment below
    | Rest
].

So they ended up with

[
    {an, element}
    |
    % comment below
    Rest
].

which seems pretty bad

[awalterschulze]

But as you say, this problem is also true for all binary operators.

[awalterschulze]

I started looking at {op, ...} and I think that would be better solved in a separate pull request. It is not as easy a fix as this one for cons_rest. Could we merge this in the mean time?

[michalmuskala]

I thought about this a bit longer and my question would be - do we need to support both positions, would it make sense to only indeed support what the user expected:

[
    Foo
    % comment
    | Bar
]

%% but no
[
    Foo
    |
    % comment
    Bar
]

The comment on a single line after single line | looks pretty weird. Similar how for regular operators that are formatted at the end of the line we only support having them before the right hand expression:

Foo +
    % comment
    Bar

%% but no

Foo
    % comment
    +
    Bar

The trailing operator case falls out naturally from how we've structured the code. It looks like we'd need to adjust how we emit comments for the "leading" operator case to match. But we wouldn't need to alter the AST:

cons_to_algebra(Head, Tail) ->
    HeadD = expr_to_algebra(Head),
    TailD = concat(<<"| ">>, maybe_wrap_in_parens(Tail, do_expr_to_algebra(Tail))),
    break(HeadD, combine_comments(Tail, TailD),).

[awalterschulze]

I can definitely try this and also look at how it affects other binary operators.

[awalterschulze]

@michalmuskala I don't know how you did it, but again this solution is much more elegant than mine.
I think this is ready to merge and solves the issue.