Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Implement 'shrink' for Docs by hand #113

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions prettyprinter/src/Data/Text/Prettyprint/Doc/Internal.hs
Original file line number Diff line number Diff line change
Expand Up @@ -1921,6 +1921,24 @@ renderShowS = \sds -> case sds of
SAnnPush _ x -> renderShowS x
SAnnPop x -> renderShowS x

valid :: Doc ann -> Bool
valid = go False
where
go mayFail doc = case doc of
Fail -> mayFail
Empty -> True
Char c -> c /= '\n'
Text l t -> l == T.length t && l >= 2 && T.all (/= '\n') t
Line -> True
FlatAlt x y -> go mayFail x && go mayFail y
Cat x y -> go mayFail x && go mayFail y
Nest _ x -> go mayFail x
Union x y -> go True x && go mayFail y
Column f -> all (go mayFail) (map f [0..80])
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how to best handle these functions. Checking them exhaustively would take too long. Even the current check with "typical" values 0..80 seems excessive.

WithPageWidth f -> all (go mayFail) (map f (Unbounded : [AvailablePerLine c r | c <- [1..80], r <- [0, 0.1 .. 1]]))
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is 0 a valid ribbon width at all? Not sure how to interpret the docs there:

= AvailablePerLine Int Double
-- ^ Layouters should not exceed the specified space per line.
--
-- - The 'Int' is the number of characters, including whitespace, that
-- fit in a line. A typical value is 80.
--
-- - The 'Double' is the ribbon with, i.e. the fraction of the total
-- page width that can be printed on. This allows limiting the length
-- of printable text per line. Values must be between 0 and 1, and
-- 0.4 to 1 is typical.

Suggested change
WithPageWidth f -> all (go mayFail) (map f (Unbounded : [AvailablePerLine c r | c <- [1..80], r <- [0, 0.1 .. 1]]))
WithPageWidth f -> all (go mayFail) (map f (Unbounded : [AvailablePerLine c r | c <- [1..80], r <- [0.1, 0.2 .. 1]]))

Nesting f -> all (go mayFail) (map f [0..80])
Annotated _ x -> go mayFail x


-- $setup
--
Expand Down
25 changes: 21 additions & 4 deletions prettyprinter/test/Testsuite/Main.hs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import System.Timeout (timeout)

import Data.Text.Prettyprint.Doc
import Data.Text.Prettyprint.Doc.Internal
import Data.Text.Prettyprint.Doc.Internal.Debug
import Data.Text.Prettyprint.Doc.Internal.Debug (diag)
import Data.Text.Prettyprint.Doc.Render.Text
import Data.Text.Prettyprint.Doc.Render.Util.StackMachine (renderSimplyDecorated)

Expand Down Expand Up @@ -94,7 +94,7 @@ tests = testGroup "Tests"

fusionDoesNotChangeRendering :: FusionDepth -> Property
fusionDoesNotChangeRendering depth
= forAllShow (arbitrary :: Gen (Doc Int)) (show . diag) (\doc ->
= forAllShrinkShow (arbitrary :: Gen (Doc Int)) shrink (show . diag) (\doc ->
forAll arbitrary (\layouter ->
let tShow = T.pack . show
render = renderSimplyDecorated id tShow tShow . layout layouter
Expand All @@ -109,11 +109,28 @@ fusionDoesNotChangeRendering depth
, "Unfused:"
, indent 4 (pretty rendered)
, "Fused:"
, indent 4 (pretty renderedFused) ]
, indent 4 (pretty renderedFused)
]

instance Arbitrary ann => Arbitrary (Doc ann) where
arbitrary = document
shrink = genericShrink -- Possibly not a good idea, may break invariants
shrink doc = filter valid $ case doc of
Fail -> [Empty]
Empty -> []
Char c -> Empty : map Char (filter (/= '\n') (shrink c))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be one of the few places where a list comprehension is nicer to read! :-)

Text _ t -> Empty : map pretty (shrink t)
Line -> Empty : [space]
FlatAlt x y -> Empty : noFail x ++ noFail y ++ map (uncurry FlatAlt) (shrink (x, y))
Cat x y -> Empty : noFail x ++ noFail y ++ map (uncurry Cat) (shrink (x, y))
Nest i x -> Empty : noFail x ++ map (flip Nest x) (shrink i)
Union x y -> Empty : noFail x ++ noFail y ++ map (uncurry Union) (shrink (x, y))
Column f -> Empty : noFail (f 0) ++ map Column (shrink f)
WithPageWidth f -> Empty : noFail (f defaultPageWidth) ++ map WithPageWidth (shrink f)
Nesting f -> Empty : noFail (f 0) ++ map Nesting (shrink f)
Annotated a x -> Empty : noFail x ++ map (uncurry Annotated) (shrink (a, x))
where
noFail Fail = []
noFail x = [x]

document :: Arbitrary ann => Gen (Doc ann)
document = (dampen . frequency)
Expand Down