Skip to content
This repository has been archived by the owner on Sep 19, 2022. It is now read-only.

REST routing syntax #23

Closed
yaworsw opened this issue Feb 1, 2014 · 8 comments
Closed

REST routing syntax #23

yaworsw opened this issue Feb 1, 2014 · 8 comments

Comments

@yaworsw
Copy link
Contributor

yaworsw commented Feb 1, 2014

This is a continuation of #19 and #17.

To sum up #19 and #17...

PR #19 added a new syntax for routes which goes something like this:

$app->addRoutes(array(
    '/'               => 'Home:index',
    'get  /posts'     => 'Posts:index',
    'get  /posts/:id' => 'Posts:show',
    'post /posts'     => array('Posts:create', function() { echo 'Middleware!'; }),
));

The http method is defined in the same string as the route and they are separated by a space.

@falmp suggested the following alternative syntax:

$app->addRoutes(array(
    '/'         => 'Home:index'
    '/posts'    => array('get'  => 'Posts:index',
                         'post' => array('Posts:create', function() { echo 'Middleware!'; })),
    '/posts:id' => 'Posts:show'
));
@falmp
Copy link
Contributor

falmp commented Feb 10, 2014

Well, just to make it clear, I'm still not sure my suggestion is better or not (in all honesty, seeing the two options side by side on your post your option doesn't look bad), I just wanted to give an alternative since I'm not a huge fan of parsing patterns on configuration structures (regardless of its simplicity, it may induce user errors), and also maybe start a discussion. :)

@yaworsw
Copy link
Contributor Author

yaworsw commented Feb 10, 2014

I do really want to have a discussion, the only reason I made those changes without opening an issue or anything is because was I was using slimcontroller in a hackathon type situation and ran into trouble making RESTful routes so I had to do something quick.

Anyway I think your suggestion is more consistent with how routing is currently done. I also agree that having to parse a config array for patterns is a little clunky.

My current plan is to implement your suggestion, release it as another RC and sit on it for a bit, if nothing better comes along I think your routing suggestion will be the new routing syntax for 0.4.0. Idk when I will be able to find time to work on this but eventually I'll get to it. Feel free to send a PR <3

@quickliketurtle
Copy link
Contributor

Hello, Just found slimcontroller yesterday. I like @falmp 's syntax. Feels cleaner to write, also clear to read, especially with short array syntax.

@yaworsw
Copy link
Contributor Author

yaworsw commented Mar 30, 2014

Hello @quickliketurtle,

I actually agree. I also think it's more consistent with how it's been before.

Unfortunately I've been pretty busy with other things and just haven't found the time to code up @falmp's version. If you could submit a PR implementing @falmp's syntax I'd be very grateful.

Lmk if you're planning to work on it. If not I guess I should force myself to find some time to get this patch done since it's so very long overdue.

@quickliketurtle
Copy link
Contributor

Hi @yaworsw I'll give it a look in the next few days and see what i come up with.

@quickliketurtle
Copy link
Contributor

Hi Again -
I've got a working version. What branch do you want me to submit the PR to?
I made an additional change so the middleware calls are either a single callback or an array of callbacks. This is for both local and global middleware.

Thanks.

@yaworsw
Copy link
Contributor Author

yaworsw commented Apr 1, 2014

Awesome! thanks,

Can you send the PR to develop? I'll then merge it in to master and release it as 0.4.0.

@falmp
Copy link
Contributor

falmp commented Apr 4, 2014

Great work! Sorry about not getting back to this, it was sitting on my TODO list for some time but I've been kinda busy lately (trying to organize a wedding overseas for August can be time consuming ;) ).

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

No branches or pull requests

3 participants