avh4 / elm-format

elm-format formats Elm source code according to a standard set of rules based on the official Elm Style Guide
BSD 3-Clause "New" or "Revised" License
1.31k stars 148 forks source link

Single/Multi-line formatting trigger discussion #647

Open rlefevre opened 4 years ago

rlefevre commented 4 years ago

This is a discussion about the way we can trigger formatting on one line vs multi-line.

I'm sorry if this has been discussed before and I did not find it, if this is the case please just point me to the discussion.

I was thinking about https://github.com/avh4/elm-format/issues/507 and https://github.com/avh4/elm-format/issues/209, and thought about the cost of converting some multi-line code to single-line code if they are implemented, particularly the case...of one.

Currently, for lists, tuples, records and lambdas, multi-line is triggered if the expression is on more than one line. This makes it very easy to get proper multi-line indentation from single-line with a single line break, but more complicated to get the reverse operation, particularly if the single line case...of patterns were implemented.

So I was thinking that using only the first indented element (symbol or expression) to trigger the wanted formatting would make both operations really easy, at a slight cost of feature discoverability. Basically, the main difference with current method is that positioning the first indented element would "suck" all the others in the multiline-line to single-line case.

Examples

For example, by supposing that the PRs above are implemented:

Leads to multi-line

Lists

[1
, 2, 3, 4]

would be formatted as

[ 1
, 2
, 3
, 4
]

Tuples

(1
)

would be formatted as

( 1
)

Records

{a=True
}

would be formatted as

{ a = True
}

Case of

case x of
  1 ->
      foo1
  2 -> bar

would be formatted as

case x of
    1 ->
        foo1

    2 ->
        bar

If then else

if cond then
t else f

would be formatted as

if cond then
    t

else
    f

Lambda

\x ->
x + 1

would be formatted as

\x ->
  x + 1

Not much surprising as this is a subset of the current behavior. The , or ], ), first case expression or then expression are on a different line, therefore the whole expression will be indented on multi-line.

Leads to single-line

This becomes quite different from current behavior.

Lists

[1,
2, 3, 4]

would be formatted as

[ 1, 2, 3, 4 ]

Tuples

(1,
2, 3)

would be formatted as

( 1, 2, 3 )

Records

{ a = True,
b = False}

would be formatted as

{ a = True, b = False }

Case of

case x of
  1 -> foo1
  2 -> 
     bar2
  3 -> 
     bar3

would be formatted as

case x of
  1 -> foo1
  2 -> bar2
  3 -> bar3

If then else

if cond then t
else f

would be formatted as

if cond then t else f

Lambda

\x -> x
+ 1

would be formatted as

\x -> x + 1

The first ,, first case expression or then expression are on the first line, therefore the whole expression would be single-line.

Comparison

Let's try to summarize a comparison of both methods using the following quite arbitrary legend:

:green_heart: great :yellow_heart: good :heart: can be cumbersome

Operation Current method First Element method
single-line to multi-line :green_heart: :green_heart:
multi-line to single-line :heart: * :green_heart:
single-line to multi-line feature discoverability :green_heart: :yellow_heart:
multi-line to single-line feature discoverability :green_heart: :yellow_heart:
Suspected performance ** :yellow_heart: :green_heart:

* It's particularly cumbersome for single line case...of patterns if they are implemented (https://github.com/avh4/elm-format/issues/507). For other cases, merging several lines to a single one is usually available in editors.

** I suspect that the proposed method performance would be slightly better, as only the first element has to be examined in the single-line case. The current method could use some optimization that makes this supposition wrong though. Also the proposed method implementation might be more complicated.

So feature discovery seems to be the main drawback of the proposed method, but it should be noted that single-line expression will be indented single-line, and multi-line ones multi-line, which is the most important in the feature discovery (only mixed single/multi-line are interpreted differently).

Use cases:

I think that there are several use cases, regular enough to justify using the proposed method:

  1. Converting some code to single line
    • if the feature is added for if...then...else and case...of
    • when removing some elements which then makes the expression fit on a single line
  2. Evaluating both indentations when coding before deciding which one to keep (if going from the single line to multi, the editor undo can help, but not with the reverse operation)
  3. A current use case I noticed I use a lot when I'm developing is to sort alphabetically case...of patterns. Even if I want multi-line indentation, I often put them on a single line, select all the cases, call an editor sort function, then indent them back as wanted. When there are a lot of cases, this is quite cumbersome.

Conclusion

The current method is fine at the moment, but would be more cumbersome for case...of if they are implemented. Therefore if https://github.com/avh4/elm-format/issues/507 is implemented I think it would make sense to switch to the proposed method.

Another solution would be to use the proposed method only for case...of, as this seems to be the only one that cannot be transformed simply to single-line with an editor generic feature.

What do you think? Does it make sense? Have I missed some important point?

Thanks

rlefevre commented 4 years ago

A major drawback of the proposed method I did not think about at first is that the formatter would not know what to do with comments in multi-line when single-line is requested. I will therefore close this issue until the design is more clear.

rlefevre commented 4 years ago

