Skip to content

Systematize builtins.fetchTree docs#9732

Merged
Ericson2314 merged 2 commits intomasterfrom
systematize-fetchTree-docs
Nov 17, 2025
Merged

Systematize builtins.fetchTree docs#9732
Ericson2314 merged 2 commits intomasterfrom
systematize-fetchTree-docs

Conversation

@Ericson2314
Copy link
Member

@Ericson2314 Ericson2314 commented Jan 9, 2024

Motivation

Render the docs nicely.

Context

I would like to use a markdown AST for this, but to avoid new deps (lowdown's AST doesn't suffice) I am just doing crude string manipulations for now.

Priorities and Process

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

@Ericson2314 Ericson2314 requested a review from roberth January 9, 2024 22:58
@github-actions github-actions bot added the fetching Networking with the outside (non-Nix) world, input locking label Jan 9, 2024
@roberth
Copy link
Member

roberth commented Jan 10, 2024

This seems more productive than what I've been up to with a self-documenting codec abstraction, which may be too ambitious for something in C++.

@Ericson2314 Ericson2314 force-pushed the systematize-fetchTree-docs branch from 7c5e178 to 23476d8 Compare January 12, 2024 23:58
@github-actions github-actions bot added the new-cli Relating to the "nix" command label Jan 12, 2024
@Ericson2314 Ericson2314 force-pushed the systematize-fetchTree-docs branch 4 times, most recently from cc8c940 to a330689 Compare January 17, 2024 00:37
@Ericson2314 Ericson2314 marked this pull request as ready for review January 17, 2024 00:48
@Ericson2314 Ericson2314 requested a review from edolstra as a code owner January 17, 2024 00:48
@Ericson2314
Copy link
Member Author

OK @roberth @fricklerhandwerk @infinisil the infra here is now set up! Not sure whether we want to

  • Just merge it
  • Hide not-yet-documented fields
  • Go through and document everything

@Ericson2314 Ericson2314 changed the title WIP systematize builtins.fetchTree docs Systematize builtins.fetchTree docs Jan 17, 2024
@Ericson2314 Ericson2314 force-pushed the systematize-fetchTree-docs branch 2 times, most recently from 80aae23 to d4e9043 Compare January 17, 2024 18:15
@Ericson2314 Ericson2314 changed the base branch from master to backport-9461-to-2.19-maintenance January 17, 2024 21:51
@Ericson2314 Ericson2314 changed the base branch from backport-9461-to-2.19-maintenance to master January 17, 2024 21:51
@fricklerhandwerk
Copy link
Contributor

fricklerhandwerk commented Jan 18, 2024

I vote to merge as is, and agree it's a useful step forward.

But I sigh loudly as well because it's adding notable tech debt. Now we have 5 (!) mechanisms for producing documentation:

  1. plain markdown via mdBook
  2. globals.hh, the Config object, and --dump-cli
  3. pre-processing in Makefiles
  4. processing in Nix language
  5. this

Especially the additional document composition code path makes it substantially more messy.

The ideal end state for me would be documentation living exclusively in C++ headers (with plain markdown files for optional free-form blurbs) and processing structured output – templating and such – happening exclusively in the Nix language, decoupled from building the all of Nix. Currently it's hacks on top of hacks and it takes forever to compile.

@Ericson2314
Copy link
Member Author

Ericson2314 commented Jan 18, 2024

I agree there are too many. I would like to replace the stuff we do with the interpreter inside Nix with this method, because then it will work with the store-only Nix.

@fricklerhandwerk
Copy link
Contributor

replace the stuff we do with the interpreter inside Nix with this method

That would make it a lot harder to contribute to for most people, and I don't see why docs can't be built by a full Nix as a separate step, but let's discuss it somewhere else.

@thufschmitt
Copy link
Member

thufschmitt commented Jan 19, 2024

Discussed during the maintainers meeting today. Deferred for the time being

Details
  • Adds an extra dep (cmark)
    • Having another markdown renderer in the closure is meh
    • @Ericson2314: Required because manipulating the lowdown AST doesn't work
    • @thufschmitt Embedding the Markdown directly probably works and is 10% of the work, although it's a bit less nice in theory
      • We're already doing it in a bunch of places
  • @Ericson2314: Would be happy to switch to cmark everywhere, but doesn't support outputing to the terminal

