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

Fantomas keeps adding new lines between two interface member implementations #569

Closed
ivan-brko opened this issue Nov 22, 2019 · 4 comments
Closed

Comments

@ivan-brko
Copy link

Description

Fantomas adds a new line between two interface member implementations each time the file is formatted

Repro code

namespace Quartz.Fsharp

module Logging =
    open Quartz.Logging
    open System

    //todo: it seems that quartz doesn't use mapped and nested context,
    //however, check if this is the best implementation for this interface
    type private QuartzLoggerWrapper(f) =
        interface ILogProvider with

            member this.OpenMappedContext(_, _) =
                { new IDisposable with
                    member this.Dispose() = () }
            member this.OpenNestedContext _ =
                { new IDisposable with
                    member this.Dispose() = () }

            member this.GetLogger _name = new Logger(f)

    let SetQuartzLoggingFunction f =
        let loggerFunction level (func: Func<string>) exc parameters =
            let wrappedFunction = Helpers.nullValuesToOptions (fun (x: Func<string>) -> (fun () -> x.Invoke())) func
            let wrappedException = Helpers.nullValuesToOptions id exc
            f level wrappedFunction wrappedException (parameters |> List.ofArray)
        LogProvider.SetCurrentLogProvider(QuartzLoggerWrapper(loggerFunction))

    let SetQuartzLogger l = LogProvider.SetCurrentLogProvider(l)

The link to fantomas tool example.

You can check if you copy the formatted code from the right to the source and format that, it will add new line after line 17, and it keeps doing this forever.

@theimowski
Copy link
Member

theimowski commented Dec 3, 2019

Minimal repro I could come up with: https://jindraivanek.gitlab.io/fantomas-ui/#?code=C4TwDgpgBAggFASigXgFCqpqBbC2BGEATlMABYCWAzgHQCyehRMKGW7FAZqUQK7TkIAOygAGKBAA2VaAEZ07XAWKlKtBsqIAhVuw7dgfAWWFQATBOnQAzAqxKmq6vUbEAwijGogA

Code

type A() =

    member this.MemberA =
        if true then 0 else 1

    member this.MemberB =
        if true then 2 else 3

    member this.MemberC = 0

Result

type A() =

    member this.MemberA =
        if true then 0 else 1


    member this.MemberB =
        if true then 2 else 3

    member this.MemberC = 0

Options

Fantomas 3.1.0

Name Value
IndentOnTryWith false
IndentSpaceNum 4
KeepNewlineAfter false
MaxIfThenElseShortWidth 40
PageWidth 120
ReorderOpenDeclaration false
SemicolonAtEndOfLine false
SpaceAfterComma true
SpaceAfterSemicolon true
SpaceAroundDelimiter true
SpaceBeforeArgument true
SpaceBeforeColon false
StrictMode false

@nojaf
Copy link
Contributor

nojaf commented Dec 3, 2019

The problem here is that Fantomas adds a newline between each found SynMemberDefn.Member.
However a blank line was found in the trivia and is also printed.

So you end up with an extra newline. We have encountered this issue on other places as well.
The fix is to use the sepNlnConsideringTriviaContentBeforeWithAttributes function instead of sepNln. I would need to debug to see where exactly this should be done...

@theimowski
Copy link
Member

theimowski commented Dec 3, 2019

I started looking at that, but I'm completely new to the code so I can hardly understand anything yet.

What I found is that there are two pattern matches on MultilineMemberDefnL in CodePrinter:

and for my above snippet I end up in the later one, while if I remove MemberC then I end up in the first one and the extra newline is not added

There are some differences in implementation between those two and I'm trying get my head around those

@theimowski
Copy link
Member

theimowski commented Dec 3, 2019

Ok I think I got it - see PR #584

theimowski added a commit to theimowski/fantomas that referenced this issue Dec 3, 2019
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.
@nojaf nojaf closed this as completed in be054f6 Dec 3, 2019
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

3 participants