Fix comments in if blocks

[brandonchinn178]

Resolves #998

The issue has two bugs associated with it, which are both solved in this PR.

Comments between then/else tokens and body

This bug happens if there's a comment between the then/else tokens and the body of the branch:

-- without comments
if cond
  then case a of
    Just b -> ...
  else
    foo
      True
      ""

-- expected
if cond
  then
    -- comment
    -- comment
    case a of
      Just b -> ...
  else
    -- comment
    -- comment
    foo
      True
      ""

-- previous behavior
if cond
  then -- comment
  -- comment
  case a of
    Just b -> ...
  else -- comment
  -- comment

    foo
      True
      ""

There are a few bugs here:

1. The comment is moved up next to the keyword instead of staying on the next line
2. Subsequent comments are not indented
3. Expressions that normally "hang" like case are not indented
4. Expressions that don't hang like the second branch here get an extra line added

The issue is that the code does the following steps:

1. Print then/else token
2. located A: Spit out comments
3. located B: Set breakpoint layout
4. placeHanging Apply hanging format
5. Print body

But we need to spit out the comments AFTER applying the hanging layout. Additionally, we need to never hang when there are comments. So I fixed this by moving "Apply hanging format" before "Spit out comments", and manually setting the breakpoint layout before:

1. Print then/else token
2. switchLayout: Set breakpoint layout
3. placeHanging Apply hanging format
4. located A: Spit out comments
5. located B: Set breakpoint layout
6. ~placeHanging Apply hanging format~
7. Print body

Comments before then/else keywords

In this bug, comments before the then/else keywords get moved to after:

-- input + expected
if cond
  -- asdf
  then foo
  -- asdf
  then bar

-- previous behavior
if cond
  then -- asdf
    foo
  else -- asdf
    bar

This happens because we only spit out comments when we call located, but we dont call located for keywords; we just do a plain txt "then".

The fix for this was relatively easy: I just wrap txt "then" in a located containing the SrcSpan for the then keyword from the annotations. I think there's future work here to always do this for all keywords, now that GHC has prettyprinting information with SrcSpans for all keywords

[brandonchinn178]

hm Thanks for the hackage diff. It's showing two diffs that I didn't expect (or want):

1. There are some function calls that are now always being put below then/else. I dont want to change behavior there
2. I should also add a test for comments on the same line as then/else. Those should not float above the then/else

I'll take a look tonight

[amesgen]

hm Thanks for the hackage diff. It's showing two diffs that I didn't expect (or want):

1. There are some function calls that are now always being put below then/else. I dont want to change behavior there

2. I should also add a test for comments on the same line as then/else. Those should not float above the then/else

I'll take a look tonight

To avoid any confusion: The diff is between the Ormolu output before and after this PR, not between the input and the Ormolu output; the original is not visible (but you can find it on Hackage ofc). Ideally, this would be a three-way diff, but I think I would have to set up a git repo for this?

[mrkkrp]

@brandonchinn178 Do you consider this ready or is there anything else you'd like to add here?

[mrkkrp]

It is nice that keyword spans are now available, it wasn't the case when I was originally implementing this.

[brandonchinn178]

This is ready on my end!

[brandonchinn178]

oh you added commits. I would appreciate it if you could avoid adding commits to my PRs without consulting me. It's a bit disorienting; if you just left a comment, I would have been perfectly happy to make the changes.

Let me take another look

[mrkkrp]

Sorry, I usually do that when I'm about to merge, but this time around I wasn't completely sure that you were done with it.

[brandonchinn178]

Sigh I wanted to see if I could improve the comment-next-to-keyword case, but it doesn't look trivial. It seems like the comments are attached to "after the else keyword", and there's no way to increase the indentation for comments.

I doubt this will happen often, and this PR is already a huge improvement, so let's merge. Thanks @mrkkrp!

[mrkkrp]

Thanks a lot!