i18n: Traverse through parent paths to find translator comment#1068
i18n: Traverse through parent paths to find translator comment#1068
Conversation
|
FWIW This worked for me and loading the POT file in Poedit displayed the relevant comments alongside the translation template for those strings. |
i18n/babel-plugin.js
Outdated
| return ( | ||
| line >= _originalNodeLine - 1 && | ||
| line <= _originalNodeLine && | ||
| REGEXP_TRANSLATOR_COMMENT.test( commentNode.value ) |
There was a problem hiding this comment.
Why not eliminate the duplicate test and match against this regex?
const commentMatch = find( ... => {
return (
... &&
commentNode.value.match( REGEXP_TRANSLATOR_COMMENT )
)
} );There was a problem hiding this comment.
An earlier iteration did this, but alas I encountered that find returns the entry (i.e. entire node) for which the callback returns true, not the return value itself. I might poke around for alternatives though, or if you have any suggestions.
There was a problem hiding this comment.
Ah, right, because find takes a predicate function rather than a map-like callback. 👍
| } | ||
|
|
||
| it( 'should not return translator comment on same line but after call expression', () => { | ||
| const comment = getCommentFromString( '__( \'Hello world\' ); // translators: Greeting' ); |
There was a problem hiding this comment.
This seems like a potentially useful variation, or is the intent to match existing behavior in WordPress?
There was a problem hiding this comment.
To be honest, this was based on a cursory scan of PHP source files always including the translator comment before the translation occurs. I'm not sure if there's in-fact a rule against it, or a limitation of the tooling. I'd have to check.
There was a problem hiding this comment.
So after scouring the documentation, the code itself, and its tests, I've come to the conclusions that:
- The comment must always occur before the translation call (see
$latest_commentinfind_function_callstoken iteration) - The PHP parsing is very non-particular about when that comment occurs. In fact, it looks like it could be many many lines prior the translate call, even if unrelated statements occur in-between. I'm not sure we care to preserve this behavior. It helps that the AST parsing is at least very liberal with considering whitespace in
leadingComments. - In the PHP implementation the
translators:is case-insensitive, which we should probably preserve here. I'll make that change shortly.
There was a problem hiding this comment.
Actually, we're already using the i flag for the RegExp matching. Should be fine there.
I guess this doesn't really help, as we test the comment to be occurring on the same or previous line. We might be able to drop this condition for iterating |
I agree.
I think this is fine - it seems a bit strange at the very least to put a blank line in between the translator comment and the (directly relevant) translation call. |
Related: #984 (comment)
This pull request seeks to improve translation string extraction to traverse through parent paths in the AST to find any leading comment which occurs on the same or previous line.
Testing instructions:
Ensure tests pass:
Run
npm run gettext-stringsand check that strings observed at #984 (comment) are included inlanguages/gutenberg.potoutput.cc @njpanderson