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.

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

First-class docstrings #1719

Open
thufschmitt opened this issue Nov 17, 2023 · 4 comments
Open

First-class docstrings #1719

thufschmitt opened this issue Nov 17, 2023 · 4 comments

Comments

@thufschmitt
Copy link
Member

Is your feature request related to a problem? Please describe.

Not a big immediate issue, but I find myself every once in a while wishing that doc strings could be generated programmatically.

An instance is defining an enable_option function in Organist similar to Nixpkgs's mkEnableOption, that would look like:

enable_option = fun name default_value => { enable | Bool | doc "Whether to enable %{name}" | default = default_value }

This currently fails at parse-time because doc doesn't allow interpolating variables

Describe the solution you'd like

A possibility for the doc metadata to be more dynamic to cover such a use-case

Describe alternatives you've considered

Don't use this kind of pattern. It's acceptable for my use-case (yet), but it could be limiting in the future.

@yannham
Copy link
Member

yannham commented Nov 17, 2023

I must admit that I'm a bit reluctant to the general idea of making metadata entirely dynamic. One of the point of documentation is to be used in part by humans reading the source code. Typically, most of programming languages' docstrings are just a special form of comments, which aren't dynamic. This also makes extraction simple and predictable (although, for Nickel, we already need to do some evaluation before we reach the metadata e.g. in nickel query, to be fair). For example, the LSP currently doesn't do any form of evaluation whatsoever, but it can still extract and refer to metadata, including documentation, in a very simple way, because they are static data stored in the AST.

That being said, I understand where you're coming from: I imagine what you do, more or less, is to dynamically generate a configuration interface, and you want to reuse the Nickel tooling for all the niceties it brings to explore and interact with this interface. Hence you'd like some form of metaprogramming 😛 it's a usage that is a bit different from writing static docstrings to describe library functions. In that context, having first-class documentation is not absurd.

One possibility would be to decouple those two notions: keep a static, easy to leverage way to document various values, and have a second one, which is picked by the tooling as well whenever possible, which is allowed to be dynamic.

A second possibility is to allow interpolation in docstrings, and if we don't want to perform evaluation (e.g. in the LSP), simply show the original uninterpolated string (which isn't great, but backward compatible and at least shows something).

@thufschmitt
Copy link
Member Author

A second possibility is to allow interpolation in docstrings, and if we don't want to perform evaluation (e.g. in the LSP), simply show the original uninterpolated string (which isn't great, but backward compatible and at least shows something).

I like that idea a lot. It keeps things readable without tooling ("Whether to enable %{name}" nearly looks like some convoluted markup and isn't much worse than "Whether to enable `name`"). Obviously that can still be abused (let docstring = some convoluted + expression in { foo | doc "%{docstring}"), but you'd have to be explicit about it.

@yannham
Copy link
Member

yannham commented Nov 27, 2023

We discussed this issue in the weekly meeting, and we generally feel a bit uncomfortable with allowing dynamic docstrings, for a number of reasons:

  • "is this docstring static or dynamic" is a fragile property. It depends on the whole docstring, if there is an interpolation somewhere in it, and can switch easily from one to the other with a small change. Something that happens a lot in the stdlib is to have escaped interpolation (to show code examples). Currently there is an error if you don't escape interpolation properly, but here there would be a risk of your documentation just becoming silently dynamic (and maybe failing at runtime with a spurious unbound identifier)
  • Now you documentation can evaluate. A bit like the internal code of contracts, this adds a new layer that isn't really exactly your config logic (or library code) that can now also fails with a whole bunch of new errors. It's probably not a big shift when using nickel query or the like, which already perform evaluation anyway, but for the LSP, it might be more annoying.

So it felt like it should at least be a different metadata, or alternatively use a dedicated builtin (while keeping the docstring written as metadata static).

Another question is: do you actually need dynamic documentation, or is it a XY problem? From the description, it really looks like you want to do metaprogramming/multi-stage evaluation. In particular you probably don't need to re-evaluate enable_option and the docstring each time you request the documentation fro nickel query, but rather that you could generate an interface with static documentation in one stage, and then always use that. Or, put differently, if you had macros, you wouldn't need dynamic documentation (and you could similarly template other static metadata such as being optional). Is that correct?

All of that being said, although it would be a tad uglier, I think a primop %with_doc% : (record: {_ : a}) -> (field: String) -> (doc: String) -> {_: a} would be an easy work around, because as long as it's strict in the docstring, there is no need to change anything to the current semantics, tooling or implementation of metadata. doc is just a string metadata stored somewhere and that can be set through the | doc "..." syntax. with_doc would just be an alternative way of setting this field.

@thufschmitt
Copy link
Member Author

Thanks for looking into that. The reasons for not wanting this make a lot of sense :)

a primop %with_doc% : (record: {_ : a}) -> (field: String) -> (doc: String) -> {_: a} would be an easy work around

That also seems pretty good. It's ugly, but that's quite acceptable for metaprogramming (especially since it would raise a bit the bar of it, which would prevent a number of abuses).

if you had macros, you wouldn't need dynamic documentation (and you could similarly template other static metadata such as being optional). Is that correct?

Yes, absolutely. Although macros seem like they are a much bigger hammer if the concern is avoiding having to evaluate stuff to introspect it.

But now you're making me dream of a full-blown macro system in Nickel (which I have zero need for right now, but I'm sure I could invent reasons for using that) 🤩

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants