Repository: elixir-gettext/gettext
Pull request #244 (2020-02-17 09:41:32)

Allow sorting strings by msgid
I have been working on one Phoenix app with a few people for 1 year now. Every time we happened to add strings to the same Gettext domain on two different feature branches, we would get merge conflicts because the standard behavior of Gettext is to append strings to the end of the file. This is a minor inconvenience for a single file, but a bit annoying if you have more than 5 languages.

We came to the conclusion that we might decrease how often conflicts happen if we changed the default behavior - instead of appending strings at the end, which guarantees conflicts every time, we want to sort them alphabetically by `msgid`.

We used that approach for a few months now, and it does work for decreasing the number of merge conflicts. Thus I want to suggest it as a option for everyone :).

The downside is that the files might change the first time you run `mix gettext.extract` after you resolved conflicts manually and didn't pay attention to ordering the strings.

angelikatyborska: Oops, I am a bit surprised by those test failures. It looks to me like the tests are already broken on master and it's not related to my changes.

Patch:
diff --git a/lib/gettext.ex b/lib/gettext.ex
index 94f7181..e8d422d 100644
--- a/lib/gettext.ex
+++ b/lib/gettext.ex
@@ -511,6 +511,10 @@ defmodule Gettext do
       reference comments will not be written when extracting translations or merging
       translations, and the ones already found in files will be discarded.
 
+    * `:sort_by_msgid` - a boolean that modifies the sorting behavior.
+      By default, the order of existing translations in a POT file is kept, and new
+      translations are appended to the file. If `:sort_by_msgid` is set to true,
+      existing and new translations will be mixed and sorted alphabetically by msgid.
   """
 
   defmodule Error do
diff --git a/lib/gettext/extractor.ex b/lib/gettext/extractor.ex
index 756d5a3..7b26e2e 100644
--- a/lib/gettext/extractor.ex
+++ b/lib/gettext/extractor.ex
@@ -298,8 +298,17 @@ defmodule Gettext.Extractor do
     # with the translations that only appear in `new`.
     unique_new = Enum.reject(new.translations, &PO.Translations.find(existing.translations, &1))
 
+    translations = old_and_merged ++ unique_new
+
+    translations =
+      if gettext_config[:sort_by_msgid] do
+        Enum.sort_by(translations, & &1.msgid)
+      else
+        translations
+      end
+
     %PO{
-      translations: old_and_merged ++ unique_new,
+      translations: translations,
       headers: existing.headers,
       top_of_the_file_comments: existing.top_of_the_file_comments
     }


Test patch:
diff --git a/test/gettext/extractor_test.exs b/test/gettext/extractor_test.exs
index 170f978..52e3194 100644
--- a/test/gettext/extractor_test.exs
+++ b/test/gettext/extractor_test.exs
@@ -3,6 +3,7 @@ defmodule Gettext.ExtractorTest do
   alias Gettext.Extractor
   alias Gettext.PO
   alias Gettext.PO.Translation
+  alias Gettext.PO.PluralTranslation
 
   @pot_path "../../tmp/" |> Path.expand(__DIR__) |> Path.relative_to_cwd()
 
@@ -65,7 +66,7 @@ defmodule Gettext.ExtractorTest do
       msgstr ""
       """)
 
-      message = "syntax_error.pot:3: syntax error before: msgid"
+      message = "tmp/syntax_error.pot:3: syntax error before: msgid"
 
       assert_raise Gettext.PO.SyntaxError, message, fn ->
         Extractor.merge_pot_files([{path, %PO{}}], [path], [])
@@ -204,6 +205,40 @@ defmodule Gettext.ExtractorTest do
       assert t1.references == [{"live_streaming.ex", 40}]
       assert t2.msgid == ["new translation"]
     end
+
+    test "order can be alphabetical if desired" do
+      # Old and new translations are mixed together and ordered alphabetically.
+      foo_translation = %Translation{msgid: ["foo"], references: [{"foo.ex", 1}]}
+      bar_translation = %Translation{msgid: ["bar"], references: [{"bar.ex", 1}]}
+
+      baz_translation = %PluralTranslation{
+        msgid: ["baz"],
+        msgid_plural: ["bazs"],
+        references: [{"baz.ex", 1}]
+      }
+
+      qux_translation = %Translation{msgid: ["qux"], references: [{"bar.ex", 1}]}
+
+      po1 = %PO{
+        translations: [
+          foo_translation,
+          qux_translation,
+          bar_translation
+        ]
+      }
+
+      po2 = %PO{
+        translations: [
+          baz_translation,
+          foo_translation,
+          bar_translation
+        ]
+      }
+
+      %PO{translations: translations} = Extractor.merge_template(po1, po2, sort_by_msgid: true)
+
+      assert Enum.map(translations, &Enum.at(&1.msgid, 0)) == ~w(bar baz foo qux)
+    end
   end
 
   test "extraction process" do
diff --git a/test/gettext/po_test.exs b/test/gettext/po_test.exs
index 50d706d..01db74b 100644
--- a/test/gettext/po_test.exs
+++ b/test/gettext/po_test.exs
@@ -193,14 +193,14 @@ defmodule Gettext.POTest do
 
     test "invalid file contents" do
       fixture_path = Path.expand("../fixtures/invalid_syntax_error.po", __DIR__)
-      msg = "invalid_syntax_error.po:4: syntax error before: msgstr"
+      msg = "test/fixtures/invalid_syntax_error.po:4: syntax error before: msgstr"
 
       assert_raise SyntaxError, msg, fn ->
         PO.parse_file!(fixture_path)
       end
 
       fixture_path = Path.expand("../fixtures/invalid_token_error.po", __DIR__)
-      msg = "invalid_token_error.po:3: unknown keyword 'msg'"
+      msg = "test/fixtures/invalid_token_error.po:3: unknown keyword 'msg'"
 
       assert_raise SyntaxError, msg, fn ->
         PO.parse_file!(fixture_path)