WhatsApp / erlfmt

An automated code formatter for Erlang
Apache License 2.0
412 stars 52 forks source link

Formatting of nested -ifdef/-ifndef macro directives #345

Open TD5 opened 2 years ago

TD5 commented 2 years ago

Sometimes I need to nest macro directives, and when I do so, it's nice to indent them as one would do with code generally - just to keep it readable - however, erlfmt seems to want to remove all indentation, for example:


-ifdef(FOO).
  -ifndef(BAR).
    -define(BAZ, true).
  -endif.
-endif.

--[apply erlfmt]-->

-ifdef(FOO).
-ifndef(BAR).
-define(BAZ, true).
-endif.
-endif.

I don't think there's any mention of this in the style guide. As far as I can see, the macro-related stuff covers various topics, but doesn't consider flow control for macro definitions, and treats it as I would treat a list of several -defines or something. Also, I am not a frequent user of erlfmt, so maybe I am doing something wrong, making this a support request rather than a feature request.

I know nesting macro directives like this is probably Not Good™, but I'd prefer not to compound that by needing to have them flattened out.

Thanks!

michalmuskala commented 2 years ago

This is a curious case, because of how inconsistent this is handled in "the wild".

For some cases, like the one you listed, the indentation is sometimes applied.

Yet for others, way more common, it almost never is. For example, I don't think I've ever saw code like:

-ifdef(FOO)
    conditional_function() ->
        ....
-endif.

Similarly, you frequently have "ifdef guards" for headers - those are never indented either.

So I think the primarily challenge of a feature like this would be to decide when to apply it, and when not to.

TD5 commented 2 years ago

Yes, that's my experience too. My personal feeling, and what I saw in code, was that at one layer of nesting, there's no indentation, but beyond that there is:

One level of macro flow-control:

-ifdef(FOO)
conditional_function() ->
    ....
-endif.
-ifdef(FOO)
-define(BAR, true).
-endif.

More than one level of macro flow-control:

-ifdef(FOO)
    -ifdef(BAR)
        conditional_function() ->
            ....
    -endif.
-endif.
-ifdef(FOO)
    -ifdef(BAR)
        -define(BAZ, true).
        -define(QUUX, true).
    -endif.
-endif.