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.

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

mhairi
Copy link
Contributor

@mhairi 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 2 commits Sep 5, 2017
Change parseQS so that if elements of the parsed query string are the
same, they are combined into a vector.
@CLAassistant
Copy link

CLAassistant commented Sep 5, 2017

CLA assistant check
All committers have signed the CLA.

@codecov-io
Copy link

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 rstudio:master Oct 13, 2017
4 checks passed
@trestletech
Copy link
Contributor

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants