quchen / prettyprinter

A modern, extensible and well-documented prettyprinter.
BSD 2-Clause "Simplified" License
293 stars 34 forks source link

Ribbon width… #142

Open sjakobi opened 4 years ago

sjakobi commented 4 years ago

This blob could use some documentation:

https://github.com/quchen/prettyprinter/blob/111ce3780eef8c96dadebc9e443a0ea3c13a3528/prettyprinter/src/Data/Text/Prettyprint/Doc/Internal.hs#L1824-L1830

The haddocks on AvailablePerLine try to explain what the "ribbon width" is. A visualization would be nice. I also don't really understand why it's tracked separately from the number of columns / page width.

https://github.com/quchen/prettyprinter/blob/111ce3780eef8c96dadebc9e443a0ea3c13a3528/prettyprinter/src/Data/Text/Prettyprint/Doc/Internal.hs#L1628-L1637

In layoutSmart, why doesn't the computation of the width for the subsequent lines take the ribbon width into account?

https://github.com/quchen/prettyprinter/blob/111ce3780eef8c96dadebc9e443a0ea3c13a3528/prettyprinter/src/Data/Text/Prettyprint/Doc/Internal.hs#L1759-L1761

sjakobi commented 4 years ago

If anyone uses a different ribbon width than the default of 1, I'd like to hear about it and know why! :)

quchen commented 4 years ago

I included ribbon width originally because it was in ansi-wl-pprint, and I didn’t research its necessity enough. I totally agree now that it’s probably not a very useful concept. Not sure whether its removal is viable though, probably not…

As for the layoutSmart question, that looks like a bug.

sjakobi commented 4 years ago

Here's an attempt to visualize how the ribbon width is interpreted in layoutWadlerLeijen.selectNicer:

layoutPretty (LayoutOptions (AvailablePerLine 40 0.5)) ("x" <> nest 15 (line <> "y"))
<------------- page width ------------->
x
               y
< indentation ><-- ribbon width -->

(AvailablePerLine 40 0.5) isn't the same thing as (AvailablePerLine 20). The "ribbon" starts only at the current indentation level. That's why the availableWidth calculation is so complicated.

sjakobi commented 4 years ago

Actually there is a bit of user documentation that I had overlooked so far:

https://github.com/quchen/prettyprinter/blob/1996a1692c1d32418626d3af0ba084a71892ed21/prettyprinter/src/Data/Text/Prettyprint/Doc.hs#L133-L138

sjakobi commented 4 years ago

If anyone uses a different ribbon width than the default of 1, I'd like to hear about it and know why! :)

Turns out that multiple packages on Stackage use or expose the ribbon fraction parameter (found via haskell-code-explorer):

I take that we should continue supporting this parameter and handle it correctly too.

What I haven't found so far is an example where layoutSmart would produce the wrong layout by ignoring the ribbon fraction…

sjakobi commented 4 years ago

What I haven't found so far is an example where layoutSmart would produce the wrong layout by ignoring the ribbon fraction…

Here's an example using the Doc internals:

> x = "x"
> y = hang 1 ("y" <> hardline <> "yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy")
> d = Data.Text.Prettyprint.Doc.Internal.Union y x
> layoutSmart (LayoutOptions (AvailablePerLine 8 1)) d -- correct
SChar 'x' SEmpty
> layoutSmart (LayoutOptions (AvailablePerLine 80 0.1)) d -- bug!
SChar 'y' 
    ( SLine 1 ( SText 45 "yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy" SEmpty ) )