Closed rs3vans closed 7 years ago
I think you want the precondition to ensure that you cannot have a :key-fn
and :keywords?
at the same time. You might also want to mention it in the docstring.
Actually, no, you can have them both at the same time (as it says in the README). The idea was that they could be combined. Maybe that's too confusing?
But as it's written, the code doesn't do that. The argument is either true
for keywords, or a function otherwise. In addition, if you allow both options to be set at once, you need to define how that behaves.
Actually, check out line no. 16 in json.clj. The redefining of key-fn
in the let bindings wraps the passed-in function so that its value is passed through the keyword
function (but only if both key-fn
and keywords?
are both non-false). Cheshire's parse-string
function takes either a key function, or true
(meaning use keywords), so that's why I did it the way I did...
Also there's a test ("transform keys with keywords"
) for exactly this combination of options.
Ah, I see. My apologies; I missed the line where you compose the two functions first.
I think that behaviour is okay then, but I'd change the way key-fn is defined to something like:
(comp (if keywords? keyword identity) (or key-fn identity))
Then we can just write:
(json/parse-string body-string key-fn)
You can do a similar thing in the tests. Instead of:
#(-> % name clojure.string/upper-case)
You can write:
(comp str/upper-case name)
Also remember to require clojure.string
in the test namespace if you're going to use it.
Suggested changes made :)
Was looking for this feature, how is it moving along or yet to be decided? Thanks for the work!
@kardan You're welcome to use my repository until it gets merged here (if it ever does).
The notification for the PR got lost or never made it to my inbox. I'll take a look over it.
Cool! You guys didn't seem to totally disagree, so thought it might slipped through the net.
@weavejester Any chance you want to merge this? It's been sitting around for a while. If not, just close it out - no hard feelings!
Added a few comments. Sorry this keeps getting lost off the bottom of my TODO list.
Once the comments are addressed, if you squash your commits, we should be good to merge.
Cheshire supports arbitrary functions that transform the map keys when going to/from JSON. This PR adds support for passing in such transformers (beyond the already-supported
:keywords?
option).