christopheradams / elixir_style_guide

A community driven style guide for Elixir
4.37k stars 299 forks source link

Brackets for multiline lists #132

Closed iskeld closed 7 years ago

iskeld commented 7 years ago

What is the preferred way of placing brackets for the lists which span multiple lines: 1.

list = [
  :first_item,
  :second_item,
]

2.

list = 
[
  :first_item,
  :second_item,
]

3.

list = 
  [:first_item,
   :second_item]
christopheradams commented 7 years ago

Great question!

No. 3 seems to be the preferred style (i.e., brackets on the same line as the first and last list items). Just look at any mix.exs file in the official repos.

I'd be interested in seeing counter-examples. I also didn't search for instances where a binding is involved (as in your examples).

christopheradams commented 7 years ago

@iskeld Does #146 solve this issue for you?

ericmj commented 7 years ago

The generator in mix new uses 1. The reason the official repos use 3. is because they haven't been changed and also because it's not an important change.

christopheradams commented 7 years ago

Thanks for the note @ericmj. Here's the example of style 1 in mix new for those who hadn't seen it. Will this go out in 1.5?

christopheradams commented 7 years ago

I've been getting some good feedback on this issue. Basically, you should use either style 1 or 3 for assignment. If it's not an assignment, 1 and 2 are indistinguishable.

The takeaway is to be consistent about the brackets: if you open a list with [ on its own line, you must close with ] on its own line. You should also put the square brackets on their own line if you have a long list and, especially, if each element is on its own line.

iskeld commented 7 years ago

Thank you @christopheradams - you're doing a great job! :) I'll stick with 1 then, since it has also an extra benefit (comparing to 3) on lesser changes while adding new items (from version control point of view).

christopheradams commented 7 years ago

@iskeld What do you think of this?

christopheradams commented 7 years ago

Ok, something like this for mutli-line lists is merged in now. Thanks for the feedback!