New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert JSON::PP::Boolean objects to JavaScript boolean primitives. #17

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
4 participants
@brad-mac

brad-mac commented Dec 14, 2015

Hello David,

I'm rounding out my year doing the Perl Pull Request Challenge (PRC) - I was assigned JavaScript::V8, and looking through open issues in RT I saw #103943: 'JSON::true / JSON::false converted to empty JavaScript object'.

I've updated V8Context.cpp to convert JSON::PP::Boolean objects to a Javascript boolean primitive - hope this helps. I've added test scripts for this use case, too.

Cheers

Brad

if (SvOBJECT(sv))
if (SvOBJECT(sv)) {
const char *Perl_class = sv_reftype(sv, 1);
if (0 == strcmp(Perl_class, "JSON::PP::Boolean"))

This comment has been minimized.

@godsflaw

godsflaw Dec 14, 2015

Use of strcmp() vs. strncmp() should be okay here.

@godsflaw

godsflaw Dec 14, 2015

Use of strcmp() vs. strncmp() should be okay here.

ok($v8context->eval('(function() { return (f ? 1 : 0) })()') == 0, 'Testing false - should return 0');
$v8context->bind( f => Types::Serialiser::true );
ok($v8context->eval('(function() { return (f ? 1 : 0) })()') == 1, 'Testing true - should return 1');
ok($v8context->eval('typeof f') eq 'boolean', 'Testing the Javascript type is a boolean');

This comment has been minimized.

@godsflaw

godsflaw Dec 14, 2015

Had to dig into the use of Types::Serialiser::true for JSON::XS here, looks good.

@godsflaw

godsflaw Dec 14, 2015

Had to dig into the use of Types::Serialiser::true for JSON::XS here, looks good.

@godsflaw

This comment has been minimized.

Show comment
Hide comment
@godsflaw

godsflaw Dec 14, 2015

I'm +1 on this.

godsflaw commented Dec 14, 2015

I'm +1 on this.

@tildedave

This comment has been minimized.

Show comment
Hide comment
@tildedave

tildedave Dec 14, 2015

Thank you so much @brad-mac ! This is a huge help

tildedave commented Dec 14, 2015

Thank you so much @brad-mac ! This is a huge help

@proller

This comment has been minimized.

Show comment
Hide comment
@proller

proller May 24, 2016

please convert JSON::XS::Boolean too

proller commented May 24, 2016

please convert JSON::XS::Boolean too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment