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

Bugfix: overlap and f?(cat|sep) #28

Open
thomie opened this issue Aug 4, 2015 · 1 comment
Open

Bugfix: overlap and f?(cat|sep) #28

thomie opened this issue Aug 4, 2015 · 1 comment

Comments

@thomie
Copy link

thomie commented Aug 4, 2015

The pretty source code currently contains two TODOs:

    -- XXX: TODO: PRETTY: Used to use True here (but GHC used False...)
     nilAboveNest False k (reduceDoc (vcat ys))
    -- XXX: TODO: PRETTY: Used to use True here (but GHC used False...)
     `mkUnion` nilAboveNest False k (fill g (y:ys))

I think we should go back to using True. From https://mail.haskell.org/pipermail/libraries/2008-June/009991.html (commit 1e50748):

2) Bugfix: overlap and f?(cat|sep)

The specification for cat/sep:
  * oneLiner (hcat/hsep ps)
   `union`
    vcat ps [*]

But currently cat, sep, fcat and fsep attempt to overlap the second  
line with the first one, i.e. they use
`foldr ($$) empty ps' instead of `foldr ($+$) empty ps' [*]. I assume  
this is a mistake.

This bug can lead to situations, where the line in the right argument  
of Union is actually longer:

 > prettyDoc$ cat [ text "a", nest 2 ( text "b") ]
 >> text "a"; union
 >>           (text "b"; empty)
 >>           (nilabove; nest 1; text "b"; empty)

 > renderStyle (Style PageMode 1 1) $ cat [ text "a", nest 2 ( text  
"b") ]
 >> "a b"

In the implementation, we call `nilAbove False' instead of `nilAbove  
True' (see patch).
@dterei
Copy link
Member

dterei commented Aug 5, 2015

The problem at this stage is that changing this value will affect the output of some existing programs. I think I'd be OK with the change if we had some good examples and understood what programs will change and why. Right now I don't have a good grasp of that.

Also, in the long run ideally we can unify with GHC. So changing away from GHC is an issue.

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

2 participants