Clojure

pprint a GregorianCalendar results in Arity exception

Details

  • Type: Defect Defect
  • Status: Open Open
  • Priority: Minor Minor
  • Resolution: Unresolved
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
  • Patch:
    Code and Test
  • Approval:
    Triaged

Description

What I was doing: attempting to pretty-print nested structures from Things mac app, which include instances of java.util.GregorianCalendar.
What I expected to happen: output should have an #inst very much like printing java.util.Date.
What happened instead: ArityException Wrong number of args (4) passed to: pprint$pretty-writer$fn

thingsplay.core=> (def nowish (java.util.GregorianCalendar.))
#'thingsplay.core/nowish
thingsplay.core=> nowish
#inst "2014-03-25T22:43:29.240-05:00"
thingsplay.core=> (require 'clojure.pprint)
nil
thingsplay.core=> (pprint nowish)
ArityException Wrong number of args (4) passed to: pprint$pretty-writer$fn  clojure.lang.AFn.throwArity (AFn.java:437)
#inst "
thingsplay.core=> (simple-dispatch nowish)
#inst "2014-03-25T22:43:29.240-05:00"nil
thingsplay.core=> nowish
#inst "2014-03-25T22:43:29.240-05:00"
thingsplay.core=> (write nowish)
ArityException Wrong number of args (4) passed to: pprint$pretty-writer$fn  clojure.lang.AFn.throwArity (AFn.java:437)
#inst "
  1. 0001-CLJ-1390.patch
    29/Mar/14 4:18 PM
    0.9 kB
    Norman Richards
  2. 0002-CLJ-1390-test.patch
    04/Apr/14 8:30 PM
    0.9 kB
    Norman Richards
  3. 0002-CLJ-1390-test2.patch
    07/Apr/14 10:49 AM
    1 kB
    Norman Richards
  4. CLJ-1390-pprint-GregorianCalendar.patch
    10/Apr/14 4:38 PM
    2 kB
    Steve Miner

Activity

Hide
Norman Richards added a comment -

The print-calendar function introduced in CLJ-928 doesn't work with clojure.pprint/pretty-writer since pretty-writer does not correctly implement the java.io.Writer interface. It only implements write(String) but print-calendar wants write(String,int,int) among others. Although pretty-writer should probably correctly implement java.io.Writer, it's pretty easy to rewrite print-calendar to use the supported subset of java.io.Writer that is implemented.

Show
Norman Richards added a comment - The print-calendar function introduced in CLJ-928 doesn't work with clojure.pprint/pretty-writer since pretty-writer does not correctly implement the java.io.Writer interface. It only implements write(String) but print-calendar wants write(String,int,int) among others. Although pretty-writer should probably correctly implement java.io.Writer, it's pretty easy to rewrite print-calendar to use the supported subset of java.io.Writer that is implemented.
Norman Richards made changes -
Field Original Value New Value
Attachment 0001-CLJ-1390.patch [ 12904 ]
Hide
Steve Suehs added a comment -

Thank you, "random person at the Austin Clojure Hack Day" "that I don't know" that has a CA in place. You are awesome!

See you at the next Austin Clojure Meetup.

-s

Show
Steve Suehs added a comment - Thank you, "random person at the Austin Clojure Hack Day" "that I don't know" that has a CA in place. You are awesome! See you at the next Austin Clojure Meetup. -s
Andy Fingerhut made changes -
Patch Code [ 10001 ]
Hide
Andy Fingerhut added a comment -

Norman, it would be good if the patch included a few test cases, especially ones that fail without the patch, and succeed with the patch.

Show
Andy Fingerhut added a comment - Norman, it would be good if the patch included a few test cases, especially ones that fail without the patch, and succeed with the patch.
Hide
Norman Richards added a comment -

Absolutely. I have no idea how test cases work on Clojure core, but I assume it should be easy enough to do.

Show
Norman Richards added a comment - Absolutely. I have no idea how test cases work on Clojure core, but I assume it should be easy enough to do.
Hide
Andy Fingerhut added a comment -

I would recommend looking at the following file in the Clojure repo, which already contains other pprint tests. It shouldn't be too difficult to get an idea from one or more of the tests there. Actually those might be slightly unusual in that many of them use a simple-tests macro defined in file test_helper.clj that most of the Clojure tests do not use, but ask questions if you have trouble, e.g. on the Clojure Google group or IRC channel.

test/clojure/test_clojure/pprint/test_pretty.clj

Show
Andy Fingerhut added a comment - I would recommend looking at the following file in the Clojure repo, which already contains other pprint tests. It shouldn't be too difficult to get an idea from one or more of the tests there. Actually those might be slightly unusual in that many of them use a simple-tests macro defined in file test_helper.clj that most of the Clojure tests do not use, but ask questions if you have trouble, e.g. on the Clojure Google group or IRC channel. test/clojure/test_clojure/pprint/test_pretty.clj
Hide
Steve Suehs added a comment -

Alright...you two are inspiring me to go work on getting my CA in.

Show
Steve Suehs added a comment - Alright...you two are inspiring me to go work on getting my CA in.
Norman Richards made changes -
Attachment 0002-CLJ-1390-test.patch [ 12917 ]
Hide
Norman Richards added a comment -

Test case attached. Apply the test patch, "mvn test" fails. Apply the fix, test passes.

Show
Norman Richards added a comment - Test case attached. Apply the test patch, "mvn test" fails. Apply the fix, test passes.
Hide
Andy Fingerhut added a comment -

It would be better if the "is" were of the form:

(is (= calculated-value "constant string to compare against") "string to show if test fails")

rather than just (is calculated-value "string to show if test fails"). The second form will fail if calculating the value throws an exception, but only the first form will calculate it, and then verify that the value is the expected one (and fail if it is not the expected one).

Show
Andy Fingerhut added a comment - It would be better if the "is" were of the form:
(is (= calculated-value "constant string to compare against") "string to show if test fails")
rather than just (is calculated-value "string to show if test fails"). The second form will fail if calculating the value throws an exception, but only the first form will calculate it, and then verify that the value is the expected one (and fail if it is not the expected one).
Hide
Norman Richards added a comment - - edited

Ok - here's an alternative test case. I'm less happy with this test case, since I have to add the TimeZone and make assumptions about how the specifics of how the pretty printer formats. But, it does test the fix adequately, so if you like the test2 patch better, that's perfectly fine with me.

Show
Norman Richards added a comment - - edited Ok - here's an alternative test case. I'm less happy with this test case, since I have to add the TimeZone and make assumptions about how the specifics of how the pretty printer formats. But, it does test the fix adequately, so if you like the test2 patch better, that's perfectly fine with me.
Norman Richards made changes -
Attachment 0002-CLJ-1390-test2.patch [ 12920 ]
Hide
Steve Miner added a comment -

I would rather fix the actual bug in pretty_writer.clj. The proxy needs to support more of the java.io.Writer interface. I think adding another arity to the write method would work. Something like:

(write 
   ...
  ([x off len]
      (.write this (subs (str x) off (+ off len)))))
Show
Steve Miner added a comment - I would rather fix the actual bug in pretty_writer.clj. The proxy needs to support more of the java.io.Writer interface. I think adding another arity to the write method would work. Something like:
(write 
   ...
  ([x off len]
      (.write this (subs (str x) off (+ off len)))))
Alex Miller made changes -
Labels print
Alex Miller made changes -
Priority Major [ 3 ] Minor [ 4 ]
Hide
Steve Miner added a comment - - edited

CLJ-1390-pprint-GregorianCalendar.patch fixes the pretty_writer.clj proxy to support the missing version of the write method. Includes the same test as the previous patch.

Show
Steve Miner added a comment - - edited CLJ-1390-pprint-GregorianCalendar.patch fixes the pretty_writer.clj proxy to support the missing version of the write method. Includes the same test as the previous patch.
Steve Miner made changes -
Steve Miner made changes -
Patch Code [ 10001 ] Code and Test [ 10002 ]
Alex Miller made changes -
Approval Triaged [ 10120 ]

People

Vote (1)
Watch (2)

Dates

  • Created:
    Updated: