#52696 closed enhancement (fixed)
Add a way to determine whether a translation exists
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.7 | Priority: | normal |
Severity: | minor | Version: | |
Component: | I18N | Keywords: | good-first-bug has-patch has-unit-tests commit has-dev-note |
Focuses: | Cc: |
Description
Currently __()
wraps pomo/translations.php::translate_entry()̀
in such a way:
<?php function translate( $singular, $context = null ) { $entry = new Translation_Entry( array( 'singular' => $singular, 'context' => $context, ) ); $translated = $this->translate_entry( $entry ); return ( $translated && ! empty( $translated->translations ) ) ? $translated->translations[0] : $singular; }
If no translation exist, the original is returned which is the more frequently desirable behavior.
But in some cases, the user may want to know whether the string obtained from __()
is actually the translation or the original.
Use-case: I'm inserting programmatically taxonomy terms translated using a 3rd-party service. I'm iterating over [__("t1"), __("t2"), ...]
and I would like to only insert those actually translated, ignoring the others.
Currently, I've no way to discriminate between the original and the translation (which may or may not be identical btw).
I'd like a function providing me this ability like. One such example
__i18n_exists($term, $context = null);
Attachments (1)
Change History (38)
#3
@
21 months ago
Hi! I'm exploring an approach like:
is_translated( $string, $domain );
is_translated_n( $string, $domain );
#4
@
21 months ago
Hi!
I've startet working on this on the WCEU contributor day. My first approach of the requested feature looks like this:
<?php function __i18n_exists( $text, $domain = 'default' ) { $translations = get_translations_for_domain( $domain ); $translation = $translations->translate( $text ); if ( empty( $translations->entries ) ) { return null; } return $translation; }
If I test it against de_DE
translations "Post" translates to "Beitrag" but en_EN
will just return null
.
Let me know if that's the kind of behaviour you're looking for.
#5
@
21 months ago
@ckanitz I'm here too! What's your name? I'll try to find you :D
About your approach, I think we should return always a bool, not a string.
So, what do you think about this:
<?php return empty( $translations->entries );
This ticket was mentioned in PR #4581 on WordPress/wordpress-develop by ckanitz.
21 months ago
#6
- Keywords has-patch added; needs-patch removed
Worked together with @samuelsilvapt (wp.org profile: https://profiles.wordpress.org/samuelsilvapt/) on this.
We updated the __
and translate()
function within /wp-includes/i18n.php
as well as Translations::translate()
function to support a new optional parameter $return_singular
.
The new parameter defaults to true
but if set to false, instead of the inputed $text
as a fallback if no translation exists for that string null
will returned instead.
We also introduced a new function has_translation( $text, $domain )
for explicit checks (like hasTranslation
is already part of the @wordpress/i18n package).
Trac ticket: https://core.trac.wordpress.org/ticket/52696
This ticket was mentioned in Slack in #core by samuelsilvapt. View the logs.
21 months ago
@swissspidy commented on PR #4581:
21 months ago
#8
Great progress so far 👍
I would just recommend that instead of adding a new parameter to the translate()
method, there should be a new has_translation()
method. Also no need for changes to the __()
function.
Something like this:
function has_translation( $text, $domain = 'default' ) {
$translations = get_translations_for_domain( $domain );
return $translations->has_translation( $text );
}
That makes everything much easier to understand and more maintainable.
#9
@
21 months ago
Thanks for the feedback! I got the idea of your recommendation and agree, that has_translation()
should be part of the Translate()
(and NOOP_Translate()
) class in translations.php
and a wrapper in l10n.php
.
But that's just for the helper functions, which is nice to have, but wouldn't be quiet "elegant" if you'd have to loop over a bunch of strings instead of just calling the known translation functions with an extra parameter tweaking the returned value.
We've started with an own function but since it was just a copy of __
we dropped it and thought about a more DRY approach with a new optional nonbreaking parameter.
If it's a real "no-go" to update the existing functions I'd propose for a new function after all but could need some help to find a better name instead of __i18n_exists
.
#10
@
21 months ago
- Keywords needs-refresh added
Repurposing __()
in such a way is a non-goal, so I would really just add a separate translation_exists()
or has_translation
function and a separate method on the translations class as described in my previous comment.
#12
@
12 months ago
- Summary changed from __() avoiding default language fallback to Add a way to determine whether a translation exists
This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.
11 months ago
#15
@
10 months ago
Any contributor is welcome to work on a patch.
Note that a lot has changed in regards to i18n as we added a new i18n library in 6.5. So this would require working on WP_Translation_Controller
now.
#16
@
9 months ago
- Keywords has-patch needs-testing added; needs-patch removed
Added a new has_translation
public method to the WP_Translation_Controller
which calls the protected locate_translation
method to determine if a translation is available for the given string (with optional textdomain and locale).
Also, exposed this in l10n.php
.
Just a side note that this is my first attempt at contributing here so apologises for any mistakes on my end.
#17
@
9 months ago
- Keywords needs-unit-tests added
Thanks, that's a great starting point!
To make collaboration and code review easier, I recommend creating a PR on GitHub. It will also make it easier to address PHPCS issues and work on unit tests.
Functionality-wise, we don't need the null === $locale
check in the has_translation()
function as WP_Translation_Controller::has_translation()
already does that. So it's redundant.
This ticket was mentioned in PR #6649 on WordPress/wordpress-develop by @tomhine.
9 months ago
#18
Added a new has_translation
public method to the WP_Translation_Controller
which calls the protected locate_translation
method to determine if a translation is available for the given string (with optional textdomain and locale).
Also, exposed this in l10n.php
.
Trac ticket: https://core.trac.wordpress.org/ticket/52696#comment:17
#19
@
9 months ago
@swissspidy
The translate function present in the WP_Translation_Controller
is already utilizing this locate_translation()
protected function but in the l10n.php file we are still using the NOOP Translate instance based translate function.
Wouldn't it be more efficient if we switch to using the WP_Translation_Controller's translate function like we are utilising the set_locale()
function from the controller?
That way we would not be required to make a separate call to the has_translation()
first and then call the translating functions.
#20
@
9 months ago
Not sure I follow. You mean you want to utilize WP_Translation_Controller's
in __()
& co? That seems like an entirely different change (with potential backward compatibility implications). And not sure how that would address this ticket.
But I might be misunderstanding your request. In that case, feel free to open a PR with a suggested code change or so.
FWIW no one is required to use both functions. I consider has_translation()
an edge case that rarely ever someone truly needs. Most of the time you should just translate.
This ticket was mentioned in PR #6658 on WordPress/wordpress-develop by @mchirag2002.
9 months ago
#21
The added function has_translate()
tackles the edge case where the user might need to check if the translation exists or not before actually translating the string.
The function returns the translation of the string if it exists and returns false
if in case the translation does not exist.
This should potentially save the multiple calls for the functions like once to check whether translation exists or not and then to actually translate.
Trac ticket: https://core.trac.wordpress.org/ticket/52696
#22
@
9 months ago
@swissspidy
Please take a look at this implementation made in the PR. This way in the edge case that the user might require to check whether the translation exists or not, they wouldn't have to make a separate function call to actually translate the ones which have translations available. The functionality was already present in the WP_Translations_Controller
.
@swissspidy commented on PR #6658:
9 months ago
#23
This does not work because you don't know if the returned string is the original or just a translation that happens to be identical to the original string.
That's why #6649 works better.
@mchirag2002 commented on PR #6658:
9 months ago
#24
@swissspidy Actually in this case the original string would never be returned. If there is no translation present, it would return a boolean false and if there exists a string which has a translation which is identical, only in such case the translated string shall be returned.
Internally the translate function also utilises the same locate_translate()
function but this reduces the number of function calls you have to make to get the translation of all the strings.
@swissspidy commented on PR #6658:
9 months ago
#25
if there exists a string which has a translation which is identical, only in such case the translated string shall be returned.
And that is exactly the problem I am describing. This is not desired here.
This ticket was mentioned in PR #6805 on WordPress/wordpress-develop by @louiswol94.
8 months ago
#26
- Keywords has-unit-tests added; needs-unit-tests removed
#27
follow-up:
↓ 28
@
8 months ago
Hi everyone,
I am sitting here at Contributor Day at WCEU, and by the tags I saw this was a good first ticket, and that only Unit tests were still needed.
I gathered that #6649 was the accepted solution, so I copied that code, and added some unit tests here: https://github.com/WordPress/wordpress-develop/pull/6805
The original author is welcome to use my tests as inspiration, or we can merge my PR, I am not sure what is the best way forward.
Kind regards
#28
in reply to:
↑ 27
@
5 months ago
Replying to louiswol94:
Hi everyone,
I am sitting here at Contributor Day at WCEU, and by the tags I saw this was a good first ticket, and that only Unit tests were still needed.
I gathered that #6649 was the accepted solution, so I copied that code, and added some unit tests here: https://github.com/WordPress/wordpress-develop/pull/6805
The original author is welcome to use my tests as inspiration, or we can merge my PR, I am not sure what is the best way forward.
Kind regards
Nice work. I'm just looking at this to see if anything is left to be done. It looks like the solution is agreed on and you have added tests.
A couple of the links included in comments here point to 6649 on trac and I think they were meant to link to the PR on GitHub: https://github.com/WordPress/wordpress-develop/pull/6649. Use full URL's to GitHub because Trac auto-links tickets if the number happens to match.
@swissspidy commented on PR #6649:
5 months ago
#30
Closing in favor of #6805 which builds upon this and adds tests
@swissspidy commented on PR #6658:
5 months ago
#31
Closing in favor of #6805 which is closer and adds tests
#32
@
5 months ago
- Milestone changed from Future Release to 6.7
- Owner set to swissspidy
- Status changed from new to reviewing
@swissspidy commented on PR #6805:
5 months ago
#35
Committed in https://core.trac.wordpress.org/changeset/59029
@louiswol94 commented on PR #6805:
5 months ago
#36
Thanks @swissspidy
This would make it consistent with the
@wordpress/i18n
JS package which has ahasTranslation
function for this purpose as well.