Closed strarsis closed 2 years ago
I just tested this with the current version in the main
branch and also the improved version from #319 and could not reproduce this. String extraction works just fine.
So I suppose #306 fixed this.
Can you try installing the latest version of this package via wp package install git@github.com:wp-cli/i18n-command.git
or by cloning this repository and see if it's still broken for you?
@swissspidy: After installing the latest version of wp i18n
(as you described), the PHP warning went away.
However, the translations strings are not extracted from the theme.json
:
{
"$schema": "https://schemas.wp.org/trunk/theme.json",
"version": 2,
"settings": {
"color": {
"palette": [
{
"slug": "primary",
"color": "#05536c",
"name": "Dark turquoise"
},
{
"slug": "turquoise-light",
"color": "#1aaba8",
"name": "Light turquoise"
},
{
"slug": "sky-blue",
"color": "#cff6ff",
"name": "Sky blue"
},
{
"slug": "beige",
"color": "#eed9c8",
"name": "Beige"
},
{
"slug": "sand",
"color": "#e2cb88",
"name": "Sand"
},
{
"slug": "grey-light",
"color": "#fff8f9",
"name": "Light grey"
},
{
"slug": "purple",
"color": "#733e90",
"name": "Purple"
},
{
"slug": "orange",
"color": "#de6a16",
"name": "Orange"
},
{
"slug": "white",
"color": "#fff",
"name": "White"
},
{
"slug": "black",
"color": "#000",
"name": "Black"
}
],
"custom": false,
"customGradient": false,
"defaultPalette": false,
"defaultGradients": false
},
"custom": {
"spacing": {},
"typography": {
"font-size": {},
"line-height": {},
"fontFamily": "El Messiri"
}
},
"spacing": {
"padding": true,
"units": [
"px",
"%",
"em",
"rem",
"vw",
"vh"
]
},
"typography": {
"customFontSize": false,
"fontSizes": [
{
"slug": "regular",
"size": "1.5rem",
"name": "Regular"
},
{
"slug": "huge",
"size": "3rem",
"name": "Huge"
},
{
"slug": "larger",
"size": "1.719rem",
"name": "Larger"
},
{
"slug": "large",
"size": "1.375rem",
"name": "Large"
},
{
"slug": "medium",
"size": "1.125rem",
"name": "Medium"
},
{
"slug": "smaller",
"size": "0.813rem",
"name": "Smaller"
},
{
"slug": "small",
"size": "0.938rem",
"name": "Small"
},
{
"slug": "tiny",
"size": "0.75rem",
"name": "Tiny"
}
],
"fontFamilies": [
{
"fontFamily": "'El Messiri', sans-serif",
"slug": "el-messiri",
"name": "El Messiri"
}
]
}
},
"styles": {
"color": {
"background": "var(--wp--preset--color--grey-light)",
"text": "var(--wp--preset--color--primary)"
}
}
}
The palette
and fontSizes
names are not extracted.
This is fixed by #319 as far as I can tell.
Actually, on the main
branch with this theme.json
you provided I get the following POT file:
# Copyright (C) 2022
# This file is distributed under the .
msgid ""
msgstr ""
"Project-Id-Version: foo theme\n"
"Report-Msgid-Bugs-To: https://wordpress.org/support/theme/foo-theme\n"
"Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
"Language-Team: LANGUAGE <LL@li.org>\n"
"MIME-Version: 1.0\n"
"Content-Type: text/plain; charset=UTF-8\n"
"Content-Transfer-Encoding: 8bit\n"
"POT-Creation-Date: 2022-05-11T06:06:44+00:00\n"
"PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
"X-Generator: WP-CLI 2.6.0\n"
"X-Domain: foo-theme\n"
#. Theme Name of the theme
msgid "foo theme"
msgstr ""
#: theme.json
msgctxt "Font size name"
msgid "Regular"
msgstr ""
#: theme.json
msgctxt "Font size name"
msgid "Huge"
msgstr ""
#: theme.json
msgctxt "Font size name"
msgid "Larger"
msgstr ""
#: theme.json
msgctxt "Font size name"
msgid "Large"
msgstr ""
#: theme.json
msgctxt "Font size name"
msgid "Medium"
msgstr ""
#: theme.json
msgctxt "Font size name"
msgid "Smaller"
msgstr ""
#: theme.json
msgctxt "Font size name"
msgid "Small"
msgstr ""
#: theme.json
msgctxt "Font size name"
msgid "Tiny"
msgstr ""
#: theme.json
msgctxt "Font family name"
msgid "El Messiri"
msgstr ""
#: theme.json
msgctxt "Color name"
msgid "Dark turquoise"
msgstr ""
#: theme.json
msgctxt "Color name"
msgid "Light turquoise"
msgstr ""
#: theme.json
msgctxt "Color name"
msgid "Sky blue"
msgstr ""
#: theme.json
msgctxt "Color name"
msgid "Beige"
msgstr ""
#: theme.json
msgctxt "Color name"
msgid "Sand"
msgstr ""
#: theme.json
msgctxt "Color name"
msgid "Light grey"
msgstr ""
#: theme.json
msgctxt "Color name"
msgid "Purple"
msgstr ""
#: theme.json
msgctxt "Color name"
msgid "Orange"
msgstr ""
#: theme.json
msgctxt "Color name"
msgid "White"
msgstr ""
#: theme.json
#: styles/foo.json
msgctxt "Color name"
msgid "Black"
msgstr ""
Looks all correct to me.
@swissspidy: Alright, the --include="app,resources"
parameter made wp i18n
ignore the theme.json
.
Adding theme.json
to the include
parameter made wp i18n
also scan the theme.json
again:
--include="app,resources,theme.json"
That include
excludes theme.json
is probably intentional?
Edit: This seems to be new, because when explicitly adding theme.json
to include
parameter,
the foreach
PHP warning appears again, but the translation strings are extracted, so it doesn't appear to cause issues in the output.
Edit 2: That phar:///usr/local/bin/wp/vendor/wp-cli/i18n-command/src/ThemeJsonExtractor.php
is shown in the path indicates that not the latest version of wp i18n
from GitHub is used, but rather from latest release instead?
I successfully ran the wp package install git@github.com:wp-cli/i18n-command.git
command.
You can also try cloning this repository, running composer install
and then vendor/bin/wp i18n make-pot
to make sure you are really using the latest and greatest.
@swissspidy: Yes! This approach worked. No PHP warnings anymore.
That theme.json
has to be explicitly listed in --include
is intentional?
When --include
is used, it acts as a whitelist, excluding everything else that is not listed?
I think so, yes
Hey! 👋
I've tried the above approach but it didn't work for me - I still get the error after installing the package manually (wp package install git@github.com:wp-cli/i18n-command.git
).
What I've noticed is that I don't see the warning when using the binary from ./vendor/bin/wp
. If I use the global wp
command installed via Brew, the warning is there. 🦀
@m1r0 This is normal because when you are using a global wp
command then you are using older version of i18n-command
.
Not sure why the global wp
isn't picking up that updated command for you. You might want to ask that question on the #wp-cli Slack.
Otherwise we're hopefully releasing a new proper version of this command (see https://github.com/wp-cli/i18n-command/milestone/23), maybe that helps.
The bug will be fixed there, so there's nothing else left to be done.
Got it, thanks!
@swissspidy: A new release would be great! Currently I have to use the cloned vendor/bin/wp
command.
Edit: That release would hopefully also include https://github.com/wp-cli/i18n-command/pull/316 for the new merge feature. that would be great.
@swissspidy: Addendum: Besides theme.json
, the patterns/
directory also has to be included explicitly when --include
is used.
Bug Report
Describe the current, buggy behavior The (valid)
theme.json
is not processed bywp i18n
, no translation strings are extracted from it. A PHP warning occurs:(Valid )
theme.json
that causes the issue:Describe how other contributors can replicate this bug
theme.json
:wp i18n
inside the theme directory:wp i18n make-pot . ./resources/lang/theme.pot
Notice the PHP warning and that translation strings intheme.json
are not extracted.Describe what you would expect as the correct outcome No PHP warning and translation strings extracted from
theme.json
.Let us know what environment you are running this on
Provide a possible solution Probably some mismatch of expected
theme.json
data structure and extractor logic that needs to be fixed.