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,
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=50224Thanks,
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
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/
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
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/
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
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=50224Thanks,
Juan BassoHi,
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
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/
Hi,
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
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/
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
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=50224Thanks,
Juan BassoHi,
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
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
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/
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 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
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/
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
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
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
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
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,
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.
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.
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
What is ofc? I never heard about it before. Do I need to do something?
Thanks.
Juan Basso
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
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/
I see. I thought it was some sort of simplified RFC. :)
Ok, I will create a RFC regarding it.
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/
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
I see. I thought it was some sort of simplified RFC. :)
Ok, I will create a RFC regarding it.
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/
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.
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
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 ofJSON_FORCE_OBJECTwouldn't be handled asJSON_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
On Wed, Nov 5, 2014 at 2:33 PM, Chris Wright cw@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
On Wed, Nov 5, 2014 at 2:33 PM, Chris Wright cw@daverandom.com wrote
I'm afraid I have to disagree here, I don't like the idea of changing this
behaviour without making it controllableIf 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/
On Wed, Nov 5, 2014 at 2:33 PM, Chris Wright cw@daverandom.com wrote
I'm afraid I have to disagree here, I don't like the idea of changing
this behaviour without making it controllableIf 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
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
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
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
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=50224Thanks,
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