-
Notifications
You must be signed in to change notification settings - Fork 48
Run the exception analysis from reanalyze and incorporate the results. #70
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! This is really cool!
at can raise Invalid_argument, but that's not documented.
What should happen here. Should it be documented, or should that case be merged into DecodeError ?
I think it should be documented. at
is virtually always used with a list literal, in which case using the empty list is a programmer error. I've seen some APIs get around this by requiring the first field name as a separate argument, but that's quite clunky. Converting to DecodeError
could hide the error as some combinators, like optional
, will discard the error message when turning it into an option
.
Is this PR intended to be merged in some form?
|
||
let char json = | ||
let s = string json in | ||
if String.length s = 1 then | ||
String.get s 0 | ||
try (String.get s 0) with Invalid_argument _ -> ' ' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like a case where it could be useful to have an annotation that says "trust me, I've checked already".
try | ||
decode (Js.Dict.unsafeGet source key) | ||
with | ||
with (* XXX This is wrong: unsafeGet does not raise exceptions *) | ||
DecodeError msg -> raise @@ DecodeError (msg ^ "\n\tin dict") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It isn't unsafeGet
we're concerned about here, but decode
. We want to catch any failures in decoding the value and then add a "trace" to it.
(* XXX why does the doc say this raises? Depends of what function is passed in *) | ||
let map f decode json = | ||
f (decode json) | ||
|
||
(* XXX why does the doc say this raises? Depends of what function is passed in *) | ||
let andThen b a json= | ||
b (a json) json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, so I guess it shouldn't.
No description provided.