After having looked more precisely at the possible implementation (thanks to https://github.com/avh4/elm-format/pull/648), comments could just force multi-line. So I reopen the discussion. Sorry for close/reopen.

rl-king commented 4 years ago

Function signatures sort of have this behaviour

view :
    Model -> Html Msg

will become

view : Model -> Html Msg

while

view : Model
          -> Html Msg

becomes

view :
    Model
    -> Html Msg
avh4 commented 4 years ago

I suspect that the proposed method performance would be slightly better, as only the first element has to be examined in the single-line case.

I think there is probably little difference since haskell is lazy. The current method does have to iterate all the items to decide how to format the outer construct, but it would have to iterate them anyway to print them out, and I believe most code is written to avoid doing the formatting computation twice.

avh4 commented 4 years ago

I'm hesitant about this as the UI for this feature makes much finer distinctions that any existing feature that modifies the user's code. The position of commas now can have a drastic impact on how things are formatted, and I worry that this will make elm-format feel more confusing to folks using it. Thus far I've tried to only add convenience features that would not cause anyway to wonder "why did that happen?" So in this subset of your comparison, I'm not sure the ❤️ -> 💚 of the new feature is worth the 💚 -> 💛 here.

Operation Current method First Element method
multi-line to single-line ❤️ * 💚
single-line to multi-line feature discoverability 💚 💛

For the use cases:

Converting some code to single line when removing some elements which then makes the expression fit on a single line

My impression from reading past discussions about single-line variants is that the majority of Elm code should prefer multi-line, and only in certain special circumstances is the single-line version preferable. I'm not sure that aggressively encouraging single-line variants whenever elements are removed is the right thing to do. Toward elm-format's goal of making all Elm code more homogenous to aid readability and to reduce thought about formatting for code authors, this use case seems like it would lead to more varied visual style for code that has the same meaning, and I'm not sure that's the ideal direction to go.

Evaluating both indentations when coding before deciding which one to keep

Similarly, I think this is a use case that isn't desired. Elm-format's goal is to eliminate these evaluations as much as possible so that the code authors will focus on what their code does instead of how it's formatted.

A current use case I noticed I use a lot when I'm developing is to sort alphabetically case...of patterns.

This is an interesting one I hadn't considered! Though the use case you describe would only work for case statements where all the branches are able to be formatted on a single line. (I'm assuming in your proposal that this:

case x of
  1 -> foo1
  2 -> 
     [ x
     , y
     ]

would have to format as

case x of
    1 ->
        foo1

    2 -> 
        [ x
        , y
        ]

because the body of the 2 branch is forced to be multiline?) So given that you would only be doing this with case expressions that could be single-line, wouldn't you just have that case expression saved as single-line (once #507) is implemented, so then there wouldn't be any need to convert back and forth?

-- in summary, I think this is an interesting feature, but because it doesn't quite align with some of elm-format's core goals, I'd be hesitant to have this implemented until #507 and possibly #209 are released for a while and it becomes obvious whether or not the UI complexity is worth the benefit to the average Elm user. Apart from case expressions, I haven't seen folks frequently needing to collapse expressions, and as you noted, editors often have a command for joining lines. Though if there are examples of types of code where converting multiline to single-line is quite frequent, it'd be great to collect some of those examples.

rlefevre commented 4 years ago

Those are good points and I think that I actually agree with them (you can actually notice that I did not use this method in #648).

My motivation was mainly to explain the method used in https://github.com/avh4/elm-format/pull/649, and it made me wondering if generalizing it would be a good idea, hence this discussion.

One thing that I'm not sure is that for your case..of example, you seem to imply that #507 would force single-line patterns formatting, whereas I thought that #507 would let the user chose (and that's how I implemented it in https://github.com/avh4/elm-format/pull/649), but this can be discussed in #507 or #649.

Feel free to close this discussion once you think it is not useful anymore for now.

avh4 commented 4 years ago

you seem to imply that #509 would force single-line patterns formatting

Sorry for not being clear. I just meant that after someone has chosen single-line, then they probably won't need to switch back and forth, I'd expect them to just leave it as single-line from then on.

rlefevre commented 4 years ago

Thank you for the clarification.

To me, the motivation to use this method for #507 (sorry for linking to #509, it was a typo) is a lot stronger that all other use cases because:

  1. Users have been forced to use multi-line case-expressions branches, even for simple enums, but may want to switch to single-line once #507 becomes available, and it's very cumbersome manually for a simple enumeration with a lot of cases. Single-line if expressions on the other hand are usually a lot more scattered and isolated (like some conditional attributes in views), and easier to switch manually.
  2. It's really not easy to use some editor feature to do it, compared to other expressions like lists, tuples, lambdas...
  3. As I said to help sorting alphanumerically when wanting to keep using multi-line (helpful only for single-line pattern bodies indeed though).
harrysarson commented 4 years ago

It's really not easy to use some editor feature to do it, compared to other expressions like lists, tuples, lambdas...

Would a find and replace from ->\n to -> not work?

rlefevre commented 4 years ago

Most likely, but from my limited experience, matching \n is not always trivial in editors find and replace. Maybe I am exaggerating the issue because I like the feature for case expressions though:sweat_smile:

harrysarson commented 4 years ago

I like the feature for case expressions though

So do I!