jfmengels / elm-review-simplify

Provides elm-review rules to simplify your Elm code
https://package.elm-lang.org/packages/jfmengels/elm-review-simplify/latest/
BSD 3-Clause "New" or "Revised" License
20 stars 9 forks source link

Proposal: Don't autofix list concatenations when it would introduce new line-formatting #150

Open jfmengels opened 12 months ago

jfmengels commented 12 months ago

One problem we have with Elm tooling is the formatting of lists. For instance, if you want to create a list that represents a grid, you could write code like this:

grid =
    [ 1, 2, 3
    , 4, 5, 6
    , 7, 8, 9
    ]

The problem is that elm-format will reformat it to

grid =
    [ 1
    , 2
    , 3
    , 4
    , 5
    , 6
    , 7
    , 8
    , 9
    ]

What you can do instead, is to use concatenation like:

List.concat
    [ [ 1, 2, 3 ]
    , [ 4, 5, 6 ]
    , [ 7, 8, 9 ]
    ]

but Simplify will automatically fix it to be the same as the first example (and have elm-format change it to the second example)

If you really want to write the code in a grid-like manner (which I'm convinced is helpful in some cases), I don't know of a good solution when you use both tools that will keep things readable.

My thinking is that we could remove the List.concat simplification when we detect that it would make elm-format re-format the code in a different way (which boils down to "are the different items on the same line or not").

Examples:

-- would stay as is
List.concat
    [ [ 1, 2, 3 ]
    , [ 4, 5, 6 ]
    , [ 7, 8, 9 ]
    ]

-- would be fixed
List.concat [ [ 1, 2 ], [ 3 ] ]
--> [ 1, 2, 3 ]

List.concat
    [ [ 1, 2 ], [ 3 ] ]
--> [ 1, 2, 3 ]

What do you think? Does this seem reasonable and useful?

cc @avh4, would love your thoughts here. It seems related to the example in https://github.com/avh4/elm-format/issues/156 (though I don't care about the requested formatting/alignment inside the list requested in the issue)

lue-bird commented 12 months ago

I guess one alternative would be using a let for the rows.

let
    replaceAliases =
        [ "ReplaceMe", "ToReplace", "REPLACEME" ]

    fixAliases =
        [ "FixMe", "ToFix", "TOFIX", "FIXME" ]

    todoAliases =
        [ "ToDo", "TODO" ]
in
List.concat [ replaceAliases, fixAliases, todoAliases ]

I'm actually not sure where one would use concat [ [...], ..., [ ... ] ] directly to build a grid. If you need a grid, why not construct a matrix or an Array2d?

jfmengels commented 12 months ago

If you need a grid, why not construct a matrix or an Array2d?

Depends on the data structure you're using. Maybe for some reason it's better to use a List (for performance or because it's simpler, no dependencies, etc.). Or you are, but an intermediate representation (before transforming it into a grid) is a list like the one

I'm actually not sure where one would use concat [ [...], ..., [ ... ] ] directly to build a grid

One example I'm thinking is writing test cases for a Tic-tac-toe solver (or Sudoku, same), where you give a grid and expect the test to tell you something about the state

[ O, B, X
, O, X, B
, O, X, B
]
  |> Solver.solve
  |> Expect (Winner O)

The above is really easy to preview for a human, but elm-format re-formats it to a list "with one column". To keep the same visuals, we could use concat

[ [ O, B, X ]
, [ O, X, B ]
, [ O, X, B ]
]
  |> List.concat
  |> Solver.solve
  |> Expect (Winner O)

which reads almost as well. But then Simplify breaks even that.


This is mostly about removing a potential pain point where we try to simplify an expression, but the result is actually harder to read. I know I've seen similar situations (during coding interviews no less).