[Ticket #2393] Text.PrettyPrint.HughesPJ: Bug fixes, performance improvement

[Neil Mitchell]

Hi Benedikt,

>  I'm not quite sure what you are asking for - I'm not the one to apply the
> patches ;)

The patch needs redoing with zeroWidthText, rather than zeroWidth. I
can't build a patch, but you are probably at the right point to do so,
and include it into the other patches you are doing. Also, since you
seem to understand the low-level details very well, it was a general
request for comment.

>  Two things should be considered though:
>
>  1) when defining zeroTextWidth as (textBeside_ (Str s) 0 Empty), the client
> looses the information that (Str s) is a zero-width TextDetail.
>  When combining the text again (see string_txt for example), it might be
>  good to know that.

>  I do not suggest it, but it /might/ be worth to consider using a different
> constructor for zero width texts. Of course, this modifies the API, as
> TextDetails is exported.

That seems like a bad idea, as I certainly don't know enough details
about the library to write the correct implementation in every case,
and to be sure there are no pattern-match errors.

>  2) One has to take precautions that future versions of the library do not
> use the text length for optimizations.
>
>  For example the law
>  > text "" <> s = elide_nests s
>  must not be used by testing against the text length (which is the obvious
> and fasted way to do so). Some regression tests would be a good idea.

True, but I guess that it is currently safe?

>  Btw: the people commenting in the thread linked above seemed to be using
> different pretty printer libraries, so maybe some comments from actual users
> would be good too.

I think the discussion proceeded enough to be fairly sure that no one
objected to it, and lots of people wanted it as part of a general
pretty printing library. I think it also showed that lots of serious
pretty-printing users had moved away from HughesPJ - its possible your
fixes and maintenance might tempt them back.

Thanks

Neil

>
> >
> > I'm guessing it isn't too much effort to tack this on while you are
> > working on it? I am currently limited to Hugs only (not enough disk
> > space to install GHC and no SSH access...), so am unable to patch any
> > libraries myself.
> >
> > Thanks
> >
> > Neil
> >
> >
> > On 6/24/08, Simon Peyton-Jones <simonpj at microsoft.com> wrote:
> >
> > > Thank you for doing this Benedict.  I've added your more detailed
> comments to ticket #2393 so that they are preserved.
> > >
> > >  Ian: would you like to apply?  I'm not sure how to integrate the
> QuickCheck tests, but I bet you know.
> > >
> > >
> > >  Benedict: while you are in the area, would you like to take a swing at
> http://hackage.haskell.org/trac/ghc/ticket/1337, and 1176?
> > >
> > >  Simon
> > >
> > >
> > >
> > >  | -----Original Message-----
> > >  | From: libraries-bounces at haskell.org
> [mailto:libraries-bounces at haskell.org] On Behalf Of Benedikt Huber
> > >  | Sent: 24 June 2008 13:12
> > >  | To: libraries at haskell.org
> > >  | Subject: [Ticket #2393] Text.PrettyPrint.HughesPJ: Bug fixes,
> performance improvement
> > >  |
> > >  | Hello,
> > >  |
> > >  | I'd like to propose bugfixes, documentation fixes and a performance
> > >  | improvement for Text.PrettyPrint.HughesPJ. The changes shouldn't
> > >  | effect the expected behaviour of the PP library.
> > >  |
> > >  | I've written a QuickCheck test suite for the pretty printer (to test
> > >  | the improvement), and found two bugs and some misconceptions/
> > >  | ambiguities in the documentation. Additionally, there is a
> > >  | microbenchmark for the suggested improvement.
> > >  | Both are available at
> http://code.haskell.org/~bhuber/Text/
> > >  | PrettyPrint/. Note that the QuickCheck tests need access to all top-
> > >  | level names in HughesPJ (i.e. ignore the export list).
> > >  |
> > >  | In summary, I propose to
> > >  | * fix a bug in fillNB and one in fillNB/sepNB
> > >  | * correct documentation on laws and invariants.
> > >  | * add more efficient implementations of vcat,hsep,hcat
> > >  |
> > >  | More specifically:
> > >  |
> > >  | (1) Bugfix fillNB: Additional case for  fillNB Empty (Empty : ys)
> > >  |
> > >  | (2) Bugfix [f](cat|sep): do not allow overlapping ($$) in vertical
> > >  | composition
> > >  |
> > >  | (3) Lazy implementations of vcat,hcat and hsep
> > >  |
> > >  | (4) Law <t2> isn't always true
> > >  |
> > >  | (5) Invariant 5 should be made stronger
> > >  |
> > >  | (6) Change the comment about negative indentation
> > >  |
> > >
> > > _______________________________________________
> > >  Libraries mailing list
> > >  Libraries at haskell.org
> > >  http://www.haskell.org/mailman/listinfo/libraries
> > >
> > >
> >
>
>