Skip to content

Include theme.json; patterns/ in translation scan command #3076

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
retlehs merged 5 commits into from
Dec 8, 2023
Merged

Include theme.json; patterns/ in translation scan command #3076

retlehs merged 5 commits into from
Dec 8, 2023

Conversation

@strarsis
Copy link
Contributor

@strarsis strarsis commented Jul 8, 2022 •
edited
Loading

It turns out that when using --include with wp i18n the theme.json file and the patterns directory
need to be explicitly included, too, as those would be skipped otherwise.

Edit:
The latest wp i18n version (>=2.4.0) is needed for some bugfixes of otherwise blocking bugs.
Until wp CLI is also newly released with the new wp i18n version bundled,
the latest wp i18n version can be installed independently from the wp CLI version by using
wp package install wp-cli/i18n-command:dev-main
and then just using it as usual (wp i18n).
Edit (October 2023): Current stable wp cli 2.8.1 contains a recent enough wp i18n.

@strarsis
Copy link
Contributor Author

strarsis commented Oct 12, 2022 •
edited
Loading

With recent bud the theme.json can also be generated by bud (which is the case by default).
For having an up-to-date theme.json scanned by wp i18n, the build npm script must run before the translate:pot (or invoked by it) command. Or a separate script that makes bud regenerate only the theme.json could be added that can be invoked before translate:pot (or by it).

@strarsis
Copy link
Contributor Author

strarsis commented Feb 22, 2023

@retlehs: Latest stable WP CLI appears to work fine with the used commands now, so this can be merged.

To make this more robust, bud should re-generate the theme.json before the translate:pot script.
This can be achieved by just calling the build script, but wouldn't be just re-generating the theme.json be better than building the whole project? Then bud needs a dedicated command that just re-generates the theme.json (when theme.json generation is enabled in bud, otherwise it can be a no-op).

@strarsis
Copy link
Contributor Author

strarsis commented Feb 23, 2023 •
edited
Loading

@kellymears: A dedicated command for bud that only/specifically re-generates the theme.json (or no-op if no theme.json generation is enabled) would be great for this! That command would run before the translation scan, for the translation source strings in the theme.json being up-to-date.

@strarsis strarsis changed the title Include theme.json in translation scan command Include theme.json; patterns/ in translation scan command Feb 26, 2023
@strarsis strarsis mentioned this pull request Mar 4, 2023
Closed
3 tasks
@strarsis
Copy link
Contributor Author

strarsis commented Oct 9, 2023 •
edited
Loading

@retlehs, @kellymears: Current stable wp cli 2.8.1 contains a recent enough wp i18n, so this PR can be merged now!

retlehs
retlehs approved these changes Dec 8, 2023
View reviewed changes
Copy link
Member

@retlehs retlehs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My apologies for the delay, thank you for this update!

@retlehs retlehs merged commit 2208c11 into roots:main Dec 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@retlehs retlehs retlehs approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@strarsis @retlehs