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.

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

Parsing query string arrays #165

Merged
merged 2 commits into from Oct 13, 2017

Conversation

Projects
None yet
4 participants
@mhairi
Contributor

mhairi commented Sep 5, 2017

When a query string has multiple of the same element. For example ‘keyword=hello&keyword=goodbye’, these form an array and should probably be parsed in R as a vector e.g. keyword = c(‘hello’, ‘goodbye’).

Currently having multiple elements of the same name in a query string just raises an error. The pull request aims to fix this and has added a test for it.

mhairi added some commits Sep 5, 2017

Change parseQS
Change parseQS so that if elements of the parsed query string are the
same, they are combined into a vector.
@CLAassistant

This comment has been minimized.

Show comment
Hide comment
@CLAassistant

CLAassistant Sep 5, 2017

CLA assistant check
All committers have signed the CLA.

CLAassistant commented Sep 5, 2017

CLA assistant check
All committers have signed the CLA.

@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Sep 5, 2017

Codecov Report

Merging #165 into master will increase coverage by 0.06%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #165      +/-   ##
==========================================
+ Coverage   88.08%   88.14%   +0.06%     
==========================================
  Files          23       23              
  Lines        1116     1122       +6     
==========================================
+ Hits          983      989       +6     
  Misses        133      133
Impacted Files Coverage Δ
R/query-string.R 94.36% <100%> (+0.52%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0577287...6b5e802. Read the comment docs.

codecov-io commented Sep 5, 2017

Codecov Report

Merging #165 into master will increase coverage by 0.06%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #165      +/-   ##
==========================================
+ Coverage   88.08%   88.14%   +0.06%     
==========================================
  Files          23       23              
  Lines        1116     1122       +6     
==========================================
+ Hits          983      989       +6     
  Misses        133      133
Impacted Files Coverage Δ
R/query-string.R 94.36% <100%> (+0.52%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0577287...6b5e802. Read the comment docs.

@trestletech trestletech merged commit 8fce7d3 into trestletech:master Oct 13, 2017

4 checks passed

codecov/patch 100% of diff hit (target 88.08%)
Details
codecov/project 88.14% (+0.06%) compared to 0577287
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details
@trestletech

This comment has been minimized.

Show comment
Hide comment
@trestletech

trestletech Oct 13, 2017

Owner

Awesome! Thanks so much for contributing, and sorry for the slow response.

Owner

trestletech commented Oct 13, 2017

Awesome! Thanks so much for contributing, and sorry for the slow response.

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