[Request for Feedback] Add ontype formatting support

[onno-vos-dev]

[Request for Feedback]

Description

This PR adds on-type formatting support. The way this works is that it is triggered by adding a .. This PR is a DRAFT PR which means we @anha0825 and myself are looking for feedback on this work.

The feature is disabled by default but can be enabled by adding format_on_type: true to your erlang_ls.config.

No tests have been added as of yet as I'm not sure how tests for this sort of feature can be written? Considering that BSP will take care of formatting going forward, this should be more or less an "interim" solution anyway.

[robertoaloi]

Hi @onno-vos-dev sorry for the late reply. Personally I don't have a problem merging this, especially if it's disabled by default initially. So, if you get a basic test in place and squash, I think we're good!

Unless @gomoripeti, @alanz or others have a different opinion...

[gomoripeti]

very nice!
just minor note that currently there is no folding_range POI created for one-line functions. These are usually short and the formatter wouldn't break them up, but there might be other formattings like missing or extra spaces that the formatter would change. Would it make sense to create a folding_range for every function and filter them out only in els_folding_range_provider?
Or is this change specifically for indenting?

[onno-vos-dev]

Opened: AdRoll/rebar3_format#295 as that's the biggest bug that exists as of now. I haven't forgotten about this PR...

Now let me go back and hide in a corner ashamed that this PR has been rotting away for so long...

[michalmuskala]

FWIW I think much bigger issue with rebar3_format is AdRoll/rebar3_format#216, which can lead to the code changing semantics in a very subtle, and hard to spot way

[onno-vos-dev]

FWIW I think much bigger issue with rebar3_format is AdRoll/rebar3_format#216, which can lead to the code changing semantics in a very subtle, and hard to spot way

Yeah I noticed that issue as well when trying to find if my issue was already reported but since the formatter currently doesn't support defines I guess we escape that bug.

[michalmuskala]

The issue is not in defines, but in any usages of ?MACRO. In particular, with no modifications to -define for the following example:

-define(MUL(A, B), A * B).

test() -> ?MUL((1 + 2), (3 + 4)).

the formatter will produce:

-define(MUL(A, B), A * B).

test() -> ?MUL(1 + 2, 3 + 4).

changing the result of test() from 21 to 11.

And while this example is fairly convoluted, there are frequently "guard-safe" macros using andalso and orelse where the order the conditions are applied is important and any violation would be quite hard to notice

[onno-vos-dev]

Seems to me that rebar3_format and macros is not ready for production anywhere in that case. These are some serious bugs that as you state, can seriously affect the outcome of running code 😞