Skip to content

bug plugin name in url are not underscore#757

Closed
bronze1man wants to merge 2 commits intocakephp:masterfrom
bronze1man:master
Closed

bug plugin name in url are not underscore#757
bronze1man wants to merge 2 commits intocakephp:masterfrom
bronze1man:master

Conversation

@bronze1man
Copy link
Copy Markdown

router::url(array('plugin'=>'FileUpload','controller'=>'FileUpload','action'=>'upload',)) return
/FileUpload/FileUpload/upload ,then cakephp will lookfor FileUpload controller FileUpload action

@ADmad
Copy link
Copy Markdown
Member

ADmad commented Aug 7, 2012

The value for plugin key in url needs to be underscored. If you pass it camel cased value its your mistake. The framework shouldn't try to fix all developer errors imo.

@ADmad
Copy link
Copy Markdown
Member

ADmad commented Aug 7, 2012

Also please don't include debugging comments/description in code patches. Your pull request cannot be merged in that case.

@markstory
Copy link
Copy Markdown
Member

Additionally the controller + action names are not automatically underscored. This would make plugin a special case, which I don't want.

@markstory markstory closed this Aug 7, 2012
@jpeltoniemi
Copy link
Copy Markdown

In my opinion controller and plugin names should be accepted as CamelCase and underscore. Two reasons why I think this:

  • Controller and Plugin names are in CamelCase. Only urls need them underscored. I'm not sure but I think I haven't had to use underscored controller names anywhere else.
  • CakePHP did at least before many things like underscore conversions "automagically". One might easily assume that case conversion for urls falls under this automagic stuff.

I stumbled across this pull request while searching for solutions for this situation. Normally I would've let it be and just typed the names underscored, but this time I'm testing a 3rd party plugin that's called JSON_REST_Tools(working title), which is kind of horrible to write in underscore(j_s_o_n__r_e_s_t__tools). I think it would be neat if we could tap into the url forming and parsing methods and add our own underscore conversions in a way we see best. I for one wouldn't mind if there was an option to have all urls CamelCased.

EDIT:
I'd also like to point out that HtmlHelper::script('PluginName.script') correctly underscores the plugin name. I do understand though that the context is a bit different. Still, if one function takes a CamelCased plugin name and outputs a correctly formatted url, it's easy to assume that other functions do that too and then get confused when the url doesn't work. That is what happened to me :)

@markstory
Copy link
Copy Markdown
Member

You can provide custom inflections. Just create a subclass of CakeRoute and generate URL's as you need within the routes.

@jpeltoniemi
Copy link
Copy Markdown

I'll look into that. Thanks :)

@xavier83ar
Copy link
Copy Markdown

I think this should be treated, because as it works right now is not coherent.
First of all, why if I put a controller name camel cased it works, but with a plugin name it doesn't?, why a plugin name is treated different?
So, I think that if urls MUST be underscored, those that are camel cased SHOULD not work at all. So this way is less confusing.

Also, thinking about how url works when used as an array, you put 'plugin' as key and the plugin NAME as value, the same with controllers, 'controller' key holds the controller NAME and so for actions. So, if the controller name is MyControllerName, why should I put my_controller_name when using it in urls? This is a bit confunsing, if I want to create a route to MyControllerName, and routing MUST be underscored, I don't see the reason why can't Router class handle this conversion.

My proposal is that Router should convert pluing and controller names automatically to its underscored forms. Also, camel cased controller names in urls should not be supported, to avoid confusing.

Router::url(array(
'plugin' => 'MyPluginName',
'controller' => 'MyControllerName',
'action' => 'SomeAction'
));
// should return
// /my_plugin_name/my_controller_name/some_action

@markstory
Copy link
Copy Markdown
Member

One reason we have been avoiding adding more automagic is its impact on runtime performance. CakePHP has not historically been at the top of the benchmark charts. One large contributing factor to this has been automagic and trying to infer what the developer meant. Removing that inference allows the framework to perform better and allow us to field fewer questions when the inference goes wrong.

@dereuromark
Copy link
Copy Markdown
Member

Allowing access both ways needs to be manually handled by the dev right now to avoid multiple ways of accessing the url and/or SEO duplicate content. Must are probably not aware of that.

Accessing those urls should all be via underscore only IMO. And if you try to access it differently, it should 404. At least in future versions of Cake (3.x then for example):

/my_plugin_name/my_controller_name/some_action => 200 (OK)
/my_plugin_name/MyControllerName/some_action => 404 (NOT FOUND)
/MyPluginName/my_controller_name/some_action => 404 (NOT FOUND)
/my_plugin_name/my_controller_name/SomeAction => 404 (NOT FOUND) if action is some_action

This would unify the behavior in a clean way and would give feedback to the dev right away (wrong link/router calls).

I agree that it should not automatically inflect as it would decrease performance.

@markstory
Copy link
Copy Markdown
Member

@dereuromark Agreed It would be great yo see that kind of change in Router for 3.0. I think a big part of the current problem is that things 'half' work right now.

@xavier83ar
Copy link
Copy Markdown

Ok, yep, I did not consider performance issues. I agree with @dereuromark, not to inflect automatically, but yes to have a unified behavior, this would avoid confusion besides to improve routing from a SEO point of view.

This is off topic, but as markstory mentioned Router for 3.0, something that would be awesome to add to Router is the ability to define some kind of alias or unique identifier when you define a route, so then you can use it only for its identifier (and parameters) instead of the array (controller, action, etc). This could be great for developers (speed up coding and better control on routing), but also I think that could improve performace, avoiding the Framework having to detect the matched route and just getting it by its identifier.

Btw, sorry for my english.

@lorenzo
Copy link
Copy Markdown
Member

lorenzo commented Aug 12, 2013

Named (or aliased) route was already implemented in 3.0 @xavier83ar

On Mon, Aug 12, 2013 at 2:09 PM, xavier83ar notifications@github.comwrote:

Ok, yep, I did not consider performance issues. I agree with @dereuromarkhttps://github.com/dereuromark,
not to inflect automatically, but yes to have a unified behavior, this
would avoid confusion besides to improve routing from a SEO point of view.

This is off topic, but as markstory mentioned Router for 3.0, something
that would be awesome to add to Router component is the ability to define
some kind of alias or unique identifier when you define a route, so then
you can use it only for its identifier (and parameters) instead of the all
array (controller, action, etc). This could be great for developer (speed
up coding and better control on routing), but also I think that could
improve performace, avoiding the Framework having to detect the matched
route and just getting it by its identifier.

Btw, sorry for my english.


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

@xavier83ar
Copy link
Copy Markdown

Cool!, I've not had time to check 3.0 yet. I'll give it a time soon to see what's new. Great work guys!.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants