nathggns / Scaffold

Lightweight PHP API Framework
Other
8 stars 2 forks source link

Router should match large to small in terms of segments. #48

Closed nathggns closed 11 years ago

nathggns commented 11 years ago

Currently the router uses the first matched route, which can create issues with something like this.

<?php
$router->all('/:controller/:id');
$router->all('/download/:name/:version', 'download', ['version' => 1]);

If you pass a URL such as /download/project/1 to that, it will match the second route, which is fine, but if you pass '/download/project' to it, it will match the first route, which is problematic if you expect $request->params['version'] to be set.

nathggns commented 11 years ago

Maybe you should have to indicate optional parts?

<?php
$router->all('/download/:name/:?version', 'download', ['version' => 1]);
nathggns commented 11 years ago

This will add 'optional segments' 82fd1a50e353cf3ece11428e544f9d0e465df77b

ClaudioAlbertin commented 11 years ago

I think it reverses the routes array at some time, so in fact it uses the last matching route.

Am 03.01.2013 um 03:23 schrieb Nathaniel Higgins notifications@github.com:

Currently the router uses the first matched route, which can create issues with something like this.

<?php $router->all('/:controller/:id'); $router->all('/download/:name/:version', 'download', ['version' => 1]); If you pass a URL such as /download/project/1 to that, it will match the second route, which is fine, but if you pass '/download/project' to it, it will match the first route, which is problematic if you expect $request->params['version'] to be set.

— Reply to this email directly or view it on GitHub.

ClaudioAlbertin commented 11 years ago

Optional segements yet have to be implemented.

Am 03.01.2013 um 03:31 schrieb Nathaniel Higgins notifications@github.com:

Maybe you should have to indicate optional parts?

<?php $router->all('/download/:name/:?version', 'download', ['version' => 1]); — Reply to this email directly or view it on GitHub.

nathggns commented 11 years ago

They have been implemented in commit 82fd1a5

ClaudioAlbertin commented 11 years ago

Yay!

Am 03.01.2013 um 07:30 schrieb Nathaniel Higgins notifications@github.com:

They have been implemented in commit 82fd1a5

— Reply to this email directly or view it on GitHub.