Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unique singular and plural messages #379

Merged
merged 1 commit into from
Dec 4, 2023
Merged

Conversation

maennchen
Copy link
Member

@maennchen maennchen commented Nov 6, 2023

@maennchen maennchen self-assigned this Nov 6, 2023
@coveralls
Copy link

coveralls commented Nov 6, 2023

Pull Request Test Coverage Report for Build 6f62f075c71eaa09e135a8b593aa74a680097b3e-PR-379

  • 7 of 9 (77.78%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+2.8%) to 93.651%

Changes Missing Coverage Covered Lines Changed/Added Lines %
lib/gettext/compiler.ex 4 6 66.67%
Totals Coverage Status
Change from base Build 002dee19c1255915f9dfe6cd4caa538e11ed2e65: 2.8%
Covered Lines: 531
Relevant Lines: 567

💛 - Coveralls

mix.exs Outdated Show resolved Hide resolved
@maennchen maennchen marked this pull request as ready for review November 6, 2023 15:34
@maennchen maennchen linked an issue Nov 6, 2023 that may be closed by this pull request
@maennchen maennchen force-pushed the jm/singular_plural_uniq branch 2 times, most recently from 8503f2b to 00af4c2 Compare November 15, 2023 09:16
lib/gettext/compiler.ex Show resolved Hide resolved
lib/gettext/extractor.ex Show resolved Hide resolved
@saveman71
Copy link

saveman71 commented Nov 16, 2023

Hi there, I wanted to test out this branch so I set the gettexts version to the tip of this PR

{:gettext, git: "https://github.com/elixir-gettext/gettext", ref: "00af4c2cc9c80886f5f917381fc3bbabe347c7f2", override: true},

Now when running the task I get this message about duplicate keys, fair enough there's a fix to be made.

** (Expo.PO.DuplicateMessagesError) priv/gettext/default.pot:2033: found duplicate on line 2033 for msgid: 'Annulé'priv/gettext/default.pot:2085: found duplicate on line 2085 for msgid: 'Utilisateur' and msgid_plural: 'Utilisateurs'priv/gettext/default.pot:2111: found duplicate on line 2111 for msgid: 'Utilisateur anonymisé' and msgid_plural: '%{count} utilisateurs anonymisés'priv/gettext/default.pot:9362: found duplicate on line 9362 for msgid: 'Formateur' and msgid_plural: 'Formateurs'priv/gettext/default.pot:9570: found duplicate on line 9570 for msgid: 'Gagné'priv/gettext/default.pot:10628: found duplicate on line 10628 for msgid: 'Indécis'priv/gettext/default.pot:10905: found duplicate on line 10905 for msgid: 'Formateur supprimé'priv/gettext/default.pot:10978: found duplicate on line 10978 for msgid: 'Invitation envoyée'priv/gettext/default.pot:16164: found duplicate on line 16164 for msgid: 'Perdu'

To merge the duplicates, run:

  mix expo.msguniq priv/gettext/default.pot

    (expo 0.5.0) lib/expo/po.ex:162: Expo.PO.parse_file!/2
    (gettext 0.24.0-dev) lib/gettext/extractor.ex:251: Gettext.Extractor.read_contents_and_parse/1
    (gettext 0.24.0-dev) lib/gettext/extractor.ex:239: Gettext.Extractor.merge_or_unchanged/3
    (gettext 0.24.0-dev) lib/gettext/extractor.ex:233: Gettext.Extractor.merge_existing_and_extracted/4
    (stdlib 5.1) maps.erl:199: :maps.merge_with_1/4
    (gettext 0.24.0-dev) lib/gettext/extractor.ex:223: Gettext.Extractor.merge_pot_files/3
    (gettext 0.24.0-dev) lib/mix/tasks/gettext.extract.ex:109: Mix.Tasks.Gettext.Extract.extract/2
    (gettext 0.24.0-dev) lib/mix/tasks/gettext.extract.ex:68: Mix.Tasks.Gettext.Extract.run/1

But when I run the suggested command:

  1. Nothing really happens, the PO/POT file is just written out on stdout. I had to go read the code to understand I need an --output-file option, which btw is written as just output above.

