Added support for nested parameters to url-encode. #69

Open
wants to merge 2 commits into from

4 participants

@sbelak

Pretty much what it says on the tin. I'm a big fan of nested params and it is sometimes useful to be able to pass them around.

@asmala
Collaborator

I like this idea. The Travis build, however, appears to be failing. As far as I can tell, it's because the order of the URL encoded parameters changes, since maps do not guarantee key order.

This raises an interesting question; is the order of URL parameters significant? It shouldn't be, I think, but one hair-pulling experience I had with the PayPal API suggests otherwise.

@sbelak

As far as I can tell, the standard is mute on the issue. Furthermore I don't think supporting custom orderings is too fringe and not worth the extra complexity (besides the status-quo is no better in this regard).

I have however updated the tests so that Travis doesn't complain.

@stig

@asmala the order of URL parameters is significant for caching.

@asmala
Collaborator

@stig, that's a good point. It's certainly a valid reason for guaranteeing consistent ordering of URL parameters. That being said, the current implementation (no nested parameters) also does not guarantee consistency when using a map. One option might be to allow a nested vector of key-value pairs to be used in cases where ordering matters. Something like:

; Non-nested maps, does not guarantee order
(url-encode {:a "1" :b "2"})

; Non-nested vectors, guarantees order
(url-encode [[:a "1"] [:b "2"]])

; Nested maps, does not guarantee order
(url-encode {:a {:b "2" :c "3"}})

; Nested vectors, guarantees order
(url-encode [[:a [[:b "2"] [:c "3"]]]])
@lsb

+1

Also, I'd like to note that the current implementation does indeed guarantee consistency; passing a sorted-map in will iterate in the proper order, and sorted maps are a map?.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment