WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#47691 closed defect (bug) (reported-upstream)

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

Reported by: garrett-eclipse Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: I18N Keywords:
Focuses: Cc:

Description

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

Change History (9)

#2 follow-up: @casiepa
2 years ago

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

#3 @SergeyBiryukov
2 years ago

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.

#4 @ocean90
2 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to reported-upstream
  • Status changed from new to closed

#5 in reply to: ↑ 2 ; follow-up: @garrett-eclipse
2 years ago

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.

#6 in reply to: ↑ 5 ; follow-up: @SergeyBiryukov
2 years ago

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.

#7 in reply to: ↑ 6 @garrett-eclipse
2 years ago

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

This ticket was mentioned in Slack in #docs by garrett-eclipse. View the logs.


2 years ago

#9 @garrett-eclipse
2 years ago

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

Note: See TracTickets for help on using tickets.