Derive sexp_of

[lukepalmer]

Using [@@deriving sexp_of] on exposed types is helpful for formatting good errors and other messages that can contain curl types.

I realize that this is possibly controversial because it's just one opinion on how to format such things (other people may prefer string over sexp, etc).

My direct application here is that I'd like to open source an curl client that works with Async. Having these derivations in the source means I don't need a bunch of boilerplate inside that client to do formatting. I can probably come up with something else if this isn't acceptable.

[ygrek]

i don't understand how this works without ppx_deriving dependency? and i definitely wouldn't want that dependency.
but we can export semi-mechanically those definitions into separate module (with equalities on Curl types definitions) and attach deriving there?

[lukepalmer]

Thanks for looking @ygrek. My apologies for missing dependencies; my build environment is unusual. I'll revisit this with vanilla dune.

Reiterating the types was what I was trying to avoid with this change. If you don't want the dependency that's entirely fair. I can pull that duplication into my client if that's the right thing (unless you have a good idea about how to do this within the bindings in an acceptable way, which I'm happy to try).

[lukepalmer]

Also, if it makes a difference, ppx_sexp_conv is going to be a build dependency only, with a runtime dependency on sexplib0, which is pretty compact.

If there is some other way to do human readability (string?) that you'd prefer I am happy to use whatever that is.

[ygrek]

i totally understand the motivation and i would feel much easier if there was default agreed upon / expected way of doing show for all types in ocaml, but by default i have aversion to dependencies especially in low-level libraries that have a lot of rev-deps.

unless you have a good idea

Regardless of approach below - I imagine it is only for error messages, then i prefer plain strings not sexp

Ideas (not sure if good) :

1. have only (ppx_deriving) attributes without any lib dependency in code and have ppx as optional dependency? then if ppx is present in environment - the consumer will have extra functions, if not then not. This is definitely possible to craft manually but i don't know if it is conveniently expressible with dune and opam (because both ppx and lib need to optional dep together). Second concern is that it is meh DX (one might expect these functions present but they may or may not be depending on opam switch)
2. as proposed in a comment above - create curl-types opam package that depends on curl and ppx_deriving and just re-imports all the types with attributes attached (ie duplication but maintained inside this repo not in client code)
3. there is always an option to just write conversion funcs by hand 🙈
4. but actually better variation of above - use maintainer-time dependency - cinaps or some other preprocessor to generate and inline such functions into source code

[lukepalmer]

How about doing this with ppx_string_conv? The only runtime dependency is 'base' which is hopefully uncontroversial. There are some small build-time dependencies.

If it's important to avoid build-time dependencies I could probably come up with a way to do code generation as part of a release build and then commit it. I don't understand why this would be helpful but happy to give it a try if you prefer.

[nojb]

How about doing this with ppx_string_conv? The only runtime dependency is 'base' which is hopefully uncontroversial.

To paraphrase what @ygrek said: it is controversial because it would mean that everyone using ocurl must also depend on base which is huge (~60k LOC, not counting dependencies). Having to incorporate all that code (and remember, not everyone uses OPAM) in order to have automatically generated conversion functions? Not nearly worth it, in my book :)

[lukepalmer]

Got it! I will see what I can come up with here as an alternative. I do appreciate the context.

[lukepalmer]

I was able to take a different approach in my client using Curl.strerror and similar. This seems fine for clients of my library, but maybe isn't quite as nice for me because it requires a little thought to go from the nicely formatted error message back to exactly which variant constructor it refers to.

Anyhow thanks for the feedback. I have a new perspective on how others think about dependencies. Closing.