Translator comments aren't picked up when started on second line of comment block

[garrett-eclipse]

Hello,

I noticed this with #47689 that the Noto Serif:400,400i,700,700i strings translator comment wasn't being picked up.

Initially I assumed it was because 'Translators:' prefix was capitalized. Doing a mass search though I found this not to be the case and instead the common denominator was the translator comments that started on the second line weren't being picked up, for example;

<?php
/*
 * Translators: Use this to specify the proper Google Font name and variants
 * to load that is supported by your language. Do not translate.
 * Set to 'off' to disable loading.
 */
$font_family = _x( 'Noto Serif:400,400i,700,700i', 'Google Font Name and Variants' );

Example of missing translator comment - ​https://translate.wordpress.org/projects/wp/dev/en-ca/default/?filters%5Bstatus%5D=either&filters%5Boriginal_id%5D=6929622&filters%5Btranslation_id%5D=59173298

I'm seeing this throughout core and the bundled themes but have a few quick questions before I supply a patch;

• How are bundled themes patched? Can I just supply one large patch file for all instances across core and themes? Or do I need separate patches, what about tickets do I need 1 ticket per theme?
• Is this better fixed by the parser to catch these instances?
• While I'm at it should I standardize the 'translators:' prefix to be lowercase for consistency?

Thank you

[ocean90]

This should probably be reported to ​https://github.com/wp-cli/i18n-command/.

But there's actually a test that shows that it should working as is, see ​https://github.com/wp-cli/i18n-command/blob/ba1cdfd9b7e99472e4599aacee4b9a1bd34fb936/features/makepot.feature#L517-L521.

[casiepa]

Whatever comes out of this, make sure to keep the documentation updated:
​https://developer.wordpress.org/themes/functionality/internationalization/#descriptions

[SergeyBiryukov]

Is this better fixed by the parser to catch these instances?

Yes. This was previously fixed in #30972 for trunk/tools/i18n, apparently the wp-cli command needs the same fix.

[ocean90]

Reported in ​https://github.com/wp-cli/i18n-command/issues/169.

[garrett-eclipse]

Thanks @ocean90 & @SergeyBiryukov for the history and reporting this upstream.

Replying to casiepa:

Whatever comes out of this, make sure to keep the documentation updated:
​https://developer.wordpress.org/themes/functionality/internationalization/#descriptions

Concerning @casiepa's comment should the documentation be updated to also provide an example of a multi-line Translator comment?

*And does it matter the capitalization of the translator: prefix. Should that be made consistent across core or is it irrelevant.

[SergeyBiryukov]

Replying to garrett-eclipse:

*And does it matter the capitalization of the translator: prefix. Should that be made consistent across core or is it irrelevant.

The capitalization doesn't matter, though it would probably make sense to be consistent in core :) Perhaps it's something WPCS could check.

[garrett-eclipse]

Replying to SergeyBiryukov:

Replying to garrett-eclipse:

*And does it matter the capitalization of the translator: prefix. Should that be made consistent across core or is it irrelevant.

The capitalization doesn't matter, though it would probably make sense to be consistent in core :) Perhaps it's something WPCS could check.

Thanks @SergeyBiryukov seems you caught the capitalization in changeset 46932. Does it matter that some start on the first line of the comment right after /* and others start on the second line?

I made a comment on WPCS github about checking capitalization here - ​https://github.com/WordPress/WordPress-Coding-Standards/issues/1760#issuecomment-527547958

Replying to casiepa :

Whatever comes out of this, make sure to keep the documentation updated:
​https://developer.wordpress.org/themes/functionality/internationalization/#descriptions

@casiepa I don't have the ability to update this document, are you able to make two tweaks?

1. Update 'It must start with the words translators: ' to be 'It must start with the word translators:, in all lowercase, '
2. Add a multi-line example something like;

   <?php
   /* 
    * translators: Replace with a city related to your locale.
    * Test that it matches the expected location and has upcoming
    * events before including it. If no cities related to your
    * locale have events, then use a city related to your locale
    * that would be recognizable to most users. Use only the city
    * name itself, without any region or country. Use the endonym
    * (native locale name) instead of the English name if possible.
    */
   ?>
   <input id="community-events-location" class="regular-text" type="text" name="community-events-location" placeholder="<?php esc_attr_e( 'Cincinnati' ); ?>" />
   

Thank you

[garrett-eclipse]

FYI - The documentation was updated to introduce the multi-line example here;
​https://developer.wordpress.org/themes/functionality/internationalization/#descriptions