Allow lists as indata for bytes fields. #66
Conversation
|
@tomas-abrahamsson - I added the following nif test, expecting it to fail, but it didn't: I thought I would have to add something here, but apparently the list must be converted to a binary before reaching this point: Am I missing something? |
lukebakken
force-pushed
the
feature/lrb/epb-encode-list-bytes-compat
branch
from
77bbbf8
to
ba7b98d
Compare
|
blush ahemm, mea culpa. Thanks! On the branch verify-separate-vm-catches-errors (to be merged to master soon) there's a fix plus a unit test to make sure such errors do not go unnoticed again. With that fix, your newly added eunit test fails as expected. A useful trick for debugging such issues is to change |
|
Easiest for you might be to either rebase your branch on top of this, or cherry-pick that commit into your branch (or just temporarily-while-developing change the line to fixes the issue) |
|
@tomas-abrahamsson thank you, I'll look further into this 😄 |
lukebakken
force-pushed
the
feature/lrb/epb-encode-list-bytes-compat
branch
from
ba7b98d
to
70d6b4f
Compare
|
@tomas-abrahamsson - all set. I rebased this branch onto |
lukebakken
force-pushed
the
feature/lrb/epb-encode-list-bytes-compat
branch
2 times, most recently
from
e3fcb91
to
6de3a0a
Compare
| @@ -1387,6 +1399,7 @@ nif_code_test_() -> | |||
| {"Nif enums in msgs", fun nif_enum_in_msg/0}, | |||
| {"Nif enums with pkgs", fun nif_enum_with_pkgs/0}, | |||
| {"Nif with strbin", fun nif_with_strbin/0}, | |||
| {"Nif with list indata for bytes", fun nif_with_list_indata_for_bytes/0}, | |||
There was a problem hiding this comment.
This line is >80 characters
|
Looks good Small issues: A comment on length of one line, and maybe we can squash the changes into one commit? (it'll help any future git bisect if there isn't a failing eunit test in one of the commits (I know it is deliberate during development; tdd)) |
|
Hmm... just remembered... the eunit test wasn't failing after all... :) Another thing: I can add a sentence to the README after I've merged them all, about iolist being ok for encoding. Since the README table rewrite is in the other branch, I figure it might be easier to add that after merge. |
Allow using a list as indata for a bytes field. add nif test Put in last bits of magic to accept lists as bytes indata
lukebakken
force-pushed
the
feature/lrb/epb-encode-list-bytes-compat
branch
from
55ee336
to
fda55c1
Compare
|
@tomas-abrahamsson - all set I think, line length addressed, commits squashed. If you could add something to the |
|
Looks good, thanks for your patience, and have a nice weekend, too! |
|
This is now included in 3.24.0 |
Another piece of
epbcompatibility.See #63
TODO: