WordPress / wporg-main-2022

A block-based child theme for WordPress.org, plus local environment
72 stars 26 forks source link

Parser: Add a note for translators when a string contains a shortcode #289

Closed ryelle closed 1 year ago

ryelle commented 1 year ago

Fixes #284 — This updates the content parser to add a translator comment on strings with shortcodes.

https://github.com/WordPress/wporg-main-2022/blob/7a56dec288104f32a0feb3daec0a01105dd56683/source/wp-content/themes/wporg-main-2022/patterns/about-requirements.php#L18-L31

How to test the changes in this Pull Request:

  1. Run the phpunit tests
  2. Build the patterns with yarn build:patterns (should be no change since this was done in https://github.com/WordPress/wporg-main-2022/commit/e962cc3984b7df09a319eaad0106673237261bce, except for the 6-3 page which is still in progress)
dd32 commented 1 year ago

The linting errors seem.. silly IMHO. Squiz.PHP.EmbeddedPhp.ContentBeforeOpen, Squiz.PHP.EmbeddedPhp.ContentAfterEnd

I'm personally in favour of excluding these where we exclude their siblings here: https://github.com/WordPress/wporg-repo-tools/blob/trunk/configs/phpcs.xml.dist#L28-L31

Instead of

- <p><?php _e( .... ); ?></p>
+ <p><?php
+ /* translators: [market_share] is a shortcode and should not be translated. */
+ _e( .... );
+ ?></p>

it wants

- <p><?php _e( '..... ?></p>
+ <p>
+ <?php
+ /* translators: [market_share] is a shortcode and should not be translated. */
+ _e( .... );
+ ?>
+ </p>

Easily fixed with \ns but just seems like a weird sniff.

ryelle commented 1 year ago

I think I'll exclude that rule for the pattern files (like this), but leave the shared projects config alone for now.