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

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 wrote:

...

Thank you for doing this Benedict. I've added your more detailed

...

...

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@haskell.org

[mailto:libraries-bounces@haskell.org] On Behalf Of Benedikt Huber

...

| Sent: 24 June 2008 13:12 | To: libraries@haskell.org | Subject: [Ticket #2393] Text.PrettyPrint.HughesPJ: Bug fixes,

comments to ticket #2393 so that they are preserved. 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@haskell.org http://www.haskell.org/mailman/listinfo/libraries