Accept binaries as struct keys.

[ewanmellor]

Have store_val accept a binary as the key, and use the same binary_to_existing_atom path to make sure that they get created as atoms when appropriate (i.e. the same as from_json).

[alavrik]

Can you tell more about your use case?

The original idea was to keep the proplist API separate from the JSON API and your change kind of blends them together. Besides, it breaks some original contracts. For instance, this is how the proplist type is currently defined in erlson.erl:

-type proplist() :: [ proplist_elem() ].
-type proplist_elem() :: atom() | {atom(), value()}.

[ewanmellor]

I'm parsing JSON received from the wire, modifying it, and then sending it on.

Received = <<"{\"definitelynotanatom\":\"B\"}">>,
Parsed = erlson:from_json(Received),
Improved = [{foo, <<"bar">>} | Parsed],
Result = erlson:from_nested_proplist(Added).

With my change, Result will contain [{foo,<<"bar">>},{<<"definitelynotanatom">>,<<"B">>}] and erlson:to_json(Result) works fine. Without my change, the from_nested_proplist call will fail with erlson_bad_proplist.

I hadn't noticed that proplist_elem was declared to allow only an atom. That obviously should be adjusted to match. Everything seems to work just fine with binaries in that position.

[alavrik]

Why wouldn't you want to merge steps 3 and 4 of your example into one and use Erlson syntax:

Result = Parsed#{foo = <<"bar">>}

Or I'm missing something in your example?

The original idea behind erlson:proplist() type is to use Erlson syntax for passing and destructuring lists of options for various Erlang functions. We can generalize it to support binaries in field names as well per your suggestion, but it could have some undesired consequences. For example, Dialyzer can start generating warnings on some existing code.

Can you give me some more examples of what you can't achieve using the existing Erlson syntax/API? It could be that there's a better solution for your problem that doesn't require modifying the existing API.

[ewanmellor]

Parsed#{foo = <<"bar">>} works. I didn't think of that.

I still think it's a bit odd that parsed values can contain binary keys, but the API won't accept them. It's strangely asymmetric. I understand that changing the API has its own issues though, so I can change my code to use Parsed#{foo = <<"bar">>}

[alavrik]

I'm glad this solution worked for you!

Actually, the only reason why parsed values can still contain binary keys is stability (or security -- depending on the context). Erlang VM doesn't have garbage collection for atoms (at least yet) and Erlson uses binary_to_existing_atom so that the VM doesn't run out of atom space. I know it is quirky, but it works well in practice: if you use Erlson syntax at least once in your program to access a path segment the VM will know about this atom. This also means that those keys that remain as binaries in the parsed representation are likely the ones you are not interested in.

[ewanmellor]

It would be great to put that paragraph above into your readme. I figured it all out in the end (the bit about using binary_to_existing_atom etc) but it took me a little while. I was converting from mochijson2 to erlson, and so I naturally had lots of code with binary keys. It took me a long time to figure out the subtle bugs that that introduced!

Anyway, I'm closing this pull request and dropping the branch, because your suggestion is the better solution.