sile / jsone

Erlang JSON library
MIT License
291 stars 71 forks source link

Support for other Erlang datatypes (pid, port, func, etc) #64

Closed menih closed 2 years ago

menih commented 3 years ago

Would be great if we can convert them all to a string. I'm trying to encode process_info(self()). It crashes on pids.

sile commented 3 years ago

You can use map_unknown_value option for that purpose as follows:

% Without `map_unknown_value`
>  jsone:encode([1,self(),foo]).
** exception error: bad argument

% With `map_unknown_value`
> jsone:encode([1,self(),foo], [{map_unknown_value, fun (X) -> {ok, list_to_binary(io_lib:format("~p", [X]))} end}]).
<<"[1,\"<0.209.0>\",\"foo\"]">>
menih commented 3 years ago

Thanks for the tip. It would be nice if this is the default behavior.

On Fri, Aug 27, 2021 at 5:58 PM Takeru Ohta @.***> wrote:

You can use map_unknown_value option for that purpose as follows:

% Without map_unknown_value> jsone:encode([1,self(),foo]).** exception error: bad argument % With map_unknown_value> jsone:encode([1,self(),foo], [{map_unknown_value, fun (X) -> {ok, list_to_binary(io_lib:format("~p", [X]))} end}]). <<"[1,\"<0.209.0>\",\"foo\"]">>

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/sile/jsone/issues/64#issuecomment-907541514, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB2FAFKU7SIGA34TURQAS3DT7AYEHANCNFSM5C4X7QSA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

sile commented 3 years ago

It would be nice if this is the default behavior.

Maybe. However, I'd like to keep jsone implementation simple as possible (it's already a bit complex though..), and I don't know how many users desire this behavior to be the default. So let me keep the implementation as-is for a while. I'll reconsider that if I receive more similar requests in the future.

menih commented 3 years ago

Somewhat fair - most transformation from erlang to json are "business data" related, where Erlang "special" datatypes most likely do not participate. But for some use cases, such as logging and diagnostics, it is all they need to. I also don't see any downside to it. Consider a newcomer (like me I guess), who would not be able to quickly figure out the subtlety, and actually know there is a relatively easy way to accommodate. Lucky for me, I posted an issue and got a quick reply (Thank you!), but I am guessing I'd be the minority.

On Fri, Aug 27, 2021 at 6:37 PM Takeru Ohta @.***> wrote:

It would be nice if this is the default behavior.

Maybe. However, I'd like to keep jsone implementation simple as possible (it's already a bit complex though..), and I don't know how many users desire this behavior to be the default. So let me keep the implementation as-is for a while. I'll reconsider that if I receive more similar requests in the future.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/sile/jsone/issues/64#issuecomment-907546656, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB2FAFLHRDQS66CQHSJNR7TT7A4URANCNFSM5C4X7QSA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

sile commented 3 years ago

Thanks for your reply. The feedback is very useful.

On the second thought, I think that it might be better to define a function, like below, and set it to the default value of map_unknown_value option.

default_map_unknown_value(X) ->
    {ok, list_to_binary(io_lib:format("~p", [X]))}.

This approach doesn't have to modify the core part of the jsone encoding logic, and users can easily opt out of the behavior (or use other encoding formats they prefer). I'll consider that more deeply and create a prototype PR when I have time.

menih commented 3 years ago

Cheers!

On Fri, Aug 27, 2021 at 7:55 PM Takeru Ohta @.***> wrote:

Thanks for your reply. The feedback is very useful.

On the second thought, I think that it might be better to define a function, like below, and set it to the default value of map_unknown_value option.

default_map_unknown_value(X) -> {ok, list_to_binary(io_lib:format("~p", [X]))}.

This approach doesn't have to modify the core part of the jsone encoding logic, and users can easily opt out of the behavior (or use other encoding formats they prefer). I'll consider that more deeply and create a prototype PR when I have time.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/sile/jsone/issues/64#issuecomment-907556266, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB2FAFMFDGDFJJENQRJS4FLT7BF2DANCNFSM5C4X7QSA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.