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

doc: fix code to generate links for headings in the manual #297687

Merged
merged 1 commit into from
Mar 21, 2024

Conversation

DanielSidhion
Copy link
Member

Description of changes

Due to unfortunate testing of #295847, we didn't realise that AnchorJS was generating the wrong links for almost all of the manual. I personally only checked the generated links with the first couple top-level headings (which have the same id as the name of the headings), and checked that the link button showed up on all some lower-level headings as well.

As a result of that, most links in the manual are wrong. For example, if you go to this section and click the anchor link, you'll see that the url changes from #chap-platform-support to #platform-support. The correct link is #chap-platform-support.

The fix comes in two parts:

  1. A fix on nixos-render-docs to generate headings with id attributes. Previously, it generated headings without id attributes even if there was an id specified. The id ended up inside a <span> element inside the heading.
  • This is needed for AnchorJS to create links pointing to the proper id. If the heading element doesn't have an id, AnchorJS generates one, which is the main reason why the current links in the manual are mostly wrong.
  1. The code that calls AnchorJS forces it to only run on headings that have an id attribute set. This prevents AnchorJS from generating ids automatically, which means that the links it generates can easily break as the content of the manual changes.

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.

@DanielSidhion
Copy link
Member Author

@pennae requested your review because this touches nrd more than other things. I kept the PR title with doc: since this also touches another bit that is not nrd-specific, but am ok changing the title to be nrd-specific as well. I have no opinion on this, so leaving it up to you if you prefer changing the title

Copy link
Contributor

@pennae pennae left a comment

Choose a reason for hiding this comment

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

assuming this works now (haven't tested the rendered manual) it's fine.

Copy link
Contributor

Successfully created backport PR for release-23.11:

@DanielSidhion DanielSidhion deleted the fix-manual-headings branch March 21, 2024 15:18
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2024-03-21-documentation-team-meeting-notes-114/41957/1

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

Successfully merging this pull request may close these issues.

None yet

4 participants