[wp-trac] [WordPress Trac] #49178: Upload image issue after deleting image from edit image page

[WordPress Trac]

#49178: Upload image issue after deleting image from edit image page
------------------------------+------------------------
 Reporter:  rnitinb           |       Owner:  joedolson
     Type:  defect (bug)      |      Status:  closed
 Priority:  normal            |   Milestone:  6.2
Component:  Media             |     Version:  5.3.2
 Severity:  normal            |  Resolution:  fixed
 Keywords:  has-patch commit  |     Focuses:
------------------------------+------------------------

Comment (by SergeyBiryukov):

 Replying to [comment:29 joedolson]:
 > One possibility for some code simplification here:
 >
 >
 > {{{
 > +     if ( 1 === $detached ) {
 > +             $message = __( 'Media file detached.' );
 > +     } else {
 > +             /* translators: %s: Number of media files. */
 > +             $message = _n( '%s media file detached.', '%s media files
 detached.', $detached );
 > +     }
 > }}}
 >
 > Each check includes an if/else structure gives a different message
 depending on whether the result is singular or plural. The value of
 `$detached` (in this example) is an absint, so it's always going to be a
 non-negative integer; so the singular return value of `_n()` here would
 never be used. I can't see any reason these couldn't be collapsed to
 remove the if/else.

 Just to follow up here, the singular return value of `_n()` would actually
 be used in languages that have more than one plural form, see a note in
 the [https://developer.wordpress.org/plugins/internationalization/how-to-
 internationalize-your-plugin/#basic-pluralization plugin i18n handbook]:
 > Note that some languages use the singular form for other numbers (e.g.
 21, 31 and so on, much like ’21st’, ’31st’ in English). If you would like
 to special case the singular, check for it specifically:
 > {{{
 > if ( 1 === $count ) {
 >     printf( esc_html__( 'Last thing!', 'my-text-domain' ), $count );
 > } else {
 >     printf( esc_html( _n( '%d thing.', '%d things.', $count, 'my-text-
 domain' ) ), $count );
 > }
 > }}}

 For more context:
 * Changesets:
  * [31941]: Decouple strings where the singular and plural form are not
 just the same string with different numbers, but essentially two different
 strings.
  * [31951]: After [31941], use the decoupled strings from
 `WP_Customize_Manager::register_controls()` on the Menus screen.
  * [32029]: After [31941], use the decoupled strings from `wp-
 admin/network/themes.php` in `wp-admin/network/site-themes.php` as well.
  * [34521]: Plugins: Don't use `_n()` for singular/plural strings which
 have no placeholder for a number.
  * [35611]: About: Don't use `_n_noop()` for singular/plural strings which
 provide no placeholder for a number.
 * Comments:
  * comment:5:ticket:28502
  * comment:23:ticket:28502
  * comment:37:ticket:28502
 * Tickets: #28502, #33239, #34127, #34307, #34308.

 Replying to [comment:30 joedolson]:
 > I also note that the function `number_format_i18n()` is not needed here,
 as this value will never be a float.

 I believe it is best practice to always include `number_format_i18n()`
 when using `sprintf()` with `_n()`, as it not only deals with float
 values, but also formats the thousands separator according to the locale.
 It might not be common to perform an action on more than a thousand of
 media items at once, but it's technically possible, and including
 `number_format_i18n()` would be more consistent with the rest of core.

 Removing the `number_format_i18n()` calls in [55178] does not appear to be
 related to that particular fix, so I have restored them in [55631] as part
 of some other cleanup :)

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/49178#comment:39>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform