fslang-design icon indicating copy to clipboard operation
fslang-design copied to clipboard

[style-guide] Parameter owner patterns should be consistent with prefix application expressions

Open auduchinok opened this issue 3 years ago • 10 comments

Union cases patterns should be consistent with their creation expressions, like this:

let _ = Foo x
let _ = Bar(1, 2)

match s with
| Foo x -> ()
| Bar(1, 2) -> ()

Currently Fantomas adds an extra space in the last pattern, which makes it inconsistent. I propose we should fix this.

auduchinok avatar Sep 26 '22 14:09 auduchinok

@nojaf My initial decision is to agree with this. Please let me know if you think this is wrong or inconsistent.

dsyme avatar Sep 29 '22 13:09 dsyme

Sounds reasonable. We currently have two settings that control the spaces before upper/lower case invocations (comment).

These could be re-used. Makes sense.

nojaf avatar Sep 29 '22 13:09 nojaf

Hi @dsyme, I've implemented this on Fantomas' side: https://github.com/fsprojects/fantomas/pull/2551 I think we should just go with this, it makes for a more consistent story.

nojaf avatar Oct 03 '22 16:10 nojaf

Great! thank you

dsyme avatar Oct 27 '22 15:10 dsyme

@nojaf @auduchinok Do we need a docs update for this?

dsyme avatar Oct 27 '22 15:10 dsyme

Well, there is no real section on formatting patterns, so maybe we Maybe we need a new section on the level of "Formatting expressions" and "Formatting declarations" called "Formatting patterns"? It could also serve to capture the eventual outcome of https://github.com/fsharp/fslang-design/issues/647.

nojaf avatar Oct 29 '22 07:10 nojaf

This is a hill I am willing to die on. I propose we adjust union case construction expressions to mirror the status quo of pattern decomposition instead. The style guide mandates that there be a space between function and tupled arguments (I'd argue this should apply to method invocation too, but that's a different matter). Since class and case constructors can be used like functions, I don't see why the Bar(1, 2) <-> func (1, 2) distinction is desirable.

kerams avatar Aug 24 '23 07:08 kerams

@kerams While being skeptical about changing such a long standing default behaviour, there's a setting that should help: https://fsprojects.github.io/fantomas/docs/end-users/Configuration.html#fsharp_space_before_uppercase_invocation

dawedawe avatar Aug 24 '23 07:08 dawedawe

I've also tried to share some context in https://github.com/dotnet/fsharp/pull/15847#discussion_r1307322325.

auduchinok avatar Aug 28 '23 13:08 auduchinok

there is also this edge case https://github.com/dotnet/fsharp/issues/15780 for fluent notation that currently prevents method and arguments to be separate.

smoothdeveloper avatar Aug 28 '23 13:08 smoothdeveloper