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

Add docFn to lib, to automatically generate documentation for lib functions #23505

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
9 participants
@moretea
Copy link
Contributor

moretea commented Mar 4, 2017

This enables generating documentation about the lib.* functions automatically, and opens doors to a poor mans auto-complete of the standard library functions.

Motivation for this change

Currently it's pretty hard to find out which library functions exist. You have to manually scroll, or grep through the files in lib.

As a proof of concept, I've moved some of the functions in lib/strings.nix to use this docFn function.
The resulting manual build can be seen here.

Implementation

It shows the top level functions made available in lib and the functions that are scoped in a sub attrset (e.g. lib.strings.y) in two different places. The first will be more commonly used than the latter.

Discussion
  • These library functions are used everywhere. This PR is implemented with functors, which might be to slow.
  • As an alternative, if this proves to be too slow, we could look into adding doc comments to Nix, that will be parsed and made inspectable just like builtins.functionArgs is today. (see the partial implementation)
Add docFn function to lib.
This enables generating documentation about the lib.* functions
automatically, and opens doors to a poor mans auto-complete with
the standard library functions.
@mention-bot

This comment has been minimized.

Copy link

mention-bot commented Mar 4, 2017

@moretea, thanks for your PR! By analyzing the history of the files in this pull request, we identified @edolstra, @zimbatm and @abbradar to be potential reviewers.

@ericsagnes

This comment has been minimized.

Copy link
Contributor

ericsagnes commented Mar 5, 2017

Interesting! I use a similar approach to document styx functions and templates.
I didn't benchmark but this didn't seem to impact performance much, but as nixpkgs is much larger and complex, it might be of real impact.
This can also be pushed one step further to generate tests from examples.

PS: Example of generated function docs with examples and arguments here.

libDocJSON = builtins.toFile "options.json" (builtins.toJSON (import ./fn-docs.nix));
libDocBook= pkgs.runCommand "options-db.xml" {} ''
${pkgs.ruby}/bin/ruby ${./lib2docbook.rb} < ${libDocJSON} > $out
'';

This comment has been minimized.

@ericsagnes

ericsagnes Mar 10, 2017

Contributor

The fn-docs.nix -> function docbook conversion is simple enough so it could be done in plain nix instead of using ruby. That will also remove the need of the json conversion.
Nit: options.json options-db.xml copy paste miss? :)

This comment has been minimized.

@moretea

moretea Mar 10, 2017

Contributor

All fixed in a801c0b. Thanks!

@zimbatm

This comment has been minimized.

Copy link
Member

zimbatm commented Mar 11, 2017

@moretea

This comment has been minimized.

Copy link
Contributor

moretea commented Mar 13, 2017

@zimbatm I would use that if it was in nixpkgs.

@moretea

This comment has been minimized.

Copy link
Contributor

moretea commented Mar 13, 2017

I'm wondering what people think about this idea, should I start drafting a RFC for this?

let documentedFunction =
{
type = "documentedFunction";
__functor = self: x: fn x;

This comment has been minimized.

@zimbatm

zimbatm Mar 14, 2017

Member

Is that a magic accessor like __toString?

This comment has been minimized.

@zimbatm

zimbatm Mar 14, 2017

Member

If yes, what are the performance implications? Given that lib is used everywhere it might be problematic.

This comment has been minimized.

@ericsagnes

ericsagnes Mar 14, 2017

Contributor

@zimbatm I was also curious about __functor performance so I made a quick little benchmark here. (Not sure if it is the right way to benchmark nix function calls and if the results have any relevance)

Anyway, the results show that the use of functor increased the number of cycles by about 3% for that test.

This comment has been minimized.

@Profpatsch

Profpatsch Jul 17, 2017

Member

I don’t know if that benchmark shows anything for nixpkgs itself. The complete evaluation of all (non-broken) package should be benchmarked, right?

@@ -16,6 +16,7 @@
<xi:include href="cross-compilation.xml" />
<xi:include href="configuration.xml" />
<xi:include href="functions.xml" />
<xi:include href="lib.xml" />

This comment has been minimized.

@zimbatm

zimbatm Mar 14, 2017

Member

I'm lazy, is there a rendered example somewhere?

This comment has been minimized.

@moretea

moretea Mar 14, 2017

Contributor

@zimbatm yes, see my top comment. The last refactor did not modify the content at all.

@FRidh

This comment has been minimized.

Copy link
Member

FRidh commented Jul 28, 2017

Just now there was a discussion again on IRC about this PR as well as #27394. Comparing the two,
this one seems neater because you won't have the problem of strings/sets of documentation ending up in your sets. As @zimbatm and @Profpatsch pointed out, there may indeed be performance implications.

@LnL7 and @grahamc just had the idea of rewriting the documented functions to just the bare functions when evaluating the fixpoint when something like docs ? false is passed.

@grahamc

This comment has been minimized.

Copy link
Member

grahamc commented Jul 29, 2017

My testing suggests this does not appreciably harm performance in some limited testing. @LnL7 also did some testing, and his results agreed with mine. I think we should continue with this route by setting up an eval on hydra to see how it really behaves. Perhaps also merging in some functionality /feedback from #27394 like:

I did also make lib a fixed-point to make it easier to replace the docsfn (master...grahamc:fixed-lib) but I'd rather not merge this.

@edolstra

This comment has been minimized.

Copy link
Member

edolstra commented Jul 30, 2017

I don't know, this approach strikes me as too much magic. Values really should not produce different results depending on how they're evaluated (i.e. the x in the expression x y can now mean something different than the x in x.y). I'm not super fond of __functor for that reason.

An approach that doesn't rely on __functor would be something like:

id =
  { docString = "bla bla...";
    examples = [ ... ];
    implementation = x: x;
  };

and do a mapAttrs (n: v: v.implementation) in lib/default.nix.

@Profpatsch

This comment has been minimized.

Copy link
Member

Profpatsch commented Aug 1, 2017

I'm not super fond of __functor for that reason.

I’m curious: In what way does __functor resemble a functor?

@danbst

This comment has been minimized.

Copy link
Contributor

danbst commented Jan 11, 2019

Approach in #53055 is less invasive and more in-line with docs-from-code approaches for other languages (Python and Haskell at least).

Closing this.

@Profpatsch probably it is more like functor from C++, rather than Functor from Haskell.

@danbst danbst closed this Jan 11, 2019

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