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

[maennchen]

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

Easy fix then :)

[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

[maennchen]

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

[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 🙃

[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 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?

[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.