ProtoDef-io / node-protodef

Describe your protocol, and read it with ease.
MIT License
31 stars 18 forks source link

Fix option sizeof and write #103

Closed Karang closed 4 years ago

rom1504 commented 4 years ago

Do you think it's possible to add a regression test for this there https://github.com/ProtoDef-io/ProtoDef/blob/master/test/conditional.json ?

rom1504 commented 4 years ago

I think we should probably be iso to https://github.com/ProtoDef-io/node-protodef/blob/master/src/datatypes/conditional.js#L47 here. (Even if this behavior is a bit weird, relying on js implicit casting for null and undefined)

Karang commented 4 years ago

Do you think it's possible to add a regression test for this there https://github.com/ProtoDef-io/ProtoDef/blob/master/test/conditional.json ?

Yes the case that was not tested was the "option not present" one. So basically an undefined value and a buffer containing only a byte at 0.

Can we do something like:

{
   "value": undefined,
   "buffer":["0x00"]
}

?

rom1504 commented 4 years ago

Could you add that ?

rom1504 commented 4 years ago

Maybe, I'm not sure, try it

Karang commented 4 years ago

Maybe, I'm not sure, try it

it works with null but we cannot test undefined