Extract strings for translation from theme.json #254

oandregal · 2021-06-21T09:38:16Z

Closes #224

Setup

See https://make.wordpress.org/cli/handbook/contributions/pull-requests/#setting-up
To run the behat tests you also need a working mysql locally:
- Install the packages if you don't have them. In linux: sudo apt install jq mysql-server mysql-client php-mysql
- composer install && composer behat => it didn't setup the database for me (mysql 8) so I did;
  - sudo mysql -u root
  - CREATE DATABASE IF NOT EXISTS wp_cli_test;
  - CREATE USER 'wp_cli_test'@'localhost' IDENTIFIED WITH mysql_native_password BY 'password1';
  - GRANT ALL PRIVILEGES ON wp_cli_test.* TO "wp_cli_test"@"localhost"

How to test

Check out this PR locally and cd to the directory.
composer install
Create a new directory called foo-theme and create a theme.json file within with the following contents:

      {
        "version": "1",
        "settings": {
          "color": {
            "palette": [
              { "slug": "black", "color": "#000000", "name": "Black" }
            ]
          }
        }
      }

Run vendor/bin/wp i18n make-pot foo-theme
Verify that there's a foo-theme/foo-theme.pot file and that the "Black" string is present as well as its context "Color name". You should see something like:

msgctxt "Color name"
msgid "Black"
msgstr ""

Stress test the network connection

Switch to offline and verify that the file is still generated properly using the local file. Upon executing the command, you should see the following message:

