FSharp.Formatting icon indicating copy to clipboard operation
FSharp.Formatting copied to clipboard

Replace `ApiDocMemberDetails` union with a record

Open MangelMaxime opened this issue 1 year ago • 2 comments

ApiDocMemberDetails is currently described as a single case DUs.

https://github.com/fsprojects/FSharp.Formatting/blob/2efb355823b8311f91a06566c0a3de86037a590d/src/FSharp.Formatting.ApiDocs/GenerateModel.fs#L292-L301

This makes accessing the information in it not easy because tuples doesn't expose a named property to accessing the different values. The user is responsible, for manually naming each variables.

let (ApiDocMemberDetails(usageHtml,
                         paramTypes,
                         returnType,
                         modifiers,
                         typars,
                         baseType,
                         location,
                         compiledName)) =
    entity.Details

I propose to change this type to a record which will exposed named properties making it easier for consumer to use the type.

type ApiDocMemberDetails =
    {
        UsageHtml: ApiDocHtml 
        ParamTypes: (Choice<FSharpParameter, FSharpField> * string * ApiDocHtml) list 
        ReturnType: (FSharpType * ApiDocHtml) option 
        Modifiers: string list 
        Typars: string list 
        ExtendedType: (FSharpEntity * ApiDocHtml) option 
        Location: string option 
        CompiledName: string option
    }

// Usage
let details : ApiDocMemberDetails = // ...

details.ParamTypes 
// etc.

MangelMaxime avatar Jul 14 '24 13:07 MangelMaxime

I see only one usage of this, is this really worth the breaking change? How many times does this bother you?

nojaf avatar Aug 05 '24 15:08 nojaf

I am not using it very often, but when doing so it is a pain to manually find the name of the properties.

A record is also more robust because you can make it evolve without breaking.

I leave it up to you (maintainers) to decide if making the switch is worth it or not. If not feel free to close :)

MangelMaxime avatar Aug 12 '24 12:08 MangelMaxime