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

Unnecessary breaks and name-sensitive alignment when using Stroustrup #3130

Open
1 of 4 tasks
cmeeren opened this issue Oct 18, 2024 · 1 comment
Open
1 of 4 tasks

Comments

@cmeeren
Copy link
Contributor

cmeeren commented Oct 18, 2024

Issue created from fantomas-online

Code and expected output

This is the output when using default settings (not Stroustrup).

let getUpdateBklrEmailEvent user =
    match user with
    | { PrimaryEmail = Some { Value = newEmail }
        PersistedState = Some { PrimaryEmail = Some { Value = oldEmail } } } when newEmail <> oldEmail ->
        Some(UpdateBklrEmail { Old = oldEmail; New = newEmail })
    | _ -> None

Result when using Stroustrup

let getUpdateBklrEmailEvent user =
    match user with
    | {
          PrimaryEmail = Some { Value = newEmail }
          PersistedState = Some {
                                    PrimaryEmail = Some { Value = oldEmail }
                                }
      } when newEmail <> oldEmail -> Some(UpdateBklrEmail { Old = oldEmail; New = newEmail })
    | _ -> None

Problem description

  1. The line break after PersistedState = Some { is unnecessary; it fits on a single line.
  2. The sub-record inside PersistedState is formatted with name-sensitive alignment. If it has to be broken into multiple lines, it should be something like:
let getUpdateBklrEmailEvent user =
    match user with
    | {
          PrimaryEmail = Some { Value = newEmail }
          PersistedState = 
              Some {
                  PrimaryEmail = Some { Value = oldEmail }
              }
      } when newEmail <> oldEmail -> Some(UpdateBklrEmail { Old = oldEmail; New = newEmail })
    | _ -> None

Or alternatively:

let getUpdateBklrEmailEvent user =
    match user with
    | {
          PrimaryEmail = Some { Value = newEmail }
          PersistedState = Some {
              PrimaryEmail = Some { Value = oldEmail }
          }
      } when newEmail <> oldEmail -> Some(UpdateBklrEmail { Old = oldEmail; New = newEmail })
    | _ -> None

Extra information

  • The formatted result breaks my code.
  • The formatted result gives compiler warnings.
  • I or my company would be willing to help fix this.
  • I would like a release if this problem is solved.

Options

Fantomas main branch at 1/1/1990

    { config with
                MultilineBracketStyle = stroustrup }

Did you know that you can ignore files when formatting by using a .fantomasignore file?
PS: It's unlikely that someone else will solve your specific issue, as it's something that you have a personal stake in.

@cmeeren
Copy link
Contributor Author

cmeeren commented Oct 25, 2024

Another and perhaps simpler example: The following fits on one line, and is the expected output (and the actual output with default settings):

type PiLangAttribute(nameGroup: string) =
    inherit ObjectFieldDescriptorAttribute()

    override this.OnConfigure(_context, descriptor, _member) =
        descriptor.Directive({ PiLangDirective.Name = Some nameGroup }) |> ignore

With Stroustrup, however, I get this:

type PiLangAttribute(nameGroup: string) =
    inherit ObjectFieldDescriptorAttribute()

    override this.OnConfigure(_context, descriptor, _member) =
        descriptor.Directive(
            {
                PiLangDirective.Name = Some nameGroup
            }
        )
        |> ignore

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant