Query url parameters should not escape + or :

[opalmer]

See #18 for details.

[opalmer]

@andygrunwald, @perolausson PTAL. So far as I can tell, this change should fix the issue brought up in #18.

I'm not sure this is the best approach but neither net/url or github.com/google/go-querystring/query appear to have a way of excluding some symbols from parsing and writing our own from scratch didn't seem ideal either.

[andygrunwald]

I am on a trip until 21st October. I try to have a look at it in between,
but I am not sure if I can afford this. The trip is quite busy and tough.

If you are sure that this will fix it and (even better) provide a unit /
regression test with it: Go for it.

@opalmer you know I trust you in this project :) so we are equal :)

Am Donnerstag, 13. Oktober 2016 schrieb opalmer :

@andygrunwald https://github.com/andygrunwald, @perolausson
https://github.com/perolausson PTAL. So far as I can tell, this change
should fix the issue discovered in #18
#18.

I'm not sure this is the best approach but neither net/url or
github.com/google/go-querystring/query appear to have a way of excluding
some symbols from parsing and writing our own from scratch didn't seem
ideal either.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#22 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AATiQOd0SDUhEZ69yy-1FvP8LpBv4o8gks5qziRCgaJpZM4KV0y2
.

[opalmer]

If you are sure that this will fix it and (even better) provide a unit / regression test with it: Go for it.

Already added a test in d703d14 for exactly this :). Prior to the fix this new unittest would have failed.

[coveralls]

Coverage increased (+0.7%) to 11.227% when pulling 33fa8c5 on fix_url_encoding into 2b0558c on master.

[opalmer]

@perolausson, have you had a chance to test this?

[perolausson]

No I'm sorry. Snowed under.

On 23 Oct 2016 16:17, "opalmer" [email protected] wrote:

@perolausson https://github.com/perolausson, have you had a chance to
test this?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#22 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AC2X-KkAwliK001joX8EiaygQeT25JhHks5q23qFgaJpZM4KV0y2
.

[opalmer]

@perolausson, any chance you could take a look at this sometime this week?

[opalmer]

I'm going to go ahead and merge this within the next couple of days if there are not any objections. The testing I've done seems to indicate this will fix the original problem without causing any new ones. If that turns out to not be the case it's pretty easy to either roll this back or apply a patch.

[andygrunwald]

Go ahead @opalmer.
Thx for your effort

[andygrunwald]

See #193 (comment)