? ? Success

User tests: Successful: Unsuccessful:

avatar yild
yild
18 Aug 2016

Pull Request for Issue # .

Summary of Changes

Simple change from string specifier %s to integer specifier %d in multiple strings.

Values for place holders are integers not strings - second param for JText::plurar() function, i.e. https://github.com/joomla/joomla-cms/blob/b2a4b483995875373da5b385011225358abeb571/libraries/legacy/controller/admin.php#L237

Testing Instructions

Code review

avatar joomla-cms-bot joomla-cms-bot - change - 18 Aug 2016
Category Language & Strings Administration
avatar yild yild - open - 18 Aug 2016
avatar yild yild - change - 18 Aug 2016
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 18 Aug 2016
Labels Added: ? ?
avatar infograf768
infograf768 - comment - 19 Aug 2016

Although this is indeed correct theoretically speaking, it has no effect whatsoever on the resulting display. Is it really worth changing?


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11662.

avatar yild
yild - comment - 24 Aug 2016

I don't know if its truly necessary - I just wanted to post this 'issue', ignore it (close) or not is not my call :)

avatar ggppdk
ggppdk - comment - 24 Aug 2016

I would be against using %d,
unless it is really 100% sure that we will never need to pass a string to it ...

e.g. i am doing this in my views (but i am not using these Joomla language strings, instead using my own)
4 out of 7 items were deleted

notice the:
4 out of 7

so if you want to make an addition to the existing code, you will have to change the language strings back to %s,

but then you have a problem:

  • think of all the translations that will have been updated to have %d

unless existing strings are causing a problem, it is best not to do this, reverting it in the future, would be problematic !

avatar yild
yild - comment - 24 Aug 2016

I was thinking about it, that best way would be to use %s in every single key when param is passed to a string. But again it would be a pain for translators OR not replace %d with %s in multiple files is a simple procedure, even at Crowdin.

avatar brianteeman
brianteeman - comment - 26 Aug 2016

I am setting this to needs review so a decision can be made.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11662.

avatar brianteeman brianteeman - change - 26 Aug 2016
Status Pending Needs Review
Labels
avatar rdeutz
rdeutz - comment - 6 Sep 2016

Seems to me not worth the change, @yild you are correct here but at the end of the day it will change nothing but it will be work for the translation team. Thanks for bringing it up.

avatar rdeutz rdeutz - change - 6 Sep 2016
Status Needs Review Closed
Closed_Date 0000-00-00 00:00:00 2016-09-06 13:41:04
Closed_By rdeutz

Add a Comment

Login with GitHub to post a comment