[style-guide] Reconsidering long tupled signatures

[dsyme]

@nojaf I'm applying fantomas latest to the signatures in the F# compiler. dotnet/fsharp#12346

I'm wondering if I made a mistake with the decision on long signature formatting. Do you know where the long discussion we had on this was, where I laid out the options? We settled on this:

    member ResolveDependencies:
        scriptDirectory: string
        * scriptName: string
        * scriptExt: string
        * timeout: int ->
        obj

When the thing is tupled+curried we have no real choice but to put the * at the start of line as I mentioned in the discussion. Howerver for things that are only tupled this is feeling more wrong to me than I realised. The most common is either everything curried or everything tupled. So it seems that for tupled-but-not-curried that we should use the nicer:

    member ResolveDependencies:
        scriptDirectory: string *
        scriptName: string *
        scriptExt: string *
        timeout: int ->
            obj

I still feel the return type should be indented too.

What do you think? If you agree I can make the adjustment to fantomas and apply to the F# style guide.

Thanks

That's just so much easier to read.

[smoothdeveloper]

we should use the nicer

I find it subjective, in my subjective experience, formatting stuff which needs a repeated separator (* here, often times , in SQL or C#) by putting at the beginning of the line, makes things more readable due to not having to scan end of line (of varying length) to know if stuff is continuing. I get same feeling with boolean operators (or just using |>, we almost always put it in front of new line rather than end of last one).

This, as yours, is just subjective opinion on formatting.

I'm super happy with what you describe as what was "settled on" and think more of the default F# formatting guidelines should end up formatting like this wrt prefixing next line with separator; although it is not the norm in regular languages.

Could you point at precise formatting issues you noticed from the dotnet/fsharp PR in relation to the formatting knob you are bringing here?

[smoothdeveloper]

IIUC, currently (🎉):

member ResolveDependencies:
    scriptDirectory: string
    * scriptName: string
    * scriptExt: string
    * timeout: int ->
    obj

@dsyme proposal (👀)

member ResolveDependencies:
    scriptDirectory: string *
    scriptName: string *
    scriptExt: string *
    timeout: int ->
        obj

adding mine (:smile:)

member ResolveDependencies:
    scriptDirectory: string
    * scriptName: string
    * scriptExt: string
    * timeout: int
    -> obj 

optionally, adding an indent, as @dsyme mentions it helps to identify return type (:rocket:):

member ResolveDependencies:
    scriptDirectory: string
    * scriptName: string
    * scriptExt: string
    * timeout: int
        -> obj 

people maybe can put the emoji on this post to rate which one they like, or suggest others?

[nojaf]

Hello @dsyme, thank you for bringing this up but we have a clear rule that style is not to be discussed here.
If the guide gets updated, Fantomas can adhere to it and no sooner.

This is a bit similar to F# language features/suggestions first need to be discussed/approved in the right repository before anything gets done dotnet/fsharp. This protects Fantomas from changing too often and without the proper protocol.
Perhaps I should update the contribution guidelines or issue template so this is better covered.

[knocte]

@smoothdeveloper IMO you should edit your comment to adjust indentation sizes: dsyme used 4 spaces, you used 2 in one place (last snippet) and 8 in all of them after the member line.

[smoothdeveloper]

@knocte, thanks for attention to details, I've adjusted it, I hope correctly.

@nojaf, thanks for the feedback, makes sense, alas, I'm unsure if formatting discussions really occur on dotnet/fsharp or if they are suitable in the other repositories. I've seen discussions occuring in PRs, and dotnet/docs most often; @dsyme, maybe it would be good to have a formatting discussion thread under dotnet/fsharp for now?

[nojaf]

@smoothdeveloper I am not at all asking for discussions to take place under dotnet/fsharp, I was merely drawing a parallel with fsharp/fslang-suggestions. The correct location is docs/fsharp/style-guide/formatting.md

[dsyme]

I'm ok with style guide discussions happening at fslang-suggestions. I'd consider @nojaf and myself joint final approvers on the actual style-guide.

Regarding this:

member ResolveDependencies:
    scriptDirectory: string
    * scriptName: string
    * scriptExt: string
    * timeout: int
        -> obj 

My problem with this is that the * creates needless difficulty when reading the argument names. For a tupled signature, it's just not useful.

[dsyme]

I note that Ionide uses

• trailing *
• indented ->
• aligned :

in its tooltips:

I can do without the alignment of : for fantomas of course.....

[nojaf]

I can do without the alignment of : for fantomas of course.....

Trust me, vanity alignment police would find you 😋.

Jokes aside, the problem I would have with the alignment of : is that you introduce a git diff for all lines the moment you change the longest name (innerExpr in this case).

[knocte]

is that you introduce a git diff for all lines the moment you change the longest name

And that's exactly the reason behind the motives of the vanity police ;) (disclaimer: it wasn't me who pushed for non-vanity alignment in the beginning 😅 )

trailing *

+1 from me, because then this way the "*" symbol gets consistent with how the "," is treated in the case of tuples.

[nojaf]

Hello @dsyme, I have some faint memories where it would be most consistent to have the * in the back (similar to long union definitions).

In order for this to always work, an extra indent would be necessary when a type was preceded by a tuple.

namespace Foo

type WithWarning =
    member ResolveDependencies:
        scriptDirectory: string * 
        scriptName: string ->
        scriptName: string

type WithoutWarning =
    member ResolveDependencies:
        scriptDirectory: string * 
        scriptName: string ->
            scriptName: string

(online tool)

And for esthetic purposes we can choose to always indent the last type, to highlight its role as the return type.

type Meh =
    member ResolveDependencies:
        scriptDirectory: string *
        scriptName: string *
        scriptExt: string *
        timeout: int ->
            obj

Does that sound about right?

[dsyme]

Yes, that sounds right