Support formatting a range from CLI

[michalmuskala]

Given architecture of erlfmt, it would be relatively easy to support formatting only a range of the file (assuming the range encompasses whole top-level forms -- functions or attributes).

There's some partial work towards that, that was merged in https://github.com/WhatsApp/erlfmt/pull/6, but the feature was never productionised.

[slaykachu]

Jumping on this task!

(assuming the range encompasses whole top-level forms -- functions or attributes).

What about formatting the smallest portion with whole top-level forms that contains the given range? Eg, if you specify just a line, it would format the enclosing function.

Rationale:

• Do the right thing TM
• Allow to chain tools. E.g. git provides the lines changed, and we only format the surrounding lines rather than the whole file (more convenient for reviews).

[awalterschulze]

erlfmt's API supports formatting per form. I think it would be really hard to format more granularly.

[awalterschulze]

Maybe try formatting per form first, even if just as an exercise to pick up context. But I wouldn't want to give someone the hard task of formatting erlfmt per line.

[slaykachu]

TODO:

• Question the pertinence of this task :)

See for instance elixir approach: "This is not an option. Mix format wants to format the entire file passed to it. I also don’t think it ever will be added as the whole point is to add consistency." This approach hinders the 'smallest possible diff' quality. Mitigation:

• (1) If the file was already formatted, this becomes a non issue.
• (2) The pretty reformatting and actual code changes should be in separate commit anyway, in my book. So the whole module/directory could/should be formatted in a preliminary commit, then goto (1).

What is that we want to achieve with the range feature?

[awalterschulze]

That is a great explanation of why we haven't implemented this feature, yet.

But to be fair, it has been a popular feature request from users and VS Code supports this feature for formatters, so I assume it is popular for most other formatters to have this.

So I would say this is a nice to have feature and shouldn't require too much effort to add. Hopefully a nice warm up task.

[slaykachu]

Do you know of any standard to pass such options? E.g. how VS Code is passing that to other formatters.

Started searching that on the interweb, with no clear result.

Oh, the answer might also be "it doesn't matter much!".

[awalterschulze]

I think first step is checking that this works, by adding some tests https://github.com/WhatsApp/erlfmt/blob/master/src/erlfmt.erl#L225

But yes you are right, next thing would be checking that this would work with VS Code and/or erlang_ls Possibly this is a useful read https://code.visualstudio.com/blogs/2016/11/15/formatters-best-practices But it seems a little old.

[slaykachu]

For the CLI syntax, I'm considering:

--lines=start[-end] [,start[-end]]*

Rationale:

• Allow to pass several ranges, which will come handy from the IDE.
• --range would be weird for unique line flavour (to be fair, the plural is weird too in that case).

Con:

• Unusual syntax? Sed and vi uses start,end. Didn't find any exemple of other formater supporting that!

Alternative:

• --range=start,end, but we can specify the option several times, one for each range. No single-line version in that case.

Regarding VS Code, we have to create the command line with its options anyway, so the format of the particular option doesn't matter.

[awalterschulze]

I like your alternative proposal --range=start,end, since you mention there is precedence in vi and sed. What is the flag name in vim and sed?

I guess a one line version of this would be <start>,<start+1> ?

[slaykachu]

That's editor syntax, not a CLI option. E.g.:

• vi :1,2d deletes lines 1 and 2
• sed -n '4,6p' prints lines 4, 5, 6

Both endpoints are inclusive, so one line would be <start>,<start>

[awalterschulze]

Maybe we can find some precedent in a command line tool that we can copy?

[michalmuskala]

FWIW prettier uses --range-start and --range-end. I like --range start,end though - we just need to make sure it works nicely with getopt that we use for processing cli args.

[slaykachu]

What about formatting the smallest portion with whole top-level forms that contains the given range? Eg, if you specify just a line, it would format the enclosing function.

I have second thoughts for that! For a IDE experience, that might not be want we want. E.g. a SWE changes a line in a big function in an unformatted file, and suddenly all the function is formatted!

• That's unexpected.
• That add noise for the code review.
• That's not how IDEs behave for other languages.

So, I vote for only modifying the lines from the passed range(s).

[awalterschulze]

I think it is okay to only format per form, at least as a starting exercise. If after that you think it would be easy to do it more granular, then I would be very happy to hear about it how we can do this.

But your argument here is still interesting and I think we can think about if we only format forms that are totally included in the range or do we also format forms that are partially included in the range. What do you think?

[slaykachu]

I think we shouldn't do what is easiest, but what is more convenient for users!

That being said you have a point, we can reach that ideal state incrementally, and I should do a first "good enough" version. With this approach, it doesn't really matter how we deal with partial forms, since it is meant to change anyway!

[awalterschulze]

@slaykachu has shown that integration with VS Code works using the range flag. So I think we can close this.