Add #controller_name method to Phoenix.Controller #1016
Conversation
|
I think we have a function in Phoenix.Naming that does exactly what you want: |
| @@ -110,6 +110,19 @@ defmodule Phoenix.Controller do | |||
| def action_name(conn), do: conn.private.phoenix_action | |||
|
|
|||
| @doc """ | |||
| Returns the controller name as a string, raises if unavailable. | |||
| """ | |||
| @spec controller_name(Plug.Conn.t) :: atom | |||
There was a problem hiding this comment.
The spec says it returns an atom but it actually returns a string.
|
Thanks, I have added some comments. It feels like this would be a good addition but I will leave it up for @chrismccord to decide. |
acconrad
force-pushed
the
controller-fn-name
branch
from
705fbab
to
0f1762b
Compare
|
Thank you @josevalim ! I updated the code based on your comments to include the correct datatype returned. |
|
Thanks @acconrad ! I think this is a welcome addition, but we need to extend it a bit first. For example, it fails for multi-world controllers, i.e.: iex(2)> Phoenix.Controller.controller_name(%{private: %{
phoenix_controller: Foo.AdminBarController
}})
"adminbar"We would need to dasherize or similar, but I don't want the code to get too complex. We also need to discuss if it should include all "namespaces" back to the root namespace, so MyApp.Admin.BarController would return |
| Returns the controller name as a string, raises if unavailable. | ||
| """ | ||
| @spec controller_name(Plug.Conn.t) :: String.t | ||
| def controller_name(conn) do |
There was a problem hiding this comment.
The implementation of this function should be just:
Phoenix.Naming.resource_name(conn.private.private_controller, "Controller")
|
@chrismccord the issues you have mentioned are solved by using About namespacing, that is a bit trickier because it would require storing the namespace in the connection too. |
|
@josevalim @chrismccord I've taken your suggestions and updated the code accordingly - this function now supports multi-word controllers. For the sake of CSS conventions, I did a simple |
|
@chrismccord I don't think we should dasherize. If we want to dasherize, we should move this to Phoenix.HTML as |
|
Now that I think about it, dasherize is only for a css specific use-case, and not something we should leak to this function |
|
@josevalim beats me at the internet again, by 3 seconds, 👍 |
|
good points re: dasherize. perhaps as a compromise, have the |
|
@josevalim @chrismccord I've reverted the dasherization pipeline and just left it as is. I still think this function would be useful on the backend, but I think it would be worth extending on I just poured through the |
|
@acconrad that's a very good point. At best, we could keep controller_name here and Phoenix.HTML would only dasherize it. |
|
@josevalim relating to my previous comment, how exactly could |
|
@chrismccord just wanted to check back in here - been a few weeks and wanted to see what else I needed to do before we can merge this in |
This is still on my list to think about and review, so hang tight! |
|
@acconrad Thanks very much for this contribution, but I'm going to wait on this until after 1.0. For now, users can provide their own helper easily by using the |
Added a
#controller_namefunction intoPhoenix.Controller.Input:
Plug.Conn.tconnectionOutput: lowercase name of the controller.
Example:
where
@connwas generated by thePageController.Why?
When dealing with CSS styling, it's useful to be able to separate out styles by page. The best way to get a page is the combination of it's controller and action. So for example, I can specifically style the homepage if I have
.page.indexas the classes to the<body>tag of a page. It also helps for modular loading of JS as well as CSS styles. Coming from Rails, it was great to have access tocontroller_nameandaction_name, and I found it strange that Phoenix only hadaction_namebut notcontroller_name, and I felt like this would be a good addition to the core functionality ofPhoenix.Controller.