PHP Warning:  file_get_contents(): php_network_getaddresses: getaddrinfo failed: Temporary failure in name resolution in /home/andres/src/i18n-command/src/ThemeJsonExtractor.php on line 106
PHP Warning:  file_get_contents(https://raw.githubusercontent.com/WordPress/gutenberg/wp/trunk/lib/experimental-i18n-theme.json): failed to open stream: php_network_getaddresses: getaddrinfo failed: Temporary failure in name resolution in /home/andres/src/i18n-command/src/ThemeJsonExtractor.php on line 106
Success: POT file successfully generated!

oandregal · 2021-06-21T10:26:36Z

src/theme-i18n.json

@@ -0,0 +1,51 @@
+{


Ideally, wp-cli would take this file from the WordPress core if it exists. If it doesn't, it shouldn't lookup for strings in the directory, as it means that the WordPress version in use doesn't support theme.json.

Note that wp i18n make-pot is usually run against a single theme's directory, with no knowledge of a WP installation around it.

Ideally the theme-i18n.json file is requested from an external API, e.g. WordPress.org, based on the version field in the theme.json file.

It shouldn't be bundled with WP-CLI for sure.

BTW, the same would need to be done for block.json.

As per #224 (comment) by @schlessera my understanding is that we want to access this from WordPress core. What would be the preferred way to do so?

(same as below, didn't see Pascal's comment when posting mine because I hadn't reloaded the browser)

It may take a bit to create and deploy an endpoint in wordpress.org. Would you think it's reasonable to bundle this file with 2.5.1 so we then can update translate.wordpress.org for these strings to be translatable for the WordPress 5.8 release? We can work on that endpoint later, with more time.

In 6d850a1 I'm pulling the file from Gutenberg's GitHub repo. I've also added a fallback to the local file if there was an issue and it is empty. I've tried using WordPress.org SVN but it reports a 403.

If this unblocks us to ship this, let's do it. My preference, however, would be to start only with the local file as it less fragile.

Added some stress testing instructions in the description.

I agree with @nosolosw that for the first version a bundled schema file should only be used to not depend on an unstable URL like it's implemented right now. It's unstable because there's no version at all. In case there's a change to the structure of the file or the file is moved we'd have to update WP-CLI anyway right?
I don't think it's responsible to block/delay the command from working in this case just because a not yet published update isn't supported.

A compromise might be to add a --prefer-local-schema argument which can be used by environments which require a stable behaviour?

@schlessera Any thoughts on a --prefer-local-schema argument?

@ocean90 Sounds useful, especially if it accepts a file path (which would default to the included one). This would let you easily experiment with different schema.

src/ThemeJsonExtractor.php

swissspidy · 2021-06-22T09:02:00Z

src/ThemeJsonExtractor.php

+	 *
+	 * @return mixed The value from the path specified.
+	 */
+	private function array_get( $array, $path, $default = null ) {


We might want to move those utils to https://github.com/wp-cli/wp-cli or at least extract them to a new abstract class so it can be used by BlockExtractor

Happy to create a class for this to be available in other places. I don't see any potential use of them in BlockExtractor, though, so perhaps we can extract them when we need them?

oandregal · 2021-06-24T08:58:39Z

@swissspidy @schlessera do you think this is ready? The only bit under discussion is whether the theme-18n.json can be bundled with this command until we find a better path forward.

oandregal · 2021-06-28T08:08:54Z

@swissspidy @schlessera what would be the next steps for this? Anything I can do to move things forward?

swissspidy · 2021-06-28T08:37:41Z

I'd love to get some additional feedback from @schlessera

Once merged, it would be good to follow up with another PR to improve the block.json extractor in similar fashion, re-using some of the code.

swissspidy · 2021-06-30T12:39:46Z

src/ThemeJsonExtractor.php

+		// Using the WordPress.org SVN repo resolved to a 403.
+		// $file_structure = self::read_json_file( 'http://develop.svn.wordpress.org/trunk/src/wp-includes/theme-i18n.json', $context );
+		$file_structure = self::read_json_file( 'https://raw.githubusercontent.com/WordPress/gutenberg/wp/trunk/lib/experimental-i18n-theme.json', $context );


https://raw.githubusercontent.com/WordPress/wordpress-develop/master/src/wp-includes/theme-i18n.json?

I've used the one in the wp/trunk branch because that's the one that only changes with WordPress core. I wish I could have used wp/5.8 but that branch is not created until WordPress 5.8 is released like WordPress.org's SVN.

So https://raw.githubusercontent.com/WordPress/wordpress-develop/5.8/src/wp-includes/theme-i18n.json perhaps?

Locking it to 5.8 doesn't help at all, then it can just as well be statically committed here. The idea of an outside source is about getting the changes from Core without needing to release another WP-CLI version in sync. If you lock the version that WP-CLI looks for, there's not much of a point.

It should be either:

always retrieve the translation info for the latest development version: or

always retrieve the translation info for the latest stable version.

Any other version is basically the same as having a hard-coded version inside of WP-CLI.

Can you share more details about what type of changes wouldn't work and why can't we make them work?

Any structural change to theme.json can affect this. A few ones I can think of right now: translatable strings need to be part of a leaf node that is an unkeyed array, the format supports variability in node names encoded via * but only to one level, can't translate several strings from the same leaf, etc.

It's not that these are planned changes. My point is that, in the current state of things, the unit that best encapsulates the behavior is the file + the code used to parse it. Hence, pulling the file from elsewhere is risky: can work with some changes, and won't work with others.

hi @schlessera is there anything I can help with to move this forward?

Not at the moment. I'll try to find the time to do a thorough final review, and then it will be good to be merged.
What I'll be looking at most of all, is ways in which this can break if the format changes, and ensure we have graceful handling for such cases (instead of causing a fatal or - even worse - succeeding with the wrong output).

Feel free to push changes to this PR if you want to. I'm also happy to add more graceful handling if we come up with anything.

oandregal · 2021-07-08T07:50:14Z

@schlessera @swissspidy I wanted to share that the last fortnight of July (15th onwards) I'll have very limited availability. JFYI so you know what to expect from me in terms of time to respond/help/feedback/etc. You are very welcome to own this PR, should I not be around.

youknowriad · 2021-07-13T10:24:33Z

Hi folks, what's the status for this one, do you think we're still on track to land this in 5.8?

src/ThemeJsonExtractor.php

swissspidy · 2021-07-14T09:47:03Z

Hi folks, what's the status for this one, do you think we're still on track to land this in 5.8?

Just awaiting final review from @schlessera

oandregal commented Jun 21, 2021

View reviewed changes

src/ThemeJsonExtractor.php Outdated Show resolved Hide resolved

oandregal commented Jun 21, 2021

View reviewed changes

src/ThemeJsonExtractor.php Show resolved Hide resolved

oandregal marked this pull request as ready for review June 21, 2021 11:12

oandregal requested a review from a team as a code owner June 21, 2021 11:12

jorgefilipecosta reviewed Jun 21, 2021

View reviewed changes

src/ThemeJsonExtractor.php Outdated Show resolved Hide resolved

src/ThemeJsonExtractor.php Show resolved Hide resolved

swissspidy reviewed Jun 22, 2021

View reviewed changes

swissspidy requested a review from schlessera June 22, 2021 09:02

swissspidy approved these changes Jun 24, 2021

View reviewed changes

swissspidy reviewed Jun 30, 2021

View reviewed changes

ocean90 reviewed Jul 14, 2021

View reviewed changes

src/ThemeJsonExtractor.php Outdated Show resolved Hide resolved

oandregal added 13 commits July 19, 2021 16:01

Add skeleton to implement string extraction from theme.json

a2f369b

Extract strings using intermediate theme-i18n file

f8a4dc7

Iterate over each preset individually

4ba7014

Fix lint issue: rename _wp_array_get to array_get

2324979

Fix variable name

e5fa7d1

Add behat tests

ac47a45

More behat tests

44206c6

Add PHPDoc

183faa0

Make array_get static

3689a1e

Pull the theme-i18n.json file from elsewhere

145db74

Use URI from wordpress-develop

f57712d

Use master from wordpress-develop

37bf31a

Should fall back to local if remote is empty

f21497a

oandregal and others added 9 commits July 19, 2021 16:01

Fix lint issues

68949cb

Clean up loop

1c4bf87

Use short array notation

50451e5

Replace is_null() with null ===

207df59

Remove unneeded variable

9f2a19b

Fix rebase conflict in feature file

df97093

Fix broken loop in path extraction

ccea6c3

Move fallback file

f286473

Use Utils\http_request for remote request

b3bede0

schlessera approved these changes Jul 19, 2021

View reviewed changes

schlessera added the command:i18n-make-pot label Jul 19, 2021

schlessera added this to the 2.2.9 milestone Jul 19, 2021

schlessera merged commit cce44b4 into wp-cli:master Jul 19, 2021

schlessera added a commit that referenced this pull request Jan 6, 2022

Merge pull request #254 from nosolosw/extract/strings-from-theme-json

c43831d

oandregal mentioned this pull request Mar 10, 2022

Translate title field from theme.json #306

Merged

oandregal mentioned this pull request Apr 14, 2023

Allow register/unregister block style variations from theme.json WordPress/gutenberg#49827

Closed

oandregal mentioned this pull request Jun 13, 2024

theme.json: find title string within styles.blocks.variations #405

Merged

oandregal mentioned this pull request Dec 19, 2024

Fix theme.json string extraction #423

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extract strings for translation from theme.json #254

Extract strings for translation from theme.json #254

oandregal commented Jun 21, 2021 •

edited

Loading

oandregal Jun 21, 2021

swissspidy Jun 22, 2021

oandregal Jun 22, 2021

oandregal Jun 22, 2021

oandregal Jun 22, 2021

oandregal Jun 30, 2021

oandregal Jun 30, 2021

ocean90 Jul 14, 2021

ocean90 Jul 20, 2021

schlessera Jul 20, 2021

swissspidy Jun 22, 2021

oandregal Jun 22, 2021

oandregal commented Jun 24, 2021

oandregal commented Jun 28, 2021

swissspidy commented Jun 28, 2021

swissspidy Jun 30, 2021

oandregal Jun 30, 2021

swissspidy Jun 30, 2021

schlessera Jul 1, 2021

schlessera Jul 1, 2021

schlessera Jul 1, 2021

oandregal Jul 1, 2021

oandregal Jul 6, 2021

schlessera Jul 6, 2021

oandregal Jul 6, 2021

oandregal commented Jul 8, 2021 •

edited

Loading

youknowriad commented Jul 13, 2021

swissspidy commented Jul 14, 2021

Extract strings for translation from theme.json #254

Extract strings for translation from theme.json #254

Conversation

oandregal commented Jun 21, 2021 • edited Loading

Setup

How to test

Stress test the network connection

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oandregal commented Jun 24, 2021

oandregal commented Jun 28, 2021

swissspidy commented Jun 28, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oandregal commented Jul 8, 2021 • edited Loading

youknowriad commented Jul 13, 2021

swissspidy commented Jul 14, 2021

oandregal commented Jun 21, 2021 •

edited

Loading

oandregal commented Jul 8, 2021 •

edited

Loading