search

brandonbloom / fipp

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

Add support for *print-meta* #15

Closed hugoduncan closed 10 years ago

hugoduncan commented 10 years ago

The meta map and value are put within a :group vector.

brandonbloom commented 10 years ago

Hey Hugo, I've merged your commit, then made some cleanups/improvements. Changes should be on master now. Please give the new master branch a try. If it works well, I'll stamp out a release.

Thanks!

hugoduncan commented 10 years ago

Thanks for looking at this, and for the improvements.

Passing {:print-meta true} results in metadata being printed, but binding *print-meta* to true doesn't seem to.

I've also tried setting :width, but it doesn't seem to limit line length with large strings in maps. e.g this was with {:width 60}.

{:dev {:plugins [[lein-pallet-release "0.1.4-SNAPSHOT"]],
       :pallet-release {:url "https://pbors:${GH_TOKEN}@github.com/palletops/lein-pallet-release.git",
                        :branch "master"}},
 :no-checkouts {:checkout-deps-shares []},
 :release {:set-version {:updates [{:path "README.md",
                                    :no-snapshot true}]}}}
brandonbloom commented 10 years ago

binding print-meta to true doesn't seem to.

D'oh! That's because I rebound print-meta to false to prevent the pr-str fallback from showing metadata. Fixed now. Please try again.

I've also tried setting :width, but it doesn't seem to limit line length with large strings in maps. e.g this was with {:width 60}.

That's expected. It's not a hard limit, it's simply the right margin setting for controlling when groups are broken. Individual elements are never broken, since that may produce invalid syntax in the output.

hugoduncan commented 10 years ago

Umm, can't get it to print metadata at all now.

The tests fail with "Unable to resolve var: p/*width* in this context" in fipp/printer-test.

I also note that there is a reflection warning "Reflection warning, fipp/edn.clj:8:1 - reference to field pretty can't be resolved."

brandonbloom commented 10 years ago

Test was stale, should be fixed now.

Reflection issue is discussed here: http://dev.clojure.org/jira/browse/CLJ-1202

I could rename the function to work around the bug...

hugoduncan commented 10 years ago

Metadata on the argument to pprint isn't getting printed due to the the call to pretty in pprint, which is outside of the dynamic scope for *options* set in pprint-document.

brandonbloom commented 10 years ago

D'oh! That's what I get for trying to write code during sessions at clojure/west.

I just pushed a change taking my own advice: explicit context, instead of dynamic variables! No promises on this code being sane, since the collective energy level at this point in the week is very low here at the conference. yawn

hugoduncan commented 10 years ago

Wish I were there to join in the fun!

Seems to be working for me for some light testing.

brandonbloom commented 10 years ago

Thanks Hugo. Pushed out 0.4.2 with these changes.