Add querystring stringify, url{en|de}codecomponent #264

Merged
merged 3 commits into from Jul 27, 2014

Conversation

Projects
None yet
4 participants
Contributor

tim-smart commented Jun 28, 2012

Including tests. The *component methods probably need a bit more
love to make them more on parity with the javascript equivilents.

Signed-off-by: Tim Smart tim@fostle.com

Add querystring stringify, url{en|de}codecomponent
Including tests. The *component methods probably need a bit more
love to make them more on parity with the javascript equivilents.

Signed-off-by: Tim Smart <tim@fostle.com>
lib/luvit/querystring.lua
+ if 'table' == vtype then
+ for i, v in ipairs(val) do
+ count = count + 1
+ table.insert(ret, table.concat({skey, querystring.urlencodecomponent(v, sep, eq)}, eq))
@tim-smart

tim-smart Jun 28, 2012

Contributor

Eww, this line should be broken up to keep it under 80 characters wide.

@philips

philips Jun 28, 2012

Contributor

Wait, does that mean you are fixing it? You commented on your own pull request. :-)

@tim-smart

tim-smart Jun 28, 2012

Contributor

Yeah it was more of a note to self.

tim-smart added some commits Jun 28, 2012

Add order array option to querystring.stringify
This should allow people who need to make sure their keys are in
alphabetical order and such to do so.

Also a bit of DRY and make sure code is less than 80 characters wide.

Signed-off-by: Tim Smart <tim@fostle.com>
Tidy up querystring.stringify
Remove a log, and save a type() call.

Signed-off-by: Tim Smart <tim@fostle.com>
Contributor

tim-smart commented Jul 6, 2012

After using this for a while, I'm happy with it as is. Do as you want with it :)

Contributor

voronianski commented Jul 27, 2014

+1 this should be available by default inside luvit.. @creationix @rphillips why this isn't merged yet?

rphillips added a commit that referenced this pull request Jul 27, 2014

Merge pull request #264 from tim-smart/feature/querystring-stringify
Add querystring stringify, url{en|de}codecomponent

@rphillips rphillips merged commit fcf4479 into luvit:master Jul 27, 2014

Member

rphillips commented Jul 27, 2014

thanks... just didn't get to it... I could use the help reviewing the other PRs as well.

Contributor

voronianski commented Jul 27, 2014

👍

Contributor

tim-smart commented Jul 27, 2014

I had completely forgotten that I wrote this. From what I remember it was a port
of the node.js code.

If I have time to fix this, I will; but at this stage I have no plans. I would
encourage someone else to fix it up.

On Sun, Jul 27, 2014 at 01:52:54PM -0700, Dmitri Voronianski wrote:

@tim-smart your test fails here - https://github.com/luvit/luvit/pull/264/files#diff-f659ae8f383ef743e8aad1100c95a2bbR52


Reply to this email directly or view it on GitHub:
#264 (comment)

rphillips pushed a commit that referenced this pull request Jul 27, 2014

Revert "Merge pull request #264 from tim-smart/feature/querystring-st…
…ringify"

This reverts commit fcf4479, reversing
changes made to eeb845a.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment