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

i18n: Extract concatenated strings of translate functions #2655

Merged
merged 1 commit into from Sep 6, 2017

Conversation

Projects
None yet
2 participants
@aduth
Member

aduth commented Sep 4, 2017

Closes #1927

This pull request seeks to enhance the i18n strings extraction parser to support string concatenation (BinaryExpression type).

Testing instructions:

Verify that concatenated strings (like that of the InvalidBlockWarning component) are included in the generated languages/gutenberg.pot file when running npm run gettext-strings.

@codecov

This comment has been minimized.

Show comment
Hide comment
@codecov

codecov bot Sep 4, 2017

Codecov Report

Merging #2655 into master will increase coverage by 0.06%.
The diff coverage is 85.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2655      +/-   ##
==========================================
+ Coverage   31.38%   31.45%   +0.06%     
==========================================
  Files         177      177              
  Lines        5413     5418       +5     
  Branches      949      950       +1     
==========================================
+ Hits         1699     1704       +5     
  Misses       3139     3139              
  Partials      575      575
Impacted Files Coverage Δ
i18n/babel-plugin.js 39.58% <85.71%> (+3.31%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 78dd8b6...12caa99. Read the comment docs.

codecov bot commented Sep 4, 2017

Codecov Report

Merging #2655 into master will increase coverage by 0.06%.
The diff coverage is 85.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2655      +/-   ##
==========================================
+ Coverage   31.38%   31.45%   +0.06%     
==========================================
  Files         177      177              
  Lines        5413     5418       +5     
  Branches      949      950       +1     
==========================================
+ Hits         1699     1704       +5     
  Misses       3139     3139              
  Partials      575      575
Impacted Files Coverage Δ
i18n/babel-plugin.js 39.58% <85.71%> (+3.31%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 78dd8b6...12caa99. Read the comment docs.

@BE-Webdesign

This comment has been minimized.

Show comment
Hide comment
@BE-Webdesign

BE-Webdesign Sep 6, 2017

Contributor

Verify that concatenated strings (like that of the InvalidBlockWarning component) are included in the generated languages/gutenberg.pot file when running npm run gettext-strings.

Verified that this works.

Contributor

BE-Webdesign commented Sep 6, 2017

Verify that concatenated strings (like that of the InvalidBlockWarning component) are included in the generated languages/gutenberg.pot file when running npm run gettext-strings.

Verified that this works.

@BE-Webdesign

This comment has been minimized.

Show comment
Hide comment
@BE-Webdesign

BE-Webdesign Sep 6, 2017

Contributor

It is however copying over the first line break as well:

#: editor/modes/visual-editor/invalid-block-warning.js:32
msgid ""
"This block appears to have been modified externally. Overwrite the external "
"changes or Convert to %s to keep your changes."
msgstr ""

Everything is working good though as far as I can tell.

Contributor

BE-Webdesign commented Sep 6, 2017

It is however copying over the first line break as well:

#: editor/modes/visual-editor/invalid-block-warning.js:32
msgid ""
"This block appears to have been modified externally. Overwrite the external "
"changes or Convert to %s to keep your changes."
msgstr ""

Everything is working good though as far as I can tell.

@aduth

This comment has been minimized.

Show comment
Hide comment
@aduth

aduth Sep 6, 2017

Member

@BE-Webdesign Thanks for the review! The behavior there can be attributed to the gettext parser I believe, specifically its foldLength option:

> var foldLine = require( 'gettext-parser/lib/shared' ).foldLine;
undefined
> foldLine( 'This block appears to have been modified externally. Overwrite the external changes or Convert to %s to keep your changes.', 76 );
[ 'This block appears to have been modified externally. Overwrite the external ',
  'changes or Convert to %s to keep your changes.' ]
Member

aduth commented Sep 6, 2017

@BE-Webdesign Thanks for the review! The behavior there can be attributed to the gettext parser I believe, specifically its foldLength option:

> var foldLine = require( 'gettext-parser/lib/shared' ).foldLine;
undefined
> foldLine( 'This block appears to have been modified externally. Overwrite the external changes or Convert to %s to keep your changes.', 76 );
[ 'This block appears to have been modified externally. Overwrite the external ',
  'changes or Convert to %s to keep your changes.' ]
@aduth

This comment has been minimized.

Show comment
Hide comment
@aduth

aduth Sep 6, 2017

Member

Just in case, I manually debugged the running parsing code and confirmed there are no newlines in the original extracted text argument.

Member

aduth commented Sep 6, 2017

Just in case, I manually debugged the running parsing code and confirmed there are no newlines in the original extracted text argument.

@aduth aduth merged commit c8b1be2 into master Sep 6, 2017

3 checks passed

codecov/project 31.45% (+0.06%) compared to 78dd8b6
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@aduth aduth deleted the update/1927-i18n-string-concat branch Sep 6, 2017

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