search

dbuenzli / fmt

OCaml Format pretty-printer combinators
http://erratique.ch/software/fmt
ISC License
71 stars 26 forks source link

Add the [@@ocaml.deprecated] annotation to deprecated functions. #47

Closed MisterDA closed 3 years ago

MisterDA commented 3 years ago

Hi! This tags deprecated functions with the ocaml.deprecated compiler annotation, so that an error or a warning may be raised at compile-time if a project uses these functions. Do you prefer ocaml.deprecated or alert deprecated? See https://caml.inria.fr/pub/docs/manual-ocaml/alerts.html.

dbuenzli commented 3 years ago

Thanks!

Do you prefer ocaml.deprecated or alert deprecated? See https://caml.inria.fr/pub/docs/manual-ocaml/alerts.html.

What's the difference ? Also is that supported in 4.05 ?

MisterDA commented 3 years ago

Thanks!

Do you prefer ocaml.deprecated or alert deprecated? See https://caml.inria.fr/pub/docs/manual-ocaml/alerts.html.

What's the difference ? Also is that supported in 4.05 ?

Since 4.08, ocaml.deprecated is legacy, and aliases to ocaml.alert deprecated which is also identical to alert. Only ocaml.deprecated works in 4.05 (tested in an Opam switch).

Note that they also accept an optional message. I haven't used it as it would duplicate the doc comment, but that may be friendlier. =

dbuenzli commented 3 years ago

Note that they also accept an optional message. I haven't used it as it would duplicate the doc comment, but that may be friendlier. =

There are often good arguments to switch to 4.08 but that doesn't seem enough to me to drop the 4.05 support. Also aren't @deprecated doc strings lifted to @@ocaml.deprecated anyways ? (just curious)

MisterDA commented 3 years ago

There are often good arguments to switch to 4.08 but that doesn't seem enough to me to drop the 4.05 support.

Of course. [@@ocaml.deprecated] is compatible with 4.05, so we’re fine.

Also aren't @deprecated doc strings lifted to @@ocaml.deprecated anyways ? (just curious)

Well, installing fmt v0.8.9 with Opam doesn't raise depreciation errors when I compile a test project, but with that patch they are raised; so it seems to me that the doc string has no effect. Maybe that could be fixed? A doc string with @deprecated could be lifted to compiler annotations…

dbuenzli commented 3 years ago

Thanks, your patch is in as 2f4dbfd0f447ff2cabd79e

kit-ty-kate commented 3 years ago

[@@ocaml.deprecated] also allows library developers to say what they want the users to replace this with. e.g.

[@@ocaml.deprecated "Please use str instead"]

Right now the error message isn't really user-friendly

dbuenzli commented 3 years ago

Would the message show up in generated documentation ? There is a good message there.

MisterDA commented 3 years ago

Right now the error message isn't really user-friendly

Does the message show up in generated documentation ? There is a good message there.

There is indeed an overlap with the documentation. I think the good way forward would be to use the compiler attribute only, because then the compiler can generate warnings; but if we can generate warnings from the doc blocks that's fine too… It's also easier when the compiler insults you to have the fix at hand without having to look it up!

To avoid duplication with documentation, odoc should pick up the attribute, but it doesn't yet. The issue is here: https://github.com/ocaml/odoc/issues/406.

dbuenzli commented 2 years ago

It's as if nothing really ever gets designed in this eco-system. I added the redundant messages in e76a883