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

fsharp_newline_between_type_definition_and_members inconsistency #42

Open
nojaf opened this issue Oct 19, 2023 · 1 comment
Open

fsharp_newline_between_type_definition_and_members inconsistency #42

nojaf opened this issue Oct 19, 2023 · 1 comment

Comments

@nojaf
Copy link
Contributor

nojaf commented Oct 19, 2023

Hi @Smaug123,

@auduchinok and I were talking about fsharp_newline_between_type_definition_and_members yesterday and we may have found a questionable case.

The setting is on by default in Fantomas:

type Foo with
    member x.Bar () = ()

type Foo2=
    member x.Bar2 () = ()

type Foo3 =
    abstract Bar3: unit -> unit

leads to

type Foo with

    member x.Bar() = ()

type Foo2 =
    member x.Bar2() = ()

type Foo3 =
    abstract Bar3: unit -> unit

Does the new line in type Foo with make sense? Is it desired by the teams?
My gut feeling is that we never discussed this case in great detail.
And the result we have today just happens to be how the implementation works.

Eugene advocated for having it only when there's a representation or non-member bindings/fields in the default code style.

type A =
    let a = 1

    member this.P = 1

type A =
    val field: int // not sure whether it's correct syntax

    member this.P = 1

type A =
    do ()

    member this.P = 1

type A =
    inherit T()

    member this.P = 1

type A =
    | Case

    member this.P = 1

type B =
    member this.P = 1

type B() =
    member this.P = 1

type B with
    member this.P = 1

I think this sounds reasonable, would be interested to know what the GR folks think about it.

@pleaseletmesearch
Copy link

(This is @Smaug123 commenting from my GR-internal GitHub account.)

I think the final snippet looks reasonable; the type Foo with\n\nmember x.Bar() = () does look a bit ugly. Omitting the first newline looks right to me. I'll just send this round in case anyone disagrees.

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

No branches or pull requests

2 participants