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

Dump flags after references in PO files #310

Merged
merged 1 commit into from
May 19, 2022
Merged

Dump flags after references in PO files #310

merged 1 commit into from
May 19, 2022

Conversation

wpiekutowski
Copy link
Contributor

Summary

This PR changes the order in which flags and references are dumped into PO files, so that flags are placed after references. This behavior seems to be common among a small sample of non-Elixir PO files I saw.

Background

In of the projects, I'm going to use transifex.com to enable easier management of translations. However, I've noticed that they output comments in PO files in a different order than Elixir's gettext. They first output file references and then flags, while Elixir's gettext does it the other way. This results in some unnecessary churn in git history, because these 2 lines are swapped back and forth. Folks at transifex.com insist that Elixir's gettext is outputing comments in the wrong order:

After investigation, our team concluded that the resulting order of the comments is not caused by a bug in our parser. Instead, our parser while following the PO file entry schematic structure, arranges the comments in the order specified in the GNU guidelines. You can find more information about this here.

Since these are the best practices for PO file handling our advice would be to follow them to avoid any future issues.

However, I wasn't able to find any explicit mention of the "standard" comment order in PO files, except for the example PO file in the linked gettext doc. My short GitHub research indicated that indeed PO files with flags (that I've found) had them placed after the references, so that gives some additional credibility to transifex claim.

@whatyouhide
Copy link
Contributor

This will potentially generate large diffs for folks with lots of PO files. It's alright, just something that we might want to call out in the CHANGELOG. 🙃

Thanks @wpiekutowski! ❤️

@whatyouhide whatyouhide merged commit b6607a6 into elixir-gettext:main May 19, 2022
@wpiekutowski
Copy link
Contributor Author

Thanks for the quick review and merge!

I agree it's a good idea to explain these large diffs in the CHANGELOG, although I suppose they won't be significantly bigger than the ones resulting from 0.19.1 ex-autogen -> elixir-autogen rename.

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.

None yet

3 participants