Support marking messages as obsolete

Implements #210

lib/gettext.ex

@@ -524,6 +524,10 @@ defmodule Gettext do
       messages are appended to the file. If `:sort_by_msgid` is set to `true`,
       existing and new messages will be mixed and sorted alphabetically by msgid.
 
+    * `:on_obsolete` - controls what happens when obsolete messages are found.
+      If `:mark_as_obsolete`, messages are kept and marked as obsolete.
+      If `:delete`, obsolete messages are deleted. Defaults to `:delete`.
+
   """
 
   defmodule Error do

lib/gettext/compiler.ex

@@ -506,8 +506,6 @@ defmodule Gettext.Compiler do
     singular_fun = :"#{locale}_#{domain}_lgettext"
     plural_fun = :"#{locale}_#{domain}_lngettext"
 
-    # Remove obsolete messages (not needed at runtime)
-    # TODO: Resolve when implementing https://github.com/elixir-gettext/gettext/issues/210
     messages = Enum.filter(messages, &match?(%{obsolete: false}, &1))
 
     mapper =

lib/gettext/merger.ex

@@ -54,7 +54,7 @@ defmodule Gettext.Merger do
   def merge(%Messages{} = old, %Messages{} = new, locale, opts)
       when is_binary(locale) and is_list(opts) do
     opts = put_plural_forms_opt(opts, old.headers, locale)
-    stats = %{new: 0, exact_matches: 0, fuzzy_matches: 0, removed: 0}
+    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)
 
@@ -103,7 +103,19 @@ defmodule Gettext.Merger do
         end
       end)
 
-    {messages, put_in(stats.removed, map_size(unused))}
+    messages = Enum.map(messages, &%{&1 | obsolete: false})
+
+    {messages, stats} =
+      case Keyword.get(opts, :on_obsolete, :delete) do
+        :mark_as_obsolete ->
+          {messages ++ (unused |> Map.values() |> Enum.map(&%{&1 | obsolete: true})),
+           put_in(stats.marked_as_obsolete, map_size(unused))}
+
+        :delete ->
+          {messages, put_in(stats.removed, map_size(unused))}
+      end
+
+    {messages, stats}
   end
 
   defp adjust_number_of_plural_forms(%Message.Plural{} = message, plural_forms)
@@ -208,7 +220,13 @@ defmodule Gettext.Merger do
       messages: Enum.map(pot.messages, &prepare_new_message(&1, plural_forms))
     }
 
-    stats = %{new: length(po.messages), exact_matches: 0, fuzzy_matches: 0, removed: 0}
+    stats = %{
+      new: length(po.messages),
+      exact_matches: 0,
+      fuzzy_matches: 0,
+      removed: 0,
+      marked_as_obsolete: 0
+    }
 
     {po, stats}
   end

lib/mix/tasks/gettext.merge.ex

@@ -98,6 +98,10 @@ defmodule Mix.Tasks.Gettext.Merge do
       new messages in the target PO files will have this number of empty
       plural forms. See the "Plural forms" section above.
 
+    * `--on-obsolete` - controls what happens when obsolete messages are found.
+      If `mark_as_obsolete`, messages are kept and marked as obsolete.
+      If `delete`, obsolete messages are deleted. Defaults to `delete`.
+
   """
 
   alias Expo.PO
@@ -109,7 +113,8 @@ defmodule Mix.Tasks.Gettext.Merge do
     locale: :string,
     fuzzy: :boolean,
     fuzzy_threshold: :float,
-    plural_forms: :integer
+    plural_forms: :integer,
+    on_obsolete: :string
   ]
 
   def run(args) do
@@ -260,11 +265,12 @@ defmodule Mix.Tasks.Gettext.Merge do
   defp validate_merging_opts!(opts, gettext_config) do
     opts =
       opts
-      |> Keyword.take([:fuzzy, :fuzzy_threshold, :plural_forms])
+      |> Keyword.take([:fuzzy, :fuzzy_threshold, :plural_forms, :on_obsolete])
       |> Keyword.put_new(:fuzzy, true)
       |> Keyword.put_new_lazy(:fuzzy_threshold, fn ->
         gettext_config[:fuzzy_threshold] || @default_fuzzy_threshold
       end)
+      |> Keyword.update(:on_obsolete, :delete, &cast_on_obsolete/1)
 
     threshold = opts[:fuzzy_threshold]
 
@@ -285,6 +291,18 @@ defmodule Mix.Tasks.Gettext.Merge do
     pluralized = if stats.new == 1, do: "message", else: "messages"
 
     "#{stats.new} new #{pluralized}, #{stats.removed} removed, " <>
-      "#{stats.exact_matches} unchanged, #{stats.fuzzy_matches} reworded (fuzzy)"
+      "#{stats.exact_matches} unchanged, #{stats.fuzzy_matches} reworded (fuzzy), " <>
+      "#{stats.marked_as_obsolete} marked as obsolete"
+  end
+
+  defp cast_on_obsolete("delete" = _on_obsolete), do: :delete
+  defp cast_on_obsolete("mark_as_obsolete" = _on_obsolete), do: :mark_as_obsolete
+
+  defp cast_on_obsolete(on_obsolete) do
+    Mix.raise("""
+    An invalid value was provided for the option `on_obsolete`.
+    Value: #{inspect(on_obsolete)}
+    Valid Choices: "delete" / "mark_as_obsolete"
+    """)
   end
 end

test/gettext/merger_test.exs

@@ -38,7 +38,43 @@ defmodule Gettext.MergerTest do
       assert message.msgid == "tomerge"
       assert message.msgstr == "foo"
 
-      assert stats == %{exact_matches: 1, fuzzy_matches: 0, new: 0, removed: 2}
+      assert stats == %{
+               exact_matches: 1,
+               fuzzy_matches: 0,
+               new: 0,
+               removed: 2,
+               marked_as_obsolete: 0
+             }
+    end
+
+    test "obsolete messages are marked as obsolete" do
+      old_po = %Messages{
+        messages: [
+          %Message.Singular{msgid: "obs_auto", msgstr: "foo", flags: @autogenerated_flags},
+          %Message.Singular{msgid: "obs_manual", msgstr: "foo"},
+          %Message.Singular{msgid: "tomerge", msgstr: "foo", obsolete: true}
+        ]
+      }
+
+      new_pot = %Messages{messages: [%Message.Singular{msgid: "tomerge", msgstr: ""}]}
+
+      assert {%Messages{
+                messages: [
+                  %Message.Singular{msgid: "tomerge", obsolete: false},
+                  %Message.Singular{msgid: "obs_auto", obsolete: true},
+                  %Message.Singular{msgid: "obs_manual", obsolete: true}
+                ]
+              },
+              stats} =
+               Merger.merge(old_po, new_pot, "en", @opts ++ [on_obsolete: :mark_as_obsolete])
+
+      assert stats == %{
+               exact_matches: 1,
+               fuzzy_matches: 0,
+               new: 0,
+               removed: 0,
+               marked_as_obsolete: 2
+             }
     end
 
     test "when messages match, the msgstr of the old one is preserved" do
@@ -193,7 +229,7 @@ defmodule Gettext.MergerTest do
 
       assert {%Messages{messages: [message]}, stats} = Merger.merge(old_po, new_pot, "en", @opts)
 
-      assert stats == %{exact_matches: 0, fuzzy_matches: 1, new: 0, removed: 0}
+      assert %{exact_matches: 0, fuzzy_matches: 1, new: 0, removed: 0} = stats
 
       assert message.msgid == "hello worlds!"
       assert message.msgstr == ["foo"]
@@ -223,7 +259,7 @@ defmodule Gettext.MergerTest do
       assert message.msgid == ["hello world!"]
       assert message.msgstr == ["foo"]
 
-      assert stats == %{exact_matches: 1, fuzzy_matches: 0, new: 0, removed: 1}
+      assert %{exact_matches: 1, fuzzy_matches: 0, new: 0, removed: 1} = stats
     end
 
     test "exact matches do not prevent fuzzy matches for other messages" do
@@ -280,7 +316,7 @@ defmodule Gettext.MergerTest do
       assert message_2.msgstr == ["foo"]
       assert Message.has_flag?(message_2, "fuzzy")
 
