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

4.x: Fix parsing of PO files without empty lines #296

Merged
merged 2 commits into from
May 18, 2024
Merged

4.x: Fix parsing of PO files without empty lines #296

merged 2 commits into from
May 18, 2024

Conversation

swissspidy
Copy link
Contributor

This resolves an issue that was brought to our attention in wp-cli/i18n-command#393

Turns out that msgfmt doesn't like empty lines between translations when merging PO files, and instead adds empty comments.

So a file like this:

# File: eme-actions.php, line: 343
# File: eme-events.php, line: 9395
msgid "An error has occurred"
msgstr "Es ist ein Fehler aufgetreten"

# File: eme-actions.php, line: 344
msgid "Clear"
msgstr "Leeren"

# File: eme-actions.php, line: 345
# File: eme-events.php, line: 9396
msgid "Mailing preferences"
msgstr "Mailing Präferenzen"

# File: eme-actions.php, line: 346
# File: eme-events.php, line: 9397
msgid "Yes, I'm sure"
msgstr "Ja, ich bin mir sicher"

Gets turned into this:

#
# File: eme-actions.php, line: 343
# File: eme-events.php, line: 9395
msgid "An error has occurred"
msgstr "Es ist ein Fehler aufgetreten"
#
# File: eme-actions.php, line: 344
msgid "Clear"
msgstr "Leeren"
#
# File: eme-actions.php, line: 345
# File: eme-events.php, line: 9396
msgid "Mailing preferences"
msgstr "Mailing Präferenzen"
#
# File: eme-actions.php, line: 346
# File: eme-events.php, line: 9397
msgid "Yes, I'm sure"
msgstr "Ja, ich bin mir sicher"

As per the spec, whitespace between entries is optional. However, Gettext v4 doesn't parse these files correctly when they look like this.

This PR addresses that.

In the WP-CLI i18n command we currently rely on Gettext v4 because of PHP version requirements, so I hope this bugfix gets accepted still.

@oscarotero
Copy link
Member

If I understood correctly, an empty comment is the same as an empty line, right?
So a simpler approach would be:

// Line 31

if ($line === "#") {
  $line = "";
}

Does it make sense?

@swissspidy
Copy link
Contributor Author

That... also works! Much simpler, thanks. Code updated ✅

@oscarotero oscarotero merged commit 7d98aba into php-gettext:4.x May 18, 2024
@oscarotero
Copy link
Member

Thanks, merged and released in v4.8.12

oscarotero added a commit that referenced this pull request May 18, 2024
@swissspidy swissspidy deleted the fix/empty-comment-lines branch May 18, 2024 15:31
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.

2 participants