Skip to content
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.

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

Support for gettext/gettext v5 #303

Closed
1 task done
strarsis opened this issue Feb 18, 2022 · 13 comments
Closed
1 task done

Support for gettext/gettext v5 #303

strarsis opened this issue Feb 18, 2022 · 13 comments

Comments

@strarsis
Copy link
Contributor

Feature Request

Describe your use case and the problem you are facing
For adding new file formats to scan like blade (that WordPress base themes use),
it would be nice to know what gettext/gettext version should be used.
Currently wp i18n uses gettext/gettext v4.8, v5 introduced some changes, Extractors were removed from the base package (and the concept renamed).

Describe the solution you'd like
What gettext/gettext release should be used in the long term? Are there plans to update to 5.x?

@swissspidy
Copy link
Member

We can‘t use v5 yet because of differing PHP version requirements. As soon as this changes we are able to upgrade. See also #217 for some context.

WP-CLI only changes its requirements when WP core does, so there is no ETA on this right now.

@strarsis strarsis changed the title Support for gettext/gettext `v5 Support for gettext/gettext v5 Feb 18, 2022
@strarsis
Copy link
Contributor Author

@swissspidy: So for attempting to add blade.php support to wp i18n, it would make sense to use the v4 branch of gettext/gettext (and Extractors)?

@swissspidy
Copy link
Member

Yeah for instance. You could also try to fork this command to use v5 if that‘s easier

@strarsis
Copy link
Contributor Author

strarsis commented Feb 21, 2022

@swissspidy: I just discovered that in the gettext 4.x branch is already a readily implemented Blade extractor:
https://github.com/php-gettext/Gettext/blob/4.x/src/Extractors/Blade.php

Hence implementing a PHP-Blade wp i18n extraction command was quite easy, PR: #304

By the way, in gettext/gettext v5.x those Extractors that had been split into Scanners and Loaders can be found in their own, separate repositories, e.g. the gettext PHP Scanner: https://github.com/php-gettext/PHP-Scanner/

@swissspidy
Copy link
Member

Closing for now since we now have Blade support and we can update to gettext v5 (and reopen this ticket) once PHP requirements allow it

@JUVOJustin
Copy link

Just wanted to ask if this is planned sometimes soon, since the minimal php requirement for wordpress is now 7.4. Having a hard time following these issues, but i think they will be fixed with the upgrade:

@swissspidy
Copy link
Member

@JUVOJ No. The minimum PHP version for WordPress is still 5.6 for the foreseeable future.

@drzraf
Copy link

drzraf commented Mar 14, 2023

@swissspidy / @schlessera: Can't we find a solution regarding this problem? Something around composer's suggest? Or some environment variable, a new i18n-command gettext v5 branch, or another creative solution that would unlock this whole stack of problem.
Keeping wp-cli i18n away from nice features (from gettext v5+) because we are stuck by PHP 5.6 is insane.
If you relax the version restriction, PHP 5.6 user would get gettext v4 while PHP 7/8 users would get the v5. So it's "just" a question of supporting both API then. Would you accept this? It would be a bit dirty code-wise but not impossible at all.

@swissspidy
Copy link
Member

If it's possible to support both versions in a maintainable way, that sounds like a viable middle ground. Maybe best to bring this up in the next office hours.

@Levdbas
Copy link

Levdbas commented Aug 1, 2023

Since we are finally dropping support for PHP 5.6 in WordPress 6.3 could you take this into consideration again @swissspidy?

https://make.wordpress.org/core/2023/07/05/dropping-support-for-php-5/

@swissspidy
Copy link
Member

Not yet, because gettextv5 requires PHP 7.2+ and WP still 7.0+

See also https://make.wordpress.org/cli/2019/01/15/wp-cli-php-requirements-strategy/

Again: If it's possible to support both versions in a maintainable way, that sounds like a viable middle ground

@drzraf
Copy link

drzraf commented Aug 8, 2023

@swissspidy : Could you please have a look at the v5 diff https://github.com/wp-cli/i18n-command/pull/355/files. The change for dual v4/v5 support are obvious (but not trivial):

Doable, but that would basically double the size of the diff.

(I'm really sad a whole community of Timber (Twig) users are waiting for this to translate Twig templates and we are all held back because of a few (very hypothetical) users of PHP 7.1 using wp-cli i18n command in their old (but shell-enabled) hosting provider.
The choice of wp-cli to blindly and inflexibly stick to WP PHP requirements is IHMO a very bad one)

@swissspidy
Copy link
Member

It seems like a good starting point 👍 Unfortunately CI logs are no longer available, but if it can be made to work, that sounds great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants