Add the :custom_flags_to_keep option (#370)

Co-authored-by: Andrea Leopardi <[email protected]> Closes #369. While merging translations with "mix gettext.merge", flags get cleared out and replaced with the exception of "fuzzy", which is kept. This change will allow the ":custom_flags_to_keep" option to add a list of flags to be kept in addition to "fuzzy" which helps prevent information loss if external tools adds their own flags to messages.
elixir-gettext · Jul 31, 2023 · d7e1bef · d7e1bef
1 parent b8eb74d
commit d7e1bef
Show file tree

Hide file tree

Showing 4 changed files with 114 additions and 38 deletions.
diff --git a/lib/gettext.ex b/lib/gettext.ex
@@ -586,6 +586,11 @@ defmodule Gettext do
       have a matching reference. You can use this pattern to prevent Gettext from
       removing messages that you have extracted using another tool.
 
+    * `:custom_flags_to_keep` - a list of custom flags that will be kept for
+      existing messages during a merge. Gettext always keeps the `elixir-format`
+      and `fuzzy` flags.
+      
+
     * `:write_reference_comments` - a boolean that specifies whether reference
       comments should be written when outputting PO(T) files. If this is `false`,
       reference comments will not be written when extracting messages or merging

diff --git a/lib/gettext/merger.ex b/lib/gettext/merger.ex
@@ -50,9 +50,9 @@ defmodule Gettext.Merger do
         by the references in the POT file
 
   """
-  @spec merge(Messages.t(), Messages.t(), String.t(), Keyword.t()) ::
+  @spec merge(Messages.t(), Messages.t(), String.t(), Keyword.t(), Keyword.t()) ::
           {Messages.t(), map()}
-  def merge(%Messages{} = old, %Messages{} = new, locale, opts)
+  def merge(%Messages{} = old, %Messages{} = new, locale, opts, gettext_config)
       when is_binary(locale) and is_list(opts) do
     opts =
       opts
@@ -61,7 +61,7 @@ defmodule Gettext.Merger do
 
     stats = %{new: 0, exact_matches: 0, fuzzy_matches: 0, removed: 0, marked_as_obsolete: 0}
 
-    {messages, stats} = merge_messages(old.messages, new.messages, opts, stats)
+    {messages, stats} = merge_messages(old.messages, new.messages, opts, gettext_config, stats)
 
     po = %Messages{
       top_comments: old.top_comments,
@@ -99,10 +99,11 @@ defmodule Gettext.Merger do
     end
   end
 
-  defp merge_messages(old, new, opts, stats) do
+  defp merge_messages(old, new, opts, gettext_config, stats) do
     fuzzy? = Keyword.fetch!(opts, :fuzzy)
     fuzzy_threshold = Keyword.fetch!(opts, :fuzzy_threshold)
     plural_forms = Keyword.fetch!(opts, :plural_forms)
+    custom_flags_to_keep = Keyword.get(gettext_config, :custom_flags_to_keep, [])
 
     old = Map.new(old, &{Message.key(&1), &1})
 
@@ -114,7 +115,9 @@ defmodule Gettext.Merger do
         case Map.fetch(old, key) do
           {:ok, exact_match} ->
             stats = update_in(stats_acc.exact_matches, &(&1 + 1))
-            {merge_two_messages(exact_match, message), {stats, Map.delete(unused, key)}}
+
+            {merge_two_messages(exact_match, message, custom_flags_to_keep),
+             {stats, Map.delete(unused, key)}}
 
           :error when fuzzy? ->
             case maybe_merge_fuzzy(message, old, key, fuzzy_threshold) do
@@ -202,37 +205,49 @@ defmodule Gettext.Merger do
   # flags: we should take the new flags and preserve the fuzzy flag
   # references: new contains the updated and most recent references
 
-  defp merge_two_messages(%Message.Singular{} = old, %Message.Singular{} = new) do
+  defp merge_two_messages(
+         %Message.Singular{} = old,
+         %Message.Singular{} = new,
+         custom_flags_to_keep
+       ) do
     %Message.Singular{
       msgctxt: new.msgctxt,
       msgid: new.msgid,
       msgstr: old.msgstr,
       comments: old.comments,
       extracted_comments: new.extracted_comments,
-      flags: merge_flags(old, new),
+      flags: merge_flags(old, new, custom_flags_to_keep),
       references: new.references
     }
   end
 
-  defp merge_two_messages(%Message.Plural{} = old, %Message.Plural{} = new) do
+  defp merge_two_messages(%Message.Plural{} = old, %Message.Plural{} = new, custom_flags_to_keep) do
     %Message.Plural{
       msgctxt: new.msgctxt,
       msgid: new.msgid,
       msgid_plural: new.msgid_plural,
       msgstr: old.msgstr,
       comments: old.comments,
       extracted_comments: new.extracted_comments,
-      flags: merge_flags(old, new),
+      flags: merge_flags(old, new, custom_flags_to_keep),
       references: new.references
     }
   end
 
-  defp merge_flags(old, new) do
-    if Message.has_flag?(old, "fuzzy") do
-      Message.append_flag(new, "fuzzy").flags
-    else
-      new.flags
-    end
+  @default_flags_to_keep ["elixir-format", "fuzzy"]
+  defp merge_flags(old_message, new_message, custom_flags_to_keep) do
+    flags_to_keep = Enum.uniq(@default_flags_to_keep ++ custom_flags_to_keep)
+
+    %{flags: flags} =
+      Enum.reduce(flags_to_keep, new_message, fn flag, message ->
+        if Message.has_flag?(old_message, flag) do
+          Message.append_flag(message, flag)
+        else
+          message
+        end
+      end)
+
+    flags
   end
 
   @doc """

diff --git a/lib/mix/tasks/gettext.merge.ex b/lib/mix/tasks/gettext.merge.ex
@@ -238,7 +238,13 @@ defmodule Mix.Tasks.Gettext.Merge do
 
   defp merge_files(po_file, pot_file, locale, opts, gettext_config) do
     {merged, stats} =
-      Merger.merge(PO.parse_file!(po_file), PO.parse_file!(pot_file), locale, opts)
+      Merger.merge(
+        PO.parse_file!(po_file),
+        PO.parse_file!(pot_file),
+        locale,
+        opts,
+        gettext_config
+      )
 
     {merged
      |> Merger.prune_references(gettext_config)

diff --git a/test/gettext/merger_test.exs b/test/gettext/merger_test.exs
@@ -8,9 +8,10 @@ defmodule Gettext.MergerTest do
   alias Gettext.Merger
 
   @opts fuzzy: true, fuzzy_threshold: 0.8
+  @gettext_config []
   @autogenerated_flags [["elixir-format"]]
 
-  describe "merge/2" do
+  describe "merge/5" do
     test "headers from the old file are kept" do
       old_po = %Messages{
         headers: [~S(Language: it\n), ~S(My-Header: my-value\n)],
@@ -19,7 +20,7 @@ defmodule Gettext.MergerTest do
 
       new_pot = %Messages{headers: ["foo"], messages: []}
 
-      assert {new_po, _stats} = Merger.merge(old_po, new_pot, "en", @opts)
+      assert {new_po, _stats} = Merger.merge(old_po, new_pot, "en", @opts, @gettext_config)
       assert new_po.headers == old_po.headers
     end
 
@@ -34,7 +35,8 @@ defmodule Gettext.MergerTest do
 
       new_pot = %Messages{messages: [%Message.Singular{msgid: "tomerge", msgstr: ""}]}
 
-      assert {%Messages{messages: [message]}, stats} = Merger.merge(old_po, new_pot, "en", @opts)
+      assert {%Messages{messages: [message]}, stats} =
+               Merger.merge(old_po, new_pot, "en", @opts, @gettext_config)
 
       assert message.msgid == "tomerge"
       assert message.msgstr == "foo"
@@ -67,7 +69,13 @@ defmodule Gettext.MergerTest do
                 ]
               },
               stats} =
