weavejester / hiccup

Fast library for rendering HTML in Clojure
http://weavejester.github.io/hiccup
Eclipse Public License 1.0
2.68k stars 174 forks source link

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

Closed sbelak closed 2 years ago

sbelak commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

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 commented 9 years ago

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

asmala commented 9 years ago

@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 commented 9 years ago

+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?.

stig commented 7 years ago

This issue seems to not be going anywhere. Can it be closed? I'm asking because it keeps stealing attention every week when I'm going through the list of pull requests I own / am reviewers for / am involved with.