From f8da08bd5318837079ccef5bf29773b49eeb650f Mon Sep 17 00:00:00 2001
From: Rodolfo Carvalho <rhcarvalho@gmail.com>
Date: Mon, 4 Dec 2023 15:52:32 +0100
Subject: [PATCH] Add missing gettext calls in core_components.ex

Depending on the context of the template file, we need to output code
with different syntaxes. To make maintenance easier and keep the
with/without Gettext strings consistent, we add a helper
`maybe_gettext/3` to contextually call gettext or skip it, while
outputting correct syntax. The helper is internal and is never exposed
to the generated code.

Both installer and Phoenix (through phx.gen.live) have a
core_components.ex template file, and those must stay in sync. We don't
want to create a dependency between Phoenix and the installer, so
`maybe_gettext/3` is implemented in both places and, similar to the
core_components.ex template itself, we ensure that `maybe_gettext/3`
implementations stay in sync with automated tests.
---
 installer/lib/phx_new/generator.ex            |  7 ++-
 installer/lib/phx_new/gettext_support.ex      | 44 +++++++++++++++++++
 .../phx_web/components/core_components.ex     | 19 ++++----
 .../test/phx_new/gettext_support_test.exs     | 21 +++++++++
 installer/test/phx_new_test.exs               | 13 ++++++
 lib/mix/phoenix.ex                            |  5 +++
 lib/mix/phoenix/gettext_support.ex            | 44 +++++++++++++++++++
 .../templates/phx.gen.live/core_components.ex | 19 ++++----
 test/mix/phoenix/gettext_support_test.exs     | 21 +++++++++
 9 files changed, 173 insertions(+), 20 deletions(-)
 create mode 100644 installer/lib/phx_new/gettext_support.ex
 create mode 100644 installer/test/phx_new/gettext_support_test.exs
 create mode 100644 lib/mix/phoenix/gettext_support.ex
 create mode 100644 test/mix/phoenix/gettext_support_test.exs

diff --git a/installer/lib/phx_new/generator.ex b/installer/lib/phx_new/generator.ex
index 1a44ffbe87..cf9ad70785 100644
--- a/installer/lib/phx_new/generator.ex
+++ b/installer/lib/phx_new/generator.ex
@@ -37,8 +37,11 @@ defmodule Phx.New.Generator do
               @external_resource unquote(path)
               @file unquote(path)
               def render(unquote(name), unquote(source), var!(assigns))
-                  when is_list(var!(assigns)),
-                  do: unquote(compiled)
+                  when is_list(var!(assigns)) do
+                var!(maybe_gettext) = &Phx.New.GettextSupport.maybe_gettext/3
+                _ = var!(maybe_gettext)
+                unquote(compiled)
+              end
             end
           else
             quote do
