Make WordPress Core

Opened 6 years ago

Closed 15 months ago

Last modified 15 months ago

#39733 closed enhancement (fixed)

List item separator should be a WP_Locale property

Reported by: sergeybiryukov's profile SergeyBiryukov Owned by: audrasjb's profile audrasjb
Milestone: 6.0 Priority: normal
Severity: normal Version:
Component: I18N Keywords: good-first-bug has-patch commit has-screenshots needs-dev-note
Focuses: Cc:


Currently, some (most?) themes have a translatable list item separator for displaying a list of categories or tags. See an example in Twenty Seventeen:

/* translators: used between list items, there is a space after the comma */
$separate_meta = __( ', ', 'twentyseventeen' );

One of ru_RU translation team contributors made a point that list item separator is a locale property, and it doesn't make much sense to translate it separately in multiple projects.

We could probably add it as a property of WP_Locale class, in addition to ::number_format['thousands_sep'] and ::number_format['decimal_point'], and there should be a public function like number_format_i18n(), e.g. get_list_item_separator(), for plugins and themes to use. Thoughts?

Attachments (5)

39733.2021091800.patch (19.9 KB) - added by rsiddharth 21 months ago.
Initial version (2021091800)
39733.2021092000.patch (16.0 KB) - added by rsiddharth 21 months ago.
Version 2021092000
39733.2022011300.patch (19.2 KB) - added by rsiddharth 17 months ago.
Capture d’écran 2022-03-13 à 19.58.38.png (176.0 KB) - added by audrasjb 15 months ago.
After patch: separator works on WP-Admin
Capture d’écran 2022-03-13 à 19.58.57.png (258.2 KB) - added by audrasjb 15 months ago.
After patch: separator works on front-end (example: Twenty Thirteen)

Download all attachments as: .zip

Change History (21)

#1 @swissspidy
6 years ago

Sounds useful. Besides the default themes there are three instances of __( ', ' ) being used in core. Perhaps more when counting instances of implode( ', ', … ) that could make use of a translatable list item separator.

#2 @swissspidy
23 months ago

  • Keywords needs-patch good-first-bug added
  • Milestone changed from Awaiting Review to Future Release

21 months ago

Initial version (2021091800)

#3 @rsiddharth
21 months ago

  • Keywords has-patch added; needs-patch removed

@swissspidy @SergeyBiryukov I've uploaded the initial version of the patch.

Are you able to tell if I'm going in the correct direction?

#4 @johnbillion
21 months ago

Just noting that I don't think it will be possible to change the default themes to use this new method because they need to remain compatible with the version of WordPress where they were introduced. A compatibility layer in each theme could be an option.

#5 @SergeyBiryukov
21 months ago

Thanks for the patch! The WP_Locale part looks good at a glance.

For the usage though, I think we'd want to avoid using the $wp_locale global in all those instances, and just wrap this in a function like wp_get_list_item_separator() that would still call $wp_locale->get_list_item_separator() internally.

As noted above, we should also think about backward compatibility for bundled themes.

21 months ago

Version 2021092000

#6 @rsiddharth
21 months ago

@johnbillion @SergeyBiryukov I uploaded the second version of the patch.


  • Added wrapper function -- wp_get_list_item_separator() -- for WP_Locale::get_list_item_separator.
  • Replaced $wp_locale->get_list_item_separator() calls with wp_get_list_item_separator() calls.
  • Added a "compatibility layer" for the twentyeleven theme. Does that look correct?
Last edited 21 months ago by rsiddharth (previous) (diff)

This ticket was mentioned in Slack in #core by rsiddharth. View the logs.

16 months ago

#8 @SergeyBiryukov
16 months ago

  • Milestone changed from Future Release to 6.0

#10 @audrasjb
15 months ago

The above PR refreshes the previous patch against trunk and updates @since mentions. It also add again a removed translator comment.

#11 @audrasjb
15 months ago

  • Owner set to audrasjb
  • Status changed from new to accepted

The patch works fine on my side using default themes or navigating through admin screens.
Self assigning for commit consideration.

15 months ago

After patch: separator works on WP-Admin

15 months ago

After patch: separator works on front-end (example: Twenty Thirteen)

#12 @audrasjb
15 months ago

  • Keywords commit has-screenshots added

Manual tests are going well. Marking for commit.

#13 @audrasjb
15 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 52929:

i18n: Define List item separator as a WP_Locale property.

The list item separator is a locale property, and it doesn't make much sense to translate it separately in multiple projects. This changeset implements the following modifications:

  • Define list item separator as a new WP_Locale property
  • Add wp_get_list_item_separator() as a wrapper for WP_Locale::get_list_item_separator
  • Replace $wp_locale->get_list_item_separator() calls with wp_get_list_item_separator()
  • Added a compatibility layer for bundled themes

Props SergeyBiryukov, swissspidy, rsiddharth, johnbillion, audrasjb.
Fixes #39733.

#14 @audrasjb
15 months ago

  • Keywords needs-dev-note added

This change deserves a dev note or at least a mention in the Misc changes dev note.

#16 @SergeyBiryukov
15 months ago

In 52933:

I18N: Move wp_get_list_item_separator() to a more appropriate place.

Follow-up to [52929].

See #39733.

Note: See TracTickets for help on using tickets.