Clojure

enhance pprint to print type for defrecord (as in pr)

Details

  • Type: Feature Feature
  • Status: Open Open
  • Priority: Minor Minor
  • Resolution: Unresolved
  • Affects Version/s: Release 1.8
  • Fix Version/s: None
  • Component/s: None
  • Labels:
  • Approval:
    Triaged

Description

pprint currently doesn't print the names of defrecord types, instead printing just the underlying map. This is in contrast to pr-str/println. This ticket proposes that the behaviour of pprint is changed to match pr-str and println's.

More discussion at https://groups.google.com/forum/#!topic/clojure-dev/lRDG6a5eE-s

user=> (defrecord myrec [a b])
user.myrec
user=> (->myrec 1 2)
#user.myrec{:a 1, :b 2}
user=> (pr-str (->myrec 1 2))
"#user.myrec{:a 1, :b 2}"
user=> (println (->myrec 1 2))
#user.myrec{:a 1, :b 2}
nil
user=> (pprint (->myrec 1 2))
{:a 1, :b 2}
nil

Approach: Add new IRecord case to pprint's simple-dispatch mode. Extract guts of pprint-map to pprint-map-kvs and call it from both existing pprint-map and new pprint-record. Set multimethod preference for IRecord version. Added test.

user=> (pprint (->myrec 1 2))
#user.myrec{:a 1, :b 2}

Patch: CLJ-1890-pprint-records-2.patch

Prescreened by: Alex Miller

Also see: CLJS-1753

Activity

Hide
Daniel Compton added a comment -

The fix for this will needed to be ported to ClojureScript too.

Show
Daniel Compton added a comment - The fix for this will needed to be ported to ClojureScript too.
Hide
Steve Miner added a comment -

Added patch to pprint records with classname.

Show
Steve Miner added a comment - Added patch to pprint records with classname.
Hide
Steve Miner added a comment -

Open question: How should pprint handle a record that has a print-method defined for it? Should the print-method be used instead of the pprint default?

The current release and my patch do not consider the print-method when calling pprint.

Show
Steve Miner added a comment - Open question: How should pprint handle a record that has a print-method defined for it? Should the print-method be used instead of the pprint default? The current release and my patch do not consider the print-method when calling pprint.
Hide
Alex Miller added a comment -

I do not think pprint should check for or use print-method. pprint has it's own simple-dispatch multimethod that you can extend, or it will ultimately fall through to pr (which can be extended via print-dup).

Example of the former:

user=> (defrecord R [a])
user=> (def r (->R 1))
user=> (pprint r)
{:a 1}

user=> (use 'clojure.pprint)
user=> (defmethod simple-dispatch user.R [r] (pr r))
#object[clojure.lang.MultiFn 0x497470ed "clojure.lang.MultiFn@497470ed"]
user=> (pprint r)
#user.R{:a 1}
Show
Alex Miller added a comment - I do not think pprint should check for or use print-method. pprint has it's own simple-dispatch multimethod that you can extend, or it will ultimately fall through to pr (which can be extended via print-dup). Example of the former:
user=> (defrecord R [a])
user=> (def r (->R 1))
user=> (pprint r)
{:a 1}

user=> (use 'clojure.pprint)
user=> (defmethod simple-dispatch user.R [r] (pr r))
#object[clojure.lang.MultiFn 0x497470ed "clojure.lang.MultiFn@497470ed"]
user=> (pprint r)
#user.R{:a 1}
Hide
Steve Miner added a comment -

Right, clojure.pprint/simple-dispatch is there for user code to customize pprint, independent of whatever they might do with print-method. No need to conflate the two. So the patch is ready to review.

Show
Steve Miner added a comment - Right, clojure.pprint/simple-dispatch is there for user code to customize pprint, independent of whatever they might do with print-method. No need to conflate the two. So the patch is ready to review.
Hide
Michael Blume added a comment -

Patch does not appear to apply cleanly to current master.

Show
Michael Blume added a comment - Patch does not appear to apply cleanly to current master.
Hide
Michael Blume added a comment -

Doesn't apply cleanly to any of the beta tags, does apply cleanly to 1.8.0, but can't rebase without a conflict. Steve, you should probably fix that.

Show
Michael Blume added a comment - Doesn't apply cleanly to any of the beta tags, does apply cleanly to 1.8.0, but can't rebase without a conflict. Steve, you should probably fix that.
Hide
Steve Miner added a comment -

revised patch for current master (Clojure 1.9 beta4)

Show
Steve Miner added a comment - revised patch for current master (Clojure 1.9 beta4)
Hide
Steve Miner added a comment -

The old patch was prescreened but I guess the new patch should be reconsidered from scratch as namespaced maps were added to Clojure in between patches. By the way, I don't think there are any tests to cover pretty-printing namespaced maps. The only test I could find was for pr-str of a namespaced map. In any case, I tried to make sure the prefix logic was maintained so it should still work. The same test for pretty-printing records is used in both versions of the patch.

Show
Steve Miner added a comment - The old patch was prescreened but I guess the new patch should be reconsidered from scratch as namespaced maps were added to Clojure in between patches. By the way, I don't think there are any tests to cover pretty-printing namespaced maps. The only test I could find was for pr-str of a namespaced map. In any case, I tried to make sure the prefix logic was maintained so it should still work. The same test for pretty-printing records is used in both versions of the patch.

People

Vote (5)
Watch (2)

Dates

  • Created:
    Updated: