Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

*date-format* never bound #120

Open
cemerick opened this issue Jun 27, 2017 · 5 comments
Open

*date-format* never bound #120

cemerick opened this issue Jun 27, 2017 · 5 comments

Comments

@cemerick
Copy link

gen/*date-format* appears to be vestigial (from cheshire.custom?): it's never bound. However, it probably should be, as there's currently no way for JSONable implementations to access the desired date format string. The same goes for the other parameters to gen/generate...

@dakrone dakrone closed this as completed in af95a5f Jul 3, 2017
@dakrone
Copy link
Owner

dakrone commented Jul 3, 2017

Pushed a commit to bind *date-format*as for the other parameters, I think for the custom encoding the parameters for generate don't need to be provided, since you can do anything in the implementation of the custom encoder, so it's considered a little lower level?

@cemerick
Copy link
Author

cemerick commented Jul 3, 2017

Confused. Having a default value for *date-format* is fine I guess, but it's never bound to the date format passed into gen/generate. This means I can do (json/generate-string data {:date-format my-format}), and any JSONable I've provided that refers to *date-format* will use ISO8601 instead of my-format. Is the intent that cheshire callers bind *date-format* in addition to providing a :date-format option?

@dakrone
Copy link
Owner

dakrone commented Jul 3, 2017

Ahh you're right, I see the problem now :-/

@dakrone dakrone reopened this Jul 3, 2017
@dakrone
Copy link
Owner

dakrone commented Jul 3, 2017

I reverted the commit for now, I'll fix this in a different way.

@bgrabow
Copy link

bgrabow commented Oct 25, 2019

I ran into this today while trying to custom encode a org.joda.time.DateTime. I'd like these to be formatted the same as my java.util.Date objects with an encoder that looks like this:

(defn- joda-time-encoder
  [^DateTime dt ^JsonGenerator jg]
  (generate/encode-date (.toDate dt) jg))

This API would work great for me but without a value bound to generate/*date-format* it doesn't work.

The caller of cheshire.core/encode could provide the binding like so:

    (binding [gen/*date-format* "yyyy-MM-dd'T'HH:mm:ss'Z'"]
      (cheshire.core/encode thing {:date-format "yyyy-MM-dd'T'HH:mm:ss'Z'"}))

but this is problematic as all callers of encode would need to provide the date format in both the binding and the opts map. Better to keep the dynamic var a private implementation detail that is fully managed by the library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants