Make WordPress Core

Opened 5 years ago

Last modified 4 months ago

#39733 new enhancement

List item separator should be a WP_Locale property

Reported by: SergeyBiryukov Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: I18N Keywords: good-first-bug has-patch
Focuses: Cc:

Description

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 (3)

39733.2021091800.patch (19.9 KB) - added by rsiddharth 4 months ago.
Initial version (2021091800)
39733.2021092000.patch (16.0 KB) - added by rsiddharth 4 months ago.
Version 2021092000
39733.2022011300.patch (19.2 KB) - added by rsiddharth 2 weeks ago.

Download all attachments as: .zip

Change History (9)

#1 @swissspidy
5 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
6 months ago

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

@rsiddharth
4 months ago

Initial version (2021091800)

#3 @rsiddharth
4 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
4 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
4 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.

@rsiddharth
4 months ago

Version 2021092000

#6 @rsiddharth
4 months ago

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

Changes:

  • 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 4 months ago by rsiddharth (previous) (diff)
Note: See TracTickets for help on using tickets.