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

Lambda functions in fluent API calls should indent further #970

Closed
2 of 5 tasks
reinux opened this issue Jul 15, 2020 · 4 comments
Closed
2 of 5 tasks

Lambda functions in fluent API calls should indent further #970

reinux opened this issue Jul 15, 2020 · 4 comments

Comments

@reinux
Copy link

reinux commented Jul 15, 2020

I propose we ... indent lambda functions in fluent API calls further.

Examples

Currently, it looks something like this:

something.Add(fun x ->
         x.a <- 1
         x.b <- 2)
         .Add2(fun y ->
         y.a <- 3
         y.b <- 4)

Or in a more practical scenario:

  services.AddAuthentication(fun options ->
          options.DefaultScheme <- "Cookies"
          options.DefaultChallengeScheme <- "oidc").AddCookie("Cookies")
          .AddOpenIdConnect(fun options ->
          options.Authority <- "http://localhost:7001"
          options.ClientId <- "mvc"
          options.ClientSecret <- "secret"
          options.ResponseType <- "code"
          options.SaveTokens <- true)

.AddOpenIdConnect and .AddAuthentication have the same indentation as the content of the function, which makes it harder to understand where the first call ends.

The indentation is nicer when the inner expression is a record, which I think should be a similar scenario visually:

something2.Add({ a_really_long_name = 1
                 another_really_long_name = 2 })
          .Add({ a_really_long_name = 3
                 another_really_long_name = 4 })

I think it should look like either:

something
    .Add(fun x ->
        x.a <- 1
        x.b <- 2)
    .Add2(fun y ->
         y.a <- 3
         y.b <- 4)

services
    .AddAuthentication(fun options ->
        options.DefaultScheme <- "Cookies"
        options.DefaultChallengeScheme <- "oidc").AddCookie("Cookies")
    .AddOpenIdConnect(fun options ->
        options.Authority <- "http://localhost:7001"
        options.ClientId <- "mvc"
        options.ClientSecret <- "secret"
        options.ResponseType <- "code"
        options.SaveTokens <- true)
 |> ignore

or:

something.Add(fun x ->
            x.a <- 1
            x.b <- 2)
         .Add2(fun y ->
            y.a <- 3
            y.b <- 4)

services.AddAuthentication(fun options ->
            options.DefaultScheme <- "Cookies"
            options.DefaultChallengeScheme <- "oidc").AddCookie("Cookies")
        .AddOpenIdConnect(fun options ->
            options.Authority <- "http://localhost:7001"
            options.ClientId <- "mvc"
            options.ClientSecret <- "secret"
            options.ResponseType <- "code"
            options.SaveTokens <- true)
 |> ignore

Pros and Cons

Pros:

  • Easier to see how expressions are nested

Cons:

  • May need to move indentation back past the dot on the outer expression
  • Otherwise the call may get too long horizontally

Extra information

Estimated cost (XS, S, M, L, XL, XXL): M

Affidavit (please submit!)

Please tick this by placing a cross in the box:

Please tick all that apply:

  • This is not a breaking change to Fantomas
  • I or my company would be willing to help implement and/or test this
  • This suggestion is part of the Microsoft style guide (please add a link to section if so)
  • This suggestion is part of the G-Research style guide (please add a link to section if so)
@nojaf
Copy link
Contributor

nojaf commented Jul 17, 2020

At first glance, this is ok to have.
I'm not sure what the impact would be for implementing this though.
Chances are the formatting of the lambda is something common and could impact other areas as well.
This should be investigated.

@reinux
Copy link
Author

reinux commented Jul 17, 2020

I see what you mean.

services.AddHttpsRedirection(Action<HttpsRedirectionOptions>(fun options ->
    options.HttpsPort <- Nullable(7002)
)) |> ignore

will end up becoming

services.AddHttpsRedirection(Action<HttpsRedirectionOptions>(fun options ->
          options.HttpsPort <- Nullable(7002)
)) |> ignore

Which is a bit jarring.

Maybe it should only apply when it's otherwise aligned with the dot. It only really looks weird when it's aligned, so that might work.

Not sure how easy that is to check for.

nojaf added a commit to nojaf/fantomas that referenced this issue Oct 25, 2020
@nojaf
Copy link
Contributor

nojaf commented Oct 25, 2020

Hey @reinux, while solving #1202 I saw an opportunity to improve the indentation of lambdas.
See 7f6a833

@reinux
Copy link
Author

reinux commented Oct 25, 2020

Nice! Thanks!

@nojaf nojaf closed this as completed in 5222b1a Oct 26, 2020
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

2 participants