-
-
Notifications
You must be signed in to change notification settings - Fork 17.5k
nixpkgs-manual: enable Markdown formatting with Prettier #400369
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
Conversation
0564f98 to
324bc43
Compare
This comment was marked as resolved.
This comment was marked as resolved.
ba91f9d to
f944481
Compare
doc/README.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is wrong. It's supposed to be a term-definition in the paragraph that is part of the outer definition.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think nested term-definition are officially supported in markdown. #investigating
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a - (list item), would it be acceptable for you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess I can mark this as resolved as I updated the current documentation before doing the formatting so that this issue won't happen again. Hope it's OK now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm that seems to be a deficiency of any given markdown implementation (there's no such thing as "official" markdown). The semantics is valid DOM, so in a sense "not our problem". Making it a list item defies the original purpose.
It also seems inappropriate to iron over the work of @DanielSidhion and others who have put effort into adding more structure, just because some tool happens not to know that structure. This may appear like a minor thing, but I think it's sort of at the heart of what we're doing with Nixpkgs: we're building a knowledge base, and that's a big data structure. Removing information because some tools can't handle it seems wrong.
Is it possible to disable that particular behavior? Since the other changes look alright (haven't checked everything).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right I hear you and fully agree. I'll write a post describing precisely what has been done so far to fix this issue at the end of the day, with screenshots and detailed explanation so we can decide what to do about this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot @drupol!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated the OP with full details, explanation and screenshots. Let me know what you think.
12ae0c0 to
3ff1b23
Compare
019dba9 to
029f3a4
Compare
commands: ``` redirects rename-identifier module-system-lib-evalModules-return-value-_configurationClass module-system-lib-evalModules-return-value--configurationClass redirects rename-identifier module-system-lib-evalModules-return-value-_module module-system-lib-evalModules-return-value--module redirects rename-identifier module-system-lib-evalModules-return-value-_type module-system-lib-evalModules-return-value--type ```
command: `cd doc; prettier --write .`
This PR:
checkworkflow on documentation #396914nixpkgs-manual.tests.check-formatting(541861cefb2be3dbdc5702f57fdc470f0751bd16) usingprettiermarkdown-code-runnerdoc/directory withprettier(822d5fc7bc48ce31005c50c081379bc425d78687)The most controversial point is the handling of definition lists, commonly written as:
This format is part of what is referred to as “Markdown Extended” 1, but it is not part of the original or CommonMark specification. As a result, tools like
prettier, which are widely adopted for Markdown formatting, do support basic definition lists, but they fail to preserve structure when those lists are nested.Example: Nested definition list before and after Prettier formatting
Before:
After:
Try it in the Prettier playground
I agree that these nested definitions add meaningful structure. Unfortunately, there is currently no way to configure
prettierto preserve this structure or disable this specific behavior.The compromise solution
Rather than remove these lists or leave them unformatted (which would fail the automated check), I chose to preserve the meaning and pass
prettierformatting by wrapping nested definitions in bullet points.Rewritten as list items: preserves meaning, passes Prettier
Before:
After (identical output):
Try it in the Prettier playground
This way:
Commit 3ae1d6b28ab0833b17f1b06c4ed499a0e86da72a applies the above compromise and updates the documentation accordingly. As seen in the screenshots hereunder, the generated HTML is slightly different, I would argue that the readability and clarity have improved.
Simple example (before, after)
Example with nested definition list (before, after)
Let me know what you think! And if you disagree, perhaps it's worth considering a compromise so that we can benefit from an automated documentation checking system. The trade-off may well be worth it: would you rather rely on nested definition lists that aren't really standard and lose automation and checks, or accept a small compromise and gain all of that?
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.