search

Pylons / pyramid_handlers

Pyramid "handlers" emulate Pylons 1 "controllers"
Other
8 stars 9 forks source link

added support for a default_action to config.add_handler #3

Open jvanasco opened 12 years ago

jvanasco commented 12 years ago

I added support for a "default_action" kwarg to add_handlers

This allows the following two config lines to be collapsed into one:

was: config.add_handler("test", "/test/", "app.handlers.test:Test", action="index") config.add_handler("test", "/test/{action}", "app.handlers.test:Test")

now: config.add_handler("test", "/test/{action}", "app.handlers.test:Test", default_action="index")

it does this by implicitly creating another route named "test-default_action" , removing the ":action" args from the pattern, and subbing in "default_action" for "action"

I wasn't sure how the inheritance worked, so I used "add_handler(self" instead of "self.add_handler(".

mmerickel commented 12 years ago

So how do I generate a url for this? The only way I see is request.route_url('route-default-action'), which will require documentation update, and special-casing code in views/templates the same as you do now. All in the name of saving 1 line of configuration code.

There is also a bug in your patch for config.add_handler('test', '/test/{action}/') the resulting route will be /test// for test-default-action.

Anyway, I'm -1 on this feature, it was even removed from Pylons (routes minimization=False) because it's "magicky".

jvanasco commented 12 years ago

Calls intended for request.route_url('route-default_action') would almost always be made to an explicitly defined request.route_url('route')

I added a fix for the "//" issue, and also pushed the unit test which I forgot to commit.

This isn't a matter of saving 1 line of configuration code - this can save dozens of lines code , as this is a common pattern / need that is present in many frameworks.

The pylons implementation was indeed magicky in how someone would implement a route - however that was because it happened on a global scale and was altering the route to make something work by minimizing it. it was very easy to have unintended consequences.

This implementation is not magicky at all and is very straightforward - if you specify a default action for a route, it supports that default action to be used in place of an unspecified action. this behavior is essentially identical to specifying a "Directory Index" on a web/proxy/cache server, however it's within pyramid -- which means it is easier to test and more consolidated to maintain.

mmerickel commented 12 years ago

So what am I missing here? Your patch simply calls add_handler twice. This saves the user from having to call it twice, but that's basically it AFAICT. This does not address the url generation issue.

With or without your patch, the following code is not valid in Pyramid:

config.add_handler('route', '/test/{action}')

request.route_url('route')