List of exported functions and types are inlined despite the default {when_over, 25} setting

[michalwski]

We tried the default_formatter in esl/exml#52 with the following settings

{format, [
    {formatter, default_formatter},
    {options, #{sub_indent => 4}}
]}.

and we observed that in most cases the list of exported function and types are in-lined. The expected behaviour is to keep one exported function or type per line.

[elbrujohalcon]

I think that this particular issue only presents itself when formatting attributes, like -type, -export_type or -export.

[elbrujohalcon]

I see what's happening here… The formatter online applies inline_items rules to the lists of items that doesn't fit in a single line, to avoid turning

[A, B] = Thing

…into…

[A,
  B] = Thing

Considering that the exports are a list… they're treated as lists and therefore if the list fits in a single row, it's written in a single row.

[elbrujohalcon]

@michalwski I added more info on the README.md in #101… and I think that should be enough, but maybe you or others would like the formatter to actually treat -export(…) clauses differently than lists.

Maybe add another option (inline_exports) and force the formatter to respect it even if it can put all items in the same row. But if that's so, we need to figure out which other similar exceptions to the general rule we want to consider (e.g. -dialyzer(…), -ignore_xref(…), record fields in assignments or patterns, record fields in record types, map fields, etc.)… Maybe tuples and lists should be the exceptions here?

[michalwski]

@elbrujohalcon thanks for looking into this. It looks like it'd worth to separate the inlining behaviour for tuples, records, maps and lists from directives like -export, -dialyzer etc.

[elbrujohalcon]

Cool. Let me grab a list of all the items that are currently affected by inline_items and we can decide which ones go where…

• tuples
• lists:
  • in code
  • in -export, -export_type and -optional_callbacks attributes
• binaries
• bitstrings
• list comprehensions
• binary comprehensions
• record expressions
• map expressions
• conjunctions (i.e. guards separated with ,)
• disjunctions (i.e. guards separated with ;)
• clause arguments (e.g. f(Arg1, Arg2, Arg3, …))
• functions in types
• maps in types
• records in types
• tuples in types
• type unions (e.g. T1 | T2 | T3…)
• applications:
  • in regular function applications (e.g. m:f(Arg1, Arg2, Arg3, …))
  • in type definitions when the type has arguments
  • in custom attributes (e.g. -my_attr(Arg1, Arg2, Arg3, …).
  • in type constraints (e.g. when Con1, Con2, Con3, …)

So… @juanbono @diegomanuel @michalwski: How would you split the list?

[elbrujohalcon]

@juanbono @diegomanuel @michalwski: Any input here?

[michalwski]

Hi @elbrujohalcon, sorry for the delay. Here's how I'd split the list:

code element	controlled by
tupels	inline_items
lists in code	inline_items
lists in -export* and -optional_callbacks	inline_directives
binaries	inline_items
bitstrings	inline_items
list comprehensions	inline_items
binary comprehensions	inline_items
record expressions	inline_items
map expressions	inline_items
conjunctions (i.e. guards separated with ,)	inline_guards
disjunctions (i.e. guards separated with ;)	inline_guards
clause arguments (e.g. f(Arg1, Arg2, Arg3, …))	inline_items
functions in types	?
maps in types	inline_types
records in types	inline_types
tuples in types	inline_items ?
type unions	inline_items ?
applications: in regular function applications (e.g. m:f(Arg1, Arg2, Arg3, …))	inline_items
applications: in type definitions when the type has arguments	inline_types
applications: in custom attributes (e.g. -my_attr(Arg1, Arg2, Arg3, …).	inline_directives
applications: in function spec's type constraints (e.g. when Con1, Con2, Con3, …)	related to #90

[elbrujohalcon]

@michalwski functions in types are things like… -type f(X) :: fun((X, pos_integer()) -> [X]).
What's affected by the parameter is the list of arguments in the function type head.

[michalwski]

@elbrujohalcon I know that. I was just not sure if we need a separate category for that, or maybe it fits some other. The table is only my opinion on the topic. Would be good to know what other people think.

[elbrujohalcon]

Oh, right! I understand now. Thanks.

[elbrujohalcon]

So, according to your list, @michalwski … we end up with 4 classes of inlining…

• inline_items
• inline_directives
• inline_guards
• inline_types

Now… I guess, inline_items should retain current behavior and only apply the inlining rule if needed (i.e. if the thing fits in a single line it should still be inlined no matter what).

Do you think all others should, by default, not be inlined, even if there are just a few elements?

As an example, if inline_guards is {when_over, 4} and we have this code:

is_positive(X) when is_integer(X), X > 0 ->
    true.

The current formatter will just leave it as it is, but forcing inline_guards to apply to all guards will result in…

is_positive(X)
    when is_integer(X),
         X > 0 ->
    true.

The same goes for inline_types, for instance for applications…

-type x() :: dict(integer(), string()).

…will become…

-type x() :: dict(integer(),
                  string()).

I think inline_directives is the one that makes more sense to me. To get things like…

-export([f1/0,
         f2/0]).

…instead of…

-export([f1/0, f2/0]).

I understand that isolated examples don't show the real problem which is consistency, i.e. either inlining all export blocks or none of them instead of inlining only those that fit in one line and not the others. But I'm still not sure what would be the best solution for this problem…

@juanbono / @diegomanuel / @facundoolano / @pbrudnick : opinions?

[michalwski]

Thanks for the examples @elbrujohalcon, this sheds new line on the formatting. I agree that inline_directives makes the most sense. And the default would be false or never which would format all the elements in new lines. The other categories inline_guards and inline_types probably doesn't make much sense and could be replaced with a specific formatting rule for these language elements, and ignore inlining options.

For instance, there could be a dedicated formatting rule for guards. What I have in mind is that they are kept in the same line as long as the line doesn't exceed the limit, only when the line is too long the formatter tries to split it, in a consistent way, for instance, the following too long line:

this_is_a_function_with_a_very_long_time(LongVarname, AnotherName) when is_binary(AnotherName), is_list(LongVarname) ->

could be formatted like this:

this_is_a_function_with_a_very_long_time(LongVarname, AnotherName) 
    when is_binary(AnotherName), is_list(LongVarname) ->

Also when the line starting with when is too long, it could be split to fit the line limit.

The same rule can apply to types, in my opinion.

[elbrujohalcon]

Yeah… I agree. I think I'll only implement inline_directives for now.
We'll eventually patch the default way to indent when… and that will be it.