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

Babel Makepot: Translator comments have been extracted from the file #9440

Merged
merged 4 commits into from
Feb 7, 2019

Conversation

akirk
Copy link
Member

@akirk akirk commented Aug 29, 2018

Description

In the POT file generated by Gutenberg the translator comments are extracted into regular comments (denoted with just a #) while they should be denoted with a #.

This is a mixup on the term "translator" comments. As can be seen in section "2.2.1 Extracted Comments" of The PO Format, the comments denoted with translators: are actually called "extracted comments" in the PO file format.

How has this been tested?

  1. Run npm run build to create the POT file in languages/gutenberg.pot
  2. Run npx pot-to-php ./languages/gutenberg.pot ./languages/gutenberg-translations.php gutenberg to generate the PHP file

Types of changes

Janitorial

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.

@akirk
Copy link
Member Author

akirk commented Aug 29, 2018

See also https://www.gnu.org/software/gettext/manual/html_node/PO-Files.html

Comment lines starting with #. contain comments given by the programmer, directed at the translator; these comments are called extracted comments because the xgettext program extracts them from the program’s source code.

@aduth aduth self-requested a review August 29, 2018 17:36
@aduth aduth added Internationalization (i18n) Issues or PRs related to internationalization efforts [Tool] Babel plugin makepot /packages/babel-plugin-makepot labels Aug 29, 2018
@gziolo gziolo requested a review from swissspidy January 31, 2019 18:10
@gziolo gziolo added the Needs Decision Needs a decision to be actionable or relevant label Feb 7, 2019
@gziolo
Copy link
Member

gziolo commented Feb 7, 2019

@swissspidy what's the future of @wordpress/babel-plugin-makepot? Does it make sense to land these changes proposed?

@gziolo gziolo added this to the 5.1 (Gutenberg) milestone Feb 7, 2019
@gziolo gziolo removed the Needs Decision Needs a decision to be actionable or relevant label Feb 7, 2019
@aduth
Copy link
Member

aduth commented Feb 7, 2019

Confusing indeed. For reference, the treatment of the extracted property occurs in gettext-parser here:

https://github.com/smhg/gettext-parser/blob/4377b96dcceef94fecac8d17bb9644d66adf8f91/lib/pocompiler.js#L64-L79

I'll defer to you two on this one, and the referenced documentation seems to reaffirm the correction.

I've force-pushed a rebased copy of the branch to be able to add a CHANGELOG entry as a fix.

I contemplated whether to consider the module.exports.getTranslatorComment removal as a breaking change, but in usage as a Babel plugin it has no effect. Those exports are used exclusively for the unit tests.

@aduth aduth merged commit 1481251 into WordPress:master Feb 7, 2019
youknowriad pushed a commit that referenced this pull request Mar 6, 2019
…9440)

* Comments have been extracted from the file

* Update POT comment behavior in PHP file generation

* Convert function names

* Babel Plugin Makepot: Add CHANGELOG entry for translator -> extracted
youknowriad pushed a commit that referenced this pull request Mar 6, 2019
…9440)

* Comments have been extracted from the file

* Update POT comment behavior in PHP file generation

* Convert function names

* Babel Plugin Makepot: Add CHANGELOG entry for translator -> extracted
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Internationalization (i18n) Issues or PRs related to internationalization efforts [Tool] Babel plugin makepot /packages/babel-plugin-makepot
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants