reprinter
reprinter copied to clipboard
camfort
Splicer and context reprinting
Remembered this was mentioned the other week but didn't get round to it, so I've rebased the changes onto current master. Really like the new Example.lhs btw, wish it was there when I was figuring out the library!
I don't think either of these changes are the best approach (at the very least the ZipperReprinting should be optional, or used in a separate reprint function) but fix two issue with the reprinter I encountered while using it. Perhaps a better approach can be use figured out with this in mind.
The splicer part allows a user defined function to be passed in which can perform extra formatting based on the Reprinting monad, in my case using a state monad to keep track of the position while we reprint, and then perform necessary line breaks where needed. This was needed because often we'd refactor lines and bring them over the 72 line limit of legacy fortran, causing errors. It kinda hijacks the existing monad, but this seemed like the simplest approach for it to work.
The ZipperReprinting allows for checks of the surrounding context by taking the zipper rather than the node as the argument, which I used to avoid printing more brackets than are necessary. In general I want explicit brackets around logical operators (i.e. ((foo .ne. 0) .and. (bar .ne. 0)), but I do not want double brackets if something like this is at the top level of a conditional or in a function argument (i.e. neither if ((foo .ne. 0)) then or foo((bar .ne. 0))).
I think a better solution might be along the lines of having the reprinter also take an AST specific argument which details the context of the section we're reprinting (indentation level, what node it's contained within etc.), and then having the reprinting function itself do formatting based on this, perhaps running on additional unchanged nodes if a line break is needed. Maybe there's some way of making this generic so the context can be updated as the AST is traversed, and mapped to user defined input.
Thanks, this looks useful! @dorchard has been saying he was keen on improving the reprinter interface. The line breaking is especially interesting.
I feel like Reprinting should be able to manage bracketing (and precedence to skip unnecessary bracketing) by storing parent expression precedence and handing it off to a pretty printer that handles it. Or if you just want bracketing on certain node types, you could handle that in a snippet before calling the pretty printer. It seems easier in my head, but I haven't tried it.
In regards to your last point, I think monadic reprinters can do a lot of that. The main difference is that they run without full context, which may restrict what you're able to do.
What do you think? Did you have issues trying to implement similar things with a monadic reprinter/without genReprinting?
Hmm yeah perhaps just the parent expression instead of the zipper would be simpler, just update it every time you go down the tree. I've not had to yet but I imagine there could be situations where you want more context than just a parent expression however.
Ah maybe that is the way to do it, I'd expanded my reprinter from the examples which use catchAll and genReprinting and hadn't considered writing the Reprinting entirely from scratch. I'll try and have a go when I get time to, as this could probably take into account the context side as well.
I think I was wrong, you probably did have problems with doing much contextual work in a monadic reprinter. I tried and couldn't figure out how to implement my example of "work on a some AST node of type node while knowing its parent Maybe node", because traverse order is managed in the algorithm.
I've forked this to work on the ZipperReprinting idea (export-zipper branch) and have managed to get something working, plus wrap existing Reprintings into ZipperReprintings. Still needs testing and consideration for the interface, will update when I've cleaned it up a bit.
Hmmm I see. Would there be some way of indicating to the monadic reprinter which movements have been taken? Although I'm unsure if that would fit nicely into how getRefactorings is recursively defined.