Perform a final review of the package before tagging the initial release

[swissspidy]

Before this package lands in WP-CLI as a bundled command, it'd be helpful to do a final review for implementation details (e.g. do we like all argument naming, etc.)

[gitlost]

This needs a lot of work.

For instance I ran it on one of my (magnificent) plugins and it failed to extract all strings, example:

<?php
/**
 * Plugin Name: Test Makepot
 */

_n( '%1$s PDF preview regenerated in %2$s seconds.', '%1$s PDF previews regenerated in %2$s seconds.', $num_updates, 'test-makepot' );
__( 'Nothing updated!', 'test-makepot' );

The 'Nothing updated!' string isn't extracted (although the fix wasn't too hard - will push PR).

Also I'm concerned about the upstream library https://github.com/oscarotero/Gettext. The comments bug php-gettext/Gettext#161 is a major showstopping bug.

Re tests, for one we'd need the core i18n phpunit tests ported over and passing with as few changes as possible.

So that's a lot of work!

[gitlost]

Before too much more work is done on this command, note the comments bug php-gettext/Gettext#161 still isn't fixed. As stated above, that's a major bug which renders the library unusable in my opinion. The fact that it exists at all raises concerns - that it hasn't been fixed in going on 2 months is a big red flag. Other PRs and issues haven't been acted on either.

So I don't think https://github.com/oscarotero/Gettext is maintained or mature enough to use.

I notice the command originally used the WordPress i18n tools https://github.com/wp-cli/i18n-command/tree/e620e2eb71b2c5e5788f848843bcfa56797dd8cc. Were there problems with this approach?

[swissspidy]

@gitlost I chatted with @ocean90, who is usually the one patching the WordPress i18n tools, about those and he heavily recommended not to use this library. It has been designed with WordPress.org as its main user in mind and is definitely not suited for this use case here. Using that library would have forced use to rewrite many parts of it, at which point we could have just rolled our own.

Thus, I'd argue https://github.com/oscarotero/Gettext is definitely more maintained, robust, and tested. Also, I wouldn't consider php-gettext/Gettext#161 a major bug that renders the library unusable. In my experience, not many projects use translator comments in that way — if they even use them.

Yes, it needs to be fixed, but that doesn't stop us from testing this command in the wild to see how people use it and like it.

[gitlost]

he heavily recommended not to use this library

That's good enough for me!

I can't see though how you can argue that https://github.com/oscarotero/Gettext is more maintained/robust/tested to be honest, the evidence isn't there in the repository.

That bug php-gettext/Gettext#161 seems to me to be major in that replicating getext behaviour is a minimum requirement of any wrapper library.

Yes, it needs to be fixed, but that doesn't stop us from testing this command in the wild to see how people use it and like it.

I'm wondering though is there a better solution than https://github.com/oscarotero/Gettext ... will do some research!

[schlessera]

The bug in php-gettext/Gettext#161 is an edge case we can at least ignore during development.

Also, the maintainer has flagged this issue with "Help wanted". I wouldn't immediately conclude the package is not maintained in this specific case.

Worst case scenario if we use this library:

• we need to provide a PR to fix the comment situation (still beats writing our own library)
• in case it doesn't get merged, we'll use our own fork of it

[swissspidy]

php-gettext/Gettext#164 has been merged now.

Let's try this again with the latest master of the library.

[swissspidy]

As mentioned in the last meeting, I wanted to write a quick summary here.

After updating the upstream gettext library I added a few more tests for the make-pot command, especially in relation to translator comments.

The i18n library in core contains a bunch of unit tests related to adding text domains and extracting gettext calls. However, we're only interested in gettext extraction. These tests can be found at http://develop.svn.wordpress.org/trunk/tools/i18n/t/ExtractTest.php

Since the tests are about extracting simple gettext calls and some translator comments, these are covered by the upstream library. There's no need to duplicate these here. The tests in this command should be for WordPress-specific stuff.

In the last few days we've set up automatic translation extraction & updates for all our projects at our company (blog post upcoming) using wp i18n make-pot and it works really well. Thus, I'd say the command is ready for everyday use and we should work on making this command ready for primetime.

[schlessera]

Thanks for the update, @swissspidy !

This sounds really good. I'll try to play with it myself in the coming week. Given we have enough time to do the fine-tuning until the next release, I don't see a problem with bundling this sooner rather than later, to let people experiment with it in nightly.