elm-format icon indicating copy to clipboard operation
elm-format copied to clipboard

Proposal: Allow only multiline `<|` and `|>` expressions

Open rtfeldman opened this issue 6 years ago • 17 comments

Motivation

A primary goal of elm-format has always been to prevent teams from having stylistic debates.

I've noticed an area where I still feel a desire to make comments in PRs for what is a purely stylistic choice: the use of inline <| and |>.

Proposal

Have elm-format rule out these stylistic debates by permitting only multiline <| and |> expressions.

Consider this expression:

List.head (Set.toList mySet)

This would be unaffected by this proposal because it uses neither <| nor |>.

Here's another way to write an equivalent expression, which elm-format currently permits:

Set.toList mySet |> List.head

This proposal would have elm-format rewrite the above to this:

Set.toList mySet
    |> List.head

Similarly, this would get rewritten...

List.head <| Set.toList mySet

...to this:

List.head <|
    Set.toList mySet

Scenarios with Clear Winners

For certain shapes of larger expressions, it's clear when <| or |> is the best way to write an expression. In scenarios where there is a clear winner, there's no meaningful room for debate.

For example, here are several ways to write the same expression:

Maybe.withDefault 0 (Maybe.map sqrt (List.head (List.reverse (getAverages (List.compact rawData)))))

Maybe.withDefault 0 <| Maybe.map sqrt <| List.head <| List.reverse <| getAverages <| List.compact rawData

Maybe.withDefault 0 <|
    Maybe.map sqrt <|
        List.head <|
            List.reverse <|
                getAverages <|
                    List.compact rawData

rawData |> List.compact |> getAverages |> List.reverse |> List.head |> Maybe.map sqrt |> Maybe.withDefault 0

rawData
    |> List.compact
    |> getAverages
    |> List.reverse
    |> List.head
    |> Maybe.map sqrt
    |> Maybe.withDefault 0

The last one is the clear winner; it reads from top to bottom as a sequence of transformations, and it reads in the order in which the transformations will be applied. It takes up more vertical space than the others, but it's been a general design principle of elm-format not to consider that a serious downside.

In practice, I haven't seen anything approaching a style debate for cases like this. People use |> in the multiline style in these situations, and it's great. This proposal wouldn't affect this case.

Here's another expression written a few different ways:

makeUser
    (if String.isEmpty username then
        "Anonymous"
     else
        username
    )
    
(if String.isEmpty username then
    "Anonymous"
 else
    username
)
    |> makeUser

    
makeUser <|
    if String.isEmpty username then
        "Anonymous"
    else
        username

Again the last one is a clear winner. <| is the only operator that removes the need for multiline parens. This also comes up when passing anonymous functions (as elm-test does) or case-expressions.

I don't think this is a source of style debates either, and the proposal doesn't affect this case either.

Scenarios with Debatable Winners

All the scenarios with clear winners involve using <| and |> in multiline expressions. What about single-line expressions?

Currently, any time you do single-line function application, you have a stylistic choice to make: write it as foo bar or foo <| bar or bar |> foo. In the specific case of foo bar it seems clear that introducing a <| or |> is silly; it makes the expression longer and does not add clarity.

What about in the case of foo (bar baz)? Here it's more debatable. Some people prefer foo <| bar baz because it doesn't use parentheses. Others prefer bar baz |> foo because depending on bar and foo, it may make more sense to think of this expression in terms of running bar baz and then running foo on the result, rather than in terms of "run foo passing the result of bar baz."

Really, if an expression is short enough to fit on a single line, how big a difference is there between these three?

Maybe.withDefault 0 (List.head list)

Maybe.withDefault 0 <| List.head list

List.head list |> Maybe.withDefault 0

There's plenty of room for debate here.

This is the case this proposal would change. It would rewrite the last two as multiline expressions—because as we've seen, it's valuable to support <| and |> in these multiline contexts—leaving the first way as the only way to write it on a single line.

In this way, elm-format would remove this source of stylistic debates by limiting the use of <| and |> to only the use cases where they are capable of being clear winners.

rtfeldman avatar Jan 13 '18 18:01 rtfeldman

I use the second example in the "debatable winners" section quite frequently. It often looks better to use the operator instead of the parentheses. Perhaps the rule could be that a single inline application could use the current behavior, but two or more in sequence are forced onto new lines?

mbuscemi avatar Jan 13 '18 21:01 mbuscemi

