brandonbloom / fipp

Fast Idiomatic Pretty Printer for Clojure
527 stars 44 forks source link

Separate records into separate visitor function #29

Closed greglook closed 9 years ago

greglook commented 9 years ago

Hey @brandonbloom, I ran into an issue which could benefit from this change. The new code in Puget adds a CanonicalPrinter which implements IVisitor. This printer should throw an exception if it encounters any type without an explicitly-defined print handler. It does this by throwing an error if visit-unknown is called. You can probably see where this is going - record types are automatically rendered by the current code as a tagged-literal, even though no such extension has been added by the printer.

This change preserves the existing behaviour, but allows implementing printers to choose what they want to do with records in visit-record instead of creating tagged literals automatically.

Not a blocker, since I can get around this in my code by wrapping visit*, checking for record?, explicitly dispatching to visit-unknown instead, and then in visit-unknown checking again for record? to generate the standard tagged-literal representation if desired. It would be nicer not to need this workaround, though. :)

brandonbloom commented 9 years ago

LGTM!

brandonbloom commented 9 years ago

Deployed 0.6.3 with this change.

greglook commented 9 years ago

Thanks!