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

Improve PHP comments extraction #161

Closed
swissspidy opened this issue Dec 14, 2017 · 6 comments
Closed

Improve PHP comments extraction #161

swissspidy opened this issue Dec 14, 2017 · 6 comments

Comments

@swissspidy
Copy link
Contributor

Noticed this while working on wp-cli/i18n-command#4.

Given the following example.php PHP file:

/* translators: TRANSLATORS!! */
_e( 'hello world', 'test-plugin' );

/* translators: TRANSLATORS!! */
$foo = __( 'foo', 'test-plugin' );

The library does not extract the second translator comment.

However, xgettext --add-comments=translators --keyword=__:1 --add-location example.php does extract that comment.

It seems like this is a case #102 / #100 didn't test against.

@oscarotero
Copy link
Member

Ok, thanks for reporting. This requires a deep change in how the comments are extracted here: https://github.com/oscarotero/Gettext/blob/master/src/Utils/PhpFunctionsScanner.php

Currently, only extract comments placed just before the function. In the second case, there's a variable definition between the comment and the function.

I'll try to fix this when I have free time. Anyway, if you want to get involved with a pull request, do not hesitate.

@ocean90
Copy link
Contributor

ocean90 commented Feb 20, 2018

I'm wondering if we can borrow some code from the PHPCS i18n sniff for WordPress even though it depends on the PHPCS' tokenizer: https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/blob/735b899c3f2971958656985098022d5c44c52811/WordPress/Sniffs/WP/I18nSniff.php#L632-L729

@jrfnl, maybe you have some thoughts on this?

@swissspidy
Copy link
Contributor Author

I've tried to dig into this in #164. Feedback welcome!

@jrfnl
Copy link

jrfnl commented Feb 21, 2018

@ocean90 Thanks for the ping.

For the sniff in WPCS, based on feedback received from various sources, including the maintainer of POEdit (see discussion threads) and tests run with various tools, we ended up with a strict check for a translators comment on the line directly above the line containing the gettext function call.

The line containing the gettext function call, however, could contain any other token before the gettext call.

So the below examples would all be fine:

/* translators: comment */
echo __( 'string' );

/* translators: comment */
$var = sprintf( __( 'string %d' ), 1 );

printf(
    /* translators: comment */
    __( 'string %d' ),
    1'
);

But these wouldn't be:

/* translators: comment not on the line above the gettext call */ __( 'string' );

/* translators: comment not on the line directly above the gettext call */


echo __( 'string' );

/* translators: comment not on the line above the gettext call */
printf(
    __( 'string %d' ),
    1'
);

Refs:
WordPress/WordPress-Coding-Standards#423
WordPress/WordPress-Coding-Standards#742

@ocean90 Does that help ? or is there anything else you'd want my input on regarding this ?

@oscarotero
Copy link
Member

v4.4.4 released.
Thanks for this great work.

@swissspidy
Copy link
Contributor Author

@oscarotero Thanks for your efforts! Hope you don't mind that I made some additional tweaks in #166 :)

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

4 participants