Formatting of long parameter lists

[cmeeren]

Online here

Input:

type RequestParser<'ctx, 'a> = internal {
  consumedFields: Set<ConsumedFieldName>
  parse: 'ctx -> Request ->  Async<Result<'a, Error list>>
  prohibited: ProhibitedRequestGetter list
} with

  static member internal Create(consumedFields, parse: 'ctx -> Request -> Async<Result<'a, Error list>>) : RequestParser<'ctx, 'a> =
    {
      consumedFields = consumedFields
      parse = parse
      prohibited = []
    }

Output:

type RequestParser<'ctx, 'a> =
  internal { consumedFields: Set<ConsumedFieldName>
             parse: 'ctx -> Request -> Async<Result<'a, Error list>>
             prohibited: ProhibitedRequestGetter list }

  static member internal Create(consumedFields,
                                parse: 'ctx -> Request -> Async<Result<'a, Error list>>): RequestParser<'ctx, 'a> =
    { consumedFields = consumedFields
      parse = parse
      prohibited = [] }

I strongly dislike code that is indented far to the right to "match up", for the following reasons

• Important code moved far to the right
• There is little space left for the actual code
• There is an unseemly gap

I usually format it like this:

  static member internal Create
      ( consumedFields
      ,  parse: 'ctx -> Request -> Async<Result<'a, Error list>>)
      : RequestParser<'ctx, 'a> =
    {
      consumedFields = consumedFields
      parse = parse
      prohibited = []
    }

• Every parameter on its own line
• Commas after each parameter (could also be placed first as suggested in Long C#-like arguments list  #232 - looks weird to me, but could probably get used to it if necessary)
• The last line has the colon and return type

[cmeeren]

For the record, dotnet/docs#21690 was just merged, so now the core issue here (avoiding vanity alignment) has style guide backing.

[nojaf]

@cmeeren for my understanding of the guide, would the expected outcome be

type RequestParser<'ctx, 'a> =
    internal
        { consumedFields: Set<ConsumedFieldName>
          parse: 'ctx -> Request -> Async<Result<'a, Error list>>
          prohibited: ProhibitedRequestGetter list }

        static member internal Create
            (
                consumedFields, parse: 'ctx -> Request -> Async<Result<'a, Error list>>
            ) : RequestParser<'ctx, 'a> =
            { consumedFields = consumedFields
              parse = parse
              prohibited = [] }

?

[cmeeren]

The style guide doesn't say this explicitly, but as I interpret it, members should be indented relative to the type declaration, not the constructor/fields (which creates one extra level of indentation without any benefit and breaks consistency with how class members are indented). So this is my interpretation:

type RequestParser<'ctx, 'a> =
    internal
        { consumedFields: Set<ConsumedFieldName>
          parse: 'ctx -> Request -> Async<Result<'a, Error list>>
          prohibited: ProhibitedRequestGetter list }

    static member internal Create
        (
            consumedFields, parse: 'ctx -> Request -> Async<Result<'a, Error list>>
        ) : RequestParser<'ctx, 'a> =
        { consumedFields = consumedFields
          parse = parse
          prohibited = [] }

However, I see that:

• The style guide examples indents parameters only 2 spaces from the opening parenthesis, without saying so explicitly
  Edit: Woops, it does indeed use 4 relative to the parenthesis. (I counted from the right side of the parenthesis and also missed one space.)
• This kind of indentation looks weird with only one parameter

I will seek clarification for these points (as well as the initial indentation question) back on the dotnet/docs repo.

[nojaf]

I will seek clarification for these points

Thank you @cmeeren!

[cmeeren]

@nojaf I think the question is resolved in dotnet/docs#21690. AFAIU the snippet I posted above should be correct.

[nojaf]

Well, I'd like Phillip to confirm here ;), so I'll wait until things are official ;)

Also, should it not be:

type RequestParser<'ctx, 'a> =
    internal
        { consumedFields: Set<ConsumedFieldName>
          parse: 'ctx -> Request -> Async<Result<'a, Error list>>
          prohibited: ProhibitedRequestGetter list }

    static member internal Create
        (
            consumedFields, 
            parse: 'ctx -> Request -> Async<Result<'a, Error list>>
        ) : RequestParser<'ctx, 'a> =
        { consumedFields = consumedFields
          parse = parse
          prohibited = [] }

I created #1300 for the static member indentation issue.
But the tuple should be split over two lines. consumedFields is just not named.

[cmeeren]

Well, I'd like Phillip to confirm here ;), so I'll wait until things are official ;)

👍

Also, should it not be:

Yes of course, my bad. I didn't notice there were two parameters.

[cartermp]

@nojaf that looks reasonable to me.