[Ticket #2393] Text.PrettyPrint.HughesPJ: Bug fixes, performance
benjovi at gmx.net
Wed Jun 25 10:03:45 EDT 2008
Neil Mitchell schrieb:
> Hi Benedikt,
> Great work, and many thanks!
> One ticket that might be interesting, while you are in the area, is:
> 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".
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
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'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.
> 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?
>> | -----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
More information about the Libraries