parse-params public #49

Merged
merged 4 commits into from Mar 22, 2012

Projects

None yet

2 participants

@rjack

Hello,

I'd like to use parse-params in my code but it's private. This change makes it public, as it can be useful as generic query strring parser.

(for example I'd like to use it to capture query string parameters in hrefs when scraping web pages with enlive).

Maybe parse-params is private for a reason, in that case sorry for the noise.

Cheers,
Giacomo

@weavejester
Collaborator

It's probably a good idea to make this functionality public, but the ring.middleware.params namespace is not the right place for it, as parse-params is not middleware.

It would probably make more sense for it to be in a ring.util.* namespace, because it's a utility function. I'm thinking perhaps ring.util.codec. We could then have two functions, ring.util.codec/param-decode and ring.util.codec/param-encode. Or perhaps form-decode and form-encode.

@rjack

It would sound strange to me, as an user, to have a query string parsing function mixed with url encoding functions.

I tried moving assoc-params and parse-params to ring.util.params instead.

How does it look?

@weavejester
Collaborator

Why would it seem odd to have functions for parsing form-urlencoded parameters in the same place as urlencoded strings?

@rjack

Having them in the same place looks good, but wouldn't be misleading for someone looking for a parser to have to find it inside a file named codec?

@weavejester
Collaborator

Don't get too hung up on the word "parse" here. The act of converting a map of parameters into a query string can be considered to be a form of encoding - indeed, the content type is literally "application/x-www-form-urlencoded".

So the most appropriate location would probably be ring.util.codec/form-decode, with an appropriate docstring explaining that this decodes HTML form data into a map. We should also write a ring.util.codec/form-encode function that does the opposite, i.e. convert a map into form-urlencoded data.

@rjack

Hello,

I implemented form-encode and form-decode plus a simple test.

Cheers,
Giacomo

@weavejester
Collaborator

Sorry for taking a while to look at this.

The assoc-param function doesn't quite seem appropriate for the codec namespace. Let me think a little about this before I merge.

Usually I'm not this particular about moving functions around, but these functions seem really hard to place in an appropriate namespace. Please bear with me :)

@weavejester
Collaborator

I'm going to merge this in so I can experiment with it a little.

@weavejester weavejester merged commit 84446f6 into mmcgrana:master Mar 22, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment