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

[style-guide] prefer leading rather than trailing operators #687

Open
smoothdeveloper opened this issue Jul 5, 2022 · 8 comments
Open

Comments

@smoothdeveloper
Copy link
Contributor

This is a suggestion regarding position of operators in chained expressions

starting from:

if
    long-thing &&
    another-long-thing  &&
    another-long-thing &&
    another-long-thing
then

I generally feel it is actually better (especially with long things) to put the operator in a leading position on the next line:

if
    long-thing
    && another-long-thing
    && another-long-thing
    && another-long-thing
then

The same rule could be extended for , as well.

let a =
    long-thing,
    another-long-thing,
    another-long-thing
let a =
    long-thing
    , another-long-thing
    , another-long-thing

Note that F# users tend to use |> operator in such manner.

Also, most languages using "fluent notation" (chaining method calls), would put the . operator in leading position of next line

a
    .Something()
    .AnotherThing()

I've consistently felt it is more readable, easier to extend code, in many languages, using this type of positioning.

@nojaf
Copy link
Contributor

nojaf commented Jul 6, 2022

The style guide is quite incomplete when it comes to infix operators.
I am however going to need more than your feelings to accept any input.
Can you at the very least try and come up with any arguments?

Next, I do not see the parallel between the separators tokens of tuples and infix operators.
This is comparing apples with oranges to me.
The only reference I could find to multi-line tuples is in https://docs.microsoft.com/en-us/dotnet/fsharp/style-guide/formatting#formatting-function-and-member-arguments where they are placed at the back. Which is consistent with how * and -> are formatted in signature files.

Chained method calls are also a completely different topic, I again do not see the parallel with both operators and tuple separators.

@smoothdeveloper
Copy link
Contributor Author

arguments:

  • scanning for the operator in various places or in a consistent place, no matter length of expression
  • when appending one more entry, only the first spot is a special case, more frequent to append than prepend in first spot, less diff
  • no need to make special distinction: commas, operators, dots, it is more consistent to have at the front of next line

This issue is to gather feedback, and most entrenched style guides of most entrenched languages, they seem to look over the points above, for some reason; maybe F# can break this line, if there is adhesion to the reasons or the overall feeling.

@auduchinok
Copy link

For some reason I always find it considerably harder to read it when operators are shifted to the next line. When they're on the left operand line, they show that there's another related expression on the next line, and it reads naturally. The only exception for me is pipe operators, but they almost look like a part of F# syntax to me.
It also looks like moving the operators to the next line is adopted much less, and it doesn't seem good to diverge from commons styles, especially in the official style guide of a language.

@nojaf
Copy link
Contributor

nojaf commented Jul 7, 2022

One disadvantage I see from putting the operator on the next line is that it can break "indentation flow"
Example:

let v =
    expr1 //
    +> expr2

if the chosen indent_size is 4 then expr2 starts at column 7. This is no longer a multitude of 4 and depending on what expr2 is, you might need to lock the entire expression to that column 7 offset.

let v =
    expr1 //
    +> if a then
           b
       else
           c

The else keyword starts at column 7. b and c at 11. This disrupts the typing code experience a bit.

A disadvantage of putting the operator on the same line after the expression is that it can lead to indentation warnings and twist the meaning of the code.

match [] with | [] -> 0 | _ -> 42 +
2

Instead of having the match + 2, the + is parsed as 42 + 2 instead.
There are a couple of left-hand expressions that will face this problem (if/then/else, try/with, ...).

Another interesting case is that some operators cannot have the right-hand side on the next line, regardless of where the operator is placed.

// not valid
"""long string

""" =
"""other long 
string"""

// also not valid
"""long string

"""
= """other long 
string"""

// valid
"""long string

""" = """other long 
string"""

// also valid
"""long string

""" =
       """other long 
string"""

@cmeeren
Copy link

cmeeren commented Jul 9, 2022

Not exactly sure what @smoothdeveloper is getting at in the OP; operators are currently by default placed at the start, aren't they?

Is this then only about ,? If so, I have never found a problem with the readability of trailing , and generally prefer that, though admittedly mostly out of habit. I can see myself getting used to "elm-style" leading separators if the F# world would turn in that direction, though I haven't seen it much in the wild. A possible (not necessarily strong) argument for trailing , is that it's more consistent with the rest of the .NET world.