Decision: Defer until commonmark/cmark#362 (comment) gets an answer. The the plan is to

  • If the answer is positive, @Ericson2314 will implement the cmark terminal renderer
  • Otherwise just concatenate markdown snippets

@tribals
Copy link

tribals commented Jan 20, 2024

Our use case includes having a self-contained nix executable directly output some pretty-pretty documentation on the terminal without relying on other programs.

Personally I want to state that such style of documentation for CLI tools is worst-case - it is neither man nor help, you're forcing user to pipe output to pager and leaving out man convetions (no one interested in following them because "help files" is written in Markdown, and typical markdown user is rarely even avare of existence of manual pages, and their conventions).

Not to mention that you're forcing user to execute your command in order to get help (rather than "viewing static files" with man).

(From here: commonmark/cmark#362).

@Ericson2314 Ericson2314 force-pushed the systematize-fetchTree-docs branch from 22112c1 to f66a761 Compare October 11, 2024 16:09
@Ericson2314 Ericson2314 force-pushed the systematize-fetchTree-docs branch 2 times, most recently from c374264 to 35f7e40 Compare November 11, 2025 15:18
@github-project-automation github-project-automation bot moved this to Triage in Nix team Nov 11, 2025
@Ericson2314 Ericson2314 requested a review from xokdvium November 11, 2025 15:19
@Ericson2314 Ericson2314 force-pushed the systematize-fetchTree-docs branch from 35f7e40 to d3a64f4 Compare November 11, 2025 15:59
Ericson2314 added a commit that referenced this pull request Nov 11, 2025
This makes the output easier to compare with the new machine-generated
lists in #9732.

The hand-curated order did have the advantage of putting more important
attributes at the top, but I don't think it is worth preserving that
when `std::map` is so much easier to work with. The right solution to
leading the reader to the more important attributes is to call them out
in the intro texts.
@Ericson2314 Ericson2314 force-pushed the systematize-fetchTree-docs branch from d3a64f4 to 8fb8329 Compare November 11, 2025 17:03
@Ericson2314
Copy link
Member Author

OK. With the aid of #14545 (which we should land first), I hand-diffed the :doc builtins.fetchTree output to make sure that no doc changes in the past 2 years were lost in the rebase. This should be good to go!

@dpulls
Copy link

dpulls bot commented Nov 12, 2025

🎉 All dependencies have been resolved !

@Ericson2314 Ericson2314 force-pushed the systematize-fetchTree-docs branch 2 times, most recently from 541b0d6 to 73f9e39 Compare November 12, 2025 23:32
@Ericson2314
Copy link
Member Author

OK @edolstra and @xokdvium it just concats strings now. I will do the rest separately as requested.

@Ericson2314 Ericson2314 force-pushed the systematize-fetchTree-docs branch from 73f9e39 to 594acd7 Compare November 14, 2025 05:50

std::string schemeDescription() const override
{
return stripIndentation(R"(
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can drop stripIndentation() here and have the caller do that?

Copy link
Member Author

Choose a reason for hiding this comment

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

I would rather not do that because we have both the doc builder caller, and the main.cc caller that dumps the machine-readable JSON info.

And also render the docs nicely.

I would like to use a markdown AST for this, but to avoid new deps
(lowdown's AST doesn't suffice) I am just doing crude string
manipulations for now.
@Ericson2314 Ericson2314 force-pushed the systematize-fetchTree-docs branch from 594acd7 to bae1ca2 Compare November 17, 2025 18:10
@Ericson2314 Ericson2314 added this pull request to the merge queue Nov 17, 2025
Merged via the queue into master with commit 958866b Nov 17, 2025
20 checks passed
@Ericson2314 Ericson2314 deleted the systematize-fetchTree-docs branch November 17, 2025 19:46
@github-project-automation github-project-automation bot moved this from Triage to Done in Nix team Nov 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fetching Networking with the outside (non-Nix) world, input locking new-cli Relating to the "nix" command

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

8 participants