Skip to content
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 from

Conversation

moretea
Copy link
Contributor

@moretea 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)

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
Copy link

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

doc/default.nix Outdated
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
'';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All fixed in a801c0b. Thanks!

@zimbatm
Copy link
Member

zimbatm commented Mar 11, 2017

Would it make sense to have a pure Nix XML builder? https://gist.github.com/anonymous/c52b3c9ca41f586a95a113d1a2bb26d3#file-to_xml-nix-L49-L63

@moretea
Copy link
Contributor Author

moretea commented Mar 13, 2017

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

@moretea
Copy link
Contributor Author

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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that a magic accessor like __toString?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@FRidh
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
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
Copy link
Member

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
Copy link
Member

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

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants