quchen / prettyprinter

A modern, extensible and well-documented prettyprinter.
BSD 2-Clause "Simplified" License
293 stars 34 forks source link

Simple example uses putDocW, but it's not referenced. #104

Open phadej opened 4 years ago

phadej commented 4 years ago

putDocW is not in Data.Text.Prettyprint.Doc module.

Relatedly, there is putDoc / putDocW convinience functions; but there should also be functions to Doc a -> String and Doc a -> Text.

sjakobi commented 4 years ago

putDocW is not in Data.Text.Prettyprint.Doc module.

I have added a reference to the definition in https://github.com/quchen/prettyprinter/pull/105. Or do you see a better way to address this?

Relatedly, there is putDoc / putDocW convinience functions; but there should also be functions to Doc a -> String and Doc a -> Text.

I what context have you wanted these functions? Do you want to use them in proper code, or just for interactive exploration?

(FYI: David asked me to help maintain this project)

phadej commented 4 years ago

I needed putDoc like function in golden tests, to render code / error messages / ... into e.g. ByteString. I could do Text.encodeUtf8 . PP.renderDoc, but I have to also remember PP.layoutPretty layoutOptions . PP.unAnnotate.

It's not bad when you know what to do. I.e. that you need to import second module for "render" (as in https://hackage.haskell.org/package/prettyprinter-1.5.1/docs/Data-Text-Prettyprint-Doc.html general workflow section).

In other words, between simple example (which uses "test" function putDocW) and general workflow, there's missing "real world" example.

sjakobi commented 4 years ago

I agree that there should be an examples that renders a Doc to Text. I've found the various parts involved in producing Text from a Doc hard to remember too!

I'd welcome a PR, or might eventually write an example myself.

I think we can also add some convenience functions for translating Doc to Text or possibly String to Doc.Util. Although I fear that would invite some bikeshedding regarding naming, layoutPretty vs. layoutSmart, etc. that I'm not too keen on! ;)

Again, a PR would be welcome, although it would probably see a bit more discussion.

quchen commented 4 years ago

The problem with convenience functions is that there are too many combinations, so we get the zoo already present in the rendering modules – hPutDoc, putDoc, render, renderIO. putDocW is yet another version of »rendering pipeline with special settings« that is very useful for doctests. I’ve encountered a couple more that I deliberately did not add to the convenience modules.

In an ideal world, I’d just get rid of the convenience modules, but that breaks a lot of code, so it will remain.

phadej commented 4 years ago

Please then document that deliberate choice, perfectly already in the Right now it looks like an accidental mistake. Perfectly somewhere close to Simple Example and General Workflow session, as the documentation leaves one without no forward pointers.

I'm sad to report that I think the switch from ansi-wl-pprint to prettyprinter in trifecta doesn't feel like improvement now.

I hope that @sjakobi would also drive the adoption (the important one is optparse-applicative: https://github.com/pcapriotti/optparse-applicative/issues/273) otherwise we (= I and RyanGlScott) won't be able to deprecate ansi-wl-pprint in good faith. Just not upgrading it (to work with newer GHCs e.g.) and forcing community to adopt doesn't feel nice. So current status is that prettyprinter tried to solve pretty-printing problem, but just repeated xkcd 927.

sjakobi commented 4 years ago

@quchen I just don't see the problem with simply adding the convenience functions that people want in Doc.Util (we have a similar discussion in https://github.com/quchen/prettyprinter/issues/88). Obviously I'm not going to implement every possible combination upfront. I'd just accept those that people request.

Of course, Doc.Util isn't going to be super easy to navigate then, but that's ok. The interesting documentation is elsewhere anyway.

sjakobi commented 4 years ago

FWIW I'd be happy to see prettyprinter get wider adoption, and eventually replace ansi-wl-pprint. I'd be happy to accomodate related requirements in the packages in this repo.

I don't currently plan to work on directly converting packages that currently use ansi-wl-pprint though, although that sounds like an interesting task.

phadej commented 4 years ago

In the case of optparse-applicative the most work is in convincing (and/or reminding) the maintainer that the change to prettyprinter is something that both maintainers of ansi-wl-pprint (me) and prettyprinter would like to see. The "hard work" of widening e.g. prettyprinter to support old GHCs down to 7.4.2 is already done. optparse-applicative would need to drop support for GHC-7.0 and GHC-7.2 but it's probably a good time at this time (but that might need more convincing).

Also porting other libraries from using ansi-wl-pprint will highlight shortcomings in prettyprinter. I don't suspect the patch from ansi-wl-pprint to prettyprinter be more than an evening of work, if even that.

This issue is a result of such porting experience.