google / mdbook-i18n-helpers

Translation support for mdbook. The plugins here give you a structured way to maintain a translated book.
Apache License 2.0
127 stars 25 forks source link

Make xgettext respect granularity for sub-chapter messages #215

Closed carreter closed 1 month ago

carreter commented 1 month ago

Related to #171.

While trying to refresh the Spanish translation of Comprehensive Rust, @henrif75 noticed that the messages.pot file I generated had line numbers that shouldn't be there (see https://github.com/google/comprehensive-rust/issues/2120).

Digging around in the code, I noticed that the xgettext code was ignoring the granularity for sub-chapters. This PR fixes that!

codecov-commenter commented 1 month ago

Codecov Report

Attention: Patch coverage is 96.55172% with 2 lines in your changes missing coverage. Please review.

Project coverage is 85.02%. Comparing base (68d2e29) to head (5cddc82). Report is 6 commits behind head on main.

Files Patch % Lines
i18n-helpers/src/xgettext.rs 96.55% 0 Missing and 2 partials :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #215 +/- ## ========================================== - Coverage 86.99% 85.02% -1.98% ========================================== Files 15 15 Lines 3229 3398 +169 Branches 3229 3398 +169 ========================================== + Hits 2809 2889 +80 - Misses 327 413 +86 - Partials 93 96 +3 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

kdarkhan commented 1 month ago

Thanks for the fix, @carreter! Can you add or update one of the existing test cases which will catch this bug please?

I think it will be good to have this tested to ensure we don't break it in the future.

carreter commented 1 month ago

Added the test as requested @kdarkhan !

kdarkhan commented 1 month ago

Please fix formatting. LGTM otherwise.

kdarkhan commented 1 month ago

I added the fix on top of your PR, thanks for the contribution.