From 87a63dfd474373af041beb1a0ad88ea8ee16c161 Mon Sep 17 00:00:00 2001 From: Johan Tell Date: Wed, 19 Oct 2022 10:13:00 +0200 Subject: [PATCH] Add `write_reference_line_numbers` option (#322) 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 --- lib/gettext.ex | 4 +++ lib/gettext/extractor.ex | 4 +-- lib/gettext/merger.ex | 39 +++++++++++++++++++++----- lib/mix/tasks/gettext.merge.ex | 4 +-- test/gettext/merger_test.exs | 51 ++++++++++++++++++++++++++++++++++ 5 files changed, 91 insertions(+), 11 deletions(-) diff --git a/lib/gettext.ex b/lib/gettext.ex index a418545e..0a2d100f 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 382a2086..15e55379 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 47fdbd53..bd3215df 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 d35fba5a..5665dd40 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 diff --git a/test/gettext/merger_test.exs b/test/gettext/merger_test.exs index 5299941d..36c8655d 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")