natefaubion/purescript-dodo-printer

AlignCurrentColumn should keep indentation consistent

srghma opened this issue · 6 comments

I have created a test srghma@d2c9a61

and the output of this test is

Data.Map.Map
(Data.Map.Map
  (Data.Map.Map
    Number
   Boolean)
 (Data.Map.Map
   (Data.Map.Map
     Number
    Boolean)
  (Data.Map.Map
    Number
   Boolean)))
Boolean

but it should be

Data.Map.Map
(Data.Map.Map
 (Data.Map.Map
  Number
  Boolean)
 (Data.Map.Map
  (Data.Map.Map
   Number
   Boolean)
  (Data.Map.Map
   Number
   Boolean)))
Boolean

because I have tested what haskell implmenetation of prettyprinter will output with this test

module TestSpec where

import           Protolude hiding (group)

import           Test.Hspec

import Data.Text.Prettyprint.Doc
import Data.Text.Prettyprint.Doc.Render.Text

test1 :: Doc a
test1 = group $
  mapType
  `appWithPar`
  ( mapType
    `appWithPar`
    ( mapType
      `appWithoutPar`
      numberType
      `appWithoutPar`
      booleanType
    )
    `appWithPar`
    ( mapType
      `appWithPar`
      ( mapType
        `appWithoutPar`
        numberType
        `appWithoutPar`
        booleanType
      )
      `appWithPar`
      ( mapType
        `appWithoutPar`
        numberType
        `appWithoutPar`
        booleanType
      )
    )
  )
  `appWithoutPar`
  booleanType

  where
    mapType = pretty ("Data.Map.Map" :: Text)
    numberType = pretty ("Number" :: Text)
    booleanType = pretty ("Boolean" :: Text)

    appWithoutPar l r = align $ l <> line <> r
    appWithPar l r = align $ l <> line <> pretty ("(" :: Text) <> r <> pretty (")" :: Text)

spec :: Spec
spec = do
  it "prettypirnter" $ do
    let output = renderStrict (layoutSmart (LayoutOptions (AvailablePerLine 12 1)) test1)
    putStrLn output
    output `shouldBe` "qwe"

I think because of this I have a weird output in purescript-codegen/purescript-ps-cst#12

2020-09-04-12:18:31-screenshot

2020-09-04-12:21:11-screenshot

here is the other test

module DodoAlignCurrentColumnShouldNotIndent where

import Dodo
import Prelude

import Effect (Effect)
import Effect.Class.Console as Console

test1 :: forall a. Doc a
test1 =
  flexGroup $ foldWithSeparator softBreak
  [ text "xxxxxx1"
  , text "||||" <>
    ( alignCurrentColumn $ flexGroup $ foldWithSeparator softBreak
      [ text "yyyyyy1"
      , text "||||" <>
        ( alignCurrentColumn $ flexGroup $ foldWithSeparator softBreak
            [ text "zzzzzz1"
            , text "zzzzzz2"
            , text "zzzzzz3"
            ]
        )
      , text "yyyyyy3"
      ]
    )
  , text "xxxxxx3"
  ]

main :: Effect Unit
main = do
  Console.log $ print plainText ({ pageWidth: 12, ribbonRatio: 1.0, indentUnit: " ", indentWidth: 1 }) test1
xxxxxx1
||||yyyyyy1
    ||||zzzzzz1
        zzzzzz2
        zzzzzz3
    yyyyyy3
xxxxxx3

but

module DodoAlignCurrentColumnShouldNotIndent where

import Dodo
import Prelude

import Effect (Effect)
import Effect.Class.Console as Console

test1 :: forall a. Doc a
test1 =
  flexGroup $ foldWithSeparator softBreak
  [ text "xxxxxx1"
  , text "||||" <>
    ( alignCurrentColumn $ flexGroup $ foldWithSeparator softBreak
      [ text "yyyyyy1"
      , text "||||" <>
        ( alignCurrentColumn $ alignCurrentColumn $ flexGroup $ foldWithSeparator softBreak
            [ text "zzzzzz1"
            , text "zzzzzz2"
            , text "zzzzzz3"
            ]
        )
      , text "yyyyyy3"
      ]
    )
  , text "xxxxxx3"
  ]

