cdepillabout / pretty-simple

pretty-printer for Haskell data types that have a Show instance
https://hackage.haskell.org/package/pretty-simple
BSD 3-Clause "New" or "Revised" License
243 stars 29 forks source link

further reduce newlines for compact output #110

Closed juhp closed 2 years ago

juhp commented 2 years ago

This seems to rather make the compact output have less newline commas: addressing #84.

eg

    ( "request"
    , ValueArray
        [ ValueString "git+https://src.fedoraproject.org/rpms/rpmbuild-order.git#6a9f3fce18040a2020d37707abb82445e1884e69"
        , ValueString "f37-build-side-54239"
        , ValueStruct
            [
                ( "fail_fast", ValueBool True )
            ,
                ( "wait_builds", ValueArray [] )
            ,
                ( "custom_user_metadata", ValueStruct [] ) ] ] )
,
    ( "result"
    , ValueStruct
        [
            ( "faultCode", ValueInt 1000 )
        ,
            ( "faultString"
            , ValueString "bad filename: A-1-1.fc37.buildreqs.nosrc.rpm (expected A-1-1.fc37.src.rpm)" ) ] )
,
    ( "start_time", ValueString "2022-05-31 02:27:37.208059+00:00" )

becomes

    ( "request", ValueArray
        [ ValueString "git+https://src.fedoraproject.org/rpms/rpmbuild-order.git#6a9f3fce18040a2020d37707abb82445e1884e69", ValueString "f37-build-side-54239", ValueStruct
            [
                ( "fail_fast", ValueBool True ),
                ( "wait_builds", ValueArray [] ),
                ( "custom_user_metadata", ValueStruct [] ) ] ] ),
    ( "result", ValueStruct
        [
            ( "faultCode", ValueInt 1000 ),
            ( "faultString", ValueString "bad filename: A-1-1.fc37.buildreqs.nosrc.rpm (expected A-1-1.fc37.src.rpm)" ) ] ),
    ( "start_time", ValueString "2022-05-31 02:27:37.208059+00:00" ),
cdepillabout commented 2 years ago

I don't generally use the compact output option, so I don't have a strong opinion on this change, but it does roughly look to me like what someone who wants more compact output would want.

@georgefst How does this look to you?

@juhp I'm wondering if any of these doctests will need to get fixed? https://github.com/cdepillabout/pretty-simple/blob/abbfd6912297029fb1ef983d9839a134141f726c/src/Text/Pretty/Simple.hs#L663. If none of these need fixing (which is somewhat surprising), could you add a new doctest that would fail on master, but would now pass with this PR?

juhp commented 2 years ago

Yea, I was a little surprised too: thanks for pointing out the doctests.

I have a added a simple doctest which is affected by this change.

Previously the output would have been:

          [
              ( "id", 123 )
          ,
              ( "state", 1 )
          ,
              ( "pass", 1 )
          ,
              ( "tested", 100 )
          ,
              ( "time", 12345 )
          ]
georgefst commented 2 years ago

Nice! As I've said elsewhere, compact mode was kind of a quick hacky afterthought. This makes it a bit more useful and intuitive.

I hope you don't mind that I've pushed two small refactors.

georgefst commented 2 years ago

RE https://github.com/cdepillabout/pretty-simple/issues/84#issuecomment-1146647685: would you be interested in also helping to improve the output of records? We can tackle that separately otherwise.

juhp commented 2 years ago

RE #84 (comment): would you be interested in also helping to improve the output of records?

I think I would prefer it to be tackled separately - perhaps I can have a look at some point.

georgefst commented 2 years ago

I think I would prefer it to be tackled separately - perhaps I can have a look at some point.

Ok, no worries!