diff --git a/installer/lib/phx_new/gettext_support.ex b/installer/lib/phx_new/gettext_support.ex
new file mode 100644
index 0000000000..c16a43cfcf
--- /dev/null
+++ b/installer/lib/phx_new/gettext_support.ex
@@ -0,0 +1,44 @@
+defmodule Phx.New.GettextSupport do
+  @moduledoc false
+
+  @doc ~S"""
+  Translates a message using Gettext if `gettext?` is true.
+
+  The role provides context and determines which syntax should be used:
+
+    * `:heex_attr` - Used in a HEEx attribute value.
+    * `:eex` - Used in an EEx template.
+
+  ## Examples
+
+      iex> ~s|<tag attr=#{maybe_gettext("Hello", :heex_attr, true)} />|
+      ~S|<tag attr={gettext("Hello")} />|
+
+      iex> ~s|<tag attr=#{maybe_gettext("Hello", :heex_attr, false)} />|
+      ~S|<tag attr="Hello" />|
+
+      iex> ~s|<tag>#{maybe_gettext("Hello", :eex, true)}</tag>|
+      ~S|<tag><%= gettext("Hello") %></tag>|
+
+      iex> ~s|<tag>#{maybe_gettext("Hello", :eex, false)}</tag>|
+      ~S|<tag>Hello</tag>|
+  """
+  @spec maybe_gettext(binary(), :heex_attr | :eex | :ex, boolean()) :: binary()
+  def maybe_gettext(message, role, gettext?)
+
+  def maybe_gettext(message, :heex_attr, gettext?) do
+    if gettext? do
+      ~s|{gettext(#{inspect(message)})}|
+    else
+      inspect(message)
+    end
+  end
+
+  def maybe_gettext(message, :eex, gettext?) do
+    if gettext? do
+      ~s|<%= gettext(#{inspect(message)}) %>|
+    else
+      message
+    end
+  end
+end
diff --git a/installer/templates/phx_web/components/core_components.ex b/installer/templates/phx_web/components/core_components.ex
index 6d5841ea48..a9d6a38cc6 100644
--- a/installer/templates/phx_web/components/core_components.ex
+++ b/installer/templates/phx_web/components/core_components.ex
@@ -73,7 +73,7 @@ defmodule <%= @web_namespace %>.CoreComponents do
                   phx-click={JS.exec("data-cancel", to: "##{@id}")}
                   type="button"
                   class="-m-3 flex-none p-3 opacity-20 hover:opacity-40"
-                  aria-label=<%= if @gettext do %>{gettext("close")}<% else %>"close"<% end %>
+                  aria-label=<%= maybe_gettext.("close", :heex_attr, @gettext) %>
                 >
                   <.icon name="hero-x-mark-solid" class="h-5 w-5" />
                 </button>
@@ -127,7 +127,7 @@ defmodule <%= @web_namespace %>.CoreComponents do
         <%%= @title %>
       </p>
       <p class="mt-2 text-sm leading-5"><%%= msg %></p>
-      <button type="button" class="group absolute top-1 right-1 p-2" aria-label=<%= if @gettext do %>{gettext("close")}<% else %>"close"<% end %>>
+      <button type="button" class="group absolute top-1 right-1 p-2" aria-label=<%= maybe_gettext.("close", :heex_attr, @gettext) %>>
         <.icon name="hero-x-mark-solid" class="h-5 w-5 opacity-40 group-hover:opacity-70" />
       </button>
     </div>
@@ -147,28 +147,29 @@ defmodule <%= @web_namespace %>.CoreComponents do
   def flash_group(assigns) do
     ~H"""
     <div id={@id}>
-      <.flash kind={:info} title="Success!" flash={@flash} />
-      <.flash kind={:error} title="Error!" flash={@flash} />
+      <.flash kind={:info} title=<%= maybe_gettext.("Success!", :heex_attr, @gettext) %> flash={@flash} />
+      <.flash kind={:error} title=<%= maybe_gettext.("Error!", :heex_attr, @gettext) %> flash={@flash} />
       <.flash
         id="client-error"
         kind={:error}
-        title="We can't find the internet"
+        title=<%= maybe_gettext.("We can't find the internet", :heex_attr, @gettext) %>
         phx-disconnected={show(".phx-client-error #client-error")}
         phx-connected={hide("#client-error")}
         hidden
       >
-        Attempting to reconnect <.icon name="hero-arrow-path" class="ml-1 h-3 w-3 animate-spin" />
+        <%= maybe_gettext.("Attempting to reconnect", :eex, @gettext) %>
+        <.icon name="hero-arrow-path" class="ml-1 h-3 w-3 animate-spin" />
       </.flash>
 
       <.flash
         id="server-error"
         kind={:error}
-        title="Something went wrong!"
+        title=<%= maybe_gettext.("Something went wrong!", :heex_attr, @gettext) %>
         phx-disconnected={show(".phx-server-error #server-error")}
         phx-connected={hide("#server-error")}
         hidden
       >
-        Hang in there while we get back on track
+        <%= maybe_gettext.("Hang in there while we get back on track", :eex, @gettext) %>
         <.icon name="hero-arrow-path" class="ml-1 h-3 w-3 animate-spin" />
       </.flash>
     </div>
@@ -479,7 +480,7 @@ defmodule <%= @web_namespace %>.CoreComponents do
           <tr>
             <th :for={col <- @col} class="p-0 pb-4 pr-6 font-normal"><%%= col[:label] %></th>
             <th :if={@action != []} class="relative p-0 pb-4">
-              <span class="sr-only"><%= if @gettext do %><%%= gettext("Actions") %><% else %>Actions<% end %></span>
+              <span class="sr-only"><%= maybe_gettext.("Actions", :eex, @gettext) %></span>
             </th>
           </tr>
         </thead>
diff --git a/installer/test/phx_new/gettext_support_test.exs b/installer/test/phx_new/gettext_support_test.exs
new file mode 100644
index 0000000000..d023bf34b5
--- /dev/null
+++ b/installer/test/phx_new/gettext_support_test.exs
@@ -0,0 +1,21 @@
+defmodule Phx.New.GettextSupportTest do
+  use ExUnit.Case, async: true
+
+  doctest Phx.New.GettextSupport, import: true
+
+  test "gettext_support.ex in sync with phoenix" do
+    in_phoenix = read_split!("../lib/mix/phoenix/gettext_support.ex")
+    in_installer = read_split!("lib/phx_new/gettext_support.ex")
+
+    assert in_phoenix.first_line == "defmodule Mix.Phoenix.GettextSupport do"
+    assert in_installer.first_line == "defmodule Phx.New.GettextSupport do"
+    assert in_phoenix.rest == in_installer.rest
+    assert in_phoenix.rest =~ "gettext("
+  end
+
+  defp read_split!(path) do
+    File.read!(path)
+    |> String.split("\n", parts: 2)
+    |> then(fn [first_line, rest] -> %{first_line: first_line, rest: rest} end)
+  end
+end
diff --git a/installer/test/phx_new_test.exs b/installer/test/phx_new_test.exs
index ae1af81647..a09572c77f 100644
--- a/installer/test/phx_new_test.exs
+++ b/installer/test/phx_new_test.exs
@@ -110,6 +110,8 @@ defmodule Mix.Tasks.Phx.NewTest do
 
       assert_file("phx_blog/lib/phx_blog_web/components/core_components.ex", fn file ->
         assert file =~ "defmodule PhxBlogWeb.CoreComponents"
+        assert file =~ ~S|aria-label={gettext("close")}|
+        assert file =~ ~S|<.flash kind={:info} title={gettext("Success!")} flash={@flash} />|
       end)
 
       assert_file("phx_blog/lib/phx_blog_web/components/layouts.ex", fn file ->
@@ -550,6 +552,17 @@ defmodule Mix.Tasks.Phx.NewTest do
     end)
   end
 
+  test "new with --no-gettext" do
+    in_tmp("new with no_gettext", fn ->
+      Mix.Tasks.Phx.New.run([@app_name, "--no-gettext"])
+
+      assert_file("phx_blog/lib/phx_blog_web/components/core_components.ex", fn file ->
+        assert file =~ ~S|aria-label="close"|
+        assert file =~ ~S|<.flash kind={:info} title="Success!" flash={@flash} />|
+      end)
+    end)
+  end
+
   test "new with binary_id" do
     in_tmp("new with binary_id", fn ->
       Mix.Tasks.Phx.New.run([@app_name, "--binary-id"])
diff --git a/lib/mix/phoenix.ex b/lib/mix/phoenix.ex
index 39d3b000a6..ff5da58383 100644
--- a/lib/mix/phoenix.ex
+++ b/lib/mix/phoenix.ex
@@ -29,6 +29,11 @@ defmodule Mix.Phoenix do
   def copy_from(apps, source_dir, binding, mapping) when is_list(mapping) do
     roots = Enum.map(apps, &to_app_source(&1, source_dir))
 
+    binding =
+      Keyword.merge(binding,
+        maybe_gettext: &Mix.Phoenix.GettextSupport.maybe_gettext/3
+      )
+
     for {format, source_file_path, target} <- mapping do
       source =
         Enum.find_value(roots, fn root ->
diff --git a/lib/mix/phoenix/gettext_support.ex b/lib/mix/phoenix/gettext_support.ex
new file mode 100644
index 0000000000..058321af83
--- /dev/null
+++ b/lib/mix/phoenix/gettext_support.ex
@@ -0,0 +1,44 @@
+defmodule Mix.Phoenix.GettextSupport do
+  @moduledoc false
+
+  @doc ~S"""
+  Translates a message using Gettext if `gettext?` is true.
+
+  The role provides context and determines which syntax should be used:
+
+    * `:heex_attr` - Used in a HEEx attribute value.
+    * `:eex` - Used in an EEx template.
+
+  ## Examples
+
+      iex> ~s|<tag attr=#{maybe_gettext("Hello", :heex_attr, true)} />|
+      ~S|<tag attr={gettext("Hello")} />|
+
+      iex> ~s|<tag attr=#{maybe_gettext("Hello", :heex_attr, false)} />|
+      ~S|<tag attr="Hello" />|
+
+      iex> ~s|<tag>#{maybe_gettext("Hello", :eex, true)}</tag>|
+      ~S|<tag><%= gettext("Hello") %></tag>|
+
+      iex> ~s|<tag>#{maybe_gettext("Hello", :eex, false)}</tag>|
+      ~S|<tag>Hello</tag>|
+  """
+  @spec maybe_gettext(binary(), :heex_attr | :eex | :ex, boolean()) :: binary()
+  def maybe_gettext(message, role, gettext?)
+
+  def maybe_gettext(message, :heex_attr, gettext?) do
+    if gettext? do
+      ~s|{gettext(#{inspect(message)})}|
+    else
+      inspect(message)
+    end
+  end
+
+  def maybe_gettext(message, :eex, gettext?) do
+    if gettext? do
+      ~s|<%= gettext(#{inspect(message)}) %>|
+    else
+      message
+    end
+  end
+end
diff --git a/priv/templates/phx.gen.live/core_components.ex b/priv/templates/phx.gen.live/core_components.ex
index 6d5841ea48..a9d6a38cc6 100644
--- a/priv/templates/phx.gen.live/core_components.ex
+++ b/priv/templates/phx.gen.live/core_components.ex
@@ -73,7 +73,7 @@ defmodule <%= @web_namespace %>.CoreComponents do
                   phx-click={JS.exec("data-cancel", to: "##{@id}")}
                   type="button"
                   class="-m-3 flex-none p-3 opacity-20 hover:opacity-40"
-                  aria-label=<%= if @gettext do %>{gettext("close")}<% else %>"close"<% end %>
+                  aria-label=<%= maybe_gettext.("close", :heex_attr, @gettext) %>
                 >
                   <.icon name="hero-x-mark-solid" class="h-5 w-5" />
                 </button>
@@ -127,7 +127,7 @@ defmodule <%= @web_namespace %>.CoreComponents do
         <%%= @title %>
       </p>
       <p class="mt-2 text-sm leading-5"><%%= msg %></p>
-      <button type="button" class="group absolute top-1 right-1 p-2" aria-label=<%= if @gettext do %>{gettext("close")}<% else %>"close"<% end %>>
+      <button type="button" class="group absolute top-1 right-1 p-2" aria-label=<%= maybe_gettext.("close", :heex_attr, @gettext) %>>
         <.icon name="hero-x-mark-solid" class="h-5 w-5 opacity-40 group-hover:opacity-70" />
       </button>
     </div>
@@ -147,28 +147,29 @@ defmodule <%= @web_namespace %>.CoreComponents do
   def flash_group(assigns) do
     ~H"""
     <div id={@id}>
-      <.flash kind={:info} title="Success!" flash={@flash} />
-      <.flash kind={:error} title="Error!" flash={@flash} />
+      <.flash kind={:info} title=<%= maybe_gettext.("Success!", :heex_attr, @gettext) %> flash={@flash} />
+      <.flash kind={:error} title=<%= maybe_gettext.("Error!", :heex_attr, @gettext) %> flash={@flash} />
       <.flash
         id="client-error"
         kind={:error}
-        title="We can't find the internet"
+        title=<%= maybe_gettext.("We can't find the internet", :heex_attr, @gettext) %>
         phx-disconnected={show(".phx-client-error #client-error")}
         phx-connected={hide("#client-error")}
         hidden
       >
-        Attempting to reconnect <.icon name="hero-arrow-path" class="ml-1 h-3 w-3 animate-spin" />
+        <%= maybe_gettext.("Attempting to reconnect", :eex, @gettext) %>
+        <.icon name="hero-arrow-path" class="ml-1 h-3 w-3 animate-spin" />
       </.flash>
 
       <.flash
         id="server-error"
         kind={:error}
-        title="Something went wrong!"
+        title=<%= maybe_gettext.("Something went wrong!", :heex_attr, @gettext) %>
         phx-disconnected={show(".phx-server-error #server-error")}
         phx-connected={hide("#server-error")}
         hidden
       >
-        Hang in there while we get back on track
+        <%= maybe_gettext.("Hang in there while we get back on track", :eex, @gettext) %>
         <.icon name="hero-arrow-path" class="ml-1 h-3 w-3 animate-spin" />
       </.flash>
     </div>
@@ -479,7 +480,7 @@ defmodule <%= @web_namespace %>.CoreComponents do
           <tr>
             <th :for={col <- @col} class="p-0 pb-4 pr-6 font-normal"><%%= col[:label] %></th>
             <th :if={@action != []} class="relative p-0 pb-4">
-              <span class="sr-only"><%= if @gettext do %><%%= gettext("Actions") %><% else %>Actions<% end %></span>
+              <span class="sr-only"><%= maybe_gettext.("Actions", :eex, @gettext) %></span>
             </th>
           </tr>
         </thead>
diff --git a/test/mix/phoenix/gettext_support_test.exs b/test/mix/phoenix/gettext_support_test.exs
new file mode 100644
index 0000000000..6fe0ec8ef5
--- /dev/null
+++ b/test/mix/phoenix/gettext_support_test.exs
@@ -0,0 +1,21 @@
+defmodule Mix.Phoenix.GettextSupportTest do
+  use ExUnit.Case, async: true
+
+  doctest Mix.Phoenix.GettextSupport, import: true
+
+  test "gettext_support.ex in sync with installer" do
+    in_phoenix = read_split!("lib/mix/phoenix/gettext_support.ex")
+    in_installer = read_split!("installer/lib/phx_new/gettext_support.ex")
+
+    assert in_phoenix.first_line == "defmodule Mix.Phoenix.GettextSupport do"
+    assert in_installer.first_line == "defmodule Phx.New.GettextSupport do"
+    assert in_phoenix.rest == in_installer.rest
+    assert in_phoenix.rest =~ "gettext("
+  end
+
+  defp read_split!(path) do
+    File.read!(path)
+    |> String.split("\n", parts: 2)
+    |> then(fn [first_line, rest] -> %{first_line: first_line, rest: rest} end)
+  end
+end