[style-guide] Question about match block having vanity indentation

[knocte]

Issue created from fantomas-online

Code

match directoryRouter.GetIdentity ()
      |> self.ServerDescriptors.TryFind with
| None -> CircuitNodeDetail.FastCreate
| Some serverDescriptor ->
    self.ConvertToCircuitNodeDetail serverDescriptor

Result

match directoryRouter.GetIdentity()
      |> self.ServerDescriptors.TryFind with
| None -> CircuitNodeDetail.FastCreate
| Some serverDescriptor -> self.ConvertToCircuitNodeDetail serverDescriptor

Expected Result

match
    directoryRouter.GetIdentity()
    |> self.ServerDescriptors.TryFind
    with
| None -> CircuitNodeDetail.FastCreate
| Some serverDescriptor -> self.ConvertToCircuitNodeDetail serverDescriptor

Extra information

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

Options

Fantomas 4.6 branch at 10/12/2021 15:28:15 - d7b4f109e13c86475ec3bb03ea7eeea7029d2e0c

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?}

[nojaf]

So, this happens because we added the exception of infix expressions in

https://github.com/fsprojects/fantomas/blob/d7b4f109e13c86475ec3bb03ea7eeea7029d2e0c/src/Fantomas/CodePrinter.fs#L3286

The main reason to have this was that a lot of tests were impacted for if/then/else.
Would you consider

if directoryRouter.GetIdentity()
   |> self.ServerDescriptors.TryFind then
    CircuitNodeDetail.FastCreate
else
    self.ConvertToCircuitNodeDetail serverDescriptor

then also as a vanity alignment?

I'm asking because I want to be a bit cautious that we don't go too far here as this will have a lot of impact on a lot of end-user code.

I've raised dotnet/docs#26507 to get some clarification.

[hvester]

I would prefer that with would be aligned with match like this

match
    directoryRouter.GetIdentity()
    |> self.ServerDescriptors.TryFind
with
| None -> CircuitNodeDetail.FastCreate
| Some serverDescriptor -> self.ConvertToCircuitNodeDetail serverDescriptor

Then the formatting would be inline with the formatting of try .. with with multiple match cases.

The following formatting of if/then/else is in my opinion visually distracting, because there is not clear enough distinction between the condition expression and the expression after then. I would prefer to put thenon the next line and align it with if.

if directoryRouter.GetIdentity()
   |> self.ServerDescriptors.TryFind then
    CircuitNodeDetail.FastCreate
else
    self.ConvertToCircuitNodeDetail serverDescriptor

[knocte]

Hey @hvester those are very good points! Do you mind creating PRs in the style-guide to address those? I'm thinking 1 PR for the match thing and another one for the if thing.

[knocte]

Do you mind creating PRs in the style-guide to address those?

And they would reference to address this issue: dotnet/docs#26507 (or rather, discuss in the issue before proposing PR).

[knocte]

I would prefer that with would be aligned with match like this

Arghhh, unfortunately that snippet doesn't compile @hvester. So the 4 spaces before the with keyword are definitely required :(

[dsyme]

I've added a comment on the style-guide question here: dotnet/docs#26507 (comment)

[dsyme]

In short I propose we make the following the default in the style guide

1. It is encouraged to avoid multi-line inputs to match .. with and if ... then

2. Multiline inputs on match are formatted with match and with like this:

match
    directoryRouter.GetIdentity ()
    |> self.ServerDescriptors.TryFind
with
| None ->
    CircuitNodeDetail.FastCreate
| Some serverDescriptor ->
    self.ConvertToCircuitNodeDetail serverDescriptor

2. Multiline inputs on if are formatted with if and then like this:

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

4. The default tolerance for the length of the single-line containging if .. then should be to end-of-line

5. The default tolerance for the length of the single-line containing match .. with should be to end-of-line. If any break is needed then the mutli-lnie formatting should be used, even if this results in three lines

   match
       long-expr
   with

   That is we should never get

   match long-expr
   with

[dsyme]

I do wonder if the default fsharp_max_infix_operator_expression should be increased as part of this. It seems to be a considerable cause of needless multi-lining of multiline inputs to match and if

[nojaf]

It seems reasonable to increase fsharp_max_infix_operator_expression, let me know over at Fantomas' side what makes sense.

[smoothdeveloper]

Relegating the boolean operators to the end of the line doesn't make it clearer to me.

I have same feeling about trailing commas rather than leading, and I think it is minority of people who get acquainted with puttingin leading position.

Still, since F# likes to put operator such as |> in leading rather than trailing position, maybe it should be evaluated if

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

is actually better than (especially with long things):

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

[nojaf]

Please open a new issue for this.
This is irrelevant in regards to the original long matchExpr question.