main :: Effect Unit
main = do
  Console.log $ print plainText ({ pageWidth: 12, ribbonRatio: 1.0, indentUnit: " ", indentWidth: 1 }) test1

outputs

xxxxxx1
||||yyyyyy1
    ||||zzzzzz1
            zzzzzz2
            zzzzzz3
    yyyyyy3
xxxxxx3

and

module DodoAlignCurrentColumnShouldNotIndent where

import Dodo
import Prelude

import Effect (Effect)
import Effect.Class.Console as Console

test1 :: forall a. Doc a
test1 =
  flexGroup $ foldWithSeparator softBreak
  [ text "xxxxxx1"
  , text "||||" <>
    ( alignCurrentColumn $ alignCurrentColumn $ flexGroup $ foldWithSeparator softBreak
      [ text "yyyyyy1"
      , text "||||" <>
        ( alignCurrentColumn $ alignCurrentColumn $ flexGroup $ foldWithSeparator softBreak
            [ text "zzzzzz1"
            , text "zzzzzz2"
            , text "zzzzzz3"
            ]
        )
      , text "yyyyyy3"
      ]
    )
  , text "xxxxxx3"
  ]

main :: Effect Unit
main = do
  Console.log $ print plainText ({ pageWidth: 12, ribbonRatio: 1.0, indentUnit: " ", indentWidth: 1 }) test1
xxxxxx1
||||yyyyyy1
        ||||zzzzzz1
                zzzzzz2
                zzzzzz3
        yyyyyy3
xxxxxx3

in haskell it doesnt matter how many times I'll use align $ - the output is always the same

module TestSpec where

import           Protolude hiding (group)

import           Test.Hspec

import Data.Text.Prettyprint.Doc
import Data.Text.Prettyprint.Doc.Render.Text

text :: Text -> Doc a
text = pretty

test1 :: Doc a
test1 = group $
  group $ concatWith (surround line')
  [ text "xxxxxx1"
  , text "||||" <>
    ( align $ align $ group $ concatWith (surround line')
      [ text "yyyyyy1"
      , text "||||" <>
        ( align $ align $ group $ concatWith (surround line')
            [ text "zzzzzz1"
            , text "zzzzzz2"
            , text "zzzzzz3"
            ]
        )
      , text "yyyyyy3"
      ]
    )
  , text "xxxxxx3"
  ]

  where
    mapType = pretty ("Data.Map.Map" :: Text)
    numberType = pretty ("Number" :: Text)
    booleanType = pretty ("Boolean" :: Text)

    appWithoutPar l r = align $ l <> line <> r
    appWithPar l r = align $ l <> line <> pretty ("(" :: Text) <> r <> pretty (")" :: Text)

spec :: Spec
spec = do
  it "prettypirnter" $ do
    let output = renderStrict (layoutSmart (LayoutOptions (AvailablePerLine 12 1)) test1)
    putStrLn output
    output `shouldBe` "qwe"
xxxxxx1
||||yyyyyy1
    ||||zzzzzz1
        zzzzzz2
        zzzzzz3
    yyyyyy3
xxxxxx3

Thanks for the report. Minimal test is:

test :: forall a. Doc a
test = flexGroup $ text "1234" <> alignCurrentColumn (alignCurrentColumn (softBreak <> text "1234"))

main :: Effect Unit
main = Console.log $ print plainText (fourSpaces { pageWidth = 0 }) test

alignCurrentColumn is implemented using withPosition and align. This is because withPosition returns the same position in both cases, but it calls align with pos.column - pos.indent, which isn't idempotent. withPosition only returns the indentation level of the current line, but in order to make this idempotent it would also need to know what the pending indent context is.

Can you try current master?

yes, it indeed does fix everything

thank you a lot, I would do it very long