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

[WIP] Add caseSensitive option for query param keys matching #63

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

faridnsh
Copy link

@faridnsh faridnsh commented Feb 7, 2017

Fixes #57.

This is basically what was requested in #57, it adds a setter, .caseSensitive. Have a look at the test case. Then when query param keys are being matched, it should take that into account.

Right now, only getQueryParamValue and getQueryParamValues are changed but I believed other places such as deleteQueryParam, hasQueryParam and replaceQueryParam should also be changed for consistent behavior.

@derek-watson Tell me what you think about this, if you think this method is OK, I'm going to complete this.

@PandaWood
Copy link

Hey Alfred,

I just added a pull request as well - partly to show a different way.
I've implemented case insensitivity for everything, as you proposed (eg hasQueryParamValue, deleteParamValue)

To prevent oodles of toLowerCase() everywhere, I stored the key lower case). I still needed a few extra toLowerCase() calls though

All the existing tests pass and I added 4 or 5 that test for case insensitivity on all those methods.

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.

2 participants