tweag / nickel

Better configuration for less
https://nickel-lang.org/
MIT License
2.23k stars 85 forks source link

Forward the metadata when re-exporting a field #1968

Open thufschmitt opened 2 weeks ago

thufschmitt commented 2 weeks ago

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

Record fields can have an attached metadata. Since this metadata is attached to the field itself and not the value, it isn't forwarded when the value is used. It's sometimes what we want, but not always. In particular, it's very natural to re-export a field from another record, but that means losing the documentation, type annotation, and so on.

For instance, in:

let r = {
  foo | doc "Some doc" = 1,
}
in
{
  foo = r.foo,
}

the exported documentation (or the LSP) won't show the documentation for foo.

Describe the solution you'd like

Have the metadata forwarded when re-exporting a field from a different record.

Describe alternatives you've considered

This is bound to be somewhat heuristic and fragile (what about { foo = let bar = r.foo in bar }?). Maybe that would be an argument for an inherit keyword that would not just forward the value, but also the metadata. That way setting { foo = r.foo } would keep the current semantics, but the “re-export” case could be handled by { inherit foo from r } (or whatever the syntax would be).

Related: https://github.com/tweag/nickel/issues/747

Additional context

This shows-up in a few places in Organist. For instance, import_nix has a documentation and a schema, but that's lost when it's re-exported (here and here).

yannham commented 2 weeks ago

This is bound to be somewhat heuristic and fragile

I agree. Also, there are some cases when the semantics of propagating metadata is really not what you want. This question of propagation has been extensively scrutinized in RFC005, see for example this section. For balance, propagation is acknowledged as desirable though in some situations here, so it was a known tradeoff when siding for static metadata that can only appear at field location.

For those reasons, I think the inherit solution is very appealing. No complex heuristics. What's more, inherit is overdue and the recursivity by default is a strong incentive to implement it, so it's a desirable and planned extension which has a good chance of landing in some future version.

There might be cases where you want to inherit without metadata, so let's think hard about it, but for now, I don't see any example.