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

Comments extraction from PHP code #102

Merged
merged 3 commits into from
Feb 19, 2016
Merged

Comments extraction from PHP code #102

merged 3 commits into from
Feb 19, 2016

Conversation

mlocati
Copy link
Member

@mlocati mlocati commented Feb 19, 2016

Follow-up of #100: what about a code like this?

* Constructor.
*
* @param string $code The php code to scan
*/
public function __construct($code)
{
$this->tokens = token_get_all($code);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's filter out white spaces, so that parsing tokens is faster

@oscarotero
Copy link
Member

It looks fine. I think is better to extract always the comments. If the user don't want them, they can be removed later. The rest of the extractors are not configurable. What do you think?

@mlocati
Copy link
Member Author

mlocati commented Feb 19, 2016

One may want only comments that start with a specific text (like xgettext), otherwise we risk to extract comments that are not related to the strings to be translated.
Think for instance at this example:

// Initialize some variables
$hi = __('Hello');
$x = 1;
$y = 2;

So, I think we should keep the same approach as xgettext: only extract comments if requested, and optionally allow to specify a "tag" to filter comments to be extracted.

@oscarotero
Copy link
Member

ah, so comments can be before the function, not neccesary inside?
Ok, in this case, maybe a static property (like public static $functions) ?

@mlocati
Copy link
Member Author

mlocati commented Feb 19, 2016

@oscarotero
Copy link
Member

Its fine to me

@mlocati
Copy link
Member Author

mlocati commented Feb 19, 2016

I forgot to answer this:

ah, so comments can be before the function, not neccesary inside?

Yes, xgettext can extract comments that are imediately before the function, provided that "comment blocks supposed to be extracted must be adjacent to keyword lines" (from gettext manual)

That means that with this code

<?php

echo __(/* i18n: Test comment 1 */'test 1');

/* i18n: Test comment 2 */
echo __('test 2');

/* i18n: Test comment 3 */

echo __('test 3');

/* i18n: Test comment 4 */
echo __(
    'test 4'
);

xgettext extracts comments 1, 2 and 3, but not the 4th:

#. i18n: Test comment 1
#: test.php:3
msgid "test 1"
msgstr ""

#. i18n: Test comment 2
#: test.php:6
msgid "test 2"
msgstr ""

#. i18n: Test comment 3
#: test.php:10
msgid "test 3"
msgstr ""

#: test.php:14
msgid "test 4"
msgstr ""

The current implementation of this pull request is a bit different: it will extracts also i18n: Test comment 4, but I don't think this will be a big problem.

@mlocati
Copy link
Member Author

mlocati commented Feb 19, 2016

Ok, I'll add a few tests and everything is ready to be merged 😉

@mlocati
Copy link
Member Author

mlocati commented Feb 19, 2016

Done.

@mlocati
Copy link
Member Author

mlocati commented Feb 19, 2016

After this has been merged I'll stop boring you and I'll be looking forward to a new release 😉

oscarotero added a commit that referenced this pull request Feb 19, 2016
@oscarotero oscarotero merged commit 65e8925 into php-gettext:master Feb 19, 2016
@oscarotero
Copy link
Member

Thanks for your hard work. I'm going to release the new 3.5.9 version soon.

@mlocati mlocati deleted the php-comments-extraction branch February 19, 2016 15:04
@mlocati
Copy link
Member Author

mlocati commented Feb 19, 2016

Thanks for your hard work.

Thanks to you!

I'm going to release the new 3.5.9 version soon.

Great!

@oscarotero
Copy link
Member

@mlocati
Copy link
Member Author

mlocati commented Feb 19, 2016

Super fast! Thank you!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants