Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[style-guide]: Multiline generic type parameters bracket alignment #756

Open
josh-degraw opened this issue Nov 13, 2023 · 3 comments
Open

Comments

@josh-degraw
Copy link

As discussed in fsprojects/fantomas#2706 and fsprojects/fantomas#2852, with the recent changes regarding multiline bracket formatting, multiline generic type parameters don't necessarily always follow the same guidelines, so we should formalize how they can/should be formatted.

For example, if using Stroustrup bracket style, you might expect

let private asJson (arm: IArmResource) =
    arm.JsonModel
    |> convertTo<{|
        kind: string
        properties: {| statisticsEnabled: bool |}
    |}>

to be formatted like this:

let private asJson (arm: IArmResource) =
    arm.JsonModel
    |> convertTo<
           {|
               kind: string
               properties: {| statisticsEnabled: bool |}
           |} 
       >

However, the placement of the angle brackets can be problematic.

For example, if you have an application expression:

let bv = unbox<Foo<'innerContextLongLongLong, 'bb -> 'b>> bf

And try to directly apply Stroustrup style to it:

let bv =
    unbox<
        Foo<
            'innerContextLongLongLong,
            'bb -> 'b
        >
    >
        bf

This code is invalid. The compiler sees these as two separate statements. So short of updating the compiler, code in this style will need to have at least one additional space on the newline before the closing angle bracket.

let bv =
    unbox<
        Foo<
            'innerContextLongLongLong,
            'bb -> 'b
-        >
+         >
-    >
+     >
        bf

Bikeshedding question: should this be the minimal change necessary (1 space)? Or should it be a multiplier of your indent_size (if you use 2 spaces, use 2 spaces)?

Also, it probably should be noted that truly "Aligned" angle bracket style doesn't work:

let private asJson (arm: IArmResource) =
    arm.JsonModel
    |> convertTo
        <
           {|
               kind: string
               properties: {| statisticsEnabled: bool |}
           |} 
        >

Some users may expect that to work but it's invalid code. We should maybe update the style to mention this (if it's not there already).

@nojaf
Copy link
Contributor

nojaf commented Nov 13, 2023

Thanks for bringing this up!

Bikeshedding question: should this be the minimal change necessary (1 space)? Or should it be a multiplier of your indent_size (if you use 2 spaces, use 2 spaces)?

Using the indent_size will make it seem more deliberate I think. A single space will indeed strike more as a bug.

@dsyme
Copy link
Contributor

dsyme commented Nov 14, 2023

From a style guide perspective, the above proposal seems fine.

@dsyme
Copy link
Contributor

dsyme commented Nov 14, 2023

This code is invalid. The compiler sees these as two separate statements. So short of updating the compiler, code in this style will need to have at least one additional space on the newline before the closing angle bracket.

I agree that ideally this should be valid for the compiler.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants