rebar3_format
rebar3_format copied to clipboard
AdRoll
Respect `ribbon` for function call as function argument
Currently, default_formatter formats this code…
IndexedIpAddresses = lists:zip(lists:seq(1, NIpAddresses), IpAddresses),
…like this…
IndexedIpAddresses =
lists:zip(
lists:seq(1, NIpAddresses), IpAddresses),
I would like it to be formatted like this…
IndexedIpAddresses =
lists:zip(lists:seq(1, NIpAddresses), IpAddresses),
(especially since it's not going over ribbon)
Sadly… it's practically impossible, at least without replacing prettypr as our pretty-printer.
I'll try to give you the best explanation I can come up with…
prettypr expects a group of "related" documents as its input. Those documents can be paragraphs, sequences, sep (which is a kind of paragraph… sorta), etc… Then it places them in the best way it sees fit (respecting paper and ribbon whenever possible).
The behaviour that you (and to be fair, I… as well) expect from these things is akin to the paragraph behavior in the sense that we want prettypr to put as many function calls in a row as possible and only apply indentation when needed.
Therefore, in prettypr terms, we want a paragraph with the words IndexedIpAddresses =, lists:zip(, lists:seq(, 1,, … etc…
And prettypr can totally understand that, but what it cannot understand is that we want those words printed without spaces between them… IndexedIpAddresses = lists:zip(lists:seq(1,…. That simply makes no sense to prettypr and therefore the best thing it can do is… IndexedIpAddresses = lists:zip( lists:seq( 1, … (notice the spaces after ().
When I was working on #153, that was my first implementation. But I noticed it would not work like that.
So, there is an alternative (which is what the erl_prettypr from OTP or otp_formatter here use) which is to put all the things we want to be together in a single "word" (lists:zip(lists:seq(). That works relatively well when you don't compose too many functions, but it is problematic on these examples (which led to #140)…
eyer_hosted_match:cluster_id(eyer_hosted_match:canonical_id(eyer_hosted_match:new([{id,
HashKey}]),
attr(?CANONICAL_ID,
AttrMap,
undefined)),
attr(?CLUSTER_ID, AttrMap, undefined))
The third alternative, which is the default one now (but guarded behind the flag I introduced in #172) is to explicitly tell prettypr that some "words" must close some paragraphs and that the next thing is in a different paragraph or sequence, that must be indented to some degree. In this case, my idea was to do that with long function names, and only when two of those are composed. But then again… there is no real concept of long function names for the formatter. So I had to come up with a heuristic, which was qualified function names. If you compose two fully qualified function calls, the second one goes into a new paragraph that is indented below the one used for the first qualified function call. That's #153, basically.
It's not ideal and it may lead to some unexpected, undesired, or seeming inconsistent behavior.
On @AdRoll's code, it looks noticeably better than not using it (we probably have a tendency to compose long function names with long module names, etc…) and that's why it's the default. But you can totally disable that behavior, using inline_qualified_function_composition => false wherever you need to.
In any case, if you would like to take a deep dive into the formatter and prettypr modules… And if you find out a better way to produce the proper formatter… I would be delighted to merge such a PR.
