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

decodeQuery does not work as described in docs.html #137

Closed
hankhero opened this issue Mar 5, 2014 · 1 comment
Closed

decodeQuery does not work as described in docs.html #137

hankhero opened this issue Mar 5, 2014 · 1 comment
Labels

Comments

@hankhero
Copy link

hankhero commented Mar 5, 2014

If you run the examples in the docs, docs.html#static-decodeQuery, they don't work as explained. Here are two, currently failing, tests I made based on docs.html that shows it. Also, the functions in this section are lacking unit-tests. I volunteer to help code and fix, but then I need some help explain how the functions are specified to work. Is the docs or the code faulty?

test("encodeQuery", function () {
    equal(URI.encodeQuery(" "), "+");
    equal(URI.encode(" "), "%20");
});

test("decodeQuery", function () {
    equal(URI.decodeQuery("+"), " ");
    equal(URI.decode("+"), "+");
});
@rodneyrehm
Copy link
Member

This is an issue with the documentation and the result of a bad API decision.

Version 1.11.0 introduced URI.escapeQuerySpace so one could configure if a querystring should use + or %20 for encoding spaces.

in Order to get the the result you're looking for, you'd have to call URI.encodeQuery(" ", true). From an API point of view this doesn't make any sense. The additional parameter was meant for internal use. It should be optional when used externally. In order to make this work, I would change

URI.encodeQuery = function(string, escapeQuerySpace) {
    var escaped = URI.encode(string + "");
    return escapeQuerySpace ? escaped.replace(/%20/g, '+') : escaped;
};

to

URI.encodeQuery = function(string, escapeQuerySpace) {
    var escaped = URI.encode(string + "");
    if (escapeQuerySpace === undefined) {
        escapeQuerySpace = URI.escapeQuerySpace;
    }
    return escapeQuerySpace ? escaped.replace(/%20/g, '+') : escaped;
};

Would that conform with your expectations?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants