#56698 closed enhancement (fixed)
Define 'Word count type' as a WP_Locale property
Reported by: | pedromendonca | Owned by: | audrasjb |
---|---|---|---|
Milestone: | 6.2 | Priority: | normal |
Severity: | normal | Version: | 4.3 |
Component: | I18N | Keywords: | has-patch needs-testing-info needs-testing has-unit-tests has-dev-note |
Focuses: | Cc: |
Description
i18n: Define Word count type as a WP_Locale property.
As commented on https://core.trac.wordpress.org/ticket/47511#comment:19, the word count type
should be added as a locale property, so it doesn't need to be translated separately across multiple projects.
Many plugins and themes could use it without the need to set their own _x( 'words', 'Word count type. Do not translate!' )
translation string.
This proposal has the exact same approach as of the recent creation of wp_get_list_item_separator()
on https://github.com/WordPress/wordpress-develop/commit/a37a72077e90a04e4fa6205c6e99fb6614d3bc0d
This proposal implements the following:
- Define
word count type
as a new WP_Locale property - Add
wp_get_word_count_type()
as a wrapper forWP_Locale::get_word_count_type
- Replace all
_x( 'words', 'Word count type. Do not translate!' )
strings withwp_get_word_count_type()
Change History (27)
This ticket was mentioned in PR #3377 on WordPress/wordpress-develop by pedro-mendonca.
2 years ago
#1
#2
@
2 years ago
- Milestone changed from Awaiting Review to 6.2
- Version changed from trunk to 4.3
Introduced in [33517].
This ticket was mentioned in Slack in #core by costdev. View the logs.
23 months ago
#4
@
23 months ago
- Keywords needs-unit-tests needs-testing-info needs-testing changes-requested added
After this ticket was reviewed in the bug scrub, I'm adding the needs-unit-tests
, needs-testing-info
and needs-testing
keywords.
In addition, I've left a requested change on PR 3377 to use str_starts_with()
instead of strpos() === 0
. Adding the changes-requested
keyword.
Additional props: @hellofromTonya
This ticket was mentioned in Slack in #core by mukeshpanchal27. View the logs.
22 months ago
#6
@
22 months ago
This ticket was discussed in the recent bug scrub.
@pedromendonca Can you update @since
annotations and add unit tests for the changes?
Props to @costdev
#7
@
22 months ago
Pedro's comment on the PR is correct, we don't need to worry about @since
tags in patches and PRs as whoever commits the change will ensure the correct value is used.
#8
@
22 months ago
@mukesh27 I've just added some tests for all possible cases.
Let me know if it's ok.
Thank you :)
This ticket was mentioned in Slack in #core by mukeshpanchal27. View the logs.
22 months ago
#10
@
22 months ago
- Keywords has-unit-tests added; needs-unit-tests removed
This ticket was discussed during the bug scrub. I'll submit a refreshed patch that addresses some work needed with the tests, and then I think this will be ready for commit
before the 6.2 Beta 1 release party later today.
Additional props: @mukesh27
This ticket was mentioned in PR #4019 on WordPress/wordpress-develop by @costdev.
22 months ago
#11
_This PR is a refresh of #3377 to improve the tests prior to 6.2 Beta 1_
Adds a word_count_type
property, so that it does not need to be translated separately across multiple projects.
- New property:
WP_Locale::word_count_type
. - New method:
WP_Locale::get_word_count_type()
. - New function:
wp_get_word_count_type()
as a wrapper forWP_Locale::get_word_count_type()
. - All
_x( 'words', 'Word count type. Do not translate!' )
strings have been replaced with a call towp_get_word_count_type()
.
Trac ticket: https://core.trac.wordpress.org/ticket/56698
#12
@
22 months ago
- Keywords commit added
I have submitted PR 4019 to address some work needed in the tests. Adding for commit
consideration.
#13
@
22 months ago
Thanks @costdev for the improvements in the tests.
I would suggest a different wording for the keys of the data provider array.
There is one a valid option
, when in fact the valid ones are three, maybe prefixing them is clearer:
valid - words
valid - characters_excluding_spaces
valid - characters_including_spaces
What do you think?
#14
@
22 months ago
- Keywords changes-requested removed
Thanks @pedromendonca! I've updated the PR to note the valid options.
This ticket was mentioned in Slack in #core by costdev. View the logs.
22 months ago
#16
@
22 months ago
- Owner set to audrasjb
- Resolution set to fixed
- Status changed from new to closed
In 55279:
@audrasjb commented on PR #4019:
22 months ago
#17
committed in https://core.trac.wordpress.org/changeset/55279
#18
follow-up:
↓ 24
@
22 months ago
- Resolution fixed deleted
- Status changed from closed to reopened
I have a case where wp_trim_words
being used in a mu-plugin before $wp_locale
is set.
wp_trim_words
is calling wp_get_word_count_type
and then directly calling on $wp_locale->get_word_count_type
.
Would there be any objection to a patch that does something like this:
function wp_get_word_count_type() { global $wp_locale; if ( ! ( $wp_locale instanceof WP_Locale ) ) { // Default value of get_word_count_type. return 'words'; } return $wp_locale->get_word_count_type(); }
This ticket was mentioned in PR #4030 on WordPress/wordpress-develop by @kraftbj.
22 months ago
#19
wp_trim_words
can be called earlier in the stack than when the WP_Locale
is currently set (during theme setup).
This adds some defensive coding to ensure a value is returned without a fatal error. The default value of words
is used.
Trac ticket: https://core.trac.wordpress.org/ticket/56698
This ticket was mentioned in Slack in #core by sergey. View the logs.
22 months ago
This ticket was mentioned in Slack in #core by sergey. View the logs.
22 months ago
#24
in reply to:
↑ 18
@
22 months ago
Replying to kraftbj:
I have a case where
wp_trim_words
being used in a mu-plugin before$wp_locale
is set.
wp_trim_words
is callingwp_get_word_count_type
and then directly calling on$wp_locale->get_word_count_type
.
Good catch, thanks!
The suggested approach looks good to me. wp_get_list_item_separator()
could use a similar change for consistency.
Commit incoming :)
@SergeyBiryukov commented on PR #4030:
22 months ago
#26
Thanks for the PR! Merged in r55351 with some minor edits.
Add word count type as a locale property, so it doesn't need to be translated separately across multiple projects. This PR implements the following modifications:
wp_get_word_count_type()
as a wrapper forWP_Locale::get_word_count_type
_x( 'words', 'Word count type. Do not translate!' )
strings withwp_get_word_count_type()
Trac ticket: 56698