Skip to content

Commit

Permalink
formatting decision for list comprehensions (#212)
Browse files Browse the repository at this point in the history
* formatting decision for list comprehensions

* added test for multi line no generator case

* unrename
  • Loading branch information
awalterschulze authored Dec 10, 2020
1 parent 82652ad commit 77f0950
Show file tree
Hide file tree
Showing 3 changed files with 193 additions and 8 deletions.
178 changes: 178 additions & 0 deletions doc/FormattingDecisionListComprehensions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,178 @@
# Formatting Decision: List Comprehensions

This is a document explaining our reasoning behind the formatting decision for list comprehensions.

This document conforms to the decisions already made for [lists](./FormattingDecisionLists.md)
and does not rehash decisions already taken in that document.

This document focuses on list comprehensions that are broken up over multiple lines, because of:
- a long function name,
- long parameters,
- long generators
- long filters
- etc.

We will choose a format that best conforms to our goals:

- Create/keep differentiation between the generator, body and filters.
- Consistent with formatting of [Lists](./FormattingDecisionLists.md)
- Handles multiline expressions consistently.

Here you can see all the other candidates we evaluated against our goals.

## Expanded

- ✅ Differentiating
- ✅ Consistent Lists
- ✅ Multiline expressions

This is how `erlfmt` formats a multiline list comprehension today:

```erlang formatted list_comp
[
function_with_long_name(A)
|| {A, B} <- Cs,
filter(B)
]
```

`erlfmt` does allow you to move some parts onto the same line, if they fit `erlfmt` will keep them there:

```erlang formatted list_comp_2
[
function_with_long_name(A)
|| {A, B} <- Cs, filter(B)
]
```

Here is an example with a multiline expression, that shows what a long argument in the function would look like:

```erlang formatted list_comp_3
[
function_with_long_name(
A,
ALongArgument
)
|| {A, B} <- Cs,
filter(B)
]
```

## Expanded with consistent 4 spacing

- ✅ Differentiating
- ✅ Consistent with lists
- ✅ Handles multiline expression consistently

The one negative in the previous example is inconsistent indentation, where `filter(B)` was indented by 3 spaces.
This alternative tries to correct that, but now we have a misalignment between `{A, B}` and `filter(B)` by a single space.

```erlang
[
function_with_long_name(A)
|| {A, B} <- Cs,
filter(B)
]
```

## Dedent double pipes

- ✅ Differentiating
- ❌ Consistent with lists
- ✅ Handles multiline expression consistently

We also considered dedenting the double pipes to the left.

```erlang
[
function_with_long_name(A)
|| {A, B} <- Cs,
filter(B)
]
```

This doesn't seem very consistent with how lists are formatted.

```erlang
[
function_with_long_name(A)
|| {A, B} <- Cs, filter(B)
]
```

## Double pipes on their own line

- ✅ Differentiating
- ❌ Consistent with lists
- ✅ Handles multiline expression consistently

We also considered giving the double pipes their own line.

```erlang
[
function_with_long_name(A)
||
{A, B} <- Cs,
filter(B)
]
```

```erlang
[
function_with_long_name(A)
||
{A, B} <- Cs, filter(B)
]
```

## Compressed

- ✅ Differentiating
- ❌ Consistent with lists
- ❌ Handles multiline expression consistently

The compressed option is inconsistent with how `erlfmt` formats lists,
but does make differentiation clear.

```erlang
[function_with_long_name(A)
|| {A, B} <- Cs,
filter(B)]
```

There is also a problem with how to indent multiline expressions consistently:

```erlang
[function_with_long_name(
A,
AlongArgument
)
|| {A, B} <- Cs,
filter(B)]
```

## Compressed with 4 spaces

- ✅ Differentiating
- ❌ Consistent with lists
- ❌ Handles multiline expression consistently

We could also consider an option where indentations are made with four spaces.

```erlang
[function_with_long_name(A)
|| A <- a_generator(),
some_check(A)]
```

Unfortunately since the first function is technically already indented by one space,
this still results in inconsistent handling of multiline expressions:

```erlang
[function_with_long_name(
A,
AlongArgument
)
|| A <- a_generator(),
some_check(A)]
```
17 changes: 9 additions & 8 deletions doc/Readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,15 @@ As always, you can preferably provide feedback, by simply trying out the `erlfmt

## Merged Formatting Decisions

- [DefaultWidth](https://github.com/WhatsApp/erlfmt/blob/master/doc/FormattingDecisionDefaultWidth.md)
- [Ignore](https://github.com/WhatsApp/erlfmt/blob/master/doc/FormattingDecisionIgnore.md)
- [Comments](https://github.com/WhatsApp/erlfmt/blob/master/doc/FormattingDecisionComments.md)
- [Pipes](https://github.com/WhatsApp/erlfmt/blob/master/doc/FormattingDecisionPipes.md)
- [Commas in Lists](https://github.com/WhatsApp/erlfmt/blob/master/doc/FormattingDecisionListCommas.md)
- [Number of Spaces](https://github.com/WhatsApp/erlfmt/blob/master/doc/FormattingDecisionSpaces.md)
- [Multiline Lists](https://github.com/WhatsApp/erlfmt/blob/master/doc/FormattingDecisionLists.md)
- [When with Multiline Guards](https://github.com/WhatsApp/erlfmt/blob/master/doc/FormattingDecisionWhenMultilineGuards.md)
- [DefaultWidth](./FormattingDecisionDefaultWidth.md)
- [Ignore](./FormattingDecisionIgnore.md)
- [Comments](./FormattingDecisionComments.md)
- [Pipes](./FormattingDecisionPipes.md)
- [Commas in Lists](./FormattingDecisionCommas.md)
- [Number of Spaces](./FormattingDecisionSpaces.md)
- [Multiline Lists](./FormattingDecisionLists.md)
- [List Comprehensions](./FormattingDecisionListComprehensions.md)
- [When with Multiline Guards](./FormattingDecisionWhenMultilineGuards.md)



Expand Down
6 changes: 6 additions & 0 deletions test/erlfmt_format_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -1138,6 +1138,12 @@ list_comprehension(Config) when is_list(Config) ->
" || {a, b} <- es\n"
"].\n"
),
?assertSame(
"[\n"
" X\n"
" || true, true, true\n"
"].\n"
),
?assertSame(
"A = [\n"
" a\n"
Expand Down

0 comments on commit 77f0950

Please sign in to comment.