https://github.com/elixir-gettext/expo/blob/8ee1c57834ffa81ffa23a0d136fd390d2467fc53/lib/mix/tasks/expo.msguniq.ex#L12-L19

  1. Even if I run it with the file in argument:
mix expo.msguniq priv/gettext/default.pot --output-file=test.pot                                                                                                                                                                                                                        
                                                                                                                                                                                                                                                                                          
09:58:06.128 [info] configure with file config/config_dev.exs                                                                                                                                                                                                                             
** (FunctionClauseError) no function clause matching in IO.binwrite/2                                                                                                                                                                                                                     
                                                                                                                                                                                                                                                                                          
    The following arguments were given to IO.binwrite/2:                                                                                                                                                                                                                                  
                                                                                                                                                                                                                                                                                          
        # 1                                                                                                                                                                                                                                                                               
        {:file_descriptor, :prim_file, %{handle: #Reference<0.2977518349.1161953330.189856>, owner: #PID<0.99.0>, r_buffer: #Reference<0.2977518349.1161953281.199120>, r_ahead_size: 65536}}                                                                                             
                                                                                                                                                                                                                                                                                          
        # 2                                                                                                                                                                                                                                                                               
        10                                                                                                                                                                                                                                                                                
                                                                                                                                                                                                                                                                                          
    Attempted function clauses (showing 1 out of 1):                                                                                                                                                                                                                                      
                                                                                                                                                                                                                                                                                          
        def binwrite(device, iodata) when is_list(iodata) or is_binary(iodata)
     
    (elixir 1.15.6) IO.binwrite/2
    (elixir 1.15.6) lib/enum.ex:1548: Enum."-reduce_into_protocol/3-lists^foldl/2-0-"/3
    (elixir 1.15.6) lib/enum.ex:1537: Enum.into_protocol/2
    (expo 0.5.0) lib/mix/tasks/expo.msguniq.ex:75: Mix.Tasks.Expo.Msguniq.run/1
    (mix 1.15.6) lib/mix/task.ex:447: anonymous fn/3 in Mix.Task.run_task/5
    (mix 1.15.6) lib/mix/cli.ex:92: Mix.CLI.run_task/2
    /home/arch/.asdf/installs/elixir/1.15.6-otp-26/bin/mix:2: (file)

Also when running to stdin there's a weird 10 that appears instead of the newline

10#, elixir-autogen, elixir-format
msgid "String 1"
msgstr ""
10#, elixir-autogen, elixir-format
msgid "String 2"
msgstr ""

am I missing something?

My understanding is that expo is pulled to match the updated gettext dependency's requirements, so I didn't touch that (neither did I have it in the dependencies anyway)

EDIT: yes it shows 0.5.0 in the lockfile

@maennchen
Copy link
Member Author

@saveman71 Oops, the error should give you the concrete instructions…

I’ll try to reproduce your issue as well and do a fix.

@maennchen
Copy link
Member Author

@saveman71 Can you send me your po file with the duplicates? I have trouble reproducing that issue.

@saveman71
Copy link

@saveman71 Can you send me your po file with the duplicates? I have trouble reproducing that issue.

Sure, let me try to tear down the po file to a single reproductible example first.

Otherwise I'll send by email

@saveman71
Copy link

saveman71 commented Nov 16, 2023

Okay here's a test file that fails on my laptop:

POT

# This is added just to be on par with the .po file (same same message id keys)
msgid ""
msgstr ""

#, elixir-autogen, elixir-format
msgid "Invitation envoyée"
msgid_plural "Invitations envoyées"
msgstr[0] ""
msgstr[1] ""

#, elixir-autogen, elixir-format
msgid "Invitation envoyée"
msgstr ""

PO

msgid ""
msgstr ""
"Content-Type: text/plain; charset=UTF-8\n"
"Language: fr\n"
"Plural-Forms: nplurals=2; plural=(n>1);"

#, elixir-autogen, elixir-format
msgid "Invitation envoyée"
msgid_plural "Invitations envoyées"
msgstr[0] "Invitation envoyée"
msgstr[1] "Invitations envoyées"

#, elixir-autogen, elixir-format
msgid "Invitation envoyée"
msgstr "Invitation envoyée"

The issue seems to come from the first lines, if I remove them it seems we're good. Not an expert in PO files but I assume they are required on the PO file to describe the encoding etc, right?

Then I added the empty ones in the POT file to be able to do this in our CI (checks the same msgids are there, checking both files are synced (side note: this is to early exit the CI instead of compiling 3 minutes for nothing if we find an "easy to find discrepancy", we do run mix gettext.extract --check-up-to-date right after).

diff <(cat priv/gettext/fr/LC_MESSAGES/default.po | grep msgid) <(cat priv/gettext/default.pot | grep msgid)

So for the POT file we're kinda the origin of the issue, but I don't see what's wrong on the PO file?

@maennchen
Copy link
Member Author

@saveman71 I got it. It's an issue with the iodata. and Enum.into/2.

@saveman71
Copy link

saveman71 commented Nov 16, 2023

Here are the outputs sorry

With the files as above:

mix expo.msguniq priv/gettext/default.pot

10:47:37.634 [info] configure with file config/config_dev.exs
# This is added just to be on par with the .po file (same same message id keys)
msgid ""
msgstr ""
10#, elixir-autogen, elixir-format
#, elixir-autogen, elixir-format
msgid "Invitation envoyée"
msgid_plural "Invitations envoyées"
msgstr[0] ""
msgstr[1] ""
Merged 1 translations

=> note the extra 10 and duplicate comment

mix expo.msguniq priv/gettext/fr/LC_MESSAGES/default.po

10:41:53.870 [info] configure with file config/config_dev.exs
msgid ""
msgstr ""
"Content-Type: text/plain; charset=UTF-8\n"
"Language: fr\n"
"Plural-Forms: nplurals=2; plural=(n>1);"
10#, elixir-autogen, elixir-format
#, elixir-autogen, elixir-format
msgid "Invitation envoyée"
msgid_plural "Invitations envoyées"
msgstr[0] "Invitation envoyée"
msgstr[1] "Invitations envoyées"
Merged 1 translations

=> note the missing new lines, the 10 added. And also the duplicate comment.

mix expo.msguniq priv/gettext/default.pot -o priv/gettext/default.pot

10:47:46.664 [info] configure with file config/config_dev.exs
** (FunctionClauseError) no function clause matching in IO.binwrite/2    
    
    The following arguments were given to IO.binwrite/2:
    
        # 1
        {:file_descriptor, :prim_file, %{handle: #Reference<0.3619720424.2511470639.71125>, owner: #PID<0.99.0>, r_buffer: #Reference<0.3619720424.2511470609.71660>, r_ahead_size: 65536}}
    
        # 2
        10
    
    Attempted function clauses (showing 1 out of 1):
    
        def binwrite(device, iodata) when is_list(iodata) or is_binary(iodata)
    
    (elixir 1.15.6) IO.binwrite/2
    (elixir 1.15.6) lib/enum.ex:1548: Enum."-reduce_into_protocol/3-lists^foldl/2-0-"/3
    (elixir 1.15.6) lib/enum.ex:1537: Enum.into_protocol/2
    (expo 0.5.0) lib/mix/tasks/expo.msguniq.ex:75: Mix.Tasks.Expo.Msguniq.run/1
    (mix 1.15.6) lib/mix/task.ex:447: anonymous fn/3 in Mix.Task.run_task/5
    (mix 1.15.6) lib/mix/cli.ex:92: Mix.CLI.run_task/2
    /home/arch/.asdf/installs/elixir/1.15.6-otp-26/bin/mix:2: (file)

=> File is truncated to

# This is added just to be on par with the .po file (same same message id keys)
msgid ""
msgstr ""

@whatyouhide whatyouhide changed the title Unique Singular/Plural Messages Unique singular and plural messages Dec 4, 2023
@maennchen maennchen merged commit 0bea16e into main Dec 4, 2023
3 checks passed
@maennchen maennchen deleted the jm/singular_plural_uniq branch December 4, 2023 14:13
@maennchen
Copy link
Member Author

@whatyouhide Are we doing a release directly? If yes: should I prepare it or will you?

@whatyouhide
Copy link
Contributor

@maennchen yes, we can do a release. Feel free to prep it up 🙃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Duplicate msgid with singular and plural form
4 participants