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

F# style guidelines: no advice given for reverse pipeline operator #21459

Open
knocte opened this issue Nov 10, 2020 · 5 comments
Open

F# style guidelines: no advice given for reverse pipeline operator #21459

knocte opened this issue Nov 10, 2020 · 5 comments
Assignees
Labels
doc-enhancement Improve the current content [org][type][category] dotnet-fsharp/svc Pri2

Comments

@knocte
Copy link
Contributor

knocte commented Nov 10, 2020

Issue description

Advice is given about how to use forward pipeline operators |> (chaining them by putting an EOL character next to them on the left side), e.g.

someElement
|> someFunc
|> otherFunc

However, what to do with reverse pipeline operators? Splitting them in the same way looks odd, e.g.:

failwith <| sprintf "foo: %s - bar: %s" foo bar

failwith
<| sprintf "foobar: %s - foobarbaz: %s" foobar foobarbaz

So I recommend that an example is given which looks like the following:

failwith <| sprintf "foo: %s - bar: %s" foo bar

failwith <| sprintf "foobar: %s - foobarbaz: %s"
                    foobar
                    foobarbaz

Document details added by adegeo


Document Details

Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

@nojaf
Copy link
Contributor

nojaf commented Nov 10, 2020

I would like to point out that the recommendation has a 'vanity' indentation problem I believe.
foobar and foorbarbaz are indented with 20 spaces in the example and I think that is determined by the length of the word failwith.

If it were failwithSuperLongBananas, the indentation would increase because of the function name.

failwithSuperLongBananas <| sprintf "foobar: %s - foobarbaz: %s"
                                foobar
                                foobarbaz

where

failwithSuperLongBananas 
<| sprintf "foobar: %s - foobarbaz: %s" foobar foobarbaz

does not have this problem.

@cmeeren, @pragmatrix, @lydell this is the place do discuss these things ;)

@lydell
Copy link

lydell commented Nov 10, 2020

For reference, the Fantomas issue mentioning vanity indentation: fsprojects/fantomas#659

@cmeeren
Copy link
Contributor

cmeeren commented Nov 10, 2020

@nojaf What do you mean by

this is the place do discuss these things ;)

Are you saying that the Fantomas repo is not the place to discuss the issue @lydell linked to above, and that unless this is rectified in the official style guidelines, vanity indentation won't be changed in Fantomas?

If so, remember, the guidelines are just that, guidelines :)

(Mandatory Captain Barbossa meme)

In any case, I think the linked issue pretty much sums up why vanity indentation might not be a good idea.

@cartermp
Copy link
Contributor

failwithSuperLongBananas 
<| sprintf "foobar: %s - foobarbaz: %s" foobar foobarbaz

Makes me giggle but I suppose it's the least friction. Happy to accept a PR!

@nojaf
Copy link
Contributor

nojaf commented Nov 11, 2020

@cmeeren, style needs to discussed over here and what gets in this style guide gets in Fantomas.
Sorry for the burden Phillip ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc-enhancement Improve the current content [org][type][category] dotnet-fsharp/svc Pri2
Projects
None yet
Development

No branches or pull requests

9 participants