Small improvements here and there

elixir-gettext · Nov 15, 2023 · 5db88cc · 5db88cc
1 parent e0fa66b
commit 5db88cc
Show file tree

Hide file tree

Showing 7 changed files with 78 additions and 73 deletions.
diff --git a/lib/expo/message.ex b/lib/expo/message.ex
@@ -149,42 +149,41 @@ defmodule Expo.Message do
   @doc """
   Merges two messages.
 
+  If both messages are `Expo.Message.Singular`, the result is a singular message.
+  If one of the two messages is a `Expo.Message.Plural`, the result is a plural message.
+  This is consistent with the behavior of GNU Gettext.
+
   ## Examples
 
-      iex> a = %Expo.Message.Singular{msgid: ["test"], flags: ["one"]}
-      ...> b = %Expo.Message.Singular{msgid: ["test"], flags: ["two"]}
-      ...> Expo.Message.merge(a, b)
+      iex> msg1 = %Expo.Message.Singular{msgid: ["test"], flags: ["one"]}
+      ...> msg2 = %Expo.Message.Singular{msgid: ["test"], flags: ["two"]}
+      ...> Expo.Message.merge(msg1, msg2)
       %Expo.Message.Singular{msgid: ["test"], flags: ["one", "two"]}
 
-      iex> a = %Expo.Message.Singular{msgid: ["test"]}
-      ...> b = %Expo.Message.Plural{msgid: ["test"], msgid_plural: ["tests"]}
-      ...> Expo.Message.merge(a, b)
+      iex> msg1 = %Expo.Message.Singular{msgid: ["test"]}
+      ...> msg2 = %Expo.Message.Plural{msgid: ["test"], msgid_plural: ["tests"]}
+      ...> Expo.Message.merge(msg1, msg2)
       %Expo.Message.Plural{msgid: ["test"], msgid_plural: ["tests"]}
 
   """
   @doc since: "0.5.0"
   @spec merge(Singular.t(), Singular.t()) :: Singular.t()
   @spec merge(t(), Plural.t()) :: Plural.t()
   @spec merge(Plural.t(), t()) :: Plural.t()
-  def merge(%mod{} = message_1, %mod{} = message_2), do: mod.merge(message_1, message_2)
-
-  def merge(%Singular{} = message_1, %Plural{} = message_2),
-    do: Plural.merge(singular_to_plural(message_1), message_2)
+  def merge(message1, message2)
 
-  def merge(%Plural{} = message_1, %Singular{} = message_2),
-    do: Plural.merge(message_1, singular_to_plural(message_2))
+  def merge(%mod{} = msg1, %mod{} = msg2), do: mod.merge(msg1, msg2)
+  def merge(%Singular{} = msg1, %Plural{} = msg2), do: Plural.merge(to_plural(msg1), msg2)
+  def merge(%Plural{} = msg1, %Singular{} = msg2), do: Plural.merge(msg1, to_plural(msg2))
 
-  defp singular_to_plural(%Singular{msgstr: msgstr} = singular) do
+  defp to_plural(%Singular{msgstr: msgstr} = singular) do
     msgstr = if IO.iodata_length(msgstr) > 0, do: %{0 => msgstr}, else: %{}
 
-    struct!(
-      Plural,
+    attributes =
       singular
       |> Map.from_struct()
-      |> Map.merge(%{
-        msgstr: msgstr,
-        msgid_plural: []
-      })
-    )
+      |> Map.merge(%{msgstr: msgstr, msgid_plural: []})
+
+    struct!(Plural, attributes)
   end
 end
diff --git a/lib/expo/message/plural.ex b/lib/expo/message/plural.ex
@@ -177,16 +177,16 @@ defmodule Expo.Message.Plural do
 
   ## Examples
 