-      assert stats == %{exact_matches: 0, new: 0, fuzzy_matches: 2, removed: 0}
+      assert %{exact_matches: 0, new: 0, fuzzy_matches: 2, removed: 0} = stats
     end
 
     test "filling in a fuzzy message preserves references" do
@@ -344,7 +380,7 @@ defmodule Gettext.MergerTest do
       assert message.comments == ["# Guyanese Cocoballs"]
       assert message.references == [{"new_file.txt", 2}]
 
-      assert stats == %{exact_matches: 0, fuzzy_matches: 1, new: 0, removed: 0}
+      assert %{exact_matches: 0, fuzzy_matches: 1, new: 0, removed: 0} = stats
     end
 
     # This has been verified with msgmerge too.
@@ -368,7 +404,7 @@ defmodule Gettext.MergerTest do
       assert {%Messages{messages: [fuzzy_message, new_message]}, stats} =
                Merger.merge(old_po, new_pot, "en", @opts)
 
-      assert stats == %{exact_matches: 0, fuzzy_matches: 1, new: 1, removed: 0}
+      assert %{exact_matches: 0, fuzzy_matches: 1, new: 1, removed: 0} = stats
 
       assert fuzzy_message.msgid == "hello worlds!"
       assert fuzzy_message.msgstr == ["cfoo"]

test/mix/tasks/gettext.merge_test.exs

@@ -58,7 +58,9 @@ defmodule Mix.Tasks.Gettext.MergeTest do
       end)
 
     assert output =~ "Wrote tmp/gettext.merge/it/LC_MESSAGES/foo.po"
-    assert output =~ "(1 new message, 0 removed, 0 unchanged, 0 reworded (fuzzy))"
+
+    assert output =~
+             "(1 new message, 0 removed, 0 unchanged, 0 reworded (fuzzy), 0 marked as obsolete)"
 
     # The POT file is left unchanged
     assert read_file("foo.pot") == pot_contents
@@ -69,6 +71,64 @@ defmodule Mix.Tasks.Gettext.MergeTest do
            """
   end
 
+  test "marks messages as obsolete" do
+    write_file("foo.pot", "")
+
+    write_file("it/LC_MESSAGES/foo.po", """
+    msgid "foo"
+    msgstr ""
+    """)
+
+    output =
+      capture_io(fn ->
+        run([
+          tmp_path("it/LC_MESSAGES/foo.po"),
+          tmp_path("foo.pot"),
+          "--on-obsolete",
+          "mark_as_obsolete"
+        ])
+      end)
+
+    assert output =~ "Wrote tmp/gettext.merge/it/LC_MESSAGES/foo.po"
+
+    assert output =~
+             "(0 new messages, 0 removed, 0 unchanged, 0 reworded (fuzzy), 1 marked as obsolete)"
+  end
+
+  test "removes obsolete messages" do
+    write_file("foo.pot", "")
+
+    write_file("it/LC_MESSAGES/foo.po", """
+    msgid "foo"
+    msgstr ""
+    """)
+
+    output =
+      capture_io(fn ->
+        run([tmp_path("it/LC_MESSAGES/foo.po"), tmp_path("foo.pot"), "--on-obsolete", "delete"])
+      end)
+
+    assert output =~ "Wrote tmp/gettext.merge/it/LC_MESSAGES/foo.po"
+
+    assert output =~
+             "(0 new messages, 1 removed, 0 unchanged, 0 reworded (fuzzy), 0 marked as obsolete)"
+  end
+
+  test "validates on-obsolete" do
+    write_file("foo.pot", "")
+    write_file("it/LC_MESSAGES/foo.po", "")
+
+    expected_message = """
+    An invalid value was provided for the option `on_obsolete`.
+    Value: "invalid"
+    Valid Choices: "delete" / "mark_as_obsolete"
+    """
+
+    assert_raise Mix.Error, expected_message, fn ->
+      run([tmp_path("it/LC_MESSAGES/foo.po"), tmp_path("foo.pot"), "--on-obsolete", "invalid"])
+    end
+  end
+
   test "passing a dir and a --locale opt will update/create PO files in the locale dir" do
     write_file("default.pot", """
     msgid "def"