I use the second example in the "debatable winners" section quite frequently. It often looks better to use the operator instead of the parentheses

I never use it that way, and think it looks worse. (Hence my desire to comment on PRs where I see it.)

Case in point that it's a source of the kind of stylistic debates elm-format aims to prevent!

rtfeldman avatar Jan 13 '18 21:01 rtfeldman

I agree with you that multiple consecutive applications should be disallowed on the same line, and also that a single application should be forced multiline if either side is a multi-line expression. Both of these features would be an improvement to elm-format.

In contrast, removing the ability to use single-line, single instance function application operators seems to me to diminish the expressiveness of the language. I can think of situations where putting such code on either a single line or multiple lines would be better, given the context. That determination is going to require a human eye.

mbuscemi avatar Jan 13 '18 21:01 mbuscemi

removing the ability to use single-line, single instance function application operators seems to me to diminish the expressiveness of the language. I can think of situations where putting such code on either a single line or multiple lines would be better, given the context. That determination is going to require a human eye.

I totally hear what you’re saying, I just subjectively don’t agree with it. I always choose parens for single-line application when writing my own code, and it always takes me longer to process what something’s doing when it’s written using <| or |> inline instead of parens.

Regardless of which we each think looks better, I presume we can agree that the delta is small here. It’s not like changing foo <| bar baz to foo (bar baz) makes a huge difference one way or the other. (Hence my position that this is the sort of minor debate elm-format ought to settle instead of distributing the debate to individual teams.)

I’d be fine with changing it to consistently do something other than what I’ve proposed here, but I think this rule is simple, consistent, and prevents the distributed debate without negatively impacting other use cases.

rtfeldman avatar Jan 13 '18 21:01 rtfeldman

In the proposal, why is it preferred to format

List.head <| Set.toList mySet

as

List.head <|
    Set.toList mySet

instead of as

List.head (Set.toList mySet)

avh4 avatar Jan 14 '18 02:01 avh4

I don't think

makeUser <|
    if String.isEmpty username then
        "Anonymous"
    else
        username

is a clear winner. It would be preferable to write that as

let
    displayName =
        if String.isEmpty username then
            "Anonymous"
        else
            username
in
makeUser displayName

avh4 avatar Jan 14 '18 02:01 avh4

A question this raises then, is why should single-line expressions with >> and << be allowed?

avh4 avatar Jan 14 '18 02:01 avh4

@avh4 I agree with both of your points. I think the parens version and the let version, respectively, are even better.

The reason I proposed turning the <| into a newline version instead of rewriting it to use parens is that I didn’t think of it. 😄 I think that would be even better.

I think the makeUser <| version is a clear winner among those presented, and I’m not sure how elm-format could rewrite that to become a let. To do that, elm-format would have to come up with a variable name, which the user would then be expected to change, right?

rtfeldman avatar Jan 14 '18 03:01 rtfeldman

A question this raises then, is why should single-line expressions with >> and << be allowed?

Now that you mention it, I’d actually question their use in multiline expressions, but that’s neither here nor there. 😄 (Maybe I should open a separate issue for that?)

So for << and >> here are three ways to write the same thing:

List.map (\user -> String.toLower user.username) users

List.map (String.toLower << .username) users

List.map (.username >> String.toLower) users

I think there’s a more compelling case for << or >> here than there is for <| and |> above for two reasons:

  1. << and >> make this code noticeably more concise, whereas <| vs () is 2 characters versus 2 different characters.
  2. Similar to pipelines, << and >> save you having to come up with a name, without (in my opinion) sacrificing clarity.

I have no defense for using << or >> in a multiline context though.

rtfeldman avatar Jan 14 '18 03:01 rtfeldman

I have no defense for using << or >> in a multiline context though.

Ping @stoeffel, this is relevant to a recent tweet of yours

zwilias avatar Jan 14 '18 10:01 zwilias

+1 for enforcing multiline |> and <|

rootulp avatar Jan 19 '18 22:01 rootulp

I agree foo <| bar baz would be the only debatable case here, but I could get used to foo (bar baz) easily. For two or more <|s, I think using multiple lines is already much clearer. So I fully support this proposal.

eriktim avatar Jan 20 '18 07:01 eriktim

elm-format loves vertically aligning things, but keeping a transform on one line allows you to read down for the intention whilst also retains the ability to read across ( right ) for the details.

