Repository: elixir-gettext/gettext (elixir)

Issue:
Possibility to remove line-numbers from extraction metadata
Would it be reasonable to add an option to exclude line numbers from the file references?

I totally love the references and their ability to communicate in which file a certain translation is used
but in most cases the file number isn't super relevant and is creating the need to re-extract the translations
even if I'm just making a newline in one of my files. It would also prevent small commits from creating a lot of changes.

I'd gladly help out by sending a PR but wanted to check in to see if it's something that would be valuable before committing my time? :)

Pull request:
Add `write_reference_line_numbers` option
Background:
Since even the smallest changes in files can cause a cascade of changed line numbers in po/pot files we're often seeing larger amounts of changes in commits causing the need to re-extract than needed.

By adding the `write_reference_line_numbers` option developers has a way to decide for themselves whether they want to include them or not.

Fixes #321

Patch:
diff --git a/lib/gettext.ex b/lib/gettext.ex
index a418545..0a2d100 100644
--- a/lib/gettext.ex
+++ b/lib/gettext.ex
@@ -519,6 +519,10 @@ defmodule Gettext do
       reference comments will not be written when extracting messages or merging
       messages, and the ones already found in files will be discarded.
 
+    * `:write_reference_line_numbers` - a boolean that specifies whether file 
+      reference comments include line numbers when outputting PO(T) files.
+      Defaults to `true`.
+
     * `:sort_by_msgid` - a boolean that modifies the sorting behavior.
       By default, the order of existing messages in a POT file is kept and new
       messages are appended to the file. If `:sort_by_msgid` is set to `true`,
diff --git a/lib/gettext/extractor.ex b/lib/gettext/extractor.ex
index 382a208..15e5537 100644
--- a/lib/gettext/extractor.ex
+++ b/lib/gettext/extractor.ex
@@ -258,7 +258,7 @@ defmodule Gettext.Extractor do
        [
          new_pot_comment(),
          new_po
-         |> Merger.maybe_remove_references(gettext_config[:write_reference_comments])
+         |> Merger.prune_references(gettext_config)
          |> add_headers_to_new_po()
          |> PO.compose()
        ]}
@@ -267,7 +267,7 @@ defmodule Gettext.Extractor do
     do:
       {path,
        po
-       |> Merger.maybe_remove_references(gettext_config[:write_reference_comments])
+       |> Merger.prune_references(gettext_config)
        |> PO.compose()}
 
   defp prune_unmerged(path, gettext_config) do
diff --git a/lib/gettext/merger.ex b/lib/gettext/merger.ex
index 47fdbd5..bd3215d 100644
--- a/lib/gettext/merger.ex
+++ b/lib/gettext/merger.ex
@@ -232,16 +232,41 @@ defmodule Gettext.Merger do
   end
 
   @doc false
-  @spec maybe_remove_references(messages :: Messages.t(), remove? :: boolean() | nil) ::
-          Messages.t()
-  def maybe_remove_references(messages, remove?)
+  @spec prune_references(messages :: Messages.t(), gettext_config :: Keyword.t()) :: Messages.t()
+  def prune_references(%Messages{messages: messages} = all, gettext_config) do
+    write_references? = Keyword.get(gettext_config, :write_reference_comments, true)
+    write_line_numbers? = Keyword.get(gettext_config, :write_reference_line_numbers, true)
+
+    cond do
+      not write_references? ->
+        messages = Enum.map(messages, &remove_references/1)
+        %Messages{all | messages: messages}
+
+      not write_line_numbers? ->
+        messages = Enum.map(messages, &remove_reference_line_numbers/1)
+        %Messages{all | messages: messages}
+
+      true ->
+        all
+    end
+  end
+
+  defp remove_references(%{references: _} = message), do: %{message | references: []}
 
