If changes to .po file are discarded (accidentally), they're not added back

[saveman71]

How to reproduce:

https://github.com/saveman71/gettext-repro-cases/tree/missing-po-if-checked-out

In gettext_mwe.ex, add a second gettext call to the gettext function:

  gettext("First string")
  gettext("Second string")

Run mix gettext.extract --merge and observe that both po and pot files are updated.

Run git checkout -- priv/gettext/fr/LC_MESSAGES/default.po to "accidentally" reset the po file.

Run mix gettext.extract --merge and observe that nothing is updated.

From that point on, if you commit and push, gettext won't catch the new strings until the code is modified.

Expected behaviour

If you change/discard the changes to default.pot, gettext will overwrite your changes based on what's found.
I expect the same thing if I change/discard changes in default.po.

[whatyouhide]

@maennchen heyo, you have any time to look at this by any chance? This week I won't, but I might do a cycle on Gettext next week if you're busy 🙃

[maennchen]

@whatyouhide I just got a big task in and not much time until the deadline. I can have a look next week the earliest as well.

[maennchen]

I had a look at the issue and I think this is working as intended:

• mix gettext.merge merges .pot => .po
• mix gettext.extract --merge does the extraction and only applies the merge for the files that changed in the extraction

[saveman71]

Thank you for investigating the issue. So I assume we can't do anything about the above scenario? If so we can close the issue

[maennchen]

We could change the behavior to always merge even if the .pot is unchanged. I'm not sure though if we should.
What do you think @whatyouhide?

[whatyouhide]

@maennchen yeah that's an easy way out and worth a try I think 👍

[whatyouhide]

@maennchen are you going to work on this by any chance?

[maennchen]

@whatyouhide I‘m also a bit busy at the moment. So I wasn’t planning to soon.

[maennchen]

@whatyouhide I looked quickly how this could be implemented.

Currently we remove unchanged files from the list in Extractor.pot_files/2:

gettext/lib/gettext/extractor.ex

Line 225 in e7a2c2e

|> Enum.reject(&match?({_, {_, :unchanged}}, &1))

The gettext.extract task does not take the priv directory path as gettext.merge does.

In conclusion I think we either have to change the Extractor.pot_files/2 function (add option to include unchanged or a breaking change) or we have to take extra arguments for the gettext.extract mix task.

How would you want to implement this?

[whatyouhide]

@maennchen what would the extra arguments for the gettext.extract task be?

[maennchen]

@whatyouhide The path to the .pot files as in gettext.merge.

But this feels quite hacky since theoretically gettext.extract could extract for multiple Gettext modules and could therefore have multiple paths.

[whatyouhide]

I re-read this issue all over again to regain context. What we're doing right now makes sense to me, because extract is about extraction and not merging. I think if you run mix gettext.merge, then we still update the .po file that was changed.

However, I think the intent of mix gettext.extract --merge was to essentially mimic gettext.extract + gettext.merge. In that case, we'll want mix gettext.extract --merge to also override the .po files, even if the POT files were unchanged. Wdyt?

[maennchen]

@whatyouhide That is correct.

It would be trivial to implement if Extractor.pot_files/2 would return [{"path1.pot", :changed, "contents..."}, {"path2.pot", :unchanged, "contents..."}] instead of just [{"path1.pot", "contents..."}].

[whatyouhide]

@maennchen Extractor.pot_files/2 is not public, so we can change it to return [{path, :changed, contents}, ...] right?

[maennchen]

@whatyouhide Oh, oops. I overlooked the @moduledoc false.

Easy fix then :)

[whatyouhide]

@maennchen I'd like to release 1.0 around the end of the year/start of new year, so let's not release a new version with this change and let's wait for 1.0 direclty.

[maennchen]

@whatyouhide I think it would make sense to tackle the umbrella issue before releasing a 1.0 eince that could potentially create larger problems / changes.