search

brandonbloom / fipp

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

Various non-idiomatic whitespace from fipp.clojure #43

Closed tiye closed 5 years ago

tiye commented 7 years ago

I have a lot of code that is generated with clojure.pprint and now trying fipp for better readability and also performance. Here are the changes after the migration: https://github.com/Quamolit/quamolit/commit/f8bc00bcebd661364f7ccbdbea26d8c46831e287?diff=unified

Some highlights:

(a
..b
..c)
(a
.b
.c)
(cond
..a
....of-a
..b
....of-b
..:else e)

Since these details are not listed in the docs, I got the questions to ask if all of them are expected(at least code runs well)? And does it help in helping fipp in achieving better results?

I don't have the number but definitely it's faster than clojure.pprint according to my experience.

brandonbloom commented 7 years ago

Just to confirm: It looks like you are using fipp.clojure/pprint, not fipp.edn/pprint, right? I should probably document the former in the README.

I don't make any promise of clojure.pprint compatibility. That said, none of the changes you identified are intentional or problematic to fix as far as I can tell. The general philosophy of the code formatter is to be heuristically idiomatic. That is, idiomatic if its achievable without non-local analysis, and formatted in a not-totally-ugly way otherwise.

I think that https://github.com/brandonbloom/fipp/blob/master/src/fipp/clojure.cljc is pretty approachable. I'm reasonably happy with the output for my use case (mainly: macro debugging), but I'm open to improvements as long as they are relatively low risk. For example, you could probably fix the extra space at the end of the ns form by adding a when to https://github.com/brandonbloom/fipp/blob/07d138e2b3b8c58305b572548fc1ff53f5dd6618/src/fipp/clojure.cljc#L121. If you'd like to take a crack at fixing any of these, I'm happy to provide guidance. Please open individual issues or pull requests for each change.

Thanks!

brandonbloom commented 5 years ago

closing this for my own tracking purposes - if you have specific fixes/problems or whatever in the future that meet the design restrictions, please open new tickets.