[PATCH] number_format enhancements

php.internals

Matt W

20 years ago
Hi, There are a couple things I don't like about how number_format() works currently, so I'm hoping the functionality changes this patch provides can be added. :-) First is that when you're working with whole numbers and/or don't want any decimal places, both decimal-related parameters are needed before the thousands separator. I always thought it would be logical to pass the thousands separator as param #2 in those cases. That's what my changes allow if it's a non-numeric string. Second is that when decimals places ARE specified, I would like to be able to have trailing zeros/point removed. (Like "(string) round(...)" but never scientific notation.) I'm sure I'm not the only one that has wanted grouped thousands and/or custom decimal point *without* trailing 0's. :-) The patch allows this by specifying negative decimal places -- e.g. -2 is 2 decimals without extra zeros. Are these enhancements all right? If they're agreed to, I'll create more number_format() tests right away if desired. Just wanted to get any thoughts on the patch first. Also, I changed PHP_ROUND_WITH_FUZZ to skip unnecessary steps if places==0 (round() is nearly 20% faster then, in my quick tests), corrected the misspelling of "separator" in the proto, and simplified some of the code. http://realplain.com/php/number_format_enhancements.diff http://realplain.com/php/number_format_enhancements_5_2.diff Thanks, Matt

Matt W

20 years ago
Hi, Haven't heard any opinions on these functionality changes... (Though I know more significant things keep you busy!) Do many other people use number_format()?? Wouldn't allowing thousands separator as param #2 simplify things? Any issues with the semantics of operation? Negative precision to specify trailing zero removal seems fine at least, even though that's different than round()'s behavior. I've now made some tests, including changes of course, in case you were just waiting for them? Hope they're enough. http://realplain.com/php/number_format.phpt Thanks, Matt ----- Original Message ----- From: "Matt W" Sent: Wednesday, July 19, 2006
> Hi, > > There are a couple things I don't like about how number_format() works > currently, so I'm hoping the functionality changes this patch provides can > be added. :-) > > First is that when you're working with whole numbers and/or don't want any > decimal places, both decimal-related parameters are needed before the > thousands separator. I always thought it would be logical to pass the > thousands separator as param #2 in those cases. That's what my changes > allow if it's a non-numeric string. > > Second is that when decimals places ARE specified, I would like to be able > to have trailing zeros/point removed. (Like "(string) round(...)" but
never
> scientific notation.) I'm sure I'm not the only one that has wanted
grouped
> thousands and/or custom decimal point *without* trailing 0's. :-) The
patch

bertrand Gugger

20 years ago
Bonsoir Matt, Matt W wrote:
> Hi, > > Haven't heard any opinions on these functionality changes... (Though I know > more significant things keep you busy!) > > Do many other people use number_format()?? Wouldn't allowing thousands > separator as param #2 simplify things? > > Any issues with the semantics of operation? Negative precision to specify > trailing zero removal seems fine at least, even though that's different than > round()'s behavior. > > I've now made some tests, including changes of course, in case you were just > waiting for them? Hope they're enough. > > http://realplain.com/php/number_format.phpt
I did not look into that .phpt link I belong only to userland , so have less to tell here. Give some basic userland examples about what you propose to change, please ... (I guess these are a part of the .phpt) Bravo for the way you propose things. Kind regards,
-- toggg

bertrand Gugger

20 years ago
bertrand Gugger wrote:
> Bonsoir Matt, > Matt W wrote: > >> Hi, >> >> Haven't heard any opinions on these functionality changes... (Though >> I know >> more significant things keep you busy!) >> >> Do many other people use number_format()?? Wouldn't allowing thousands >> separator as param #2 simplify things? >> >> Any issues with the semantics of operation? Negative precision to >> specify >> trailing zero removal seems fine at least, even though that's >> different than >> round()'s behavior. >> >> I've now made some tests, including changes of course, in case you >> were just >> waiting for them? Hope they're enough. >> >> http://realplain.com/php/number_format.phpt > > I did not look into that .phpt link > I belong only to userland , so have less to tell here. > > Give some basic userland examples about what you propose to change, > please ... > (I guess these are a part of the .phpt) > > Bravo for the way you propose things.
I did : $ wget http://snaps.php.net/php5.2-200607222030.tar.bz2 $ tar xjf php5.2-200607222030.tar.bz2 $ cd php5.2-20060722203 $ cat ../php-5.1.4/config.nice #! /bin/sh # # Created by configure './configure' \ '--with-zlib' \ '--without-pear' \ '--without-xmlrpc' \ '--program-suffix=5' \ '--enable-cli' \ '--disable-cgi' \ '--enable-memory-limit' \ "$@" $ ./configure < ../php-5.1.4/config.nice $ make $ sapi/cli/php run-tests.php ~/A/mattW/number_format.phpt which naturally failed $ cat ~/A/mattW/number_format.diff 010+ 123 011+ 1,234,567,890 010- 0123 011- 12,345,67890 020+ 10 020- 9.877 022+ 1 235 022- 1 234.51 025+ 1,235 026+ 12,345 025- 1235 027- 12 345 028+ 9,876,543 028- 9.876.543 I hope these are tests , sorry for the userland phrasing. I guess target is test does not fail. à+
-- toggg

Matt W

20 years ago
Hi, ----- Original Message ----- From: "bertrand Gugger" Sent: Saturday, July 22, 2006
> Bonsoir Matt, > Matt W wrote: > > Hi, > > > > Haven't heard any opinions on these functionality changes... (Though I
know
> > more significant things keep you busy!) > > > > Do many other people use number_format()?? Wouldn't allowing thousands > > separator as param #2 simplify things? > > > > Any issues with the semantics of operation? Negative precision to
specify
> > trailing zero removal seems fine at least, even though that's different
than
> > round()'s behavior. > > > > I've now made some tests, including changes of course, in case you were
just
> > waiting for them? Hope they're enough. > > > > http://realplain.com/php/number_format.phpt > I did not look into that .phpt link > I belong only to userland , so have less to tell here. > > Give some basic userland examples about what you propose to change, > please ... > (I guess these are a part of the .phpt) > > Bravo for the way you propose things. > > Kind regards, > -- > toggg
In the .phpt file, it is userland code where you can see examples of how the changes work. :-) I'm on Windows, so I don't know how you apply the .diff to your source download -- shouldn't diff work somehow? :-/ Well, I uploaded the full math.c file for 5.2 and 6 if you want to grab that to replace yours (ext/standard/math.c) and experiment with the changes. http://realplain.com/php/math.c-number_format_enhancements.tar.gz (5.2/6.0 directories with the file.) Glad you like the idea. :-) I just hope functionality like this can be added -- using my patch or not, or slightly different semantics... All I want is a no extra 0's option and not be forced to include 2 unnecessary decimal-related parameters to "get to" thousands separator when I don't want any decimals! ;-D Matt

bertrand Gugger

20 years ago
Matt W wrote:
> Hi, >>Give some basic userland examples about what you propose to change, >>please ... >>(I guess these are a part of the .phpt) > > In the .phpt file, it is userland code where you can see examples of how the > changes work. :-)
Sure, but I was meaning , in so far you extend the functionality of number_format , then you will need to extend the doc. Some draft of this doc extension would be usefull , even in this test phase and especially for "userland" people.
> > I'm on Windows, so I don't know how you apply the .diff to your source > download -- shouldn't diff work somehow? :-/ Well, I uploaded the full > math.c file for 5.2 and 6 if you want to grab that to replace yours > (ext/standard/math.c) and experiment with the changes. > > http://realplain.com/php/math.c-number_format_enhancements.tar.gz (5.2/6.0 > directories with the file.)
I applied the patch http://realplain.com/php/number_format_enhancements_5_2.diff to the snap: [php5.2-200607222030]$ patch -p3 -b -i /home/bertrand/A/mattW/number_format/5.2/number_format_enhancements_5_2.diff No problem , I could check I get the right ext/standard/math.c by diffing whith the math.c you published in this tar.gz. $ diff ext/standard/math.c ~/A/mattW/number_format/5.2/ 981c981 < int is_negative=0, trim_dec=0; ---
> int is_negative = 0, trim_dec = 0;
so just a litle cleaning difference. As a note , the patch also corrected ext/standard/basic_functions.c (seperator ==> separator) which I guess is needed too :) make clean , configure , make Then , using your original .phpt : $ export TEST_PHP_EXECUTABLE=/home/bertrand/php/php5.2-200607222030/sapi/cli/php $ sapi/cli/php run-tests.php ~/A/mattW/number_format.phpt ===================================================================== CWD : /home/bertrand/php/php5.2-200607222030 PHP : /home/bertrand/php/php5.2-200607222030/sapi/cli/php PHP_SAPI : cli PHP_VERSION : 5.2.0-dev ZEND_VERSION: 2.2.0 PHP_OS : Linux - Linux ancilla.toggg.net 2.6.12-1.1381_FC3 #1 Fri Oct 21 03:46:55 EDT 2005 i686 INI actual : /usr/local/lib/php.ini More .INIs : Extra dirs : ===================================================================== Running selected tests. PASS number_format() function [/home/bertrand/A/mattW/number_format.phpt] Nice ! Just not sure if the .phpt wouldn't need some more cases ("classical" ones).
> > Glad you like the idea. :-) I just hope functionality like this can be > added -- using my patch or not, or slightly different semantics... All I > want is a no extra 0's option and not be forced to include 2 unnecessary > decimal-related parameters to "get to" thousands separator when I don't want > any decimals! ;-D
Actually , I was talking about the way you *propose* things , not about the things themselves. Errmmm.... not sure I'm not boring the list with such "userland" blabla, I just hope it can serve some of my "ignorant" colleagues. Cheers
-- toggg

Matt W

20 years ago
Hi bertrand, ----- Original Message ----- From: "bertrand Gugger" Sent: Sunday, July 23, 2006
> Matt W wrote: > > Hi, > >>Give some basic userland examples about what you propose to change, > >>please ... > >>(I guess these are a part of the .phpt) > > > > In the .phpt file, it is userland code where you can see examples of how
the
> > changes work. :-) > Sure, but I was meaning , in so far you extend the functionality of > number_format , then you will need to extend the doc. > Some draft of this doc extension would be usefull , even in this test > phase and especially for "userland" people.
Oh, I see. I don't know anything about changing docs themselves (as far as patch-type stuff goes). :-) Thought that was handled by someone else... Are you one of "the PHP people?" I *could* do a draft of doc changes, of course, *if* I knew the changes would be applied. But I've not heard from anyone but you; maybe since it's just a "little ol' function." :-/
> > I'm on Windows, so I don't know how you apply the .diff to your source > > download -- shouldn't diff work somehow? :-/ Well, I uploaded the full > > math.c file for 5.2 and 6 if you want to grab that to replace yours > > (ext/standard/math.c) and experiment with the changes. > > > > http://realplain.com/php/math.c-number_format_enhancements.tar.gz
(5.2/6.0
> > directories with the file.) > I applied the patch > http://realplain.com/php/number_format_enhancements_5_2.diff to the snap: > [php5.2-200607222030]$ patch -p3 -b -i >
/home/bertrand/A/mattW/number_format/5.2/number_format_enhancements_5_2.diff
> > No problem , I could check I get the right ext/standard/math.c by > diffing whith the math.c you published in this tar.gz. > $ diff ext/standard/math.c ~/A/mattW/number_format/5.2/ > 981c981 > < int is_negative=0, trim_dec=0; > --- > > int is_negative = 0, trim_dec = 0; > so just a litle cleaning difference. > As a note , the patch also corrected ext/standard/basic_functions.c > (seperator ==> separator) which I guess is needed too :)
Oops, yeah, you found the whitespace changes I made in the patch. :-P Well, basic_functions.c typo fix isn't needed for the functionality changes, of course. :-)
> make clean , configure , make > > Then , using your original .phpt : > $ export > TEST_PHP_EXECUTABLE=/home/bertrand/php/php5.2-200607222030/sapi/cli/php > $ sapi/cli/php run-tests.php ~/A/mattW/number_format.phpt > > ===================================================================== > CWD : /home/bertrand/php/php5.2-200607222030 > PHP : /home/bertrand/php/php5.2-200607222030/sapi/cli/php > PHP_SAPI : cli > PHP_VERSION : 5.2.0-dev > ZEND_VERSION: 2.2.0 > PHP_OS : Linux - Linux ancilla.toggg.net 2.6.12-1.1381_FC3 #1 Fri > Oct 21 03:46:55 EDT 2005 i686 > INI actual : /usr/local/lib/php.ini > More .INIs : > Extra dirs : > ===================================================================== > Running selected tests. > PASS number_format() function [/home/bertrand/A/mattW/number_format.phpt] > > Nice ! > Just not sure if the .phpt wouldn't need some more cases ("classical"
ones). Yeah, I don't know how many variations are desired...
> > > > Glad you like the idea. :-) I just hope functionality like this can be > > added -- using my patch or not, or slightly different semantics... All
I
> > want is a no extra 0's option and not be forced to include 2 unnecessary > > decimal-related parameters to "get to" thousands separator when I don't
want
> > any decimals! ;-D > Actually , I was talking about the way you *propose* things , not about > the things themselves.
Ooooh! Hehe Thanks then. :-)
> Errmmm.... not sure I'm not boring the list with such "userland" blabla, > I just hope it can serve some of my "ignorant" colleagues. > > Cheers > -- > toggg
Matt

bertrand Gugger

20 years ago
Matt W wrote:
>> >>Sure, but I was meaning , in so far you extend the functionality of >>number_format , then you will need to extend the doc. >>Some draft of this doc extension would be usefull , even in this test >>phase and especially for "userland" people. > > > Oh, I see. I don't know anything about changing docs themselves (as far as > patch-type stuff goes). :-)
Yes , it's heavy docbook thing , so you need to be a freak yourself :)
> Thought that was handled by someone else...
Sure , there's a great team of people there , translators , with their own list , etc.
> Are you one of "the PHP people?"
I belong only to one of the first "userland" layer , pear , though I'm currently inactive there.
> > I *could* do a draft of doc changes, of course, *if* I knew the changes > would be applied. But I've not heard from anyone but you; maybe since it's > just a "little ol' function." :-/
Yes , case it's accepted , just a .txt (I mean no docbook) with the needed additions to the manual page, sent or bugged to the doc team would be helpfull.
-- toggg

bertrand Gugger

20 years ago
Matt W wrote:
> Hi, > > Haven't heard any opinions on these functionality changes... (Though I know > more significant things keep you busy!) > > Do many other people use number_format()?? Wouldn't allowing thousands > separator as param #2 simplify things? > > Any issues with the semantics of operation? Negative precision to specify > trailing zero removal seems fine at least, even though that's different than > round()'s behavior. > > I've now made some tests, including changes of course, in case you were just > waiting for them? Hope they're enough. > > http://realplain.com/php/number_format.phpt
Sorry , I did not come better than the attached test copies http://toggg.info/number_format.phpt & http://toggg.info/number_format.diff (you don't need "\n") I wonder why lines are fliping , I guess it's a diff effect Let me know how I apply your patch , so I get it blank :)
-- toggg