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

Neil Mitchell schrieb:

Hi Benedikt,

Great work, and many thanks!

One ticket that might be interesting, while you are in the area, is:

http://hackage.haskell.org/trac/ghc/ticket/1217

The code is attached, I went through all the API review months and months ago, and everyone agreed that the function should be added, but under the name "zeroWidthText". Hi Neil,

Thanks ! Concerning #1217: http://www.haskell.org/pipermail/libraries/2007-March/007028.html I'm not quite sure what you are asking for - I'm not the one to apply the patches ;) From a technical point of view, zeroWidthText wouldn't cause any bugs in the current implementation, as far as I can see. 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. Btw, is there any use for the (PStr :: String -> TextDetails) constructor ? 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. --

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. cheers, benedikt

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 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@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, 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