-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update generators with new Gettext API #5902
Update generators with new Gettext API #5902
Conversation
layouts: [html: <%= @web_namespace %>.Layouts] | ||
<%= if @gettext do %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new lines are tricky to get right, but I am almost sure this is what is needed. You have to replicate this below.
layouts: [html: <%= @web_namespace %>.Layouts] | |
<%= if @gettext do %> | |
layouts: [html: <%= @web_namespace %>.Layouts]<%= if @gettext do %> | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With --gettext
:
quote do
# Translation
use Gettext, backend: GtestWeb.Gettext
# HTML escaping functionality
import Phoenix.HTML
# Core UI components
import GtestWeb.CoreComponents
# Shortcut for generating JS commands
alias Phoenix.LiveView.JS
# Routes generation with the ~p sigil
unquote(verified_routes())
end
With --no-gettext
:
quote do
# HTML escaping functionality
import Phoenix.HTML
# Core UI components
import GtestWeb.CoreComponents
# Shortcut for generating JS commands
alias Phoenix.LiveView.JS
# Routes generation with the ~p sigil
unquote(verified_routes())
end
I think we're good? I would love to keep the use
above import
as it seems like it's a pretty common ordering. 🙃
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(this is with changes I’m about to push to be clear!)
installer/templates/phx_umbrella/apps/app_name_web/lib/app_name.ex
Outdated
Show resolved
Hide resolved
installer/templates/phx_umbrella/apps/app_name_web/lib/app_name.ex
Outdated
Show resolved
Hide resolved
@@ -14,10 +14,10 @@ defmodule <%= @web_namespace %>.CoreComponents do | |||
|
|||
Icons are provided by [heroicons](https://heroicons.com). See `icon/1` for usage. | |||
""" | |||
use Phoenix.Component | |||
use Phoenix.Component<%= if @gettext do %> | |||
use Gettext, backend: <%= @web_namespace %>.Gettext<% end %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is perfect!
@@ -14,10 +14,10 @@ defmodule <%= @web_namespace %>.CoreComponents do | |||
|
|||
Icons are provided by [heroicons](https://heroicons.com). See `icon/1` for usage. | |||
""" | |||
use Phoenix.Component | |||
use Phoenix.Component<%= if @gettext do %> | |||
use Gettext, backend: <%= @web_namespace %>.Gettext<% end %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is perfect!
In the meantime, tested again with the latest Gettext changes and it looks great:
|
@josevalim this is ready for another review. I posted some comments with results of the formatting and I think it looks good! |
@whatyouhide some integration tests are failing because the generated file is not formatted. :) I will try to drop some comments but it seems to be trailing whitespace. |
@josevalim let me run those locally and I'll figure it out |
quote do<%= if @gettext do %> | ||
# Translation | ||
use Gettext, backend: <%= @web_namespace %>.Gettext | ||
<% end %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will leave trailing whitespace. I think the <%= end %>
needs to be at the end of the previous line.
import <%= @web_namespace %>.Gettext<% end %> | ||
<%= if @gettext do %> | ||
use Gettext, backend: <%= @web_namespace %>.Gettext | ||
<% end %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will leave trailing whitespace before and after. I think the if
and <%= end %>
needs to be at the end of the previous line.
quote do<%= if @gettext do %> | ||
# Translation | ||
use Gettext, backend: <%= @web_namespace %>.Gettext | ||
<% end %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will leave trailing whitespace. I think the <%= end %>
needs to be at the end of the previous line.
|
||
import Plug.Conn<%= if @gettext do %> | ||
import <%= @web_namespace %>.Gettext<% end %> | ||
<%= if @gettext do %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will leave trailing whitespace. I think the <%= end %>
needs to be at the end of the previous line.
This will be a massive improvement to Phoenix apps. Thank you @whatyouhide and @maennchen! 💚 💙 💜 💛 ❤️ |
See:
cc @josevalim @maennchen