balanceView =
    [ ( "Name: ", Model.getName model |> Alfred.Dates.toDate |> View.Components.readonlyInput )
    , ( "Due: ", Model.getDue model |> Maybe.Extra.unwrap "-" Alfred.Dates.toDate |> View.Components.readonlyInput )
    , ( "Effective Date: ", anotherModel.effDate |> Alfred.Dates.toDate |> View.Components.readonlyInput )
    , ( "Balance: ", Model.getBalance model |> Maybe.Extra.unwrap "-" Alfred.toMoney |> View.Components.readonlyInput )
    ]

Take the above example. You can read down to see the main content being displayed. You can then go across to see how it's doing it. It's concise and doesn't matter if it scrolls out of view because the important bits always starts on the left. This also extends to >> for editable components.

Another reason is that, when doing multiple line pipes, all arguments of a function is forced onto separate lines and brackets gets forced onto the next line again:

Consider this example where we insert into the dependant into dependants if it satisfies the predicate:

Alfred.List.insertByComparer (.dependantId >> (==) dependantId) dependant dependants

Applying the above rule would make it:

    Alfred.List.insertByComparer
        (.dependantId
            >> (==) dependantId       -- forcing the >> operator
        )                                          -- also forces the brackets
        rejoinRequestDependant   -- domino effect and forces the rest
        rejoinRequestDependants

Another example:

    List.Extra.find (.dependantId >> (==) dependantId) dependants
        |> Maybe.Extra.unwrap [] .name

-- turns into

    List.Extra.find
        (.dependantId
            >> (==) dependantId
        )
        dependants
        |> Maybe.Extra.unwrap [] .name

My writing style has always focused on keeping logical units of code in one place and clearly stating the source of the tranformation. The above could have also been written as:

    dependants
        |> List.Extra.find (.dependantId >> (==) dependantId)
        |> Maybe.Extra.unwrap [] .name

-- but forcing the >> would turn it into

    dependants
        |> List.Extra.find
            (.dependantId
                >> (==) dependantId
            )
        |> Maybe.Extra.unwrap [] .name

Which would 'force' me to make the comparison into a let..in helper ( needing to be in the same function closure ) leading to many more trivial functions:

let
    matchDependant dependant =
        dependant.dependantId == dependantId
in
    dependants
        |> List.Extra.find matchDependant
        |> Maybe.Extra.unwrap [] .name

Which begs the question, what about the -> because that's prolly what I'll reach for next.

    dependants
        |> List.Extra.find (\dependant -> dependant.dependantId == dependantId)
        |> Maybe.Extra.unwrap [] .name

Thinking about |> it seems to used for two separate intents: When describing a set of transformations, filter, map, unwrap, etc, then you want to put these on new lines. When used as a replacement for grouping function arguments (f (g x)) then you'd want it on the same line. x |> (f >> g). Imagine how that would read...

x
  |> (f
    >> g
  )

mordrax avatar Feb 21 '18 22:02 mordrax

I don't think I have a single instance of multi-line >>. 50% of my use case is with |> List.map (function1 >> function2) forcing them to multi-line would be horrible. The other 50% is to redefine quickly functions from other modules:

height : Float -> Svg.Attribute a
height =
    toString >> Svg.Attributes.height

Having that broken in multiple lines would look a bit pointless.

TL;DR: I would very much prefer to keep single-line >>. I don't think I use << anywhere.

xarvh avatar Jun 09 '18 21:06 xarvh

Single-line |> or <| are IMHO the least ugly way to use not, especially inside an anonymous function:

|> List.filter (\input -> not <| Dict.member input statesByKey)

vs

|> List.filter (\input -> not (Dict.member input statesByKey))

xarvh avatar Jun 09 '18 21:06 xarvh

Having this elm-css example.

css
            [ padding (calculateStuff (px 16))
            , paddingLeft (px 24)
            , paddingRight (px 24)
            , marginLeft (px 50)
            , marginRight auto
            ]

I personally prefer the first line to be written without nested parentheses.

padding <| calculateStuff <| px 16

Currently it's matter of choice and opinion. This PR would disallow it, because it would end in a bunch of line breaks. Since this proposal is marked for discussion, I'd vote against it.

andys8 avatar Nov 04 '18 02:11 andys8

strong vote against this too: stopping debates is a great goal (altough imo elm-format takes this to far generally), but code-style shouldn't be enforced in a way that can hurt expressivity or readability.

elkowar avatar Sep 11 '19 17:09 elkowar