nix-community / nixdoc

Tool to generate documentation for Nix library functions [maintainer=@infinisil]
GNU General Public License v3.0
131 stars 14 forks source link

feat: Introduce digging behaviour #98

Open ein-shved opened 11 months ago

ein-shved commented 11 months ago

I am not sure, do you need this functionality within mainstream. So I made this WIP PR to ask you, should I make it more clear and add tests to be merged?

This may be needed when the project has not such flat structure, as nixpkgs. When different libraries produces their outputs not at toplevel, but dipper.

You may specify common templates to dig into in command arguments. Eg --dig submodule, then this will generate documentation for files like:

 {
   submodule = {...}: {
     /* Doc for foo */
     foo = 1;
     /* Doc for bar */
     bar = 2;
   }
 }

You may force documentation to dig into attrset withing its comment to with DocDig! directive:

 {
   /* DocDig! */
   myDeepLib = {...}: {
     /* Doc for foo */
     foo = 1;
     /* Doc for bar */
     bar = 2;
   }
 }
infinisil commented 10 months ago

This doesn't seem very sound, because it would just ignore the function argument which could have important semantics. In particular, right now the examples you gave just generate this:

# test {#sec-functions-library-test}

## `lib.test.foo` {#function-library-lib.test.foo}

Doc for foo

## `lib.test.bar` {#function-library-lib.test.bar}

Doc for bar

With no mention of myDeepLib. And even lib.test.myDeepLib.{foo,bar} would not be right, because that looks like a direct attribute, when it's really nested in a function.

I think for an example like this:

{
  /*
    Add or subtract two numbers.
  */
  addSub = { a, b }: {
    /* Doc for add */
    add = a + b;
    /* Doc for sub */
    sub = a - b;
  };
}

generating something like this would make more sense:

# test {#sec-functions-library-test}

## `lib.test.addSub` {#function-library-lib.test.addSub}

**Type**: `Attrs -> Attrs`

Add or subtract two numbers.

**Argument**
structured function argument

: `a`

  : Function argument

  `b`

  : Function argument

**Returns**

`add`

: Doc for add

`sub`

: Doc for sub

This doesn't work very well if you have more complicated nested doc comments though, so maybe both **Returns** and **Argument** should also be headings.

Ping @hsjobeki as this is relevant for https://github.com/NixOS/rfcs/pull/145 and https://github.com/nix-community/nixdoc/pull/91

ein-shved commented 10 months ago

@infinisil, I agree with you, but only when we are talking about automatic collecting of deeper fields. My PR is about different behaviour. The most close example is flake:

{
  outputs = { self, nixpkgs }: {
    /* Doc for abc */
    result1 = "abc";
    /* Just nixpkgs */
    result2 = nixpkgs;
  };
}

My main goal is to generate doc like next:

# my-flake {#sec-functions-library-flake}

## `lib.my-flake.result1` {#function-library-lib.flake.result1}

Doc for abc

## `lib.my-flake.result2` {#function-library-lib.flake.result2}

Just nixpkgs

For now nixdoc could generate documentation only for lib.my-flake.outputs, which is not very useful in such situations. We do not really want to mention the outputs part - it is obvious. We are interested in nested fields as we could access them right as above documentation said my-flake.result2 from another flake.

I think, it is better to drop the /* DocDig! */ part from PR to left the --dig option and implement automatic documentation collection for function nested fields (as you said in the comment) later.

hsjobeki commented 10 months ago

Regarding the 'DocDig!' directive, I would say that your implementation is rather limited. It only supports direct attributes and direct let...in expressions.

Any expression other than let ... in and attrset will break it. Imagine:

{
  /* DocDig! */
  deep = a: b: c: if true then {
    /* Nested is 1*/
    nested = 1;
  } else {
    /* Nested is 2*/
    nested = 2;
  };
}

I think a complete implementation of this would collect all doc-comments recursively until the end of the parent expression. It should also track the path of more nested attributes. But that is super complex, and I suspect implementing it comes close to reimplementing a real nix evaluator.

In comparison, I'll explain how https://noogle.dev works with this problem.

Imagine the following 2 files:

init.nix

 {
   /** 
     This function is used to build our 'lib'
   */
   myDeepLib = {...}: {
     /**
       Docs for foo
     */
     foo = x: x;
      /**
       Docs for bar
     */
     bar = x: x;
   }
 }

lib.nix

{
  lib = myDeepLib {...};
}

We can the observe:

nix-repl > lib
{ foo = <lambda@init.nix>; bar = <lambda@init.nix> }

From that position, we can statically analyze the init.nix file at the given position to retrieve the documentation.

This currently works for attributes and lambdas. It gets a little more complex with partially applied functions.

Unfortunately, this doesn't work with nixdoc because it is a static (ast) only tool. And the nix language is too complex for complete static analysis.

infinisil commented 10 months ago

The most close example is flake:

{
  outputs = { self, nixpkgs }: {
    /* Doc for abc */
    result1 = "abc";
    /* Just nixpkgs */
    result2 = nixpkgs;
  };
}

You can do this instead:

{
  outputs = inputs: import ./outputs.nix inputs;
  # This doesn't work because of https://github.com/NixOS/nix/issues/4945
  # outputs = import ./outputs.nix;
}

Then you can run nixdoc on ./outputs.nix

ein-shved commented 10 months ago

You can do this instead:

{
  outputs = inputs: import ./outputs.nix inputs;
  # This doesn't work because of https://github.com/NixOS/nix/issues/4945
  # outputs = import ./outputs.nix;
}

Then you can run nixdoc on ./outputs.nix

Breaking the code architecture to fix documentation tool drawbacks is a very bad idea

hsjobeki commented 10 months ago

Personally i'd not want to maintain the docDig! directive. I am afraid that this may end up in nixpkgs. Opening up directives that do special "Walk" operations on the AST is not something that i imagined when writing this rfc https://github.com/NixOS/rfcs/pull/145.

Maybe we can find a solution that solves your problem in another way. Do you have some code example / repository that you could show to us?

ein-shved commented 10 months ago

@hsjobeki I do not actually need for DocDig! directive. I've added it as PoC and about to remove it from this PR (I said that previously). Without this option will the functionality of this PR suites this tool?

P.S. The --dig command-line option will left...

infinisil commented 10 months ago

Rethinking this a bit, I think this could be made to work like this:

{
  outputs =
    { self, nixpkgs }:
    /* nixdoc!outputs */
    {
      /* Doc for abc */
      result1 = "abc";
      /* Just nixpkgs */
      result2 = nixpkgs;
    };
}

Here, nixdoc would search specifically for /* nixdoc!<prefix> in the AST, and render documentation for the found node under the given <prefix>.

This fixes the problem brought up by @hsjobeki in https://github.com/nix-community/nixdoc/pull/98#issuecomment-1882917954.