prettyprinter icon indicating copy to clipboard operation
prettyprinter copied to clipboard

Ribbon width…

Open sjakobi opened this issue 5 years ago • 6 comments
trafficstars

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 avatar Feb 02 '20 18:02 sjakobi

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

sjakobi avatar Feb 03 '20 20:02 sjakobi

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.

quchen avatar Feb 03 '20 22:02 quchen

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 avatar Feb 04 '20 22:02 sjakobi

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 avatar Feb 09 '20 01:02 sjakobi

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

  • trifecta uses a ribbon fraction of 0.8 internally: 1, 2

  • logging-effect exposes the ribbon fraction parameter via withFDHandler

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 avatar Feb 11 '20 14:02 sjakobi

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

sjakobi avatar Feb 11 '20 17:02 sjakobi