-               Merger.merge(old_po, new_pot, "en", @opts ++ [on_obsolete: :mark_as_obsolete])
+               Merger.merge(
+                 old_po,
+                 new_pot,
+                 "en",
+                 @opts ++ [on_obsolete: :mark_as_obsolete],
+                 @gettext_config
+               )
 
       assert stats == %{
                exact_matches: 1,
@@ -82,7 +90,8 @@ defmodule Gettext.MergerTest do
       old_po = %Messages{messages: [%Message.Singular{msgid: "foo", msgstr: "bar"}]}
       new_pot = %Messages{messages: [%Message.Singular{msgid: "foo", msgstr: ""}]}
 
-      assert {%Messages{messages: [message]}, _stats} = Merger.merge(old_po, new_pot, "en", @opts)
+      assert {%Messages{messages: [message]}, _stats} =
+               Merger.merge(old_po, new_pot, "en", @opts, @gettext_config)
 
       assert message.msgid == "foo"
       assert message.msgstr == "bar"
@@ -103,7 +112,8 @@ defmodule Gettext.MergerTest do
         ]
       }
 
-      assert {%Messages{messages: [message]}, _stats} = Merger.merge(old_po, new_pot, "en", @opts)
+      assert {%Messages{messages: [message]}, _stats} =
+               Merger.merge(old_po, new_pot, "en", @opts, @gettext_config)
 
       assert message.msgid == "foo"
       assert message.comments == ["# existing comment"]
