Different infix operators not aligned/broken correctly

cmeeren commented 1 year ago

Code

let lens =
    type_
    >-> ObservationType.publishedObservation_
    >?> PublishedObservationData.hasExtendedVisibility_

Result

let lens =
    type_ >-> ObservationType.publishedObservation_
    >?> PublishedObservationData.hasExtendedVisibility_

Problem description

The initial code is how I would expect it to be formatted.

Note that it is formatted correctly if using the same operator for both (whether >->, >?>, or something else, like >=>).

(For those interested, these operators are from Aether.)

Extra information

[ ] The formatted result breaks my code.
[ ] The formatted result gives compiler warnings.
[ ] I or my company would be willing to help fix this.

Options

Fantomas main branch at 1/1/1990

Default Fantomas configuration

_{Did you know that you can ignore files when formatting from fantomas-tool or the FAKE targets by using a .fantomasignore file?}

seanamos commented 1 year ago

I resorted to the suggestion to use a comment to force a newline, not exactly pretty =/

josh-degraw commented 10 months ago

So short answer here is that at this point, this is actually a feature, not a bug: there's a specific handling for if the infix operators are the same. If guess this is mainly there to treat pipe-like operations the same but it doesn't account for situations like this where there are several similar but different operations like these.

If this behavior is unwanted the best way to work around it for now is probably by utilizing fsharp_max_infix_operator_expression.

If that is not sufficient for your use case, please feel free to open another issue if you'd like to suggest a new feature to handle this differently.

cmeeren commented 10 months ago

Reducing fsharp_max_infix_operator_expression causes too many other unwanted changes in my code. Also, it doesn't catch the case where the operator arguments are sufficiently short.

I think this can be solved by https://github.com/fsprojects/fantomas/issues/988#issuecomment-670365098:

I think this may be solved by Fantomas knowing about operator precedence.

Quasi algorithm:

If line is too long, find lowest precedence operator and break on that (break on all operators with the same precedence)

If some of the broken lines are still too long, find the next lowest precedence operator(s) in those lines and break on those, but indent as demonstrated in this issue

Do this recursively, of course.

In summary, I think this can be solved by Fantomas considering operator precedence. Instead of breaking chains on identical operators as you describe, break on operators of equal precedence. The operator precedence rules for F# are AFAIK well-defined (also for custom operators) and could be hardcoded in Fantomas.

eithermonad commented 10 months ago

I've been running into this same issue. Just to add some additional examples:

When using |>> as a generic map operator and mixing it with |>, this code:

let test (x: int) =
    x
    |> computationOne 
    |> computationTwo
    |>> Option.map ((+) 3)
    |>> Option.toValidOr "Problem"

becomes

let test (x: int) =
    x |> computationOne |> computationTwo
    |>> Option.map ((+) 3)
    |>> Option.toValidOr "Problem"

Which, in my opinion, is much less readable.

Similarly, applicative-style expressions are affected, with:

let result =
    create
    <!> validate 1
    <*> validate 2
    <*> validate 3
    <*> validate 4
    <*> validate 5
    <*> validate 6

which is fine as-is, becoming

let result =
    create <!> validate 1
    <*> validate 2
    <*> validate 3
    <*> validate 4
    <*> validate 5
    <*> validate 6

Setting fsharp_max_infix_operator_expression to a lower number isn't really an option for me because it affects non-pipeline-like expressions. I, too, will likely resort to using an // to force a line break, but it's less than ideal and adds noise.

fsprojects / fantomas