-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Doc comments #11072
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
Doc comments #11072
Changes from all commits
63f520f
e5af7cb
7fae378
e68234c
491b9cf
d4f576b
cef11b2
f9243ec
77e9f9e
71cb8bf
8a85529
6bbd493
131b6cc
2181747
ac89df8
6a125e6
ce31a04
03d3370
61a4d3d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| --- | ||
| synopsis: "`nix-repl`'s `:doc` shows documentation comments" | ||
| significance: significant | ||
| issues: | ||
| - 3904 | ||
| - 10771 | ||
| prs: | ||
| - 1652 | ||
| - 9054 | ||
| - 11072 | ||
| --- | ||
|
|
||
| `nix repl` has a `:doc` command that previously only rendered documentation for internally defined functions. | ||
| This feature has been extended to also render function documentation comments, in accordance with [RFC 145]. | ||
|
|
||
| Example: | ||
|
|
||
| ``` | ||
| nix-repl> :doc lib.toFunction | ||
| Function toFunction | ||
| … defined at /home/user/h/nixpkgs/lib/trivial.nix:1072:5 | ||
|
|
||
| Turns any non-callable values into constant functions. Returns | ||
| callable values as is. | ||
|
|
||
| Inputs | ||
|
|
||
| v | ||
|
|
||
| : Any value | ||
|
|
||
| Examples | ||
|
|
||
| :::{.example} | ||
|
|
||
| ## lib.trivial.toFunction usage example | ||
|
|
||
| | nix-repl> lib.toFunction 1 2 | ||
| | 1 | ||
| | | ||
| | nix-repl> lib.toFunction (x: x + 1) 2 | ||
| | 3 | ||
|
|
||
| ::: | ||
| ``` | ||
|
|
||
| Known limitations: | ||
| - It does not render documentation for "formals", such as `{ /** the value to return */ x, ... }: x`. | ||
| - Some extensions to markdown are not yet supported, as you can see in the example above. | ||
|
|
||
| We'd like to acknowledge Yingchi Long for proposing a proof of concept for this functionality in [#9054](https://github.com/NixOS/nix/pull/9054), as well as @sternenseemann and Johannes Kirschbauer for their contributions, proposals, and their work on [RFC 145]. | ||
|
|
||
| [RFC 145]: https://github.com/NixOS/rfcs/pull/145 |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -27,6 +27,7 @@ | |||||
| #include "local-fs-store.hh" | ||||||
| #include "print.hh" | ||||||
| #include "ref.hh" | ||||||
| #include "value.hh" | ||||||
|
|
||||||
| #if HAVE_BOEHMGC | ||||||
| #define GC_INCLUDE_NEW | ||||||
|
|
@@ -616,6 +617,38 @@ ProcessLineResult NixRepl::processLine(std::string line) | |||||
|
|
||||||
| else if (command == ":doc") { | ||||||
| Value v; | ||||||
|
|
||||||
| auto expr = parseString(arg); | ||||||
| std::string fallbackName; | ||||||
| PosIdx fallbackPos; | ||||||
| DocComment fallbackDoc; | ||||||
| if (auto select = dynamic_cast<ExprSelect *>(expr)) { | ||||||
| Value vAttrs; | ||||||
| auto name = select->evalExceptFinalSelect(*state, *env, vAttrs); | ||||||
| fallbackName = state->symbols[name]; | ||||||
|
|
||||||
| state->forceAttrs(vAttrs, noPos, "while evaluating an attribute set to look for documentation"); | ||||||
| auto attrs = vAttrs.attrs(); | ||||||
| assert(attrs); | ||||||
| auto attr = attrs->get(name); | ||||||
| if (!attr) { | ||||||
| // When missing, trigger the normal exception | ||||||
| // e.g. :doc builtins.foo | ||||||
| // behaves like | ||||||
| // nix-repl> builtins.foo | ||||||
| // error: attribute 'foo' missing | ||||||
| evalString(arg, v); | ||||||
| assert(false); | ||||||
| } | ||||||
| if (attr->pos) { | ||||||
| fallbackPos = attr->pos; | ||||||
| fallbackDoc = state->getDocCommentForPos(fallbackPos); | ||||||
| } | ||||||
|
|
||||||
| } else { | ||||||
| evalString(arg, v); | ||||||
| } | ||||||
|
|
||||||
| evalString(arg, v); | ||||||
| if (auto doc = state->getDoc(v)) { | ||||||
| std::string markdown; | ||||||
|
|
@@ -633,6 +666,19 @@ ProcessLineResult NixRepl::processLine(std::string line) | |||||
| markdown += stripIndentation(doc->doc); | ||||||
|
|
||||||
| logger->cout(trim(renderMarkdownToTerminal(markdown))); | ||||||
| } else if (fallbackPos) { | ||||||
| std::stringstream ss; | ||||||
| ss << "Attribute `" << fallbackName << "`\n\n"; | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
etc.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've tried this, but unfortunately it doesn't work, as these go into the markdown renderer, instead of the terminal. We could switch that to go direct to the terminal, but then we should do the same for the primops, to get a consistent look. Could be done in a follow-up and I've left a reverted commit to cherry pick. |
||||||
| ss << " … defined at " << state->positions[fallbackPos] << "\n\n"; | ||||||
| if (fallbackDoc) { | ||||||
| ss << fallbackDoc.getInnerText(state->positions); | ||||||
| } else { | ||||||
| ss << "No documentation found.\n\n"; | ||||||
| } | ||||||
|
|
||||||
| auto markdown = ss.str(); | ||||||
| logger->cout(trim(renderMarkdownToTerminal(markdown))); | ||||||
|
|
||||||
| } else | ||||||
| throw Error("value does not have documentation"); | ||||||
| } | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -559,6 +559,54 @@ std::optional<EvalState::Doc> EvalState::getDoc(Value & v) | |||||
| .doc = doc, | ||||||
| }; | ||||||
| } | ||||||
| if (v.isLambda()) { | ||||||
| auto exprLambda = v.payload.lambda.fun; | ||||||
|
|
||||||
| std::stringstream s(std::ios_base::out); | ||||||
| std::string name; | ||||||
| auto pos = positions[exprLambda->getPos()]; | ||||||
| std::string docStr; | ||||||
|
|
||||||
| if (exprLambda->name) { | ||||||
| name = symbols[exprLambda->name]; | ||||||
| } | ||||||
|
|
||||||
| if (exprLambda->docComment) { | ||||||
| docStr = exprLambda->docComment.getInnerText(positions); | ||||||
| } | ||||||
|
|
||||||
| if (name.empty()) { | ||||||
| s << "Function "; | ||||||
| } | ||||||
| else { | ||||||
| s << "Function `" << name << "`"; | ||||||
| if (pos) | ||||||
| s << "\\\n … " ; | ||||||
| else | ||||||
| s << "\\\n"; | ||||||
| } | ||||||
| if (pos) { | ||||||
| s << "defined at " << pos; | ||||||
| } | ||||||
| if (!docStr.empty()) { | ||||||
| s << "\n\n"; | ||||||
| } | ||||||
|
|
||||||
| s << docStr; | ||||||
|
|
||||||
| s << '\0'; // for making a c string below | ||||||
| std::string ss = s.str(); | ||||||
|
|
||||||
| return Doc { | ||||||
| .pos = pos, | ||||||
| .name = name, | ||||||
| .arity = 0, // FIXME: figure out how deep by syntax only? It's not semantically useful though... | ||||||
| .args = {}, | ||||||
| .doc = | ||||||
| // FIXME: this leaks; make the field std::string? | ||||||
| strdup(ss.data()), | ||||||
| }; | ||||||
| } | ||||||
| return {}; | ||||||
| } | ||||||
|
|
||||||
|
|
@@ -1367,6 +1415,22 @@ void ExprSelect::eval(EvalState & state, Env & env, Value & v) | |||||
| v = *vAttrs; | ||||||
| } | ||||||
|
|
||||||
| Symbol ExprSelect::evalExceptFinalSelect(EvalState & state, Env & env, Value & attrs) | ||||||
| { | ||||||
| Value vTmp; | ||||||
| Symbol name = getName(attrPath[attrPath.size() - 1], state, env); | ||||||
|
|
||||||
| if (attrPath.size() == 1) { | ||||||
| e->eval(state, env, vTmp); | ||||||
| } else { | ||||||
| ExprSelect init(*this); | ||||||
| init.attrPath.pop_back(); | ||||||
| init.eval(state, env, vTmp); | ||||||
| } | ||||||
| attrs = vTmp; | ||||||
| return name; | ||||||
| } | ||||||
|
|
||||||
|
|
||||||
| void ExprOpHasAttr::eval(EvalState & state, Env & env, Value & v) | ||||||
| { | ||||||
|
|
@@ -2828,13 +2892,37 @@ Expr * EvalState::parse( | |||||
| const SourcePath & basePath, | ||||||
| std::shared_ptr<StaticEnv> & staticEnv) | ||||||
| { | ||||||
| auto result = parseExprFromBuf(text, length, origin, basePath, symbols, settings, positions, rootFS, exprSymbols); | ||||||
| DocCommentMap tmpDocComments; // Only used when not origin is not a SourcePath | ||||||
| DocCommentMap *docComments = &tmpDocComments; | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
|
||||||
| if (auto sourcePath = std::get_if<SourcePath>(&origin)) { | ||||||
| auto [it, _] = positionToDocComment.try_emplace(*sourcePath); | ||||||
| docComments = &it->second; | ||||||
| } | ||||||
|
|
||||||
| auto result = parseExprFromBuf(text, length, origin, basePath, symbols, settings, positions, *docComments, rootFS, exprSymbols); | ||||||
|
|
||||||
| result->bindVars(*this, staticEnv); | ||||||
|
|
||||||
| return result; | ||||||
| } | ||||||
|
|
||||||
| DocComment EvalState::getDocCommentForPos(PosIdx pos) | ||||||
| { | ||||||
| auto pos2 = positions[pos]; | ||||||
| auto path = pos2.getSourcePath(); | ||||||
| if (!path) | ||||||
| return {}; | ||||||
|
|
||||||
| auto table = positionToDocComment.find(*path); | ||||||
| if (table == positionToDocComment.end()) | ||||||
| return {}; | ||||||
|
|
||||||
| auto it = table->second.find(pos); | ||||||
| if (it == table->second.end()) | ||||||
| return {}; | ||||||
| return it->second; | ||||||
| } | ||||||
|
|
||||||
| std::string ExternalValueBase::coerceToString(EvalState & state, const PosIdx & pos, NixStringContext & context, bool copyMore, bool copyToStore) const | ||||||
| { | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -130,6 +130,8 @@ struct Constant | |||||
| typedef std::map<std::string, Value *> ValMap; | ||||||
| #endif | ||||||
|
|
||||||
| typedef std::map<PosIdx, DocComment> DocCommentMap; | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
(unless this needs to be sorted) |
||||||
|
|
||||||
| struct Env | ||||||
| { | ||||||
| Env * up; | ||||||
|
|
@@ -329,6 +331,12 @@ private: | |||||
| #endif | ||||||
| FileEvalCache fileEvalCache; | ||||||
|
|
||||||
| /** | ||||||
| * Associate source positions of certain AST nodes with their preceding doc comment, if they have one. | ||||||
| * Grouped by file. | ||||||
| */ | ||||||
| std::map<SourcePath, DocCommentMap> positionToDocComment; | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would you like to inspect memory impact of this?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is hard to assess until doc comments are used more widely in real code.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
(See #11092.) |
||||||
|
|
||||||
| LookupPath lookupPath; | ||||||
|
|
||||||
| std::map<std::string, std::optional<std::string>> lookupPathResolved; | ||||||
|
|
@@ -771,6 +779,8 @@ public: | |||||
| std::string_view pathArg, | ||||||
| PosIdx pos); | ||||||
|
|
||||||
| DocComment getDocCommentForPos(PosIdx pos); | ||||||
|
|
||||||
| private: | ||||||
|
|
||||||
| /** | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| #include "lexer-tab.hh" | ||
| #include "lexer-helpers.hh" | ||
| #include "parser-tab.hh" | ||
|
|
||
| void nix::lexer::internal::initLoc(YYLTYPE * loc) | ||
| { | ||
| loc->beginOffset = loc->endOffset = 0; | ||
| } | ||
|
|
||
| void nix::lexer::internal::adjustLoc(yyscan_t yyscanner, YYLTYPE * loc, const char * s, size_t len) | ||
| { | ||
| loc->stash(); | ||
|
|
||
| LexerState & lexerState = *yyget_extra(yyscanner); | ||
|
|
||
| if (lexerState.docCommentDistance == 1) { | ||
| // Preceding token was a doc comment. | ||
| ParserLocation doc; | ||
| doc.beginOffset = lexerState.lastDocCommentLoc.beginOffset; | ||
| ParserLocation docEnd; | ||
| docEnd.beginOffset = lexerState.lastDocCommentLoc.endOffset; | ||
| DocComment docComment{lexerState.at(doc), lexerState.at(docEnd)}; | ||
| PosIdx locPos = lexerState.at(*loc); | ||
| lexerState.positionToDocComment.emplace(locPos, docComment); | ||
| } | ||
| lexerState.docCommentDistance++; | ||
|
|
||
| loc->beginOffset = loc->endOffset; | ||
| loc->endOffset += len; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| #pragma once | ||
|
|
||
| namespace nix::lexer::internal { | ||
|
|
||
| void initLoc(YYLTYPE * loc); | ||
|
|
||
| void adjustLoc(yyscan_t yyscanner, YYLTYPE * loc, const char * s, size_t len); | ||
|
|
||
| } // namespace nix::lexer |
Uh oh!
There was an error while loading. Please reload this page.