@@ -116,7 +126,8 @@ defmodule Gettext.MergerTest do
 
       new_pot = %Messages{messages: [%Message.Singular{msgid: "foo"}]}
 
-      assert {%Messages{messages: [message]}, _stats} = Merger.merge(old_po, new_pot, "en", @opts)
+      assert {%Messages{messages: [message]}, _stats} =
+               Merger.merge(old_po, new_pot, "en", @opts, @gettext_config)
 
       assert Message.has_flag?(message, "fuzzy")
     end
@@ -137,7 +148,8 @@ defmodule Gettext.MergerTest do
         ]
       }
 
-      assert {%Messages{messages: [message]}, _stats} = Merger.merge(old_po, new_pot, "en", @opts)
+      assert {%Messages{messages: [message]}, _stats} =
+               Merger.merge(old_po, new_pot, "en", @opts, @gettext_config)
 
       assert message.extracted_comments == ["#. new comment"]
     end
@@ -155,7 +167,8 @@ defmodule Gettext.MergerTest do
         ]
       }
 
-      assert {%Messages{messages: [message]}, _stats} = Merger.merge(old_po, new_pot, "en", @opts)
+      assert {%Messages{messages: [message]}, _stats} =
+               Merger.merge(old_po, new_pot, "en", @opts, @gettext_config)
 
       assert message.references == [{"bar.ex", 1}]
     end
@@ -169,7 +182,8 @@ defmodule Gettext.MergerTest do
         ]
       }
 
-      assert {%Messages{messages: [message]}, _stats} = Merger.merge(old_po, new_pot, "en", @opts)
+      assert {%Messages{messages: [message]}, _stats} =
+               Merger.merge(old_po, new_pot, "en", @opts, @gettext_config)
 
       assert message.flags == @autogenerated_flags
     end
@@ -194,7 +208,8 @@ defmodule Gettext.MergerTest do
         ]
       }
 
-      assert {%Messages{messages: messages}, _stats} = Merger.merge(old_po, new_pot, "en", @opts)
+      assert {%Messages{messages: messages}, _stats} =
+               Merger.merge(old_po, new_pot, "en", @opts, @gettext_config)
 
       assert [
                %Message.Singular{msgid: "foo", msgctxt: "context", msgstr: "with context"},
@@ -226,7 +241,8 @@ defmodule Gettext.MergerTest do
         ]
       }
 
-      assert {%Messages{messages: [message]}, stats} = Merger.merge(old_po, new_pot, "en", @opts)
+      assert {%Messages{messages: [message]}, stats} =
+               Merger.merge(old_po, new_pot, "en", @opts, @gettext_config)
 
       assert %{exact_matches: 0, fuzzy_matches: 1, new: 0, removed: 0} = stats
 
@@ -243,7 +259,8 @@ defmodule Gettext.MergerTest do
                  old_po,
                  new_pot,
                  "en",
-                 @opts ++ [store_previous_message_on_fuzzy_match: true]
+                 @opts ++ [store_previous_message_on_fuzzy_match: true],
+                 @gettext_config
                )
 
       assert message.previous_messages == [old_message]
@@ -263,7 +280,8 @@ defmodule Gettext.MergerTest do
 
       # Let's check that the "hello worlds!" message is discarded even if it's
       # a fuzzy match for "hello world!".
-      assert {%Messages{messages: [message]}, stats} = Merger.merge(old_po, new_pot, "en", @opts)
+      assert {%Messages{messages: [message]}, stats} =
+               Merger.merge(old_po, new_pot, "en", @opts, @gettext_config)
 
       refute Message.has_flag?(message, "fuzzy")
       assert message.msgid == ["hello world!"]
@@ -287,7 +305,7 @@ defmodule Gettext.MergerTest do
       }
 
       assert {%Messages{messages: [message_1, message_2]}, stats} =