-      iex> a = %Expo.Message.Plural{msgid: ["test"], msgid_plural: ["one"], flags: ["one"], msgstr: %{0 => "une"}}
-      ...> b = %Expo.Message.Plural{msgid: ["test"], msgid_plural: ["two"], flags: ["two"], msgstr: %{2 => "deux"}}
-      ...> Expo.Message.Plural.merge(a, b)
+      iex> msg1 = %Expo.Message.Plural{msgid: ["test"], msgid_plural: ["one"], flags: ["one"], msgstr: %{0 => "une"}}
+      ...> msg2 = %Expo.Message.Plural{msgid: ["test"], msgid_plural: ["two"], flags: ["two"], msgstr: %{2 => "deux"}}
+      ...> Expo.Message.Plural.merge(msg1, msg2)
       %Expo.Message.Plural{msgid: ["test"], msgid_plural: ["two"], flags: ["one", "two"], msgstr: %{0 => "une", 2 => "deux"}}
 
   """
   @doc since: "0.5.0"
   @spec merge(t(), t()) :: t()
-  def merge(message_1, message_2) do
-    Map.merge(message_1, message_2, fn
+  def merge(message1, message2) do
+    Map.merge(message1, message2, fn
       key, value_1, value_2 when key in [:msgid, :msgid_plural] ->
         if IO.iodata_length(value_2) > 0, do: value_2, else: value_1
 
@@ -205,9 +205,9 @@ defmodule Expo.Message.Plural do
     end)
   end
 
-  defp merge_msgstr(msgstrs_1, msgstrs_2) do
-    Map.merge(msgstrs_1, msgstrs_2, fn _key, msgstr_1, msgstr_2 ->
-      if IO.iodata_length(msgstr_2) > 0, do: msgstr_2, else: msgstr_1
+  defp merge_msgstr(msgstrs1, msgstrs2) do
+    Map.merge(msgstrs1, msgstrs2, fn _key, msgstr1, msgstr2 ->
+      if IO.iodata_length(msgstr2) > 0, do: msgstr2, else: msgstr1
     end)
   end
 end
diff --git a/lib/expo/message/singular.ex b/lib/expo/message/singular.ex
@@ -168,28 +168,28 @@ defmodule Expo.Message.Singular do
 
   ## Examples
 
-      iex> a = %Expo.Message.Singular{msgid: ["test"], flags: ["one"]}
-      ...> b = %Expo.Message.Singular{msgid: ["test"], flags: ["two"]}
-      ...> Expo.Message.Singular.merge(a, b)
+      iex> msg1 = %Expo.Message.Singular{msgid: ["test"], flags: ["one"]}
+      ...> msg2 = %Expo.Message.Singular{msgid: ["test"], flags: ["two"]}
+      ...> Expo.Message.Singular.merge(msg1, msg2)
       %Expo.Message.Singular{msgid: ["test"], flags: ["one", "two"]}
 
   """
   @doc since: "0.5.0"
   @spec merge(t(), t()) :: t()