-  def maybe_remove_references(%Messages{messages: messages} = all, true) do
-    messages = Enum.map(messages, &%{&1 | references: []})
-    %Messages{all | messages: messages}
+  defp remove_reference_line_numbers(%{references: references} = message) do
+    references_without_line_numbers =
+      Enum.map(references, fn reference_list ->
+        Enum.map(reference_list, &remove_reference_line_number/1)
+      end)
+
+    %{message | references: references_without_line_numbers}
   end
 
-  def maybe_remove_references(messages, _remove?), do: messages
+  defp remove_reference_line_number({file, line_number})
+       when is_binary(file) and is_integer(line_number),
+       do: file
+
+  defp remove_reference_line_number(file), do: file
 
   defp headers_for_new_po_file(locale, plural_forms) do
     [
diff --git a/lib/mix/tasks/gettext.merge.ex b/lib/mix/tasks/gettext.merge.ex
index d35fba5..5665dd4 100644
--- a/lib/mix/tasks/gettext.merge.ex
+++ b/lib/mix/tasks/gettext.merge.ex
@@ -210,7 +210,7 @@ defmodule Mix.Tasks.Gettext.Merge do
       {new_po, stats} = Merger.new_po_file(po_file, pot_file, locale, opts)
 
       {new_po
-       |> Merger.maybe_remove_references(gettext_config[:write_reference_comments])
+       |> Merger.prune_references(gettext_config)
        |> PO.compose(), stats}
     end
   end
@@ -220,7 +220,7 @@ defmodule Mix.Tasks.Gettext.Merge do
       Merger.merge(PO.parse_file!(po_file), PO.parse_file!(pot_file), locale, opts)
 
     {merged
-     |> Merger.maybe_remove_references(gettext_config[:write_reference_comments])
+     |> Merger.prune_references(gettext_config)
      |> PO.compose(), stats}
   end
 


Test patch:
diff --git a/test/gettext/merger_test.exs b/test/gettext/merger_test.exs
index 5299941..36c8655 100644
--- a/test/gettext/merger_test.exs
+++ b/test/gettext/merger_test.exs
@@ -461,6 +461,57 @@ defmodule Gettext.MergerTest do
     end
   end
 
+  describe "prune_references/2" do
+    test "prunes all references when `write_reference_comments` is `false`" do
+      po = %Messages{
+        messages: [
+          %Message.Singular{msgid: "a", references: [[{"path/to/file.ex", 12}]]},
+          %Message.Plural{msgid: "a", msgid_plural: "ab", references: [[{"path/to/file.ex", 12}]]}
+        ]
+      }
+
+      config = [write_reference_comments: false]
+
+      assert %Messages{
+               messages: [
+                 %Message.Singular{references: []},
+                 %Message.Plural{references: []}
+               ]
+             } = Merger.prune_references(po, config)
+    end
+
+    test "prunes reference line numbers when `write_reference_line_numbers` is `false`" do
+      po = %Messages{
+        messages: [
+          %Message.Singular{msgid: "a", references: [[{"path/to/file.ex", 12}]]},
+          %Message.Plural{msgid: "a", msgid_plural: "ab", references: [[{"path/to/file.ex", 12}]]}
+        ]
+      }
+
+      config = [write_reference_line_numbers: false]
+
+      assert %Messages{
+               messages: [
+                 %Message.Singular{references: [["path/to/file.ex"]]},
+                 %Message.Plural{references: [["path/to/file.ex"]]}
+               ]
+             } = Merger.prune_references(po, config)
+    end
+
+    test "does nothing per default" do
+      po = %Messages{
+        messages: [
+          %Message.Singular{msgid: "a", references: [[{"path/to/file.ex", 12}]]},
+          %Message.Plural{msgid: "a", msgid_plural: "ab", references: [{"path/to/file.ex", 12}]}
+        ]
+      }
+
+      config = []
+
+      assert po == Merger.prune_references(po, config)
+    end
+  end
+
   test "new_po_file/2" do
     pot_path = Path.join(@pot_path, "new_po_file.pot")
     new_po_path = Path.join(@pot_path, "it/LC_MESSAGES/new_po_file.po")