-
Notifications
You must be signed in to change notification settings - Fork 52
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
ease inheritance in order to support Twig/Timber #52
Comments
I just read through these issues as well as timber/timber#1754 and https://timber.github.io/docs/guides/internationalization/. Is Twig so popular in WordPress space and is Timber the de facto standard for it? Where would we draw the line when someone else comes with their templating engine and wants this included in I am not really sure whether this is something WP-CLI should handle. The Gettext library we use already supports Twig extraction: https://github.com/oscarotero/Gettext/blob/eea87605224d25125a39a83fbc9bc1c28d16bb7a/src/Extractors/Twig.php If we can easily use that to make it work with Timber, I'm open for looking at it. Otherwise I'm not sure it's worth the effort. Also, good to keep in mind that with this we'd need to scan (not necessarily load) every PHP file twice, once for good old gettext and once for Twig. |
I'll try to offer a PR, but I hope bootstrap'ing WP is not a showstopper...
|
Personally I really don‘t want to load WP just because some projects might use Twig. People should be able to use this command in environments where they need to create POT files on the fly, e.g. on Travis CI, the WordPress.org infrastructure, or in projects like https://github.com/wearerequired/traduttore Perhaps a feature flag or a new sub-command could be used for this, but I feel like in that case this should be handled in a new command offered by Timber. That command can of course extend this one‘s code, so there shouldn‘t be much duplicate code. @schlessera thoughts? |
I fully understand the concern, but how to handle writing to one common PO file (in one run) while having two different bootstrap levels? |
You mean one common POT file? Let's say your custom command is Then, you can run Of course you could combine these commands, e.g. |
Then, you can run `wp i18n make-pot --merge=twig.pot` to create one
single POT file containing all translations from Twig, regular PHP
code and JavaScript code.
(I wondered whether `make-pot --twig` could have be run after_wp_load
while keep `make-pot` before_wp_load).
It'd be annoying not being able to reuse the hundreds of useful lines already in
that package.
=> #54
|
I wondered the same yesterday, but I'm not sure it's possible. Also, it could lead to unexpected results for users and would need thorough documentation.
That's why I suggested extending this command. I bet it's possible to extend the Perhaps you'd need to require this package in composer and use Either way, you wouldn't have to rewrite things from scratch. I don't know the internals of WP-CLI that well yet, hence pinging Alain before to get his thoughts to figure out the best way of extension here. |
Moving argument handling outside |
Makes sense. I‘ll try to have a look over the weekend. If you have a proposed solution in mind, feel free to create a PR :) |
Thank you! In the meantime, here is my working code for Twig extraction: <?php
use Gettext\Translations;
use Gettext\Extractors\Twig;
use WP_CLI\I18n;
final class TwigExtractor extends Twig {
use WP_CLI\I18n\IterableCodeExtractor;
public static $options = [
'twig' => NULL,
'extractComments' => [ 'translators', 'Translators' ],
'constants' => [],
'functions' => [
'__' => 'text_domain',
'_e' => 'text_domain',
'_x' => 'text_context_domain',
'_ex' => 'text_context_domain',
'_n' => 'single_plural_number_domain',
'_nx' => 'single_plural_number_context_domain',
'_n_noop' => 'single_plural_domain',
'_nx_noop' => 'single_plural_context_domain'
]
];
public static function fromString($string, Translations $translations, array $options = []) {
$options += static::$options;
$twig = $options['twig'] ?: self::createTwig();
$source = $twig->compileSource(new Twig_Source($string, ''));
$functions = new I18n\PhpFunctionsScanner( $source );
$functions->saveGettextFunctions( $translations, $options );
}
private static function createTwig() {
$twig = new Twig_Environment(new Twig_Loader_Array(['' => '']));
$twig->addExtension(new Twig_Extensions_Extension_I18n());
$timber = new TimberTwig();
$timber->add_timber_functions( $twig );
$timber->add_timber_filters( $twig );
return static::$options['twig'] = $twig;
}
}
|
Could you also drop " |
I don't think Twig or Timber is popular enough to warrant bundling functionality for them with standard WP-CLI, especially if it needs major changes to the way the command runs. This is perfectly fine as a third-party command, though, and we can make sure that it is straight-forward enough to extend the bundled command to enable said third-party command. Note: You can extend a bundled command with a third-party command and have that override the bundled command when installed through the package manager. This could be something like this (untested): class TwigAwareMakePotCommand extends MakePotCommand {
public function __invoke( $args, $assoc_args ) {
$twig = Utils\get_flag_value( $assoc_args, 'twig', false );
// For Twig handling, defer execution to after WP was loaded.
if ( $twig ) {
WP_CLI::add_hook( 'after_wp_load', [ $this, 'process_twig' ] );
}
// Continue with bundled command execution.
parent::__invoke( $args, $assoc_args );
}
} |
Timber is intended as a library/theme-dependency/... Not being a plugin, it currently can not bundle Problems with extending
|
Looking at https://make.wordpress.org/cli/handbook/commands-cookbook/#wp_cliadd_commands-third-args-parameter and https://github.com/wp-cli/wp-cli/blob/b00cdfa4fcbe54a352ce7bc1431c3fe47989d2ae/php/WP_CLI/Dispatcher/CommandFactory.php it seems like it should be possible to get the doc block from
I'm not sure I can follow. Could you perhaps share some example code so I can better understand what's not working? |
<?php
// functions.php: if ( defined( 'WP_CLI' ) && WP_CLI ) WP_CLI::add_command( 'timber', 'My_Timber' );
class My_Timber extends \WP_CLI\I18n\MakePotCommand {
/**
* {@inheritdoc}
*
* [--twig]
* : Do additional Twig extraction before saving Po file
*/
public function __invoke( $args, $assoc_args ) { // attempt 1
// error: option definition is wrong, inheritdoc won't work, and even less with intend of synopsis extension.
}
/**
* Create a po file ...
*
* ## OPTIONS
*
* <source>
* : Directory to scan for string extraction.
* [...] full synopsis copy/paste from MakePotCommand
*
* ## EXAMPLES
* $ wp timber make-pot . languages/my-plugin.pot
* @subcommand make-pot
* @when init
*/
public function __invoke( $args, $assoc_args ) { // attempt 2
/* - With `wp timber make-pot . languages/my-plugin.pot`
error: Too many positional arguments
Because __invoke() is not even called. [make-pot is not a correctly registered command
- Only `wp timber . languages/my-plugin.pot`
is accepted, which is undesirable */
}
public function __invoke( $args, $assoc_args ) { // attempt 3
// Make it a no-op, but define a subcommand below...
}
/**
* Create a po file ...
*/
public function my_make_pot( $args, $assoc_args ) {
// never registered, because __invoke() is a no-op.
}
}
class My_Timber extends \WP_CLI_Command { // attempt 4
/**
* Create a po file ...
*
* [full synopsis, possibly with additional options, like --twig]
*
* @subcommand make-pot
* @when init
*/
public function my_make_pot( $args, $assoc_args ) {
$twig = \WP_CLI\Utils\get_flag_value( $assoc_args, 'twig', false );
// Continue with bundled command execution.
$main_pot = new \WP_CLI\I18n\MakePotCommand();
$main_pot->handle_arguments( $args, $assoc_args );
// we would like to access $source, $destination, $translations, as processed by MakePotCommand, this is not currently possible
// we would like to append to $main_pot->translations before storing the file, this is not currently possible (#63)
$this->makeTwigPot();
}
} |
@drzraf Small note regarding the above code: The idea of overriding the existing command was to add the
I don't know yet how best to handle the synopsis, I'll have to experiment with that myself. But know that you can also define the synopsis through the |
If you instead want to make the command be named WP_CLI::add_command( 'timber make-pot', 'My_Timber' );
class My_Timber {
public function __invoke() { /* ... */ }
} See https://make.wordpress.org/cli/handbook/commands-cookbook/#required-registration-arguments for details about the magic method |
@swissspidy I could inherit the way you suggested (but still copying the full synopsis). Then I tried synopsis "inheritance". Command creation accept $reflection = new \ReflectionClass( "WP_CLI\I18n\MakePotCommand" );
$docparser = new \WP_CLI\DocParser( $reflection->getMethod( '__invoke' )->getDocComment() );
$ld = explode("\n", $docparser->get_longdesc());
$new_ld = '';
$isopt = 0;
foreach($ld as $l) {
if($isopt==1 && strpos($l, '##') === 0) {
$new_ld .= <<<EOF
[--twig]
: With Twig
EOF
. PHP_EOL . PHP_EOL;
}
if($l=='## OPTIONS') $isopt=1;
$new_ld .= $l . PHP_EOL;
}
$args = ['shortdesc' => $docparser->get_shortdesc(),
'longdesc' => $new_ld,
'synopsis' => $docparser->get_synopsis(),
'when' => 'init' /*$docparser->get_tag('when')*/ ];
WP_CLI::add_command( 'timber make-pot', 'My_Timber', $args); Providing a couple of helpers to |
@drzraf Yes, I agree, this should be easier to do, and methods for I'll accept PRs for that if you care to provide them. |
Is there anything left here for the WP-CLI / i18-command side of things? |
Chronologically:
These actually are ... a big stack of painful workarounds.
¹ In the same POT file. |
Not sure I completely understood all issues in here. It's probably true that Twig/Timber is not used enough to deserve being part of the wp-cli core packages, but I think Twig it's used enough (it's the de facto PHP template language in frameworks like symphony or CMS like Drupal) that wp-cli supports enough so it can be extended to leverage Twig extraction (maybe Timber specifics could come later). |
About Twig: Timber is the implementation of Twig for WordPress (function binding/gettext wrapping/helper about templating and ACF/...) About string extraction, you may want to look at this stack: |
Hello there, This feature has been requested since quite a long time. I'm happy to announce that we finally manage to handle WordPress translations extraction from Twig file using WP-CLI: https://github.com/timber/wp-i18n-twig Technical details (WP-CLI)In order to provide a consistent DX, the command overrides the default
Just mentioning those details in order to improve extending WP-CLI commands in the future, if that's something you might consider. Technical details (Twig)Although it's released by the Timber team, the package is Timber agnostic, should be working with any Twig implementation (see limitations) and outside the project if needed (CI, etc.). The key here is to avoid compiling Twig files (which would lead to many problems), the logic has been borrowed from Symfony Translation extractor and consists in only parsing Twig templates and adding a "visitor" that collects all translations found in nodes. The collected translation function calls are shaped as an array just like what the one expected by the gettext Sadly, Twig comments are completely skipped by the lexer and can't be found in the parsed tokens. Translator comments extraction was made possible at the cost of really bad code practice 🙈 but I felt it was still better than writing/overriding/maintaining a whole Lexer class just to grab comments that were already available and "easy" to be picked up here. The benefit is that the package covers 100% of Twig and WP-CLISince I would obviously understand if you're not, this would ring another scope to maintain, etc. I'm fine keeping it as a third party package 😄 |
It would be nice for
wp-cli make-pot
to support twig template out of the box.Some of the building blocks:
timber/timber#1753
umpirsky/Twig-Gettext-Extractor#60
The text was updated successfully, but these errors were encountered: