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

decode embedded paramaters of RouteTokenFormatter #499

Merged
merged 2 commits into from May 28, 2014

Conversation

Projects
None yet
5 participants
@rdwallis
Contributor

rdwallis commented May 28, 2014

So if you have nameToken = "/myplace/{myparam}/"

and then you set myparam = "Two Words"

Route Token formatter produces the url = "/myplace/Two+Words/"

But then if you request the parameters from that url you get back "Two+Words"

So you've put in "Two Words" and you get back "Two+Words"

This fixes that issue.

@jDramaix

This comment has been minimized.

Show comment
Hide comment
@jDramaix

jDramaix May 28, 2014

Contributor

LGTM. Thanks for this contribution

Contributor

jDramaix commented May 28, 2014

LGTM. Thanks for this contribution

@olafleur

This comment has been minimized.

Show comment
Hide comment
@olafleur

olafleur May 28, 2014

Member

LGTM. @Chris-V ?

Member

olafleur commented May 28, 2014

LGTM. @Chris-V ?

@christiangoudreau

This comment has been minimized.

Show comment
Hide comment
@christiangoudreau

christiangoudreau May 28, 2014

Member

Could you add a unit test for this before we merge it? I do not want this to be broken in the future.

Thanks,

Member

christiangoudreau commented May 28, 2014

Could you add a unit test for this before we merge it? I do not want this to be broken in the future.

Thanks,

@rdwallis

This comment has been minimized.

Show comment
Hide comment
@rdwallis

rdwallis May 28, 2014

Contributor

ok will do

On 28 May 2014 22:17, christiangoudreau notifications@github.com wrote:

Could you add a unit test for this before we merge it? I do not want this
to be broken in the future.

Thanks,


Reply to this email directly or view it on GitHubhttps://github.com//pull/499#issuecomment-44458778
.

Contributor

rdwallis commented May 28, 2014

ok will do

On 28 May 2014 22:17, christiangoudreau notifications@github.com wrote:

Could you add a unit test for this before we merge it? I do not want this
to be broken in the future.

Thanks,


Reply to this email directly or view it on GitHubhttps://github.com//pull/499#issuecomment-44458778
.

@rdwallis

This comment has been minimized.

Show comment
Hide comment
@rdwallis

rdwallis May 28, 2014

Contributor

ok done, 10 minutes on the patch 40 minutes on code style, will your ide's break if there's trailing whitespace?

Contributor

rdwallis commented May 28, 2014

ok done, 10 minutes on the patch 40 minutes on code style, will your ide's break if there's trailing whitespace?

@olafleur

This comment has been minimized.

Show comment
Hide comment
@olafleur

olafleur May 28, 2014

Member

All the tests seem to pass now.

Member

olafleur commented May 28, 2014

All the tests seem to pass now.

@christiangoudreau

This comment has been minimized.

Show comment
Hide comment
@christiangoudreau

christiangoudreau May 28, 2014

Member

Nope, Build is fine, it will be perfect.

LGTM

Member

christiangoudreau commented May 28, 2014

Nope, Build is fine, it will be perfect.

LGTM

@Chris-V

This comment has been minimized.

Show comment
Hide comment
@Chris-V

Chris-V May 28, 2014

Member

Thank you!

Member

Chris-V commented May 28, 2014

Thank you!

Chris-V added a commit that referenced this pull request May 28, 2014

Merge pull request #499 from rdwallis/rtffix
decode embedded paramaters of RouteTokenFormatter

@Chris-V Chris-V merged commit 7a241b3 into ArcBees:master May 28, 2014

1 check passed

default TeamCity Build GWT-Platform (GWTP) :: Monitor Pulls finished: Tests passed: 108
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment