Pretty Printing with position logging

[copton]

Hi

I have added a feature to the Pretty library and I think others could need this as well. That's why I want to show it to you and I am eager to here what you think about it. It's about collecting a log of which Docs ended up where in the output string.

Motivation: The Ocram compiler [1] is a source to source compiler that amongst other things needs to keep track of the mapping of input rows and columns to output rows and columns. It uses the Language.C library which in turn uses the Pretty library for printing.

To perform its task the Ocram compiler needs to know which parts of the AST have been rendered where in the output document. To this end I have patched the Pretty library to support augmenting Doc objects with callbacks. When such a Doc is rendered the callback is called with the current row and column of the output document. Now I can change the Pretty instances to augment the Docs of the AST objects in question and I get a nice, accurate log of which part of the output AST ended where.

The current status of the patch is complete, as I am already using it for Ocram. But it lacks more test cases and anything else you would premise in order to accept the patches for upstream. I am willing to perform the additional work, as I would like to avoid inofficial dependencies for my project.

What do you think?

Greetings

Alex

[1] http://github.com/copton/ocram

[dterei]

Thanks for the code!

So I'm snowed under right now and may be some time before I can really look at this. In general I have two issues:

1) (unrelated to this pull request). There are some changes I want to make with the code and benchmark and testsuite I want to add. Until then I don't feel that confident accepting new features.

2) I don't know how keen I am on changing the Doc type and all interfaces for this feature which 95% of users probably won't use but now will be affected by this code due to type changes. I'd have to think if there are better ways to do this which leave a lot of the core the same.

[copton]

Hi David,

thanks for considering accepting my patches. Let me answer to your remarks.

1) Just go ahead and finish your pipeline and I can re-create the patches accordingly. Just let me know when you are ready.

2) Actually, I managed to keep downwards compatibility, as

• type Doc = DocLog () [1]
• fullRender ... = fst $ fullRenderWithLog ... [2]

Greetings, Alex

[1] https://github.com/copton/pretty/blob/master/src/Text/PrettyPrint/HughesPJ.hs#L186 [2] https://github.com/copton/pretty/blob/master/src/Text/PrettyPrint/HughesPJ.hs#L879

[copton]

Hi David,

I just wanted to ping you and ask for a status update.

I have some free time currently and I would love to get some productive work done :) Maybe I could be of help in adding test cases and benchmarks to the library?

I would really love to see this pull request being accepted. With the compatibility issue out of the way, is there anything else that would prevent you from considering these patches?

Greetings,

Alex

[dterei]

Hi Alex,

yes very sorry about this, I just am not finding time to work on pretty at the moment.

So, what if we went with something a little more generic? I like the code and having logging, but pretty is a fairly used library that hasn't changed much a all in the last 5 years. What if rather than put logging into the core, we simply changed Doc to carry around an abstract type and the rest of the logging infrastructure was in a separate module. Basically, what is the min you need to change in the core to add logging in a another module?

[copton]

Hi David,

thank you for your response.

As far as I can see, logging support needs to touch display and easy_display at least, because it is required to keep track of the row and column numbers while rendering. Reviewing the current pull request actually shows me little potential to reduce changes to the main module - at least as far as I can see. The patches touch many source code lines, but most changes are very systematic, like replacing Doc with DocL, for instance.

But I understand your reluctance. Maybe the following approach would be a good solution:

Let's add this test suite you have in your pipeline anyway first. Only when we feel comfortable about the test coverage, we will add the logging code and verify that the tests still pass. This should give us a strong indication that logging does not break existing code. The obvious next step would be to extend the tests to cover the logging aspect as well. Overall, its the same amount of work for me, because - as I already said previously - tests are still missing in the current pull request.

So, if you feel like this is a good way, let me know what you had in mind regarding the test suite and I will go ahead and do it.

Greetings,

Alex

[dterei]

Alex,

OK. So there is a folder already called "tests". It is a little messy right now but has the following:

• whitebox version of pretty: PrettyTestVersion.hs
• quickcheck based tests suite: Test.hs, TestGenerators.hs, TestStructures.hs
• ghc test suite: all.T, pp1.hs, pp1.stdout, T3911.hs, T3911.stdout

The quickcheck stuff is pretty good and comprehensive. I'd like to see the Test.hs file split up a little though, the comments at the start of the file explain that it covers 3 different areas. I'd like each area to be its own file, especially the laws be in their own file.

The GHC testsuite checks should be incorporated into the quickcheck Test.hs framework. Either as quickcheck tests if this makes sense, or just straight hand-written test code otherwise.

Pretty also needs a good benchmark suite going forward. This is a concern for your pull request as we also want to make sure that it doesn't degrade performance. The only thing so far for this is a hastily written file in tests called Bench1.hs. It isn't very good. So if you have the time and motivation than a good benchmark suite would be awesome! Feel free to start with a clean slate here. Just custom code and lots of microbenchmarks may be the best, but you could also imagine testing with something more real. E.g., are there already some haskell packages that would be suitable for driving pretty as a benchmark? a library that features serialization of XML in a pretty form for example...

Finally. It would be great to get a simple example of how to use the logging code this pull requests adds. I'd like to see this to feel more confident in its purpose and the API you are exposing.

Thanks for working on this Alex!

[copton]

Hi David,

sounds good to me. To sum it up, the individual steps are:

• clean up and complete test suites
• add a benchmark suite
• incorporate position logging

The last step will in particular:

• make sure that position logging sustains compatibility
• measure the performance costs of position logging
• show how to use the position logging API

Expect a pull request for the first item (hopefully) soon.

Greetings,

Alex

[dterei]

Alex,

I'm going to close this PR at this time as it's been a while now. Happy to reconsider this again if you want to rebase and open a new PR.

Cheers, David