-  def merge(message_1, message_2) do
-    Map.merge(message_1, message_2, fn
-      key, value_1, value_2 when key in [:msgid, :msgstr] ->
-        if IO.iodata_length(value_2) > 0, do: value_2, else: value_1
+  def merge(message1, message2) do
+    Map.merge(message1, message2, fn
+      key, value1, value2 when key in [:msgid, :msgstr] ->
+        if IO.iodata_length(value2) > 0, do: value2, else: value1
 
       :msgctxt, _msgctxt_a, msgctxt_b ->
         msgctxt_b
 
-      key, value_1, value_2
+      key, value1, value2
       when key in [:comments, :extracted_comments, :flags, :previous_messages, :references] ->
-        Enum.concat(value_1, value_2)
+        Enum.concat(value1, value2)
 
-      _key, _value_1, value_2 ->
-        value_2
+      _key, _value1, value2 ->
+        value2
     end)
   end
 end
diff --git a/lib/expo/po.ex b/lib/expo/po.ex
@@ -97,16 +97,6 @@ defmodule Expo.PO do
       iex> Expo.PO.parse_string!("msgid")
       ** (Expo.PO.SyntaxError) 1: no space after 'msgid'
 
-      iex> Expo.PO.parse_string!(\"""
-      ...> msgid "test"
-      ...> msgstr ""
-      ...>
-      ...> msgid "test"
-      ...> msgstr ""
-      ...> \""")
-      ** (Expo.PO.DuplicateMessagesError) 4: found duplicate on line 4 for msgid: 'test'
-      Run mix expo.msguniq with the input file to merge the duplicates
-
   """
   @spec parse_string!(String.t(), [parse_option()]) :: Messages.t()
   def parse_string!(string, opts \\ []) do

diff --git a/lib/expo/po/duplicate_translations_error.ex b/lib/expo/po/duplicate_translations_error.ex
@@ -6,6 +6,9 @@ defmodule Expo.PO.DuplicateMessagesError do
   alias Expo.Message
   alias Expo.Messages
 
+  @typedoc """
+  The type for this exception struct.
+  """
   @type t :: %__MODULE__{
           file: Path.t() | nil,
           duplicates: [
@@ -19,21 +22,26 @@ defmodule Expo.PO.DuplicateMessagesError do
 
   @impl Exception
   def message(%__MODULE__{file: file, duplicates: duplicates}) do
-    file = if file, do: Path.relative_to_cwd(file)
+    file = file && Path.relative_to_cwd(file)
 
     prefix = if file, do: [file, ":"], else: []
 
     fix_description =
-      if file,
-        do: ["Run mix expo.msguniq ", file, " to merge the duplicates"],
-        else: "Run mix expo.msguniq with the input file to merge the duplicates"
+      if file do
+        """
+        To merge the duplicates, run:
+
+          mix expo.msguniq #{file}
+        """
+      else
+        ~s(To merge the duplicates, run "mix expo.msguniq" with the input file)
+      end
 
-    IO.iodata_to_binary([
+    errors =
       Enum.map(duplicates, fn {_message, error_message, new_line, _old_line} ->
         [prefix, Integer.to_string(new_line), ": ", error_message]
-      end),
-      "\n",
-      fix_description
-    ])
+      end)
+
+    IO.iodata_to_binary([errors, "\n\n", fix_description])
   end
 end
diff --git a/lib/mix/tasks/expo.msguniq.ex b/lib/mix/tasks/expo.msguniq.ex
@@ -1,20 +1,22 @@
 defmodule Mix.Tasks.Expo.Msguniq do
-  @shortdoc "Unify duplicate translations in message catalog"
+  @shortdoc "Unifies duplicate translations in message catalog"
 
   @moduledoc """
   Unifies duplicate translations in the given PO file.
 
   By default, this task outputs the file on standard output. If you want to
   *overwrite* the given PO file, pass in the `--output` flag.
 
+  *This task is available since v0.5.0.*
+
   ## Usage
 
       mix expo.msguniq PO_FILE [--output=OUTPUT_FILE]
 
   ## Options
 
-  * `--output-file` (`-o`) - Default: `-` - File to store the output at. `-` for
-    STDOUT.
+  * `--output-file` (`-o`) - File to store the output in. `-` for
+    standard output. Defaults to `-`.
 
   """
   @moduledoc since: "0.5.0"
@@ -26,18 +28,14 @@ defmodule Mix.Tasks.Expo.Msguniq do
   alias Expo.PO
   alias Expo.PO.DuplicateMessagesError
 
-  @switches [
-    output_file: :string
-  ]
-  @aliases [
-    o: :output_file
-  ]
+  @switches [output_file: :string]
+  @aliases [o: :output_file]
   @default_options [output_file: "-"]
 
   @impl Mix.Task
   def run(args) do
-    Application.ensure_all_started(:expo)
-    {opts, argv} = OptionParser.parse!(args, switches: @switches, aliases: @aliases)
+    {:ok, _} = Application.ensure_all_started(:expo)
+    {opts, argv} = OptionParser.parse!(args, strict: @switches, aliases: @aliases)
 
     opts = Keyword.merge(@default_options, opts)
 

diff --git a/test/expo/po_test.exs b/test/expo/po_test.exs
@@ -921,8 +921,13 @@ defmodule Expo.POTest do
     test "file with duplicate messages" do
       fixture_path = "test/fixtures/po/duplicate_messages.po"
 
-      msg =
-        "file:4: found duplicate on line 4 for msgid: 'test'\nRun mix expo.msguniq file to merge the duplicates"
+      msg = """
+      file:4: found duplicate on line 4 for msgid: 'test'
+
+      To merge the duplicates, run:
+
+        mix expo.msguniq file
+      """
 
       assert_raise DuplicateMessagesError, msg, fn ->
         PO.parse_string!(File.read!(fixture_path), file: "file")
@@ -1061,8 +1066,13 @@ defmodule Expo.POTest do
     test "file with duplicate messages" do
       fixture_path = "test/fixtures/po/duplicate_messages.po"
 
-      message =
-        "#{fixture_path}:4: found duplicate on line 4 for msgid: 'test'\nRun mix expo.msguniq #{fixture_path} to merge the duplicates"
+      message = """
+      #{fixture_path}:4: found duplicate on line 4 for msgid: 'test'
+
+      To merge the duplicates, run:
+
+        mix expo.msguniq #{fixture_path}
+      """
 
       assert_raise DuplicateMessagesError, message, fn ->
         PO.parse_file!(fixture_path)