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

Consider Trivia Content before multiline members - fixes #569 #584

Merged
merged 3 commits into from
Dec 3, 2019

Conversation

theimowski
Copy link
Member

The pattern matching above uses nested sepMember function
to consider Trivia Content. The difference between those
pattern matching cases is that latter one matches on when
there are trailing non-multiline members.
There's some code duplication between those two now, which
I'm happy to refactor - feedback welcome.

The pattern matching above uses nested `sepMember` function
to consider Trivia Content. The difference between those
pattern matching cases is that latter one matches on when
there are trailing non-multiline members.
There's some code duplication between those two now, which
I'm happy to refactor - feedback welcome.
This makes all tests still happy, but I'm not sure if I didn't do
something stupid here. Good thing is it clears up the code a bit.
Copy link
Contributor

@nojaf nojaf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also add a unit test for the original reproduction

src/Fantomas/CodePrinter.fs Outdated Show resolved Hide resolved
src/Fantomas.Tests/ClassTests.fs Outdated Show resolved Hide resolved
Plus addressing review comments
@theimowski
Copy link
Member Author

Addressed the comments

I've also found 3 more cases where extra unwanted newline is being added - gonna tackle them as they're the outstanding issues preventing me from starting using fantomas in my project.
Already got 3 failing unit tests for them:

 [<Test>]
 let ``no extra new line before nested module with attribute, 569``() =
     let original = """module A =
     let x = 0

     [<RequireQualifiedAccess>]
     module B =
         let y = 1
 """

     formatSourceString false original config |> should equal original

 [<Test>]
 let ``no extra new line after elif expression, 569``() =
     let original = """let x =
     if true then printfn "a"
     elif true then printfn "b"

     if true then 1 else 0
 """

     formatSourceString false original config |> should equal original

 [<Test>]
 let ``no extra new line before abstract member with attribute, 569``() =
     let original = """type A =

     [<EmitConstructor>]
     abstract Create: Unit -> A

     abstract b: Unit -> Unit
 """

     formatSourceString false original config |> should equal original

Do you want me to combine those 3 more cases with this PR and issue, or shall I create a separate one?

@nojaf
Copy link
Contributor

nojaf commented Dec 3, 2019

Hey @theimowski , thanks for the changes.
Could you create a new issue of these other problems?
One would suffice for all three cases.
Many thanks for these contributions!

@nojaf nojaf self-requested a review December 3, 2019 15:40
@nojaf nojaf merged commit be054f6 into fsprojects:master Dec 3, 2019
@theimowski
Copy link
Member Author

Ok, created #586

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

Successfully merging this pull request may close these issues.

2 participants