Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Enhancement request: of_yojson throw runtime exception rather than return result #57

Closed
tomjridge opened this issue Aug 28, 2017 · 4 comments
Closed

Enhancement request: of_yojson throw runtime exception rather than return result #57

tomjridge opened this issue Aug 28, 2017 · 4 comments

Comments

@tomjridge
Copy link

tomjridge commented Aug 28, 2017 •
edited

This would be useful in cases where it is known that the json is of the right format, i.e., the exception will not in fact be thrown. Possibly also useful if you prefer exceptions to results (perhaps there are a few situations where an exception is the right choice?)
@hcarty
Copy link
Contributor

hcarty commented Aug 28, 2017

Two (relatively) simple ways this could be added. One is to add an option within the annotation to specify if you want the exception-raising or the result-returning version. Another is to generate two functions (of_yojson and of_yojson_exn).

For now, if you want to assume the format is correct or directly convert the result to an exception if it's an Error you can use something like http://erratique.ch/software/rresult/doc/Rresult.R.html#VALget_ok

@tomjridge
Copy link
Author

I guess I probably prefer the of_yojson_exn version rather than the annotation. For the moment I am just pattern matching on the result which is not a big issue really.

@whitequark
Copy link
Collaborator

PR for of_yojson_exn will be accepted.

@stevebleazard stevebleazard mentioned this issue Jan 10, 2018
Merged
@stevebleazard
Copy link
Contributor

This can now be closed as #68 resolved this

gasche added a commit to gasche/opam-repository that referenced this issue Dec 15, 2018
@gasche
ppx_deriving_yojson.3.3
815e59c 
(3.2 broke backward-compatibility so it was not released)

3.3
---

  * Make `_exn` functions opt-in (`[@@deriving yojson { exn = true }]`)
    to preserve backward-compatibility for fully-manual implementations
    of the [@@deriving yojson] interface.
    (ocaml-ppx/ppx_deriving_yojson#86)
    Gabriel Scherer

3.2
---

  * Add `let _ = to_yojson / of_yojson` to generated code to avoid warnings when
    they aren't used
    (ocaml-ppx/ppx_deriving_yojson#68)
    Steve Bleazard
  * Fix bug where doing [@@deriving of_yojson] causes an unused rec warning
    (ocaml-ppx/ppx_deriving_yojson#68)
    Steve Bleazard
  * Add generated `ty_of_yojson_exn` to raise an exception rather than return an
    error
    (ocaml-ppx/ppx_deriving_yojson#57, ocaml-ppx/ppx_deriving_yojson#68)
    Steve Bleazard
  * Port `ppx_deriving_yojson` to `dune`
    (ocaml-ppx/ppx_deriving_yojson#69, ocaml-ppx/ppx_deriving_yojson#85)
    Rudi Grinberg, Antonio Nuno Monteiro
  * Added deriver option `fields` to generate a `Yojson_meta` module containing
    all JSON key names.
    (ocaml-ppx/ppx_deriving_yojson#70)
    Steve Bleazard
  * Remove cppo that included support for versions no longer supported by
    `ppx_deriving_yojson`
    (ocaml-ppx/ppx_deriving_yojson#75)
    Rudi Grinberg
@gasche gasche mentioned this issue Dec 15, 2018
Merged
mseri pushed a commit to ocaml/opam-repository that referenced this issue Dec 16, 2018
@gasche @mseri
ppx_deriving_yojson.3.3 (#13150)
ec795aa 
(3.2 broke backward-compatibility so it was not released)

3.3
---

  * Make `_exn` functions opt-in (`[@@deriving yojson { exn = true }]`)
    to preserve backward-compatibility for fully-manual implementations
    of the [@@deriving yojson] interface.
    (ocaml-ppx/ppx_deriving_yojson#86)
    Gabriel Scherer

3.2
---

  * Add `let _ = to_yojson / of_yojson` to generated code to avoid warnings when
    they aren't used
    (ocaml-ppx/ppx_deriving_yojson#68)
    Steve Bleazard
  * Fix bug where doing [@@deriving of_yojson] causes an unused rec warning
    (ocaml-ppx/ppx_deriving_yojson#68)
    Steve Bleazard
  * Add generated `ty_of_yojson_exn` to raise an exception rather than return an
    error
    (ocaml-ppx/ppx_deriving_yojson#57, ocaml-ppx/ppx_deriving_yojson#68)
    Steve Bleazard
  * Port `ppx_deriving_yojson` to `dune`
    (ocaml-ppx/ppx_deriving_yojson#69, ocaml-ppx/ppx_deriving_yojson#85)
    Rudi Grinberg, Antonio Nuno Monteiro
  * Added deriver option `fields` to generate a `Yojson_meta` module containing
    all JSON key names.
    (ocaml-ppx/ppx_deriving_yojson#70)
    Steve Bleazard
  * Remove cppo that included support for versions no longer supported by
    `ppx_deriving_yojson`
    (ocaml-ppx/ppx_deriving_yojson#75)
    Rudi Grinberg
fdopen added a commit to fdopen/opam-repository-mingw that referenced this issue Dec 18, 2018
@fdopen
ppx_deriving_yojson.3.3 (#13150)
913e758 
(3.2 broke backward-compatibility so it was not released)

3.3
---

  * Make `_exn` functions opt-in (`[@@deriving yojson { exn = true }]`)
    to preserve backward-compatibility for fully-manual implementations
    of the [@@deriving yojson] interface.
    (ocaml-ppx/ppx_deriving_yojson#86)
    Gabriel Scherer

3.2
---

  * Add `let _ = to_yojson / of_yojson` to generated code to avoid warnings when
    they aren't used
    (ocaml-ppx/ppx_deriving_yojson#68)
    Steve Bleazard
  * Fix bug where doing [@@deriving of_yojson] causes an unused rec warning
    (ocaml-ppx/ppx_deriving_yojson#68)
    Steve Bleazard
  * Add generated `ty_of_yojson_exn` to raise an exception rather than return an
    error
    (ocaml-ppx/ppx_deriving_yojson#57, ocaml-ppx/ppx_deriving_yojson#68)
    Steve Bleazard
  * Port `ppx_deriving_yojson` to `dune`
    (ocaml-ppx/ppx_deriving_yojson#69, ocaml-ppx/ppx_deriving_yojson#85)
    Rudi Grinberg, Antonio Nuno Monteiro
  * Added deriver option `fields` to generate a `Yojson_meta` module containing
    all JSON key names.
    (ocaml-ppx/ppx_deriving_yojson#70)
    Steve Bleazard
  * Remove cppo that included support for versions no longer supported by
    `ppx_deriving_yojson`
    (ocaml-ppx/ppx_deriving_yojson#75)
    Rudi Grinberg
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@whitequark @hcarty @tomjridge @stevebleazard