Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Single/Multi-line formatting trigger discussion #647

Open
rlefevre opened this issue Nov 16, 2019 · 11 comments
Open

Single/Multi-line formatting trigger discussion #647

rlefevre opened this issue Nov 16, 2019 · 11 comments

Comments

@rlefevre
Copy link
Contributor

rlefevre commented Nov 16, 2019

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 #507 and #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:

💚 great
💛 good
❤️ can be cumbersome

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

* It's particularly cumbersome for single line case...of patterns if they are implemented (#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
  1. 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)
  2. 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 #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 rlefevre changed the title One liners trigger discussion Single/Multi-line formatting trigger discussion Nov 16, 2019
@rlefevre
Copy link
Contributor Author

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
Copy link
Contributor Author

After having looked more precisely at the possible implementation (thanks to #648), comments could just force multi-line. So I reopen the discussion. Sorry for close/reopen.

@rl-king
Copy link

rl-king commented Nov 17, 2019

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
Copy link
Owner

avh4 commented Nov 17, 2019

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
Copy link
Owner

avh4 commented Nov 17, 2019

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
Copy link
Contributor Author

rlefevre commented Nov 17, 2019

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 #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 #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
Copy link
Owner

avh4 commented Nov 17, 2019

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
Copy link
Contributor Author

rlefevre commented Nov 17, 2019

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 Single-line case-expressions branches? #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
Copy link
Contributor

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
Copy link
Contributor Author

rlefevre commented Nov 18, 2019

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
Copy link
Contributor

I like the feature for case expressions though

So do I!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants