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] Parameter owner patterns should be consistent with prefix application expressions #712

Open
auduchinok opened this issue Sep 26, 2022 · 10 comments

Comments

@auduchinok
Copy link
Contributor

auduchinok commented Sep 26, 2022

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.

@dsyme
Copy link
Contributor

dsyme commented Sep 29, 2022

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

@nojaf
Copy link
Contributor

nojaf commented Sep 29, 2022

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
Copy link
Contributor

nojaf commented Oct 3, 2022

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

@dsyme
Copy link
Contributor

dsyme commented Oct 27, 2022

Great! thank you

@dsyme
Copy link
Contributor

dsyme commented Oct 27, 2022

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

@nojaf
Copy link
Contributor

nojaf commented Oct 29, 2022

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 #647.

@kerams
Copy link
Contributor

kerams commented Aug 24, 2023

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.

@dawedawe
Copy link
Contributor

@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

@auduchinok
Copy link
Contributor Author

I've also tried to share some context in dotnet/fsharp#15847 (comment).

@smoothdeveloper
Copy link
Contributor

there is also this edge case dotnet/fsharp#15780 for fluent notation that currently prevents method and arguments to be separate.

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

6 participants