Code formatter incorrectly formats multiline lists when there is a trailing comma

[pragdave]

The docstring says

As mentioned in the previous section, lists, tuples, bitstrings, maps, and
structs will be broken into multiple lines if they are followed by a newline in
the opening bracket and preceded by a new line in the closing bracket lines.

However, this code:

  @deps [
    { :private, "> 0.0.0" },
    { :ex_doc,  "~> 0.14", only: :dev, runtime: false },
  ]

gets formatted as:

  @deps [{:private, "> 0.0.0"}, {:ex_doc, "~> 0.14", only: :dev, runtime: false}]

Cheers

Dave

[fishcakez]

This looks like an issue with multiline lists combined with a trailing comma. The formatter will remove the trailing commas but then not honor the multiline list. Rather it should remove trailing comma and honor the multiline list.

[pragdave]

Fwiw, multi line lists are exactly where a trailing comma is good...

[fishcakez]

Fwiw, multi line lists are exactly where a trailing comma is good...

Please try to stay objective :). A topic like trailing commas can invite lots of opinions and not many facts. Elixir current style does not use trailing commas and the formatter follows the existing style. If you can show that it would be of benefit please propose it separately.

[pragdave]

Objectively: 1. A trailing comma never hurts. 2. A trailing comma makes it easier to add 3. A trailing comma means you can sort a kv literal on key.

[fishcakez]

Right, it makes refactoring easier, and works nicely for history as this creates the minimal diff because a change has the smallest area of effect. I am unsure if it has a slight negative effect clarity of code for those who are not used to it. Perhaps that becomes moot, except for beginners, if the formatting is widespread. I think a similar example is piping into a case where it is not obvious until you have seen it a few times, and then it is obvious.

[pragdave]

So will the formatter not strip trailing commas going forward? Again, this is the compromise solution. If people don't use them, the formatter is happy. If people do use them, it's also happy.

[fishcakez]

I think this (and #6647) bring up a point about where the line is drawn between consistent style and flexibility. The current method is quite rigid and mandates the style, which can improve productivity as there is no discussion or leg work to be had on each project or change. On other hand being more flexible allows people to express they code how they feel is clearest and make more trade offs for themselves. Personally I don't know where this line should be.

[pragdave]

Well, if you decide the formatter should strip trailing commas because that's the Elixir way, perhaps the parser should be changed no longer to support them.

[josevalim]

Thanks for the report @pragdave! It has been fixed on 9b5af33.

Our goal with the formatter is to mandate a style, reducing the amount of discussion on each project or change and also ensuring everyone in the community follows the same guidelines.

We will likely end-up adding more flexibility but it is still too early to tell which ones. Everyone is going to have preferences on which formatting rules they would like to keep and if we adopt all of them then the formatter loses its goal of unifying the styles across the community.

[pragdave]

Our goal with the formatter is to mandate a style, reducing the amount of discussion on each project or change and also ensuring everyone in the community follows the same guidelines. I wonder if a linter would meet these goals better than a formatter?

[josevalim]

@pragdave I am not sure. We have linters that are flexible when it comes to rules and some that are strict. Similarly, I would expect some formatters to be more flexible than others on keeping the user input.

In any case, we are quite happy with the formatter so far exactly because it automates the rules and we don't need to explicitly act on the feedback of a linter, which is a much slower development cycle.

[hadfieldn]

Came here looking to see if there's a way to force trailing commas. For the reasons @pragdave gives I'm disappointed this isn't an option.

[dorian-marchal]

Something I rarely see mentioned in these discussions is the ability to easily work with multicursors when using trailing commas:

with.mov

vs. the awkward:

without.mov

[stefanchrobot]

The other thing I'd like to mention is the ability to manually control the line breaks with commas like Dart does it (I think they might have borrowed it from Google's Python formatter):

• Trailing comma forces line breaks; this

[1, 2, 3,]

is always formatted as:

[
  1,
  2,
  3,
]

• No trailing comma collapses line breaks, unless the list does not fit into one line.

I know the same thing can be achieved manually, but the trick with commas is much, much faster - you basically add/remove the trailing comma and let the formatter do the job (which I assume for most people is just Cmd+S or similar), as opposed to manually entering line breaks or removing them.

I think this is a really useful feature and could be adopted into the Elixir formatter.

[stefanchrobot]

And a demo:

[wojtekmach]

Trailing commas is one of the most requested addition to the formatter but as was mentioned many times (in this issue, in similar issues, on ML, etc) it is unlikely to be added.

You may want to look into https://github.com/marcandre/freedom_formatter.

[dorian-marchal]

An other use case made difficult: the formatter removes the trailing comma when commenting the last line of an array.

Screen.Recording.2021-05-25.at.20.48.25.mov