-               Merger.merge(old_po, new_pot, "en", @opts)
+               Merger.merge(old_po, new_pot, "en", @opts, @gettext_config)
 
       assert message_1.msgid == ["hello world"]
       assert message_1.msgstr == ["foo"]
@@ -316,7 +334,7 @@ defmodule Gettext.MergerTest do
       }
 
       assert {%Messages{messages: [message_1, message_2]}, stats} =
-               Merger.merge(old_po, new_pot, "en", @opts)
+               Merger.merge(old_po, new_pot, "en", @opts, @gettext_config)
 
       assert message_1.msgid == ["hello world 1"]
       assert message_1.msgstr == ["foo"]
@@ -350,7 +368,8 @@ defmodule Gettext.MergerTest do
         ]
       }
 
-      assert {%Messages{messages: [message]}, _stats} = Merger.merge(old_po, new_pot, "en", @opts)
+      assert {%Messages{messages: [message]}, _stats} =
+               Merger.merge(old_po, new_pot, "en", @opts, @gettext_config)
 
       assert Message.has_flag?(message, "fuzzy")
       assert message.msgid == ["hello worlds!"]
@@ -381,7 +400,8 @@ defmodule Gettext.MergerTest do
         ]
       }
 
-      assert {%Messages{messages: [message]}, stats} = Merger.merge(old_po, new_pot, "en", @opts)
+      assert {%Messages{messages: [message]}, stats} =
+               Merger.merge(old_po, new_pot, "en", @opts, @gettext_config)
 
       assert Message.has_flag?(message, "fuzzy")
       assert message.msgid == ["Here is a cocoa ball."]
@@ -412,7 +432,7 @@ defmodule Gettext.MergerTest do
       }
 
       assert {%Messages{messages: [fuzzy_message, new_message]}, stats} =
-               Merger.merge(old_po, new_pot, "en", @opts)
+               Merger.merge(old_po, new_pot, "en", @opts, @gettext_config)
 
       assert %{exact_matches: 0, fuzzy_matches: 1, new: 1, removed: 0} = stats
 
@@ -439,7 +459,7 @@ defmodule Gettext.MergerTest do
       }
 
       assert {%Messages{messages: [message, plural_message]}, _stats} =
-               Merger.merge(old_po, new_pot, "en", @opts)
+               Merger.merge(old_po, new_pot, "en", @opts, @gettext_config)
 
       assert message.msgid == "a"
 
@@ -464,7 +484,7 @@ defmodule Gettext.MergerTest do
       stderr =
         capture_io(:stderr, fn ->
           assert {%Messages{messages: [message, plural_message]}, _stats} =
-                   Merger.merge(old_po, new_pot, "en", @opts)
+                   Merger.merge(old_po, new_pot, "en", @opts, @gettext_config)
 
           assert message.msgid == "a"
 
@@ -492,7 +512,7 @@ defmodule Gettext.MergerTest do
       stderr =
         capture_io(:stderr, fn ->
           assert {%Messages{messages: [message, plural_message]}, _stats} =
-                   Merger.merge(old_po, new_pot, "en", opts)
+                   Merger.merge(old_po, new_pot, "en", opts, @gettext_config)
 
           assert message.msgid == "a"
 
@@ -504,6 +524,36 @@ defmodule Gettext.MergerTest do
       assert stderr =~ "warning"
       assert stderr =~ "The --plural-forms and :plural_forms options are deprecated"
     end
+
+    test "custom flags defined by :custom_flag_to_keep config are kept" do
+      old_po = %Messages{
+        messages: [
+          %Message.Singular{
+            msgid: "a",
+            flags: [["elixir-format", "fuzzy", "custom-flag", "other-custom-flag"]]
+          }
+        ]
+      }
+
+      new_po = %Messages{
+        messages: [
+          %Message.Singular{
+            msgid: "a",
+            flags: [["elixir-format"]]
+          }
+        ]
+      }
+
+      gettext_config = [custom_flags_to_keep: ["custom-flag"]]
+
+      {merged_message, _stats} = Merger.merge(old_po, new_po, "en", @opts, gettext_config)
+
+      assert %Messages{
+               messages: [
+                 %Message.Singular{flags: [["elixir-format", "fuzzy", "custom-flag"]]}
+               ]
+             } = merged_message
+    end
   end
 
   describe "prune_references/2" do