search

brandonbloom / fipp

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

Implement code dispatch #19

Closed mkremins closed 9 years ago

mkremins commented 9 years ago

(This PR addresses issue #9. Also bringing in @arrdem since he expressed interest in this earlier.)

This probably isn't actually ready to be merged yet. Some design questions are still open, and several fairly common special forms and core macros are still missing special-cased pretty formatting. However, I'm hoping this can be a jumping-off point for further discussion, review and revision.

I'm currently making no attempt to validate input. This implementation should do the right thing so long as it's handed reasonable code, but if you pass in something like (let (:a :b :c) (fn 42)), all bets are off. In the next few days I'm hoping to make this handle such cases by skipping the special-cased formatting and treating weird inputs like ordinary lists.

I've mostly been referencing clojure.pprint, although this is by no means a 1:1 port. Is it acceptable for us to diverge a little, or should I make getting the same formatted output as clojure.pprint for the same input more of a priority? (FWIW we're already pretty close most of the time, and out-of-the-box clojure.pprint is missing a number of special cases that I've included here.)

brandonbloom commented 9 years ago

Cool! Thanks for taking a crack at this. It looks like a sensible start, but I really need some time to study it. I'll add give it a quick once over and add some quick notes. Can take a deeper look at it (or another revision) later.

mkremins commented 9 years ago

Made the requested updates from the preliminary review and squashed everything together into one commit.

brandonbloom commented 9 years ago

Thanks. I'll take a deeper look at it on Wed or Thurs.

On Sun, Oct 19, 2014 at 10:35 PM, Max Kreminski notifications@github.com wrote:

Made the requested updates from the preliminary review and squashed everything together into one commit.

— Reply to this email directly or view it on GitHub https://github.com/brandonbloom/fipp/pull/19#issuecomment-59677312.

brandonbloom commented 9 years ago

OK, studied this a bit more and it looks like a pretty good start. Thanks!

I'll probably tweak it a bunch before documented in in the README and cutting a release.

brandonbloom commented 9 years ago

Even though I've merged this, I'm still going to add some line-comments to this PR for tracking future work. Feedback welcome!