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

[Benedikt Huber]

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