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

Implicit unit else results in extra lines with each reformat #1187

Closed
1 of 3 tasks
renngar opened this issue Oct 8, 2020 · 1 comment · Fixed by #1212
Closed
1 of 3 tasks

Implicit unit else results in extra lines with each reformat #1187

renngar opened this issue Oct 8, 2020 · 1 comment · Fixed by #1212

Comments

@renngar
Copy link

renngar commented Oct 8, 2020

Issue created from fantomas-online

Code

let fn () =
    if true then
        DoSomething ()
        DoSomethingElse()
    elif shouldWe then
        Whatever()

let  nextfunc () =
    printf "Hi"

Result

let fn () =
    if true then
        DoSomething()
        DoSomethingElse()
    elif shouldWe then
        Whatever()


let nextfunc () = printf "Hi"

Problem description

Nested ifs or if-else chains can result in additional blank lines being inserted with each reformat. This seems to happen only when they have an implicit unit-returning else tail.

I have many variants of this in my code. Some occur at the end of a function others in the middle of a function. In the latter case, the rest of the function is pushed further down the page with each iteration. In the former, the same happens to the following function.

Extra information

  • The formatted result breaks by code.
  • The formatted result gives compiler warnings.
  • I or my company would be willing to help fix this.

I am not familiar with the Fantomas code, but would be will to try to fix this with some pointers or hints.

In places I've had to resort to things like:

let private setState (host: IViewHost) retryMsg program (state: Model) dispatch =
    // create new view and update the host only if new model is not equal to a prev one
    let stateDiffers = (Some state).Equals(!stateRef) |> not

    if stateDiffers then
        let updated = state.ThrottleUpdates() |> not
        // Only build a new view if the screen has been updated with the previous one
        if updated then
            stateRef := Some state
            let view = ((Program.view program) state dispatch)
            host.Update(Some(view :> IView))
        elif retryMsgInFlight |> not then
            // Otherwise, dispatch a message after a while so that we can retry updating with the latest.
            DispatcherTimer.RunOnce(Action(fun () -> retryMsg |> dispatch), TimeSpan.FromMilliseconds 50.0)
            |> ignore

            retryMsgInFlight <- true
        else
            () // Avoid Fantomas formatting problems
    else
        () // Avoid Fantomas formatting problems

Interestingly, if I only put the comment on one of the elses, Fantomas will insert it on both.

Options

Fantomas Master at 10/02/2020 17:06:53 - 31305c2

Default Fantomas configuration

@nojaf
Copy link
Contributor

nojaf commented Oct 10, 2020

Hello @renngar, thanks for reporting this.
I believe we are adding a newline too much somewhere in CodePrinter.
Sorry that this doesn't really narrow this down a lot.

The learn about how Fantomas works internally, you can check out some videos on YouTube.
And check out our little Contributing guide section.

As for tackling this, I propose you create a unit test in IfThenElseTests.fs and do some debugging to see which code path is taken.

Don't hesitate to open a draft PR and ask some further questions down the road.

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