search

janestreet / ppx_sexp_conv

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

OPAM description doesn't mention ppx_deriving #24

Closed mars0i closed 4 years ago

mars0i commented 6 years ago

The short description of ppx_sexp_conv in the OPAM repository doesn't mention ppx_deriving. This means that it's difficult to find this lib through an OPAM search if you're looking for a ppx_deriving sexp converter. You would have to guess that it might be relevant and click through to the github page and start reading there. (Since there are several sexp libs and several deriving-related libs, this is not an efficient strategy. :-)

I learned of ppx_sexp_conv from the RWO2 chapter on sexps after giving up on OPAM.

ghost commented 6 years ago

It's because it doesn't use ppx_deriving. But I guess we could at least mention [@@deriving] so that you can find it by doing opam search deriving

mars0i commented 6 years ago

Great. Thanks. Seems important, since one likely reason for searching for ppx_sexp_conv is to use it with @@deriving.

I thought I'd create a PR that you could use if you wanted, but the description is already 69 characters, so little rewording is necessary, if it should remain in 80 chars. Not sure what would make sense. Maybe:

Generation of S-expression conversion functions from types; can be used with [@@deriving]

ghost commented 6 years ago

We can make the description multi-lines. For instance:

Generation of S-expression conversion functions from types.

This ppx rewriter provides the sexp, sexp_of and of_sexp derivers. For instance attaching [@@deriving sexp] to a type declaration will automatically generate functions for converting between this type and S-expressions.

mars0i commented 6 years ago

The opam search doesn't check subsequent lines. For example, if you search on "Jane Street", ppx_sexp_conv won't come up, even though that string is in the additional line of text that's currently part of the description.

(Maybe the search should have an option to check the rest of the text, though.)

ghost commented 6 years ago

Ah, ok. I submitted the change internally and will backport it once it passes the review process.

mars0i commented 6 years ago

OK. Thanks.

aalekseyev commented 4 years ago

Looks like opam search deriving can now find ppx_sexp_conv, so I'm closing this issue.