From d7e1bef09134c58132a20c6270ceb0c4f1f256ab Mon Sep 17 00:00:00 2001 From: Johan Tell Date: Mon, 31 Jul 2023 17:42:24 +0200 Subject: [PATCH] Add the :custom_flags_to_keep option (#370) Co-authored-by: Andrea Leopardi 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. --- lib/gettext.ex | 5 ++ lib/gettext/merger.ex | 45 ++++++++++------ lib/mix/tasks/gettext.merge.ex | 8 ++- test/gettext/merger_test.exs | 94 ++++++++++++++++++++++++++-------- 4 files changed, 114 insertions(+), 38 deletions(-) diff --git a/lib/gettext.ex b/lib/gettext.ex index d5a8dcf5..a043d363 100644 --- 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 index 996cacab..81582207 100644 --- 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,19 +205,23 @@ 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, @@ -222,17 +229,25 @@ defmodule Gettext.Merger do 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 index 3f7d9681..9b8d0d01 100644 --- 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 index 7e6916b3..e05daa3d 100644 --- 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