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

Scan for esc_xml_*() localization helpers #221

Merged
merged 2 commits into from Jul 8, 2020
Merged

Conversation

@pbiron
Copy link
Contributor

@pbiron pbiron commented Jul 6, 2020

Addresses #220.

This the first time I've looked at the i18n make-pot code, so I'm not positive this PR does what I think it does :-)

I looked at the existing unit tests and couldn't figure out how to add a test for esc_xml__(), etc. Pointers on how to write that test greatly appreciated.

@pbiron pbiron requested a review from wp-cli/committers as a code owner Jul 6, 2020
@swissspidy
Copy link
Member

@swissspidy swissspidy commented Jul 7, 2020

Most of the functionality is covered by Behat tests, not unit tests. Those should be rather self explanatory.

@pbiron
Copy link
Contributor Author

@pbiron pbiron commented Jul 7, 2020

Most of the functionality is covered by Behat tests, not unit tests. Those should be rather self explanatory.

maybe self explanatory if I knew how to write behat tests :-) I'll guess I'll have to learn.

@swissspidy
Copy link
Member

@swissspidy swissspidy commented Jul 7, 2020

@pbiron You can basically just amend this part with esc_xml__ & co:

Scenario: Extract all supported functions
Given an empty foo-plugin directory
And a foo-plugin/foo-plugin.php file:
"""
<?php
/**
* Plugin Name: Foo Plugin
*/
__( '__', 'foo-plugin' );
esc_attr__( 'esc_attr__', 'foo-plugin' );
esc_html__( 'esc_html__', 'foo-plugin' );
_e( '_e', 'foo-plugin' );
esc_attr_e( 'esc_attr_e', 'foo-plugin' );
esc_html_e( 'esc_html_e', 'foo-plugin' );
_x( '_x', '_x_context', 'foo-plugin' );
_ex( '_ex', '_ex_context', 'foo-plugin' );
esc_attr_x( 'esc_attr_x', 'esc_attr_x_context', 'foo-plugin' );
esc_html_x( 'esc_html_x', 'esc_html_x_context', 'foo-plugin' );
_n( '_n_single', '_n_plural', $number, 'foo-plugin' );
_nx( '_nx_single', '_nx_plural', $number, '_nx_context', 'foo-plugin' );
_n_noop( '_n_noop_single', '_n_noop_plural', 'foo-plugin' );
_nx_noop( '_nx_noop_single', '_nx_noop_plural', '_nx_noop_context', 'foo-plugin' );
// Compat.
_( '_', 'foo-plugin' );
// Deprecated.
_c( '_c', 'foo-plugin' );
_nc( '_nc_single', '_nc_plural', $number, 'foo-plugin' );
__ngettext( '__ngettext_single', '__ngettext_plural', $number, 'foo-plugin' );
__ngettext_noop( '__ngettext_noop_single', '__ngettext_noop_plural', 'foo-plugin' );
__unsupported_func( '__unsupported_func', 'foo-plugin' );
__( 'wrong-domain', 'wrong-domain' );
"""
When I run `wp i18n make-pot foo-plugin`
Then STDOUT should be:
"""
Plugin file detected.
Success: POT file successfully generated!
"""
And the foo-plugin/foo-plugin.pot file should exist
And the foo-plugin/foo-plugin.pot file should contain:
"""
msgid "__"
"""
And the foo-plugin/foo-plugin.pot file should contain:
"""
msgid "esc_attr__"
"""
And the foo-plugin/foo-plugin.pot file should contain:
"""
msgid "esc_html__"
"""

Copy & paste :-)

@pbiron
Copy link
Contributor Author

@pbiron pbiron commented Jul 8, 2020

the behat tests have been updated and, yeah, travis was happy...so I guess I did the mods correctly :-)

@schlessera schlessera added this to the 2.2.5 milestone Jul 8, 2020
@schlessera schlessera merged commit b02ecdc into wp-cli:master Jul 8, 2020
3 checks passed
3 checks passed
@github-actions
Lint PHP files
Details
@github-actions
PHPCS
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@schlessera schlessera changed the title Scan for esc_xml_*() l10n helpers. Scan for esc_xml_*() localization helpers Jul 8, 2020
@schlessera
Copy link
Member

@schlessera schlessera commented Jul 8, 2020

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants