Comment removed in multi-case pattern matching

[theimowski]

Issue created from fantomas-online

When there's a comment between two pattern matching cases that end in some branch, it gets removed

Code

let f (a: A) (b: B) =
    match a, b with
    | A, B
    // this comment is lost
    | C, D -> Some ()
    | _ -> None

Result

let f (a: A) (b: B) =
    match a, b with
    | A, B
    | C, D -> Some()
    | _ -> None

Options

Fantomas Next - 4.0.0-alpha-001-1/1/1990

Name	Value
IndentSpaceNum	4
PageWidth	120
SemicolonAtEndOfLine	false
SpaceBeforeParameter	true
SpaceBeforeLowercaseInvocation	true
SpaceBeforeUppercaseInvocation	false
SpaceBeforeClassConstructor	false
SpaceBeforeMember	false
SpaceBeforeColon	false
SpaceAfterComma	true
SpaceBeforeSemicolon	false
SpaceAfterSemicolon	true
IndentOnTryWith	false
SpaceAroundDelimiter	true
MaxIfThenElseShortWidth	40
MaxInfixOperatorExpression	50
MaxRecordWidth	40
MaxArrayOrListWidth	40
MaxLetBindingWidth	40
MultilineBlockBracketsOnSameColumn	false
NewlineBetweenTypeDefinitionAndMembers	false
StrictMode	false

[nojaf]

The found comment is assigned to the | token before C, D.

And no trivia is generated around PatOr.

Most likely enterNodeTokenByName before the pipe might do the trick.

Check out this video to learn more about trivia.

[kontomondo]

@nojaf Thank you for this project! This is my first time seeing this codebase, but I figured I could try tackling an issue, I had a test added in CommentTest.fs:

[<Test>]
let ``comments in pattern matching cases should not be removed, 813``() =
    formatSourceString false """
let f (a: A) =
    match a with
    // comment for case A
    | A
    // comment for case C
    | C -> Some()
    | D -> Another() // inline comment
    // comment for case _
    | _ -> None""" config
    |> prepend newline
    |> should equal """
let f (a: A) =
    match a with
    // comment for case A
    | A
    // comment for case C
    | C -> Some()
    | D -> Another() // inline comment
    // comment for case _
    | _ -> None
"""

and added enterNodeTokenByName as you suggested, however I hit an issue with an extra new line being copied after "comment for case C"

    | PatOr(p1, p2) ->
        genPat astContext p1
        +> enterNodeTokenByName pat.Range "BAR"
        +> sepNln -- "| "
        +> genPat astContext p2

so I went with sepNone instead of sepNln

    | PatOr(p1, p2) ->
        genPat astContext p1
        +> enterNodeTokenByName pat.Range "BAR"
        +> sepNone -- "| "
        +> genPat astContext p2

but that may introduce its own issue I figure?

[nojaf]

Hello @kontomondo, thanks for your kind words.
It is hard to say what the impact is of changing sepNln to sepNone, I would think that you might need to make a decision based on the trivia of the BAR token to choose one of the other.

Could you maybe make a (draft) PR with this code? Then I can easily check it out and we can see if all the other tests still pass.

[kontomondo]

@nojaf Thank you for the reply! I've opened #834 draft PR

[kontomondo]

@nojaf this issue should be closed since the PR is merged.

[nojaf]

It seems like the linking of the PR with the issue didn't happen alright.