elixir-gettext / gettext

Internationalization and localization support for Elixir.
https://hexdocs.pm/gettext
461 stars 87 forks source link

check-up-to-date fails even though files are just extracted #334

Closed krns closed 1 year ago

krns commented 1 year ago

I was just about to try the new version (0.21.0) and noticed that the pipeline is failing:

** (Mix) mix gettext.extract failed due to --check-up-to-date.
The following POT files were not extracted or are out of date:

I tried deleting all the POT files and extracting them again, with the same result.

My gettext configuration looked like:

    [
      write_reference_comments: false,
      sort_by_msgid: true
    ]

Because of the sort_by_msgid deprecation I tried false, :case_insensitive and :case_sensitive – same result.

Am I missing something on my side?

maennchen commented 1 year ago

@krns Do you get a changed file running the extract task? If yes: what is different?

Edit: nevermind, read the issue description again and i can answer my own question.

krns commented 1 year ago

Some clarifications because my problem description might be a bit misleading:

When I talk about "same result", I mean that the warning remains. The newly extracted POT files are actually slightly different from the POT file extracted with gettext version 0.20.0:

 ## This file is a PO Template file.
 ##
 ## "msgid"s here are often extracted from source code.
-## Add new translations manually only if they're dynamic
-## translations that can't be statically extracted.
+## Add new message manually only if they're dynamic
+## message that can't be statically extracted.
 ##
 ## Run "mix gettext.extract" to bring this file up to
 ## date. Leave "msgstr"s empty as changing them here has no
whatyouhide commented 1 year ago

@krns thanks for the report!

First of all, I don't think that comment change is necessary, and it's going to cause a bunch of unnecessary changes in people's projects. @maennchen, I'll revert that to the previous comment.

@krns, do you happen to have a minimal reproducing project that I could use to debug this?

maennchen commented 1 year ago

@whatyouhide That change was part of an effort to only use the term „message“ and not „translation“ in the whole codebase. This should make understanding the documentation simpler, since we always use the same term for messages.

„message“ is the name that gettext itself uses.

In this case, I personally think consistency is worth more than a tiny change produced in a file. I think it would be reasonable for a user of this library to update the .pot? files before expecting them to be up to date.

WDYT?

krns commented 1 year ago

This is a minimal reproducing project: https://github.com/krns/elixir-gettext-0.21.0-test

Steps to reproduce:

maennchen commented 1 year ago

@whatyouhide We miss-interpreted the issue:

The new comment is not forced onto users at all. It will only matter for new .pot files. The issue instead comes from #335 (write_reference_comments => false)

krns commented 1 year ago

Sorry for the confusion and thanks for the fix! I have just successfully tested the main branch in my application.

maennchen commented 1 year ago

@whatyouhide Should we directly do a patch release? I think this might impact a few users...