WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 7 years ago

#5131 closed enhancement (fixed)

wp_tag_cloud(): title='1 topics', should be 1 topic

Reported by: atppp Owned by: westi
Milestone: 2.5 Priority: normal
Severity: normal Version: 2.3
Component: General Keywords: has-patch
Focuses: Cc:

Description

I am picky...

Attachments (2)

category-template.php.diff (842 bytes) - added by webrocker 7 years ago.
category-template.php.diff.txt (795 bytes) - added by webrocker 7 years ago.

Download all attachments as: .zip

Change History (17)

comment:1 webrocker7 years ago

  • Keywords has patch added
  • Milestone changed from 2.5 to 2.3.1
  • Type changed from defect to enhancement
  • Version set to 2.3

comment:2 webrocker7 years ago

been there done that :)

comment:3 follow-up: Nazgul7 years ago

  • Keywords has patch removed

This patch kills right-to-left language support.

comment:4 foolswisdom7 years ago

  • Milestone changed from 2.3.1 to 2.5

Please don't set the milestone, leave that for a core contributor.

comment:5 follow-up: westi7 years ago

Replying to Nazgul:

This patch kills right-to-left language support.

Indeed, patch should be reworked to use __ngettext rather than __

comment:6 in reply to: ↑ 3 ; follow-up: webrocker7 years ago

Replying to Nazgul:

This patch kills right-to-left language support.

Ooops. Is this because of the way how the digit is 'handwired' in front of the 'topics' label (like '1 topic'), whereas a right-to-left needs something like 'topic 1'? In the original it is sprintf( __('%d topics'), $count ), so the digit is in front of the label anyway? Sorry, I don't get the difference between a hardcoded 'topics' after the %d and the variable used in my patch sprintf( __('%d '.$topictxt), $count ).

Replying to foolswisdom:

Please don't set the milestone, leave that for a core contributor.

Ok, sorry for that.

Replying to westi:

Indeed, patch should be reworked to use __ngettext rather than __

well the original used __ , so I didn't messed with that.

comment:7 in reply to: ↑ 5 ; follow-up: webrocker7 years ago

Replying to westi:

Indeed, patch should be reworked to use __ngettext rather than __

I see... would this work
original:
attribute_escape( sprintf( __('%d topics'), $count ) )
patch:
attribute_escape( __ngettext( 'topic', 'topics', $count )
?

comment:8 in reply to: ↑ 6 foolswisdom7 years ago

Replying to webrocker:

Replying to foolswisdom:

Please don't set the milestone, leave that for a core contributor.

Ok, sorry for that.

No serious problem of course! ;-) Keep up the great work!

comment:9 in reply to: ↑ 7 ; follow-up: westi7 years ago

  • Owner changed from anonymous to westi
  • Status changed from new to assigned

Replying to webrocker:

Replying to westi:

Indeed, patch should be reworked to use __ngettext rather than __

I see... would this work
original:
attribute_escape( sprintf( __('%d topics'), $count ) )
patch:
attribute_escape( __ngettext( 'topic', 'topics', $count )
?

Nearly __ngettext allows you to use plural forms in your translations e.g.:

__ngettext( '%d topic', '%d topics', $count)

This will output '1 topic' or '3 topics' giving you the correct plural form while still allowing translators to handle the "right to left" issues by translating the '%d topic' as something like 'topic %d'.

comment:10 JeremyVisser7 years ago

Not a good idea to go in 2.3.1 – string changes are bad for translators.

comment:11 in reply to: ↑ 9 webrocker7 years ago

Replying to westi:

__ngettext( '%d topic', '%d topics', $count)

This will output '1 topic' or '3 topics' giving you the correct plural form while still allowing translators to handle the "right to left" issues by translating the '%d topic' as something like 'topic %d'.

Ok, I think I'm getting it... so it still needs to be used inside the sprintf() function, so that the %d thingie gets replaced with $count, while the __ngettext() needs another instance of $count to determine if $single or $plural is to be used?
So this should work:
attribute_escape( sprintf( __ngettext( '%d topic', '%d topics', $count ), $count ) )

comment:12 webrocker7 years ago

  • Keywords has-patch added

I have tested the above in my 2.3 install, and it works, so I attached another patch-file.

comment:13 foolswisdom7 years ago

  • Milestone changed from 2.5 to 2.4

comment:14 westi7 years ago

Relates to #5172 and #5171 all tag cloud changes.

I will get these all in soon.

comment:15 westi7 years ago

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

(In [6227]) Translation fix for wp_tag_cloud(). Fixes #5131 props webrocker

Note: See TracTickets for help on using tickets.