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

use a state monad for keeping track of the output state #25

Closed cdepillabout closed 4 years ago

cdepillabout commented 6 years ago

I talked a little about this in my response in #24.

This would look similar to how it is done in hindent.

sureyeaah commented 4 years ago

Hey, I would like to work on this. Any particular things I should keep in mind?

cdepillabout commented 4 years ago

@sureyeaah Thanks for taking a look at this!

Any particular things I should keep in mind?

I'd take a glance at the hindent source code and see how it uses StateT (in its Printer newtype). I think a similar thing could be done for the pretty-simple output code.

Also, the printing code for pretty-simple is pretty hacky, so feel free to make big changes.

There are lots(?) of tests for pretty-simple, so you should feel confident in making changes.

cdepillabout commented 4 years ago

@sureyeaah Oh, I did think of one more thing you should keep in mind.

pretty-simple is able to parse and pretty-print lazy data structures, so it is important that this property is kept.

When you are working on this, just make sure that pretty-printing lazy data structures (like [1..]) still works.

sureyeaah commented 4 years ago

The ExprToOutput module uses a state monad already. Where exactly is it that we need to make changes then?

cdepillabout commented 4 years ago

@sureyeaah The Text.Pretty.Simple.Internal.OutputPrinter module contains the render function, which turns a list of Output elements into Text.

It does this by writing to a Text.Builder directly. Instead, it would probably be easier to use a State monad, similar to the ExprToOutput module. This should also be similar to other Haskell pretty-printing libraries, like hindent.

cdepillabout commented 4 years ago

Another thing that might help simplify this is actually using a proper pretty-printing library:

https://hackage.haskell.org/package/prettyprinter

Although a big thing to keep in mind is whether this would be sufficiently lazy.

sjakobi commented 4 years ago

Another thing that might help simplify this is actually using a proper pretty-printing library:

https://hackage.haskell.org/package/prettyprinter

Oh, no! ;)

I actually rely on pretty-simple to debug prettyprinter. This would introduce a circular dependency!

(I could probably work around a circular dependency though…)

sjakobi commented 4 years ago

Note BTW that there's already

http://hackage.haskell.org/package/show-prettyprint

which is a bit like a pretty-simple built on top of prettyprinter. I think its output is less nice than pretty-simple's though.

cdepillabout commented 4 years ago

Ah, I see :-)

I guess keeping the dependencies of pretty-simple minimal would be a good idea, so maybe using a different pretty-printing library, or re-implementing a simple pretty-printing library in pretty-simple would make sense.

georgefst commented 4 years ago

@sjakobi out of interest, would you be able to get away with:

cab install pretty-simple -f buildexe

pp :: Show a => a -> IO ()
pp = putStrLn <=< readProcess "pretty-simple" [] . show

I've used this trick before to avoid actually depending on pretty-simple.

I think there's a reasonable argument for using an actual pretty-printing library, and prettyprinter itself has minimal dependencies.

georgefst commented 4 years ago

Anyway, @sureyeaah, did you make any progress here?

I figure it may simplify #66, as well as the existing #56 and #43.

sjakobi commented 4 years ago

@georgefst Thanks, that looks like a nice trick!

I think I can also recommend using prettyprinter at this stage. It's pretty polished and there are no plans to adopt any dependencies that aren't core libraries. There have even been some ideas to somewhat "standardize" its use, so libraries like ansi-wl-pprint could eventually be deprecated.