Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

querystring: Parse multiple separator characters #3807

Closed
wants to merge 1 commit into from
Closed

querystring: Parse multiple separator characters #3807

wants to merge 1 commit into from

Conversation

yosuke-furukawa
Copy link
Member

When I explained querystring API for node beginners, I found a weird problem.

var q = qs.stringify({foo: 'bar', bar: 'baz'}, '&&', '=>');
console.log(q); // foo=>bar&&bar=>baz

var parsedQs = qs.parse('foo=>bar&&bar=>baz', '&&', '=>');
console.log(parsedQs); // { foo: '>bar', bar: '>baz' }

I expect {foo: 'bar', bar: 'baz'} but the actual result is {foo: '>bar', bar: '>baz'}.

This PR solves the problem.

cjihrig
cjihrig reviewed Nov 13, 2015
View reviewed changes
test/parallel/test-querystring-multichar-separator.js
@@ -0,0 +1,23 @@
'use strict';
const assert = require('assert');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please require('../common');

@cjihrig
Copy link
Contributor

cjihrig commented Nov 13, 2015

LGTM with a few small comments.

@mscdex mscdex added the querystring Issues and PRs related to the built-in querystring module. label Nov 13, 2015
@mscdex
Copy link
Contributor

mscdex commented Nov 13, 2015

LGTM

1 similar comment
@jasnell
Copy link
Member

jasnell commented Nov 13, 2015

LGTM

@yosuke-furukawa
Copy link
Member Author

Thank you @cjihrig. I fixed your comment.
And thank you @mscdex and @jasnell .

yosuke-furukawa added a commit that referenced this pull request Nov 13, 2015
@yosuke-furukawa @jasnell
querystring: Parse multiple separator characters
a776a86 
Fix querystring.parse to handle multiple separator characters

PR-URL: #3807
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell
Copy link
Member

jasnell commented Nov 13, 2015

Landed in a776a86

@jasnell jasnell closed this Nov 13, 2015
yosuke-furukawa added a commit that referenced this pull request Nov 17, 2015
@yosuke-furukawa @Fishrock123
querystring: Parse multiple separator characters
fd3f0d8 
Fix querystring.parse to handle multiple separator characters

PR-URL: #3807
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
yosuke-furukawa added a commit that referenced this pull request Nov 30, 2015
@yosuke-furukawa
querystring: Parse multiple separator characters
4ca83c2 
Fix querystring.parse to handle multiple separator characters

PR-URL: #3807
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
yosuke-furukawa added a commit that referenced this pull request Dec 4, 2015
@yosuke-furukawa @rvagg
querystring: Parse multiple separator characters
fe9baa7 
Fix querystring.parse to handle multiple separator characters

PR-URL: #3807
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell jasnell mentioned this pull request Dec 17, 2015
Closed
yosuke-furukawa added a commit that referenced this pull request Dec 17, 2015
@yosuke-furukawa @jasnell
querystring: Parse multiple separator characters
803a56d 
Fix querystring.parse to handle multiple separator characters

PR-URL: #3807
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
yosuke-furukawa added a commit that referenced this pull request Dec 23, 2015
@yosuke-furukawa @jasnell
querystring: Parse multiple separator characters
8d03ec9 
Fix querystring.parse to handle multiple separator characters

PR-URL: #3807
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@targos targos deleted the yosuke-patch-1 branch April 27, 2016 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
querystring Issues and PRs related to the built-in querystring module.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@yosuke-furukawa @cjihrig @mscdex @jasnell @MylesBorins