janestreet / ppx_sexp_conv

Generation of S-expression conversion functions from type definitions
MIT License
88 stars 17 forks source link

#require doesn't work in toplevel #30

Open yminsky opened 4 years ago

yminsky commented 4 years ago

I can enable ppx_sexp_value if I #require "ppx_jane", but not if I #require "ppx_sexp_value".

utop # #require "ppx_jane";;
─( 19:25:31 )─< command 1 >─────────────────────────────────────────────────────────────{ counter: 0 }─
utop # open Base;;
─( 19:25:36 )─< command 2 >─────────────────────────────────────────────────────────────{ counter: 0 }─
utop # [%sexp_of: int];;
- : int -> Sexp.t = <fun>
utop # #require "ppx_sexp_value";;
─( 19:23:52 )─< command 1 >─────────────────────────────────────────────────────────────{ counter: 0 }─
utop # [%sexp_of: int];;
Line 1, characters 2-9:
Error: Uninterpreted extension 'sexp_of'.

Any idea what's going on?

ghost commented 4 years ago

This is expected. ppx_sexp_conv being a dependency is:

We then carefully crafted the dependencies between these libraries to reflect this fact. So ppx_sexp_value not providing [%sexp_of: ...] is voluntary. You need to explicitly depend on ppx_sexp_conv to get access to it.

yminsky commented 4 years ago

Hmm. I can't figure out how to enable [%sexp_of] or [@@deriving sexp] in utop except by #require "ppx_jane". How should I do it?

ghost commented 4 years ago

It should work with: #require "ppx_sexp_conv"

yminsky commented 4 years ago

It doesn't seem to work for me:

utop # #require "base";;
─( 12:49:49 )─< command 1 >───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────{ counter: 0 }─
utop # #require "ppx_sexp_conv";;
No such package: ppx_deriving - required by `ppx_sexp_conv'
ghost commented 4 years ago

Ah, that's a deeper issue... For historical reasons, the toplevel relies on an older and more complicated way of composing and running ppx rewriters. Currently, you have to install the ppx_deriving package in order to use [@@deriving] plugins in the toplevel.

The reason it works with ppx_jane is a bit long to explain.

/cc @emillon, we should really look into changing this

yminsky commented 4 years ago

Is a reasonable short-term hack to make ppx_deriving a dependency of the various ppxs? That way, when you install a PPX, you'll be able to actually use it in the toplevel without installing anything else.

ghost commented 4 years ago

Such changes never have a 0 cost as everybody cares about dependencies and we have been in this situations for years. So I'm more inclined to keep it as a know quirk until we switch to something more straightforward.

yminsky commented 4 years ago

Understood

On Tue, May 19, 2020, 10:49 AM Jérémie Dimino notifications@github.com wrote:

Such changes never have a 0 cost as everybody cares about dependencies and we have been in this situations for years. So I'm more inclined to keep it as a know quirk until we switch to something more straightforward.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/janestreet/ppx_sexp_conv/issues/30#issuecomment-630871038, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFOUJXV7K4GDKIGQRZZ273RSKMBHANCNFSM4MYMSZAQ .

emillon commented 4 years ago

Ah, that's a deeper issue... For historical reasons, the toplevel relies on an older and more complicated way of composing and running ppx rewriters. Currently, you have to install the ppx_deriving package in order to use [@@deriving] plugins in the toplevel.

The reason it works with ppx_jane is a bit long to explain.

/cc @emillon, we should really look into changing this

Yeah, curious to hear why it works with ppx_jane. I suppose it's related to the fact that ppx_jane is installed as a precompiled driver usable with #ppx as described in the README.