WhatsApp/erlfmt

Compatibility with Emacs erlang-mode

zuiderkwast opened this issue ยท 10 comments

"We, the OTP team have the Emacs mode as the standard for indentation." (Kenneth Lundin, head of OTP, in a comment erlang/otp#2451 (comment))

Note that Emacs does indentation only; no other formatting. Given its status, I think it would be good to be compatible.

Example 1: In your PR on OTP to test erlfmt, you indented a function in dialyzer.erl like this:

cl(Opts) ->
    F = fun() ->
        {Ret, _Warnings} = dialyzer_cl:start(Opts),
        Ret
    end,
    doit(F).

... although Emacs and most Erlang code that I've seen (within Ericsson and outside) would indent 'end' under the corresponding 'case', 'fun', 'receive' or 'try', i.e. like this:

cl(Opts) ->
    F = fun() ->
            {Ret, _Warnings} = dialyzer_cl:start(Opts),
            Ret
        end,
    doit(F).

Example 2: The multi-line function call in the REAMDE is indented like this by Emacs:

    scenario(
      dial_phone_number(),
      ring(),
      hello(mike),
      hello(joe),
      hello(robert),
      system_working(),
      seems_to_be()
     ).

Though, to be honest, I've never seen the above in a real project. It's more common to start the first arg (or list element, tuple element, etc.) on the same line and then Emacs indents it like this:

    scenario(dial_phone_number(),
             ring(),
             hello(mike),
             hello(joe),
             hello(robert),
             system_working(),
             seems_to_be()).

Thank you for reporting.

erlfmt does not align any text on arbitrary column numbers. It only uses indentation.
It does this to avoid big diffs resulting from renaming a variable or function name.

Sure, I understand the problems with alignment and that most (at least most non-functional) languages avoid alignment. Unfortunately, this decision makes it hard to use this formatter when contributing to projects using the OTP style.

Just an idea: If you insert a linebreak before fun, case, if, receive in these cases, it will be compatible with the OTP style without requiring alignment. Example:

    SomeVariable =
        case Expr of
            Pattern1 -> Value1;
            Pattern2 -> Value2
        end,
    do_stuff(SomeVariable).

@zuiderkwast we did that on rebar3_format's default_formatter but we had to adjust that later to contemplate this scenario:

C = case small:var() of
        that -> you;
        dont -> want;
        to -> indent
    end

@zuiderkwast thank you for the case example, this shows how erlfmt and aligning can be compatible, in a way that I had not previously considered.
If we apply this to fun, receive, etc. as you say and preserve newlines after = we can create more places where aligning and erlfmt are compatible, for example, currently only the first test passes, but we can consider making the second one pass, what do you think @michalmuskala ?

     ?assertSame(
        "cl(Opts) ->\n"
        "    F = fun() ->\n"
        "        {Ret, _Warnings} = dialyzer_cl:start(Opts),\n"
        "        Ret\n"
        "    end,\n"
        "    doit(F).\n"
    ),
    ?assertSame(
        "cl(Opts) ->\n"
        "    F =\n"
        "        fun() ->\n"
        "            {Ret, _Warnings} = dialyzer_cl:start(Opts),\n"
        "            Ret\n"
        "        end,\n"
        "    doit(F).\n"
    ),

Nice!

This would also solve the same alignment problem with multiline lists, tuples, maps, function calls, etc. after =, e.g.

Variable =
    #{
        something_long => on_one_line,
        another_long_thing => {on, the, second, line}
    }.

You want to preserve the linebreak after = but not insert one automatically if it wasn't there before? Then you might end up with two different styles... Not a problem per se but I guess the point of a formatter is to produce a uniform style.

How about always inserting a linebreak between = and case/if/receive/try?

How about generalizing it even further and always automatically insert a linebreak before any multiline expression? (It would require test-formatting lists, etc. to see if an expression is multiline or not, but maybe you already have that?)


For short variables (which are rare in Erlang anyway), you would get slightly ugly (but nicely aligned) stuff like @elbrujohalcon pointed out:

C =
    case
        ...
    end.

... which is arguably (IMHO) less ugly than the alternative ...

C = case
    ....
end.

This is definitely something to consider.

How about always inserting a linebreak between = and case/if/receive/try?

This is how erlfmt works today.

The things that work differently are function calls, single-clause fun and collections like lists, maps, etc.

I think this would solve the issue #248
Not sure if we should merge this, but thought it might be good to look some consequences and check with everyone if this is something we want.

We will soon merge to preserve the newline after equals, which will fix this issue to achieve emacs compatibility.