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

Dont apply translators comments multiple times again #338

Open
kkmuffme opened this issue Aug 13, 2022 · 4 comments
Open

Dont apply translators comments multiple times again #338

kkmuffme opened this issue Aug 13, 2022 · 4 comments

Comments

@kkmuffme
Copy link

Bug Report

WP-CLI 2.7.0-alpha-9fcc59a

#154 is partially back (or maybe never fully fixed?)

// translators: %1$s: provider, %2$s: collaborations settings link
printf( esc_html__( 'The %1$s URL provided in the %2$s is empty.', 'my-textdomain' ), esc_html( ucfirst( $error ) ), '<a href="' . esc_url( admin_url( '/admin.php?page=collab-settings' ) ) . '">' . esc_html__( 'settings', 'my-textdomain' ) . '</a>' );

// translators: %1$s: provider, %2$s: authentication settings link
printf( esc_html__( 'The %1$s key provided in the %2$s is invalid.', 'my-textdomain' ), esc_html( ucfirst( $error ) ), '<a href="' . esc_url( admin_url( '/admin.php?page=authentication-settings' ) ) . '">' . esc_html__( 'settings', 'my-textdomain' ) . '</a>' );

Will give error:

Warning: The string "settings" has 2 different translator comments. (includes/class-some-file.php:1178)
translators: %1$s: provider, %2$s: collaborations settings link
translators: %1$s: provider, %2$s: authentication settings link

in the .pot I see:

#. translators: %1$s: provider, %2$s: collaborations settings link
#. translators: %1$s: provider, %2$s: authentication settings link
#: includes/class-some-file.php:1178
#: includes/class-some-file.php:1181
msgid "settings"
msgstr ""

Issues:

  1. the translators comment is applied to multiple strings, instead of only to the strings that has that placeholder (%1$s)/the first string
  2. the error message only contains the first location/line number in that file instead of all
@swissspidy
Copy link
Member

This is an ambiguous case, because it's not clear whether the translator comment is meant for the first call or the second. What if both your __() calls contained translator comments for example?

I'd recommend writing these statements on multiple lines. This will help both developers and tools like this CLI command.

@kkmuffme
Copy link
Author

This is an ambiguous case, because it's not clear whether the translator comment is meant for the first call or the second.

Even then, translators comments should not be applied to multiple strings.
Can you please quickly check the old issue #154 as we already discussed this exact problem there.

What if both your __() calls contained translator comments for example?

Apply the translators comments in the order they appear to the strings in the order they appear.

If I have x translators comments, but x+y strings, then apply the translators comments in the order they appear but skip strings that do not match the placeholders (and apply the translators comment to the next string where the placeholders match).

I'd recommend writing these statements on multiple lines.

See #154 (comment) (and the previous one)
I only checked PHP. Maybe this works correctly in JS already? (and the PHP-gettext bug-fix is buggy)

@swissspidy
Copy link
Member

There will always be ambiguity when there are multiple i18n functions on a single line, because there can only be 1 preceeding translator comment on the line before.

Using multi-line statements is a reasonable workaround to avoid all potential issues. Have you tried that?

@kkmuffme
Copy link
Author

Translators comments should still only be applied once.

Using multi-line statements is a reasonable workaround to avoid all potential issues. Have you tried that?

Yes, in some cases that isn't possible though, as it would make maintenance much harder.
e.g. I have about 20 email templates for about 15 different websites, which all contain identical and non-identical markup. Changing it to multi-line is tons of work, but mostly it will make maintenance harder - if I want to check for a specific markup string to change everywhere, I now have to do a multi-line regex search and a multi-line replacement.

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

No branches or pull requests

2 participants