fsharp / fslang-design

RFCs and docs related to the F# language design process, see https://github.com/fsharp/fslang-suggestions to submit ideas
520 stars 143 forks source link

[style-guide] RFC: inconsistency on curried params invocation vs signature? #643

Closed knocte closed 2 years ago

knocte commented 2 years ago

Issue created from fantomas-online

Code

module Foo =
    let Bar (baz1: int) (baz2: string) =
        FooBarBaz (someFunc x) (someOtherFunc y)

Result

module Foo =
    let Bar (baz1: int) (baz2: string) =
        FooBarBaz(someFunc x) (someOtherFunc y)

Problem description

Not sure if this contradicts the F# formatting guidelines in ms docs, but looks inconsistent, because there's a space after Bar but there is no space after FooBarBaz, despite all _spacebefore settings being uniformly set to the same value.

Don't get me wrong, I'm not advocating for fantomas removing the space after Bar here, really; just calling out the inconsistency.

Extra information

Options

Fantomas 4.6 branch at 11/07/2021 14:33:13 - a4d21fe91323c394fdb2a0f97e0c980ff5378eaf

    { config with
                SpaceBeforeParameter = false
                SpaceBeforeLowercaseInvocation = false
                MaxFunctionBindingWidth = 0 }
nojaf commented 2 years ago

SpaceBeforeParameter is currently only taken into account when there is a single parameter. The documentation is on point for this.

Add a space after the name of a function and before the opening parenthesis of the first parameter.

Any changes on the matter require input from both style guides.

dsyme commented 2 years ago

@nojaf Maybe a tag for "style-guide-question"? Also feel free to @dsyme me on all these

dsyme commented 2 years ago

@nojaf I created the label (actually renamed "style-request-change")

nojaf commented 2 years ago

@dsyme Maybe we should instead encourage people to create these style guide clarification issues directly on the relevant style guide repos?

dsyme commented 2 years ago

@dsyme Maybe we should instead encourage people to create these style guide clarification issues directly on the relevant style guide repos?

I'll think about that. Probably fslang-suggestions will be best. dotnet/docs isn't the right place for this. Agreed it shouldn't be in fantomas

knocte commented 2 years ago

SpaceBeforeParameter is currently only taken into account when there is a single parameter. The documentation is on point for this.

Hey @nojaf, actually the documentation mentions the first parameter, not single. ~Also, from the example it seems it only affects function signatures, but not clear if it affects function invocations as well.~ I'm happy to open a PR to clarify the docs first, just let me know how I should do it? (After we're clear on that front, I'll follow-up about the original issue.)

knocte commented 2 years ago

but not clear if it affects function invocations as well.

Oops, the above statement is not true actually. Now, should fantomas also respect that setting for function invocations? I mean, would a PR to make fantomas do so be accepted?

nojaf commented 2 years ago

Taking a step back, I think there should always be spaces when there are multiple arguments passed to a function. The current guide isn't really addressing this.

@dsyme what are your thoughts on the matter?

dsyme commented 2 years ago

Taking a step back, I think there should always be spaces when there are multiple arguments passed to a function. The current guide isn't really addressing this.

Yes, I agree, curried invocations should always have spaces

knocte commented 2 years ago

Thanks, I opened this PR: https://github.com/dotnet/docs/pull/28141 . After it gets merged, we'll open PR for fantomas.