separate translation extraction from Po file writing#63
separate translation extraction from Po file writing#63swissspidy merged 1 commit intowp-cli:masterfrom
Conversation
src/MakePotCommand.php
Outdated
| @@ -295,7 +298,7 @@ protected function get_main_file_data() { | |||
| * | |||
| * @return bool True on success, false otherwise. | |||
There was a problem hiding this comment.
That's now incorrect with this change.
Instead of making translations a property of the MakePotCommand, I think we can just use $translations = new Translations() and return that object at the end (like you did already).
We could get rid of set_default_headers() and move these ~10 lines into makepot().
Also, since makepot() doesn't really create a POT file anymore, we could call it extract_strings() or something.
src/MakePotCommand.php
Outdated
| $this->handle_arguments( $args, $assoc_args ); | ||
| if ( ! $this->makepot() ) { | ||
| $this->makepot(); | ||
| if ( !$this->translations ) { |
There was a problem hiding this comment.
Can we keep the space after the !? 🙂
src/MakePotCommand.php
Outdated
| if ( !$this->translations ) { | ||
| WP_CLI::warning( 'No translation found' ); | ||
| } | ||
| if ( !PotGenerator::toFile( $this->translations, $this->destination ) ) { |
There was a problem hiding this comment.
Can we keep the space after the !? 🙂
|
Thanks for this PR, @drzraf! I like where this is heading. |
src/MakePotCommand.php
Outdated
| */ | ||
| protected function makepot() { | ||
| $this->translations = new Translations(); | ||
| public function extract_strings() { |
There was a problem hiding this comment.
Note that every public method on a class is registered as a subcommand of the command. See https://make.wordpress.org/cli/handbook/commands-cookbook/#required-registration-arguments
I don't think we want this here :-) — protected should work fine for the sake of extensibility.-
There was a problem hiding this comment.
Ouch....
The workaround expressed in #52 (comment) (attempt 4) relies on the fact that instead of inheriting the command, I would be able to instantiate it and grab $translations by calling the [public] extract_strings() method. For handle_arguments it was not such a problem. Isn't the role of phpdoc to register commands?
There was a problem hiding this comment.
PHPDoc is used for documenting a command and perhaps making aliases. Registration is done via WP_CLI::add_command().
src/MakePotCommand.php
Outdated
| $this->set_default_headers(); | ||
| $meta = $this->get_meta_data(); | ||
|
|
||
| $translations->setHeader( 'Project-Id-Version', $meta['name'] . ' ' . $meta['version'] ); |
There was a problem hiding this comment.
Was there a specific reason why this block was inlined instead of remaining in a separate method? I'd consider this is a regression in terms of code quality.
There was a problem hiding this comment.
I suggested it in an inline comment. I wanted to some refactoring in another PR after this one.
There was a problem hiding this comment.
Because it was requested^^
I restored the function.
|
Unfortunately there's now a minor merge conflict because of another recent PR. @drzraf Would you mind merging Afterwards I'll happily merge this PR :) Thanks in advance! |
…nsummer of this class) may do post processing on translation array or append new translation fetched with different extractor, to be store in the same destination po file
|
rebased + squashed |
separate translation extraction from Po file writing
3 participants
split translation extraction from po file writing.
That way child class (or code instantiating this class) can do post processing on
$translationsarray or append new translation (possibly fetched with different extractors), but intended to be stored in the same destination po file