Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

treewide: Fix AST of Nix in all markdown files #299554

Merged

Conversation

dasJ
Copy link
Member

@dasJ dasJ commented Mar 27, 2024

Description of changes

This PR does 2 things:

  • It adds nix-fencing to nix code that had no syntax specifier
  • It fixes all Nix ASTs to be parsable at least by nixfmt

The end-goal of this PR is to make the ASTs parsable by the formatter to get RFC-166 into the manual. This is important since the manual is likely to be one of the first contact points of new users and also a place where code is likely to be copied from. This might also make the code compliant with other parsers as well, allowing for future functionality like checking for renamed options.

As outlined above, this does not reformat the code with nixfmt (yet). This is the preparation to do such things in the future. There are however a lot of manual reformats to make the code look "normal" (like when adding {} around something that has to be indented now).

I am also aware that there are other places where documentation can be found, which are not in .md files. There's toPretty and the examples from the manual but the PR is probably big enough as-is.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 馃憤 reaction to pull requests you find important.

This should help us with highlighting and future formatting.
This allows for correct highlighting and maybe future automatic
formatting. The AST was verified to work with nixfmt only.
@dasJ dasJ changed the title treewide: Fix AST of all markdown files treewide: Fix AST of Nix in all markdown files Mar 27, 2024
CONTRIBUTING.md Outdated Show resolved Hide resolved
@dasJ
Copy link
Member Author

dasJ commented Mar 27, 2024

Since I mentioned this multiple times in the description: This is what the manual would look like if we nixfmt'ed it: helsinki-systems@8c506ddd8436

Copy link
Contributor

@fricklerhandwerk fricklerhandwerk left a comment

Choose a reason for hiding this comment

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

Nice! You may want to note in the commit message why we're using angle brackets there.

Copy link
Member

@DanielSidhion DanielSidhion left a comment

Choose a reason for hiding this comment

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

Neat! Thank you for doing this. Do you have anything documented on how you did this? I'm assuming you built a tool to do this work. Is it available anywhere? I'd like to add this as part of the documentation team's toolset.

We use angle brackets since they look a lot like a placeholder while
also being valid nix code, as suggested by roberth here: NixOS#299554 (comment)
@dasJ dasJ force-pushed the fix/markdownnix-formatting branch from 7b6a05e to 96075d9 Compare March 28, 2024 08:12
@dasJ
Copy link
Member Author

dasJ commented Mar 28, 2024

Changed the commit message that @fricklerhandwerk suggested and also linked Robert's comment there.

@DanielSidhion partly. I wrote a script that finds the issues, the fixing of the 600+ issues was a manual process. The script does not exist anymore because it has since grown into a full utility to analyze the nix formatting in markdown files and to automatically fix it if needed. I plan to upstream this along a full reformat (see #299554 (comment)) and a GitHub workflow that ensures that people don't accidentially break it again :)

@fricklerhandwerk fricklerhandwerk merged commit bc5ee2b into NixOS:master Mar 28, 2024
7 of 8 checks passed
@dasJ dasJ deleted the fix/markdownnix-formatting branch March 28, 2024 08:28
@drupol
Copy link
Contributor

drupol commented Mar 28, 2024

impressive work! Cool !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants