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

exclude actual root from deparam #644

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
2 participants
@stevenvachon
Contributor

stevenvachon commented Jan 6, 2014

URLs with a root set to anything other than "/" results in an empty {} returned from can.route.deparam.

stevenvachon added some commits Jan 6, 2014

Update route.js
```javascript
can.route.bindings.pushstate.root = "/app/";
can.route("", {section:""});
can.route(":section/");
can.route.ready();
			
console.log( can.route.deparam(can.route.bindings.pushstate.root+"test") );
```
@stevenvachon

This comment has been minimized.

Show comment
Hide comment
@stevenvachon

stevenvachon Jan 9, 2014

Contributor

If we had:

can.route.bindings.pushstate.root = "/app/";
can.route(":section/");
can.route.deparam("test/");

Ideally, should deparam expect "test/" or "/app/test/"? I'm thinking "test/" so that root could change without affecting anything. In which case, this PR is wrong.

Contributor

stevenvachon commented Jan 9, 2014

If we had:

can.route.bindings.pushstate.root = "/app/";
can.route(":section/");
can.route.deparam("test/");

Ideally, should deparam expect "test/" or "/app/test/"? I'm thinking "test/" so that root could change without affecting anything. In which case, this PR is wrong.

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Jan 9, 2014

Contributor

@stevenvachon deparam should get the "matching part of the url" like:

https://github.com/bitovi/canjs/blob/master/route/route.js#L426

matchingPartOfTheUrl returns the part of the url will match routes like ":/section".

Contributor

justinbmeyer commented Jan 9, 2014

@stevenvachon deparam should get the "matching part of the url" like:

https://github.com/bitovi/canjs/blob/master/route/route.js#L426

matchingPartOfTheUrl returns the part of the url will match routes like ":/section".

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Jan 9, 2014

Contributor

So yeah, deparam expects: "test/" so closing ...

Contributor

justinbmeyer commented Jan 9, 2014

So yeah, deparam expects: "test/" so closing ...

@stevenvachon stevenvachon deleted the stevenvachon:patch-1 branch Jan 23, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment