Re: [PHP-DEV] Add a new flag for json_encode
Ok guys, sorry, but I am giving up on it. I opened the PR in April and all the code necessary with technical implications are done and registered on the PR. I brought the topic to this email list in November as requested in the PR and almost 1 month after you guys requested me to write a RFC. I tried to create a wiki account to write the RFC 15 days ago[1] and I had no answer in create a simple wiki account. I have no idea how long it will take and how long the RFC will be on discussion, but seems the whole process will take over 1 year. Unfortunately I don't have patience enough for it. I guess I put my contribution on the project proposing the code, discussing the technical part and executing benchmarks. If you want to use it, use, but if you want to just ignore because I didn't complete all the steps fine too, just close the PR and this topic is automatically closed. I would like to contribute more with PHP core, but if this is the process to contribute with PHP, I am out. I will help other projects that give more attention and love for who wants to contribute and not making the person to worry about the bureaucratic part. Sorry. [1] http://news.php.net/php.webmaster/20312 Juan Basso On Sun, Nov 30, 2014 at 11:58 PM, Juan Basso jrba...@gmail.com wrote: I see. I thought it was some sort of simplified RFC. :) Ok, I will create a RFC regarding it. On Sun, Nov 30, 2014 at 8:49 PM, Andrea Faulds a...@ajf.me wrote: On 1 Dec 2014, at 01:48, Juan Basso jrba...@gmail.com wrote: What is ofc? I never heard about it before. Do I need to do something? “ofc” is just an abbreviation for “of course”. -- Andrea Faulds http://ajf.me/
Re: [PHP-DEV] Add a new flag for json_encode
On Mon, Dec 15, 2014 at 6:16 PM, Juan Basso jrba...@gmail.com wrote: Ok guys, sorry, but I am giving up on it. I opened the PR in April and all the code necessary with technical implications are done and registered on the PR. I brought the topic to this email list in November as requested in the PR and almost 1 month after you guys requested me to write a RFC. I tried to create a wiki account to write the RFC 15 days ago[1] and I had no answer in create a simple wiki account. I have no idea how long it will take and how long the RFC will be on discussion, but seems the whole process will take over 1 year. Unfortunately I don't have patience enough for it. I guess I put my contribution on the project proposing the code, discussing the technical part and executing benchmarks. If you want to use it, use, but if you want to just ignore because I didn't complete all the steps fine too, just close the PR and this topic is automatically closed. I would like to contribute more with PHP core, but if this is the process to contribute with PHP, I am out. I will help other projects that give more attention and love for who wants to contribute and not making the person to worry about the bureaucratic part. Sorry. [1] http://news.php.net/php.webmaster/20312 Hi, I'm sorry for the negative experience, and I agree that we did a bad job handling this PR. ps: I've just replied to your mail, but I can understand if you aren't going to put more effort into getting this merged. -- Ferenc Kovács @Tyr43l - http://tyrael.hu
Re: [PHP-DEV] Add a new flag for json_encode
What is ofc? I never heard about it before. Do I need to do something? Thanks. Juan Basso On Sat, Nov 29, 2014 at 5:45 AM, Ferenc Kovacs tyr...@gmail.com wrote: On Sat, Nov 29, 2014 at 5:39 AM, Levi Morrison le...@php.net wrote: On Fri, Nov 28, 2014 at 1:21 PM, Juan Basso jrba...@gmail.com wrote: Over 20 days since the last comment and seems there is a consensus about accept it to the next 5.6 patch version, right? Can it be merged? After reading over the thread, there doesn't seem to be a consensus; I am not sure how you arrived at this conclusion. If I have misread the thread and there is indeed a consensus then I apologize for contributing noise. Levi: only Jakub was arguing against it, and even he was ok with the new behavior, he just argued to make it default, and that if we really need the option to disable it. Kris: the general consensus seems to be that not every minor pull request needs an RFC but ofc. usually it helps to move forward and get consensus, so it won't hurt if we have one for this. -- Ferenc Kovács @Tyr43l - http://tyrael.hu
Re: [PHP-DEV] Add a new flag for json_encode
On 1 Dec 2014, at 01:48, Juan Basso jrba...@gmail.com wrote: What is ofc? I never heard about it before. Do I need to do something? “ofc” is just an abbreviation for “of course”. -- Andrea Faulds http://ajf.me/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Add a new flag for json_encode
I see. I thought it was some sort of simplified RFC. :) Ok, I will create a RFC regarding it. On Sun, Nov 30, 2014 at 8:49 PM, Andrea Faulds a...@ajf.me wrote: On 1 Dec 2014, at 01:48, Juan Basso jrba...@gmail.com wrote: What is ofc? I never heard about it before. Do I need to do something? “ofc” is just an abbreviation for “of course”. -- Andrea Faulds http://ajf.me/
Re: [PHP-DEV] Add a new flag for json_encode
On Nov 3, 2014 7:13 PM, Juan Basso jrba...@gmail.com wrote: Hi, I opened a pull request[1] in order to solve the bug 50224[2] and it ended creating this pull request to add a new flag called JSON_PRESERVE_FRACTIONAL_PART on json_encode function. This flag will make the json encode to output float number always with decimal part, even when it is 0. Currently if you try to convert 10.0 using json_encode it outputs 10. It means if you decode it it will give an integer instead a float. In PHP words, json_decode(json_encode(10.0)) !== 10.0. After some researches and discussions it is not considered a bug because JSON specs treat integer and floats as number. Looking how other languages treat this encoding I could find it: - C (using lib jansson) and Ruby the output contains the decimal portion; - Python and Javascript outputs without the decimal portion. So it is kind of common to have different behaviors since JSON specs define it as just number. The idea of the new flag is allow PHP to behave the both ways. In the pull request Stanislav Malyshev suggested to merge it in the 5.6, but just want to see if someone else has any objection. Ferenc Kovacs and Jakub Zelenka also are in favor of merging on 5.6. Jakub completed suggesting to have this option enabled by default on PHP 7. Anyone has any objection on merging it on 5.6? Some comments about enabling it by default in 7? As a side note, with the pull request the encode of floats are about 20% faster, even after the flag check. This improvement just affect float encoding and has no impact on the other types. [1] https://github.com/php/php-src/pull/642 [2] https://bugs.php.net/bug.php?id=50224 Thanks, Juan Basso Despite some tangential disagreements regarding the default behavior, there does appear to be a general consensus with regard to the optional argument. Could you please post an RFC for this if you haven't already? Thanks! --Kris
Re: [PHP-DEV] Add a new flag for json_encode
On Sat, Nov 29, 2014 at 5:39 AM, Levi Morrison le...@php.net wrote: On Fri, Nov 28, 2014 at 1:21 PM, Juan Basso jrba...@gmail.com wrote: Over 20 days since the last comment and seems there is a consensus about accept it to the next 5.6 patch version, right? Can it be merged? After reading over the thread, there doesn't seem to be a consensus; I am not sure how you arrived at this conclusion. If I have misread the thread and there is indeed a consensus then I apologize for contributing noise. Levi: only Jakub was arguing against it, and even he was ok with the new behavior, he just argued to make it default, and that if we really need the option to disable it. Kris: the general consensus seems to be that not every minor pull request needs an RFC but ofc. usually it helps to move forward and get consensus, so it won't hurt if we have one for this. -- Ferenc Kovács @Tyr43l - http://tyrael.hu
Re: [PHP-DEV] Add a new flag for json_encode
Over 20 days since the last comment and seems there is a consensus about accept it to the next 5.6 patch version, right? Can it be merged? Juan Basso On Wed, Nov 5, 2014 at 9:37 PM, Juan Basso jrba...@gmail.com wrote: After the comments I see about deprecating the constant. Makes completely sense to keep it. Thanks for the clarification. I like the idea to have it enabled by default on 7.0, but since it is not a major issue, probably keeping it as optional would makes more sense and doesn't introduce another unnecessary BC break. But if you guys wants to postpone this decision I also understand. Juan Basso
Re: [PHP-DEV] Add a new flag for json_encode
On Fri, Nov 28, 2014 at 1:21 PM, Juan Basso jrba...@gmail.com wrote: Over 20 days since the last comment and seems there is a consensus about accept it to the next 5.6 patch version, right? Can it be merged? After reading over the thread, there doesn't seem to be a consensus; I am not sure how you arrived at this conclusion. If I have misread the thread and there is indeed a consensus then I apologize for contributing noise. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Add a new flag for json_encode
Levi, From what I understood it is a consensus to apply the change to PHP 5.6 as it is (new flag and disabled by default). There is no consensus about how should be in PHP 7, but was agreed to discuss that later. On Fri, Nov 28, 2014 at 11:39 PM, Levi Morrison le...@php.net wrote: On Fri, Nov 28, 2014 at 1:21 PM, Juan Basso jrba...@gmail.com wrote: Over 20 days since the last comment and seems there is a consensus about accept it to the next 5.6 patch version, right? Can it be merged? After reading over the thread, there doesn't seem to be a consensus; I am not sure how you arrived at this conclusion. If I have misread the thread and there is indeed a consensus then I apologize for contributing noise.
Re: [PHP-DEV] Add a new flag for json_encode
On 4 November 2014 17:07, Jakub Zelenka bu...@php.net wrote: On Tue, Nov 4, 2014 at 2:57 PM, Ferenc Kovacs tyr...@gmail.com wrote: On Tue, Nov 4, 2014 at 4:13 AM, Juan Basso jrba...@gmail.com wrote: Hi, I opened a pull request[1] in order to solve the bug 50224[2] and it ended creating this pull request to add a new flag called JSON_PRESERVE_FRACTIONAL_PART on json_encode function. This flag will make the json encode to output float number always with decimal part, even when it is 0. Currently if you try to convert 10.0 using json_encode it outputs 10. It means if you decode it it will give an integer instead a float. In PHP words, json_decode(json_encode(10.0)) !== 10.0. After some researches and discussions it is not considered a bug because JSON specs treat integer and floats as number. Looking how other languages treat this encoding I could find it: - C (using lib jansson) and Ruby the output contains the decimal portion; - Python and Javascript outputs without the decimal portion. So it is kind of common to have different behaviors since JSON specs define it as just number. The idea of the new flag is allow PHP to behave the both ways. In the pull request Stanislav Malyshev suggested to merge it in the 5.6, but just want to see if someone else has any objection. Ferenc Kovacs and Jakub Zelenka also are in favor of merging on 5.6. Jakub completed suggesting to have this option enabled by default on PHP 7. Anyone has any objection on merging it on 5.6? Some comments about enabling it by default in 7? As a side note, with the pull request the encode of floats are about 20% faster, even after the flag check. This improvement just affect float encoding and has no impact on the other types. [1] https://github.com/php/php-src/pull/642 [2] https://bugs.php.net/bug.php?id=50224 Thanks, Juan Basso Hi, just a slight correction: as far as I can tell, Jakus was ok with the general idea and even with turning this into the default, but he stated that it should only happen in a major version (as it would cause a slight BC, maybe this could cause problems for some people who are expecting the zeroes to be truncated from integer values) and he also stated that he thinks that there is no reason for a hurry to have this in a micro release, so this could wait until 7.0. Hey, Yeah exactly! I would really like to see it as default in the next major as it is a useful feature. However I don't like adding new flag. The reason for that is that it becomes useless after 7 is realesed. It would be useful only for 5.6.3 and 7.0 though. The only use case in PHP 7 would be just to disable the flag but that won't be very nice and completely different than other json constant usage It means doing something like $options ~JSON_PRESERVE_FRACTIONAL_PART ... It's a bit messy IMHO. Think that it would be much better to wait till 7 with this feature. I'm afraid I have to disagree here, I don't like the idea of changing this behaviour without making it controllable, and your logic is also slightly flawed - it's not that you'd have to do an ~ to disable, it's that it wouldn't be enabled unless you added it to your flags argument. Passing a value of JSON_FORCE_OBJECT wouldn't be handled as JSON_FORCE_OBJECT | JSON_PRESERVE_FRACTIONAL_PART, you'd just need to explicitly pass 0 to the flags argument to disable it. The other advantage of making it into a flag is that any code that is already passing in flags (suggesting they need finer control over the output) will be unaffected by the change. Cheers Jakub
Re: [PHP-DEV] Add a new flag for json_encode
I also prefer to use different flags if we enable by default. So if this behavior is enabled by default in 7.0 the JSON_PRESERVE_FRACTIONAL_PART is deprecated and a new flag is created to disable it. In resume of this thread, seems everyone if fine in having the flag JSON_PRESERVE_FRACTIONAL_PART added on 5.6 and leave this flag off by default to keep the BC. The only discussion is enable or not this behavior in 7.0. Andrea, I see your concerns about the bigint changes, but I am not sure if they are related. The PR just affect the encoding, not the decoding part. So if some Python system encodes a JSON using the decimal part it will be the same of PHP encoding using this new flag. So, if you think the decode can cause an issue on bigint, it needs a big concern for system interoperability on this part. Also, what I am proposing is not to change all integers in floats. What I am proposing with this flag is to make floats values (PHP zval) output as float in the JSON. It doesn't affect anything in the PHP variables. Juan Basso
Re: [PHP-DEV] Add a new flag for json_encode
On Wed, Nov 5, 2014 at 2:33 PM, Chris Wright c...@daverandom.com wrote: I'm afraid I have to disagree here, I don't like the idea of changing this behaviour without making it controllable, and your logic is also slightly flawed - it's not that you'd have to do an ~ to disable, it's that it wouldn't be enabled unless you added it to your flags argument. Passing a value of JSON_FORCE_OBJECT wouldn't be handled as JSON_FORCE_OBJECT | JSON_PRESERVE_FRACTIONAL_PART, you'd just need to explicitly pass 0 to the flags argument to disable it. That was just only possible use case for JSON_PRESERVE_FRACTIONAL_PART if it was made default in the PHP 7 but when I think about it's a bit stupid. It could theoretically work if all constants were ORed with that value but that's nonsens. Jakub
Re: [PHP-DEV] Add a new flag for json_encode
On 5 Nov 2014, at 16:23, Juan Basso jrba...@gmail.com wrote: Andrea, I see your concerns about the bigint changes, but I am not sure if they are related. The PR just affect the encoding, not the decoding part. Yes, I realise that. So if some Python system encodes a JSON using the decimal part it will be the same of PHP encoding using this new flag. So, if you think the decode can cause an issue on bigint, it needs a big concern for system interoperability on this part. Also, what I am proposing is not to change all integers in floats. What I am proposing with this flag is to make floats values (PHP zval) output as float in the JSON. It doesn't affect anything in the PHP variables. JSON doesn’t have floats. It just has numbers, but unfortunately some poorly-designed JSON parsers treat the existence of a decimal point as significant. Again, I don’t see the benefit of adding .0 by default if it’s an integral float value. For dealing with poorly-written JSON parsers, there should of course be an option, but there’s no good reason which I can see to make it the default. If it’s the default, then it causes interoperability issues with 32-bit and 64-bit PHP, and forwards-compatibility issues with bigints. If it’s not the default, nothing changes and everything keeps working well. -- Andrea Faulds http://ajf.me/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Add a new flag for json_encode
On Wed, Nov 5, 2014 at 4:23 PM, Juan Basso jrba...@gmail.com wrote: I also prefer to use different flags if we enable by default. So if this behavior is enabled by default in 7.0 the JSON_PRESERVE_FRACTIONAL_PART is deprecated and a new flag is created to disable it. In resume of this thread, seems everyone if fine in having the flag JSON_PRESERVE_FRACTIONAL_PART added on 5.6 and leave this flag off by default to keep the BC. The only discussion is enable or not this behavior in 7.0. No! If it should become default in 7.0, then it doesn't make to add a new flag. What's the point of adding new flag to 5.6.x and then deprecate it in 7.0...? That's just mess IMHO. It makes sense only if it doesn't become default though. Cheers Jakub
Re: [PHP-DEV] Add a new flag for json_encode
On Wed, Nov 5, 2014 at 2:33 PM, Chris Wright c...@daverandom.com wrote I'm afraid I have to disagree here, I don't like the idea of changing this behaviour without making it controllable If we make it default, then we could of course add a new constants that would allow the old behaviour. What I'm trying to say is that it doesn't make sense to add something to micro release and then change it (deprecate it) in PHP 7. I don't have problem with a new flag if the current behaviour stays but it should be decided before we make this change. There is no big hurry that we need to add it immediately. Cheers Jakub
Re: [PHP-DEV] Add a new flag for json_encode
On 5 Nov 2014, at 16:45, Jakub Zelenka bu...@php.net wrote: On Wed, Nov 5, 2014 at 2:33 PM, Chris Wright c...@daverandom.com wrote I'm afraid I have to disagree here, I don't like the idea of changing this behaviour without making it controllable If we make it default, then we could of course add a new constants that would allow the old behaviour. What I'm trying to say is that it doesn't make sense to add something to micro release and then change it (deprecate it) in PHP 7. There’s actually nothing wrong with this. It’s not like PHP 7 will come out tomorrow, it’ll be a year or so yet. So for the meantime, people who want this behaviour could specify it. Come PHP 7, their code keeps working and they need not change anything, as we wouldn’t deprecate the flag, it’d just be redundant. That being said, I don’t think it should be the default for reasons I’ve elaborated previously. -- Andrea Faulds http://ajf.me/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Add a new flag for json_encode
On 5 November 2014 16:45, Jakub Zelenka bu...@php.net wrote: On Wed, Nov 5, 2014 at 2:33 PM, Chris Wright c...@daverandom.com wrote I'm afraid I have to disagree here, I don't like the idea of changing this behaviour without making it controllable If we make it default, then we could of course add a new constants that would allow the old behaviour. What I'm trying to say is that it doesn't make sense to add something to micro release and then change it (deprecate it) in PHP 7. I don't have problem with a new flag if the current behaviour stays but it should be decided before we make this change. There is no big hurry that we need to add it immediately. I'm sorry, but I don't understand why it would need to be deprecated. For me making it the default behaviour would mean changing the default value of the $flags argument to JSON_PRESERVE_FACTIONAL_PART. The flag would still function in exactly the same way - old code which pass flags explicitly would continue to work in the same way, and any code which did not want the new behaviour would only have to pass 0 to flags to get that behaviour back. Cheers Jakub
Re: [PHP-DEV] Add a new flag for json_encode
On Wed, Nov 5, 2014 at 5:34 PM, Jakub Zelenka bu...@php.net wrote: On Wed, Nov 5, 2014 at 4:23 PM, Juan Basso jrba...@gmail.com wrote: I also prefer to use different flags if we enable by default. So if this behavior is enabled by default in 7.0 the JSON_PRESERVE_FRACTIONAL_PART is deprecated and a new flag is created to disable it. In resume of this thread, seems everyone if fine in having the flag JSON_PRESERVE_FRACTIONAL_PART added on 5.6 and leave this flag off by default to keep the BC. The only discussion is enable or not this behavior in 7.0. No! If it should become default in 7.0, then it doesn't make to add a new flag. What's the point of adding new flag to 5.6.x and then deprecate it in 7.0...? That's just mess IMHO. It makes sense only if it doesn't become default though. Cheers Jakub I think we should postpone the decision about changing the default value, and keep the scope about having JSON_PRESERVE_FRACTIONAL_PART as an optional setting. As far as I can see, everybody seems to be okay with that. I don't think that the possibility of changing the default settings in a future release(eg. 7.0) change anything about this option: The $options for json_encode is already a bitmask, and even if we change the default, you will still need the constant to be able to disable this behavior if you don't need it. I don't think that anybody else from the list favors this option in such depths that he/she would agree with you that we should even remove the option to opt-out from JSON_PRESERVE_FRACTIONAL_PART, for two reason: 1, for most apps/systems doesn't care if they receive 1.0 or 1, but sending out 1 instead of 1.0 makes the encoded form shorter so maybe some people would prefer to keep that more compact. 2, there is a chance that there are some exotic apps/systems which are already depending on the fact that php will/was used to send out 1 instead of 1.0, and while we can break BC in a major release, the migration for those people would be much easier if they just have to change/set some options for their json_encode calls. -- Ferenc Kovács @Tyr43l - http://tyrael.hu
Re: [PHP-DEV] Add a new flag for json_encode
On Wed, Nov 5, 2014 at 4:52 PM, Chris Wright c...@daverandom.com wrote: I'm sorry, but I don't understand why it would need to be deprecated. For me making it the default behaviour would mean changing the default value of the $flags argument to JSON_PRESERVE_FACTIONAL_PART. The flag would still function in exactly the same way - old code which pass flags explicitly would continue to work in the same way, and any code which did not want the new behaviour would only have to pass 0 to flags to get that behaviour back. Then passing any flag will automatically disable it unless all current constants will be ORed with that value. It means if you set default as (options = JSON_PRESERVE_FACTIONAL_PART in zif_json_encode) and you pass just PHP_JSON_UNESCAPED_UNICODE then fractional part will be disabled (unless the constants are ORed). It's not really default like other defaults in json_encode because it's changed by any flag. That being said if it's just me who doesn't like it and there are no other objection, I don't mind if it gets in. Let's just wait few days if there isn't anyone who also thinks that adding such feature to micro release is not the best idea... Cheers Jakub
Re: [PHP-DEV] Add a new flag for json_encode
On 5 Nov 2014, at 17:01, Ferenc Kovacs tyr...@gmail.com wrote: 2, there is a chance that there are some exotic apps/systems which are already depending on the fact that php will/was used to send out 1 instead of 1.0, and while we can break BC in a major release, the migration for those people would be much easier if they just have to change/set some options for their json_encode calls. It’d be even easier if we change nothing so they don’t need to touch their JSON code. -- Andrea Faulds http://ajf.me/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Add a new flag for json_encode
On 5 November 2014 17:21, Jakub Zelenka bu...@php.net wrote: On Wed, Nov 5, 2014 at 4:52 PM, Chris Wright c...@daverandom.com wrote: I'm sorry, but I don't understand why it would need to be deprecated. For me making it the default behaviour would mean changing the default value of the $flags argument to JSON_PRESERVE_FACTIONAL_PART. The flag would still function in exactly the same way - old code which pass flags explicitly would continue to work in the same way, and any code which did not want the new behaviour would only have to pass 0 to flags to get that behaviour back. Then passing any flag will automatically disable it unless all current constants will be ORed with that value. It means if you set default as (options = JSON_PRESERVE_FACTIONAL_PART in zif_json_encode) and you pass just PHP_JSON_UNESCAPED_UNICODE then fractional part will be disabled (unless the constants are ORed). It's not really default like other defaults in json_encode because it's changed by any flag. Indeed, but this is how anything else in PHP with a $flags argument works... if I pass ENT_QUOTES to htmlspecialchars() it isn't handled as ENT_COMPAT | ENT_HTML401 | ENT_QUOTES. When you pass a $flags argument to a function explicitly, you are saying I don't want the default behaviour, I want more control, so I would be a little surprised if my controlled options were suddenly changed and now I have to add another flag to get the behaviour I wanted (and had before the default value was changed). Of course, this is just my view :-) Thanks, Chris That being said if it's just me who doesn't like it and there are no other objection, I don't mind if it gets in. Let's just wait few days if there isn't anyone who also thinks that adding such feature to micro release is not the best idea... Cheers Jakub
Re: [PHP-DEV] Add a new flag for json_encode
On Wed, Nov 5, 2014 at 6:24 PM, Andrea Faulds a...@ajf.me wrote: On 5 Nov 2014, at 17:01, Ferenc Kovacs tyr...@gmail.com wrote: 2, there is a chance that there are some exotic apps/systems which are already depending on the fact that php will/was used to send out 1 instead of 1.0, and while we can break BC in a major release, the migration for those people would be much easier if they just have to change/set some options for their json_encode calls. It’d be even easier if we change nothing so they don’t need to touch their JSON code. of course, I'm just arguing that even if we later on decide that we want this to be the default behavior, we would still keep the JSON_PRESERVE_FACTIONAL_PART flag, so that decision shouldn't affect whether or not we introduce the flag now. -- Ferenc Kovács @Tyr43l - http://tyrael.hu
Re: [PHP-DEV] Add a new flag for json_encode
After the comments I see about deprecating the constant. Makes completely sense to keep it. Thanks for the clarification. I like the idea to have it enabled by default on 7.0, but since it is not a major issue, probably keeping it as optional would makes more sense and doesn't introduce another unnecessary BC break. But if you guys wants to postpone this decision I also understand. Juan Basso
Re: [PHP-DEV] Add a new flag for json_encode
On Tue, Nov 4, 2014 at 4:13 AM, Juan Basso jrba...@gmail.com wrote: Hi, I opened a pull request[1] in order to solve the bug 50224[2] and it ended creating this pull request to add a new flag called JSON_PRESERVE_FRACTIONAL_PART on json_encode function. This flag will make the json encode to output float number always with decimal part, even when it is 0. Currently if you try to convert 10.0 using json_encode it outputs 10. It means if you decode it it will give an integer instead a float. In PHP words, json_decode(json_encode(10.0)) !== 10.0. After some researches and discussions it is not considered a bug because JSON specs treat integer and floats as number. Looking how other languages treat this encoding I could find it: - C (using lib jansson) and Ruby the output contains the decimal portion; - Python and Javascript outputs without the decimal portion. So it is kind of common to have different behaviors since JSON specs define it as just number. The idea of the new flag is allow PHP to behave the both ways. In the pull request Stanislav Malyshev suggested to merge it in the 5.6, but just want to see if someone else has any objection. Ferenc Kovacs and Jakub Zelenka also are in favor of merging on 5.6. Jakub completed suggesting to have this option enabled by default on PHP 7. Anyone has any objection on merging it on 5.6? Some comments about enabling it by default in 7? As a side note, with the pull request the encode of floats are about 20% faster, even after the flag check. This improvement just affect float encoding and has no impact on the other types. [1] https://github.com/php/php-src/pull/642 [2] https://bugs.php.net/bug.php?id=50224 Thanks, Juan Basso Hi, just a slight correction: as far as I can tell, Jakus was ok with the general idea and even with turning this into the default, but he stated that it should only happen in a major version (as it would cause a slight BC, maybe this could cause problems for some people who are expecting the zeroes to be truncated from integer values) and he also stated that he thinks that there is no reason for a hurry to have this in a micro release, so this could wait until 7.0. -- Ferenc Kovács @Tyr43l - http://tyrael.hu
Re: [PHP-DEV] Add a new flag for json_encode
On 4 Nov 2014, at 14:57, Ferenc Kovacs tyr...@gmail.com wrote: Hi, just a slight correction: as far as I can tell, Jakus was ok with the general idea and even with turning this into the default, but he stated that it should only happen in a major version (as it would cause a slight BC, maybe this could cause problems for some people who are expecting the zeroes to be truncated from integer values) and he also stated that he thinks that there is no reason for a hurry to have this in a micro release, so this could wait until 7.0. Why would this cause a BC break? If we’re just adding a new, optional, off-by-default option, it shouldn’t break anything. -- Andrea Faulds http://ajf.me/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Add a new flag for json_encode
On Tue, Nov 4, 2014 at 5:35 PM, Andrea Faulds a...@ajf.me wrote: On 4 Nov 2014, at 14:57, Ferenc Kovacs tyr...@gmail.com wrote: Hi, just a slight correction: as far as I can tell, Jakus was ok with the general idea and even with turning this into the default, but he stated that it should only happen in a major version (as it would cause a slight BC, maybe this could cause problems for some people who are expecting the zeroes to be truncated from integer values) and he also stated that he thinks that there is no reason for a hurry to have this in a micro release, so this could wait until 7.0. Why would this cause a BC break? If we’re just adding a new, optional, off-by-default option, it shouldn’t break anything. please re-read my previous mail: __turning this into the default__, but he stated that it __should only happen in a major version__ -- Ferenc Kovács @Tyr43l - http://tyrael.hu
Re: [PHP-DEV] Add a new flag for json_encode
On Tue, Nov 4, 2014 at 2:57 PM, Ferenc Kovacs tyr...@gmail.com wrote: On Tue, Nov 4, 2014 at 4:13 AM, Juan Basso jrba...@gmail.com wrote: Hi, I opened a pull request[1] in order to solve the bug 50224[2] and it ended creating this pull request to add a new flag called JSON_PRESERVE_FRACTIONAL_PART on json_encode function. This flag will make the json encode to output float number always with decimal part, even when it is 0. Currently if you try to convert 10.0 using json_encode it outputs 10. It means if you decode it it will give an integer instead a float. In PHP words, json_decode(json_encode(10.0)) !== 10.0. After some researches and discussions it is not considered a bug because JSON specs treat integer and floats as number. Looking how other languages treat this encoding I could find it: - C (using lib jansson) and Ruby the output contains the decimal portion; - Python and Javascript outputs without the decimal portion. So it is kind of common to have different behaviors since JSON specs define it as just number. The idea of the new flag is allow PHP to behave the both ways. In the pull request Stanislav Malyshev suggested to merge it in the 5.6, but just want to see if someone else has any objection. Ferenc Kovacs and Jakub Zelenka also are in favor of merging on 5.6. Jakub completed suggesting to have this option enabled by default on PHP 7. Anyone has any objection on merging it on 5.6? Some comments about enabling it by default in 7? As a side note, with the pull request the encode of floats are about 20% faster, even after the flag check. This improvement just affect float encoding and has no impact on the other types. [1] https://github.com/php/php-src/pull/642 [2] https://bugs.php.net/bug.php?id=50224 Thanks, Juan Basso Hi, just a slight correction: as far as I can tell, Jakus was ok with the general idea and even with turning this into the default, but he stated that it should only happen in a major version (as it would cause a slight BC, maybe this could cause problems for some people who are expecting the zeroes to be truncated from integer values) and he also stated that he thinks that there is no reason for a hurry to have this in a micro release, so this could wait until 7.0. Hey, Yeah exactly! I would really like to see it as default in the next major as it is a useful feature. However I don't like adding new flag. The reason for that is that it becomes useless after 7 is realesed. It would be useful only for 5.6.3 and 7.0 though. The only use case in PHP 7 would be just to disable the flag but that won't be very nice and completely different than other json constant usage It means doing something like $options ~JSON_PRESERVE_FRACTIONAL_PART ... It's a bit messy IMHO. Think that it would be much better to wait till 7 with this feature. Cheers Jakub
Re: [PHP-DEV] Add a new flag for json_encode
On 4 Nov 2014, at 16:41, Ferenc Kovacs tyr...@gmail.com wrote: On Tue, Nov 4, 2014 at 5:35 PM, Andrea Faulds a...@ajf.me wrote: On 4 Nov 2014, at 14:57, Ferenc Kovacs tyr...@gmail.com wrote: Hi, just a slight correction: as far as I can tell, Jakus was ok with the general idea and even with turning this into the default, but he stated that it should only happen in a major version (as it would cause a slight BC, maybe this could cause problems for some people who are expecting the zeroes to be truncated from integer values) and he also stated that he thinks that there is no reason for a hurry to have this in a micro release, so this could wait until 7.0. Why would this cause a BC break? If we’re just adding a new, optional, off-by-default option, it shouldn’t break anything. please re-read my previous mail: __turning this into the default__, but he stated that it __should only happen in a major version__” Right, my bad. I read it as “This [including possibly turning this into the default] should only happen in a major version”, I see what you meant. Sorry for the misunderstanding, in retrospect the other interpretation would’ve made more sense. -- Andrea Faulds http://ajf.me/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Add a new flag for json_encode
On 4 Nov 2014, at 17:07, Jakub Zelenka bu...@php.net wrote: I would really like to see it as default in the next major as it is a useful feature. However I don't like adding new flag. The reason for that is that it becomes useless after 7 is realesed. It would be useful only for 5.6.3 and 7.0 though. The only use case in PHP 7 would be just to disable the flag but that won't be very nice and completely different than other json constant usage It means doing something like $options ~JSON_PRESERVE_FRACTIONAL_PART ... It's a bit messy IMHO. Think that it would be much better to wait till 7 with this feature. I really don’t like the idea of making this the default. On 32-bit systems, floats give you, effectively, extra integer range up until you hit 2^52, and indeed all JS integer values are just floats within that range. I don’t think they should be represented as floats. It’d go weirdly with 64-bit PHP, which can decode such values to integers, which would then not handle the range beyond 2^52 nicely. Also, it wouldn’t go well for cross-version compatibility with my bigint RFC here. -- Andrea Faulds http://ajf.me/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Add a new flag for json_encode
Hi, On Tue, Nov 4, 2014 at 6:14 PM, Andrea Faulds a...@ajf.me wrote: On 4 Nov 2014, at 17:07, Jakub Zelenka bu...@php.net wrote: I would really like to see it as default in the next major as it is a useful feature. However I don't like adding new flag. The reason for that is that it becomes useless after 7 is realesed. It would be useful only for 5.6.3 and 7.0 though. The only use case in PHP 7 would be just to disable the flag but that won't be very nice and completely different than other json constant usage It means doing something like $options ~JSON_PRESERVE_FRACTIONAL_PART ... It's a bit messy IMHO. Think that it would be much better to wait till 7 with this feature. I really don’t like the idea of making this the default. On 32-bit systems, floats give you, effectively, extra integer range up until you hit 2^52, and indeed all JS integer values are just floats within that range. I don’t think they should be represented as floats. It’d go weirdly with 64-bit PHP, which can decode such values to integers, which would then not handle the range beyond 2^52 nicely. That's a good point. However I still think that there are more advantages for making this the default. I like mainly the idea that json_decode(json_encode($a)) = $a for all floats which is not the case at the moment. Also it allow some other things that were mentioned before. Think that we will probably need an RFC for that as we apparently don't have a consensus about this. Cheers Jakub
Re: [PHP-DEV] Add a new flag for json_encode
On 4 Nov 2014, at 20:06, Jakub Zelenka bu...@php.net wrote: That's a good point. However I still think that there are more advantages for making this the default. I like mainly the idea that json_decode(json_encode($a)) = $a for all floats which is not the case at the moment. Also it allow some other things that were mentioned before. It won’t be with or without your changes, as JSON can’t encode Infinity or NaN. Also, I don’t think it matters. PHP isn’t a strictly-typed language where there’s an important difference between float(7000) and int(7000). They act exactly the same in all contexts. There’s no benefit to this change that I can see within PHP (perhaps with other languages that have weird JSON parsers), and it causes problems as I’ve said above. Thus, I can’t support it being a default. -- Andrea Faulds http://ajf.me/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Add a new flag for json_encode
Hi, I opened a pull request[1] in order to solve the bug 50224[2] and it ended creating this pull request to add a new flag called JSON_PRESERVE_FRACTIONAL_PART on json_encode function. This flag will make the json encode to output float number always with decimal part, even when it is 0. Currently if you try to convert 10.0 using json_encode it outputs 10. It means if you decode it it will give an integer instead a float. In PHP words, json_decode(json_encode(10.0)) !== 10.0. After some researches and discussions it is not considered a bug because JSON specs treat integer and floats as number. Looking how other languages treat this encoding I could find it: - C (using lib jansson) and Ruby the output contains the decimal portion; - Python and Javascript outputs without the decimal portion. So it is kind of common to have different behaviors since JSON specs define it as just number. The idea of the new flag is allow PHP to behave the both ways. In the pull request Stanislav Malyshev suggested to merge it in the 5.6, but just want to see if someone else has any objection. Ferenc Kovacs and Jakub Zelenka also are in favor of merging on 5.6. Jakub completed suggesting to have this option enabled by default on PHP 7. Anyone has any objection on merging it on 5.6? Some comments about enabling it by default in 7? As a side note, with the pull request the encode of floats are about 20% faster, even after the flag check. This improvement just affect float encoding and has no impact on the other types. [1] https://github.com/php/php-src/pull/642 [2] https://bugs.php.net/bug.php?id=50224 Thanks, Juan Basso
