nickel doc appears not to work for Let x in y expressions

[harris-chris]

Hello - running nickel doc -f test.ncl --stdout for this:

# test.ncl
{
  port
    | doc "a  port"
}

works fine, and outputs the documentation I'd expect. But if I change the file to this:

# test.ncl
let Port = {
  port
    | doc "a  port"
}
in Port

then I get an error:

error: no documentation found
  ┌─ test.ncl:1:1
  │  
1 │ ╭ let Port = {
2 │ │   port
3 │ │     | doc "a  port"
4 │ │ }
5 │ │ in Port
  │ ╰───────^
  │  
  = documentation can only be collected from a record.

I'd expect to get the same behaviour from both of these examples, I'd have thought that the second one would just resolve to the record shown in the first one.

Unless there's something I'm missing, this stops documentation being generated for schemas which use imports.

Thanks!

[vkleen]

This is absolutely a bug, you're not missing anything. I'll try looking into it.

[yannham]

I wouldn't be surprised if nickel doc doesn't do any form of evaluation. That is, the file must syntactically be a record literal. Which is not well documented and quite limiting (the initial motivation coming from generating the documentation for the stdlib). I think it would actually be rather easy to change that, and it sounds like a good idea: evaluate the term (weakly, i.e "lazily"), substitute the environment and generate the doc.

[yannham]

By the way, if you don't specify --output, nickel additionally creates an empty file ./nickel/doc/out.md although there is an error. We should probably not mutate the filesystem in this case.

[vkleen]

I almost feel like this needs to be a special evaluation mode. We need to not only evaluate to WHNF but also descend into record fields to collect documentation recursively. At the same time there's no need for the share normal form transformation and we don't really want to evaluate every field fully.

[yannham]

The issue is that I fear the evaluation will just blow up if we don't do the share normal form transformation, there's just too many parts of the evaluation (in particular merging) that heavily relies on this transformation being applied. I think.

Once again, share normal form wouldn't be an issue with a runtime AST repr that could encode closures directly, as we could just say goodbye to those heading let-bindings 🤷‍♂️

We did have different evaluation modes for metadata pre-RFC005. The code has been scrapped but shouldn't be too hard to revive, and I think provided a somehow principled approach to this idea (in particular the fact that being in one mode, you might want to evaluate a subterm in another mode - I don't know if that's the case here, but it was for metadata evaluation mode). It's not out of the question.