search

quchen / prettyprinter

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

Stack machine is unnecessarily opaque #163

Closed georgefst closed 4 years ago

georgefst commented 4 years ago

Perhaps it's specific to my use case, but Render.Util.StackMachine.renderSimplyDecorated seems like it would be more useful if it actually passed the current stack to the first argument (the text function).

I've found myself using renderSimplyDecoratedA with a State that just reimplements the stack. This feels quite silly.

sjakobi commented 4 years ago

I'm not very familiar with this part of the code. If you have a change in mind, could you simply submit it as a PR? That should simplify the discussion.

georgefst commented 4 years ago

Done.

For a use case, see here, where I am already using the modified form as renderSimplyDecorated'.

georgefst commented 4 years ago

Perhaps it's specific to my use case

Actually, it kind of is. And there's a better approach I could take anyway.

sjakobi commented 4 years ago

And there's a better approach I could take anyway.

I'm curious about this, so I've started watching https://github.com/georgefst/prettyprinter-graphviz for now. :)

georgefst commented 4 years ago

I'm curious about this

Basically, by using the whole annotation stack to render each substring, I'm neglecting the fact that the graphviz labels use a HTML-esque tree structure, so I should really be able to handle the annotations one at a time as they're pushed and popped. This would be more elegant and result in more concise outputs in general.

It might not currently be possible to do that with the current Haskell graphviz library, so I may need to make changes over there. Admittedly, it's not much of a priority for me, since prettyprinter-graphviz is otherwise essentially complete, and AFAIK nobody's using it, except me in one project at work.

So you'll probably get a notification some time next year once you've forgotten why you were ever watching it in the first place.