In any case, my two cents about some of the points in @nojaf's latest comment:

match [] with | [] -> 0 | _ -> 42 +
2

Instead of having the match + 2, the + is parsed as 42 + 2 instead.

I think this is clearly a case of "bad code formats badly" and an acceptable limitation. Anecdotally, I've been programming F# full-time for five years, and regardless of the operator placement, I can't tell with reasonable certainty how that expression should evaluate. I would never publish it without testing it, which means I would refactor it so I wouldn't need to test it.

if the chosen indent_size is 4 then expr2 starts at column 7. This is no longer a multitude of 4 and depending on what expr2 is, you might need to lock the entire expression to that column 7 offset.

let v =
    expr1 //
    +> if a then
           b
       else
           c

The else keyword starts at column 7. b and c at 11. This disrupts the typing code experience a bit.

It does disrupt it a bit, but in the very few cases where I use a multiline expression in this manner, I am prepared to live with that as a limitation. What you show in that example is my preferred formatting.

@smoothdeveloper
Copy link
Contributor Author

@auduchinok

It also looks like moving the operators to the next line is adopted much less, and it doesn't seem good to diverge from commons styles

I found a compelling explanation under the "How to indent operations" of https://ocaml.org/docs/guidelines

Justification: When the operator starts the line, it is clear that the operation continues on this line.

A style being less adopted is just like a language which is less adopted, it doesn't mean something is bad about it.

@nojaf

you might need to lock the entire expression to that column 7 offset.
This disrupts the typing code experience a bit.

Agreed, but F# causes the indentation challenges, and advanced editors and formatters evolve to incorporate the more advanced logic.

Especially there are few constructs in F# that cause this kind of outcome, depending brace placement, etc. I think it is ok to overlook the particular offset that wouldn't match an integer multiple of the default indent size.

@cmeeren

Not exactly sure what @smoothdeveloper is getting at in the OP; operators are currently by default placed at the start

I created this issue as follow-up to this comment: #646 (comment) and I am happy to see fantomas already does that for many cases.

I'd personally like F# style-guide adopt same recommendation as what it says in the OCaml guidelines, but I know it is not the default, but for very few languages.

Some C# and C++ formatters support this for comma now :

image
from
https://youtrack.jetbrains.com/issue/RSRP-380962/Line-break-formatting-does-not-support-beginning-lines-with-a-comma#focus=Comments-27-1873499.0-0

@kspeakman
Copy link

kspeakman commented Aug 4, 2022

Here's what Elm-style looks like, same for records (curly brace instead of paren). I've had a few cases where this made sense, usually method/constructor calls where I include parameter names for clarity and the line is too long.

let madeUpTuple =
    ( moderate sized expression
    , 42
    , pipe |> into |> all |> the |> things
    )

Any way you choose requires 2-3 different selection approaches to copy/paste, depending if the item is first, middle, or last. And it's always a pain to move first item to last position and vice versa. I try to avoid multi-line tuples.

Lead/trailing infix operators are subjective. I cannot stand trailing, with few exceptions. Probably built-up hate from VB line continuation chars. Also because operators stand out, they look out of place at the end to me vs lining them up at beginning.

I feel like some F# styles already use non-tab-aligned code. (People who use this might also prefer 2-space tabs instead of 4 to avoid the issue.)

let record =
    { Name = "foo"
      IsActive = true }

Same for lists. Always have to fix spacing after unindent in VS pro. 2 selection strategies required. I like just clicking line numbers and dragging to select. So I rarely use this style.

@En3Tho
Copy link

En3Tho commented Aug 4, 2022

It's only my personal opinion but I feel like all operators should generally behave the same: e.g. if pipe operator is moved to new line then and/or e.t.c. should be moved too.

  1. It is consistent

  2. Like with pipe operator it means take result of previous line and apply it via operator and it works with every binary operator (you don't have to take precedence into account, new line is here to help)

  3. You don't have to move your eyes up + right to see/understand what operation is applied. New line starts with it so you just see it naturally.

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

No branches or pull requests

7 participants