-
Notifications
You must be signed in to change notification settings - Fork 476
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
removeQuery accepts RegExp that filters the keys #204
Conversation
is |
Yes, it is already supported by hasQuery, and I don't think it makes sense in add or set |
I see Should both methods accept RegExps for both name and value? |
I don't think so. When I look at changing hasQuery to support that, I don't know how it should behave. If the regexp matches multiple keys then should it test all the values? To make it support a regexp for the key parameter and also support all the other ways you can use it would be quite complex and I can't think of any use cases for it, so it would just be extra code for no reason. If you look at the way removeQuery works when you give it a single string, the way I have done it fits in nicely with that ie it filters the keys, not the values. I don't think it makes the overall API inconsistent. |
The one method accepts RegExp for keys, the other method accepts RegExp for values. How is that not inconsistent?
neither can I for the |
What I mean is that in hasQuery, the first parameter is the key selector, the second is the value selector, so if the first argument is always a key selector in the methods, the API is consistent. I can appreciate that someone may want to filter out an option in the query if a value matches a regexp. That would be an additional way of using removeQuery where the value regexp is passed as the second argument, rather than a replacement for this. I can submit that in another pr if it is also useful? |
yes, there was a time when I did this regularly.
You could also just add it do this PR, after all it's just extending the RegExp support you introduced here, no? |
Ok, I am away until Wednesday so I can't add that until then. |
I merged what you had and added the missing bit myself. thanks for your support! |
I added RegExp support to the removeQuery function. It is a bit like passing a string, except if uses the regexp to filter out keys that match the expression. I added a test and updated the docs too.