nixdoc icon indicating copy to clipboard operation
nixdoc copied to clipboard

feat: Introduce digging behaviour

Open ein-shved opened this issue 1 year ago • 8 comments

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;
   }
 }

ein-shved avatar Dec 29 '23 14:12 ein-shved

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

infinisil avatar Jan 08 '24 21:01 infinisil

@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.

ein-shved avatar Jan 09 '24 06:01 ein-shved

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:

  • import
  • assert
  • if ... then ... else
  • partial application
  • lists
  • ...
{
  /* 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 = <[email protected]>; bar = <[email protected]> }

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.

hsjobeki avatar Jan 09 '24 11:01 hsjobeki

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

infinisil avatar Jan 15 '24 21:01 infinisil

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

ein-shved avatar Jan 16 '24 08:01 ein-shved

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?

hsjobeki avatar Jan 16 '24 09:01 hsjobeki

@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...

ein-shved avatar Jan 16 '24 09:01 ein-shved

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.

infinisil avatar Jan 31 '24 20:01 infinisil