-
-
Notifications
You must be signed in to change notification settings - Fork 14
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
Fix/headings & check: nixdoc can always build nixpks manual #121
Conversation
Nixpkgs is a first class consumer. Which is guaranteed to be supported via this check
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/2024-04-25-documentation-team-meeting-notes-122/44080/1 |
@infinisil I know you where busy with some other work. I'd be delighted If you could find time to review this PR or tell me what to do with it. |
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.
Overall looking good! Just some minor points
``` nix | ||
```nix |
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 looks wrong
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 actually correct.
The expected output is that it should not change the code fences, input is this:
### h3-heading
```nix
# A comment should not be shifted
```
So the output is correct, comrak crate that i used there before insered whitespace which is also allowed but a little ugly.
This behavior is removed anyways and never made it to the nixpkgs manual. :)
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.
Ahh I see!
let mut curr_fence: Option<(usize, char)> = None; | ||
for raw_line in raw.split_inclusive('\n') { | ||
// Code blocks can only start with backticks or tildes | ||
let fence_line = &trim_leading_whitespace(raw_line, 3); |
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.
Where does the 3 come from? A comment would be good :)
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.
Added a comment here.
3 whitespaces are okay before code fences according to commonmark. Used this spec. to detect code fences.
Since for headings the same rule applies, we can also handle indented code blocks easily.
This PR fixes the problems that where introduced with using any commonmark parser to shift the headings.
Currently nixdoc cannot be used to build the nixpkgs manual. This PR fixes that and also ensures that it remains buildable.
Even though headings might be detected and can be operated on, it became nearly impossible to serialize the parsed tokens/ast of any parser (the rest of the document). Any parser must support ALL underlying extensions that we use in nixpkgs docs. OR alternatively any parser should preserve whitespace and formatting, but i couldnt find any parser amongst the rust ecosystem with such properties.
Solution
The solution now involves to manually track whether we are inside or outside of code-blocks and shift only headings that are not inside a code block and start right at the beginning of the line.
A rather simple state tracking should be a pretty solid solution for the problem.
Also added an additional check to the flake where we build the nixpkgs manual against the current state of nixdoc.
ping @infinisil
Limitations
https://spec.commonmark.org/0.28/#fenced-code-blocks
Commonmark also specifies how unclosed codeblocks behave. We don't support unclosed code blocks.