ingydotnet / yaml-libyaml-pm

Perl Binding to libyaml
http://search.cpan.org/dist/YAML-LibYAML/
33 stars 37 forks source link

Turn off POK for numbers #107

Closed ingydotnet closed 1 year ago

ingydotnet commented 1 year ago

Fix for #68

Using @perlpunk 's test code:

use v5.16;
use YAML::XS qw/ Load /;
use JSON::PP qw/ encode_json /;
use Devel::Peek qw/ Dump /;
my $yaml = "foo: 23";
my $data = Load $yaml;
my $json = encode_json($data);
say $json;
Dump $data->{foo};

before:

{"foo":23}
SV = PVIV(0x556868e926c8) at 0x556868e8b130
  REFCNT = 1
  FLAGS = (IOK,POK,pIOK,pPOK)
  IV = 23
  PV = 0x556869234380 "23"\0
  CUR = 2
  LEN = 10

after:

{"foo":23}
SV = PVNV(0x55a2dce90580) at 0x55a2dcead130
  REFCNT = 1
  FLAGS = (IOK,pIOK)
  IV = 23
  NV = 23
  PV = 0x55a2dd255c90 "23"\0
  CUR = 2
  LEN = 10

Note that POK is now turned off, though pIOK remains as does the PV.

Also the JSON number was not quoted on my machine.

@karenetheridge can you test if this fixes your issue?

perlpunk commented 1 year ago

Also the JSON number was not quoted on my machine.

That's because of https://github.com/ingydotnet/yaml-libyaml-pm/issues/68#issuecomment-656337437

karenetheridge commented 1 year ago

🥳

perlpunk commented 1 year ago

Why is this PR still open and not marked as merged?

ingydotnet commented 1 year ago

Closing.

perlpunk commented 1 year ago

For the record: merged as b6b1661868a5690a63603edd8899c004e93f51ae

If a PR is merged manually then at least adding the commit helps for later investigation if something was actually merged or not.