docs: experimental re-write #2884
Conversation
|
I completly agree that there is too much nesting! |
MattSturgeon
force-pushed
the
docs/modular
branch
9 times, most recently
from
7d976d6 to
d3f4c0a
Compare
|
I think this is ready for review. Not finished necessarily, but I don't think that's important as this is intended to be marked "beta" or "experimental" for now. |
|
I'd suggest moving the |
|
CI failure is due to the current docs impl not supporting This is a consequence of us hand-rolling evaluating the option tree, instead of leveraging the nixpkgs tooling (i.e. Assuming the new docs & old docs will live alongside each other for a while, I'll fix the old impl in another PR before merging this one. |
MattSturgeon
force-pushed
the
docs/modular
branch
6 times, most recently
from
003271d to
964d98a
Compare
|
I've addressed the feedback so far, and done some additional cleanup. |
There was a problem hiding this comment.
I'm a bit confused why we have:
- The docs.{files,platform,options} options
- Alongside the docs.*.menu.section
I'd assume for example that docs.platforms.menu.section must always be "platforms" or that docs.options.menu.section should be "options"
Also I would find it better if the "docs" documentation was in "Contributors" or "Developpement" section instead of the main "Options" section, as it would not be very relevant to most users
If you could provide a high level overview of the docs module architecture somewhere it could be quite helpful for the review, I still have not built the complete picture in my head (due to going at it only bit by bit)
| docs.options.${docsfile} = { | ||
| title = lib.last loc; | ||
| description = lib.concatLines ( | ||
| [ | ||
| "**URL:** [${url}](${url})" | ||
| "" | ||
| ] | ||
| ++ lib.optionals (maintainers != [ ]) [ | ||
| "**Maintainers:** ${maintainersString}" | ||
| "" | ||
| ] | ||
| ++ lib.optionals (description != null && description != "") [ | ||
| "---" | ||
| "" | ||
| description | ||
| ] | ||
| ); | ||
| }; |
There was a problem hiding this comment.
As a gut reaction it feels a bit strange to have a function called mkMetaModule that sets other things than meta arguments.
It feels a "duplicate" of the metadata in another form, can't we extract it with a well placed library function? (Not entirely sure, as the metadata is a bit of a complicated beast --')
There was a problem hiding this comment.
There is a lot of deliberately duplicated stuff, with the idea being that the "old" way will eventually be removed.
| options.docs._utils = lib.mkOption { | ||
| type = with lib.types; lazyAttrsOf raw; | ||
| description = "internal utils, modules, functions, etc"; | ||
| default = { }; | ||
| internal = true; | ||
| visible = false; | ||
| }; | ||
|
|
There was a problem hiding this comment.
Why do we go with a "utils" module instead of additions to the nixvim lib or to a separate "docs" library? (I'm not suggesting it's better, I genuinely don't know why we should choose one or the other)
There was a problem hiding this comment.
My thought process was that I wanted to avoid bloating our lib with docs-specific things.
I also wanted things in this util to be able to depend on options and config if needed.
Not saying this is the best approach, though.
| docsPageLiberalType = lib.types.submodule [ | ||
| { _module.check = false; } | ||
| docsPageModule | ||
| ]; |
There was a problem hiding this comment.
What is the difference between this and a freeform option?
There was a problem hiding this comment.
Ah this is an "extendable" submodule in some sense?
There was a problem hiding this comment.
A freeform submodule allows config attrs to be defined in a freeform way.
On the other hand, this "liberal" submodule just discards any definitions that don't correspond to a declared option.
I.e. option existence checks are disabled.
| _allInputs = lib.mkOption { | ||
| type = with lib.types; listOf str; | ||
| description = "`docs.*` option names that should be included in `docs.all`."; | ||
| defaultText = config.docs._allInputs; | ||
| default = [ ]; | ||
| internal = true; | ||
| visible = false; | ||
| }; | ||
| all = lib.mkOption { | ||
| type = with lib.types; listOf docsPageLiberalType; | ||
| description = '' | ||
| All enabled doc pages defined in: | ||
| ${lib.concatMapStringsSep "\n" (name: "- `docs.${name}`") config.docs._allInputs}. | ||
| ''; | ||
| visible = "shallow"; | ||
| readOnly = true; | ||
| }; |
There was a problem hiding this comment.
This dance is to have a readonly option whose definitions are split around the nixvim source right? Is the readonly really important? If it were absent this could be simplified a bit with an apply right?
There was a problem hiding this comment.
Kinda.
I marked it as read-only because it seemed unlikely that someone should define a page directly here, without using one of the more specific implementations.
The other options listed in _allInputs all provide some additional functionality, built on top of the base docsPageModule.
Also, unlike all, they don't set _module.check = false, so their config definitions must strictly match their respective option declarations.
The various While we could potentially have a single submodule with a bunch of specialised nullable options, it felt cleaner to keep it separate. I could be persuaded either way, though.
Some page options set a sane default for
This can be done by setting
Sorry, I'll try and get to that. I know I've been a bit lazy documenting and explaining my design & implementation here! |
Don't be sorry, I always find your PR messages very useful & much better than the ones I write |
Introduce various `docs.*` options where doc pages, menu entries, etc can be defined. The options themselves are responsible for rendering this to markdown and HTML.
MattSturgeon
force-pushed
the
docs/modular
branch
from
1e11f8f to
fc972cb
Compare
I've spent a couple hours putting together a proof-of-concept for a simpler, more modular & cachable, implementation of the docs.
An attempt is made to lean more heavily on upstream tooling (nixos-render-docs), and to minimise the amount of custom option-traversial.
Instead, the idea is that nixvim's modules are evaluated once, and the resulting
optionsset is partitioned up into smaller sub-sets that will each be rendered individually.This allows us to retain a multi-page architecture while still using the upstream options rendering.
However, it is my opinion that the current docs have too much nesting, so in this new design we have only one level of nesting per options sub-set.
Menu
colorschemes
options
plugins
This could be improved on by having a few exceptions to the rule, e.g. lsp servers. Perhaps a
metaoption could be introduced to indicate whether an option-set should be nested further?This is an incomplete proof-of-concept. Some things are missing, such as the per-platform options docs.
Once the basics are in place, I'd like to have this merged so that users and contributors can experiment with using the beta-docs alongside the "stable" docs.
I don't think all the rough edges and design decisions need to be resolved before merging the initial beta. But feedback is of course encouraged!
Testing
You can test by building
beta-docsordocs.passthru.beta-docs.Or by building
docsas normal and navigating to/beta/index.html.Performance
The eval/build is still quite expensive, but it feels faster than the current docs?