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

Private record definition error with stroustrup style #2481

Closed
3 tasks done
sheridanchris opened this issue Sep 7, 2022 · 5 comments · Fixed by #2484
Closed
3 tasks done

Private record definition error with stroustrup style #2481

sheridanchris opened this issue Sep 7, 2022 · 5 comments · Fixed by #2484

Comments

@sheridanchris
Copy link
Contributor

Issue created from fantomas-online

Code

type Person = private {
    FirstName: string
    LastName: string
}

Result

type Person = private
    {
        FirstName: string
        LastName: string
    }

Problem description

Formatting the above code with the below options results in a compiler error.
I expected the code to remain the same after format, which was not the case. Although, I may be mistaken on the expected output

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.

Options

Fantomas master branch at 2022-09-07T06:05:21Z - b0916a4

    { config with
                MultilineBlockBracketsOnSameColumn = true
                ExperimentalStroustrupStyle = true }

Did you know that you can ignore files when formatting from fantomas-tool or the FAKE targets by using a .fantomasignore file?

@nojaf
Copy link
Contributor

nojaf commented Sep 7, 2022

Hello,

Thank you for reporting this issue.
This one is tricky, if you can fix this in an elegant way I guess we can store the original formatting.

What I mean by elegant is that the fix is a tweak in existing code, without having to rewrite everything we currently have. If that is not possible then we will have to fallback MultilineBlockBracketsOnSameColumn when the type has an access modifier.

Are you interested in submitting a PR for this?

@sheridanchris
Copy link
Contributor Author

Hello,

Thank you for reporting this issue. This one is tricky, if you can fix this in an elegant way I guess we can store the original formatting.

What I mean by elegant is that the fix is a tweak in existing code, without having to rewrite everything we currently have. If that is not possible then we will have to fallback MultilineBlockBracketsOnSameColumn when the type has an access modifier.

Are you interested in submitting a PR for this?

I'm not familiar with the codebase, but I am interested in submitting a PR. I'll take a look and see if I can fix the issue.

@nojaf
Copy link
Contributor

nojaf commented Sep 7, 2022

Great 😊, you can find some notes on the codebase over at https://fsprojects.github.io/fantomas/docs/contributors/Index.html.
Going through those pages should give you a general idea of how it works.

In practice you will most likely need to make a change around:

and genMultilineSimpleRecordTypeDefnAlignBrackets astContext openingBrace withKeyword ms ao' fs closingBrace =
// the typeName is already printed
opt (indent +> sepNln) ao' genAccess
+> enterNodeFor SynTypeDefnSimpleRepr_Record_OpeningBrace openingBrace
+> sepOpenSFixed
+> indentSepNlnUnindent (
atCurrentColumn (
leaveNodeFor SynTypeDefnSimpleRepr_Record_OpeningBrace openingBrace
+> col sepNln fs (genField astContext "")
)
)
+> sepNln
+> genTriviaFor SynTypeDefnSimpleRepr_Record_ClosingBrace closingBrace sepCloseSFixed
+> optSingle (fun _ -> unindent) ao'
+> onlyIf (List.isNotEmpty ms) sepNln
+> sepNlnBetweenTypeAndMembers withKeyword ms
+> genMemberDefnList astContext ms

opt (indent +> sepNln) ao' genAccess will add the additional indent if the access modifier is present.
If ExperimentalStroustrupStyle is active you want to avoid this.

When submitting your PR you want to check https://github.com/fsprojects/fantomas/blob/master/CONTRIBUTING.md#pull-request-ground-rules first.

Don't hesitate to reach out if you have any questions.

@sheridanchris
Copy link
Contributor Author

Is MultilineBlockBracketsOnSameColumnRecordTests.fs an appropriate place for tests? Does this warrant a new file?

@nojaf
Copy link
Contributor

nojaf commented Sep 7, 2022

I would add the test in SynTypeDefnSimpleReprRecordTests.fs.

@sheridanchris sheridanchris changed the title Private record constructor error with stroustrup style Private record definition error with stroustrup style Sep 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants