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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrite GitHub anchor links to user-content- #372

Merged
merged 2 commits into from Nov 3, 2022

Conversation

yukiisbored
Copy link
Member

@yukiisbored yukiisbored commented Oct 22, 2022

Due to an Sphinx issue, linkcheck fails on GitHub anchors for
GitHub rendered documents.

This change rewrites the anchors to the ones that can be verified by
linkcheck.

@zupo
Copy link
Contributor

zupo commented Oct 23, 2022

I don't understand why we want this, can you explain a bit please?

@yukiisbored
Copy link
Member Author

I don't understand why we want this, can you explain a bit please?

For some reason, the linkcheck builder fails when it tries to check heading anchors on a GitHub page with Markdown

This PR simply removes the GitHub specific cases with a more generic "ignore all pages from GitHub".

For example, cross-compiling refers to https://github.com/NixOS/nix.dev/blob/master/source/tutorials/building-bootable-iso-image.md?plain=1#L4 which contains a link to https://github.com/nix-community/nixos-generators#cross-compiling

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/2022-10-28-documentation-team-meeting-13/22806/1

@yukiisbored
Copy link
Member Author

yukiisbored commented Oct 28, 2022

Found the upstream (Sphinx) issue on this: sphinx-doc/sphinx#9016
Following the conversation there's a workaround but it is disabled: https://github.com/sphinx-doc/sphinx/blob/master/sphinx/builders/linkcheck.py#L585-L587 (sphinx-doc/sphinx#9435)

It seems the regex option to capture github.com URLs won't work because linkcheck_anchors_ignore only captures the anchor part of the URL: https://github.com/sphinx-doc/sphinx/blob/master/sphinx/builders/linkcheck.py#L585-L587

@yukiisbored
Copy link
Member Author

yukiisbored commented Oct 28, 2022

There's a workaround where user-content-<heading> is used over <heading> (e.x https://github.com/cachix/install-nix-action#user-content-how-can-i-run-nixos-tests instead of https://github.com/cachix/install-nix-action#user-content-how-can-i-run-nixos-tests).

Though, the heading won't be visible as it's covered by some GitHub UI element.

@yukiisbored
Copy link
Member Author

There are two options as far as I see:

  1. Disable anchor link checks altogether.
  2. Use user-content- anchors. Hopefully until Sphinx fix the issue.

I'm gonna opt for the second option on this pull-request for now and add a note in CONTRIBUTING.md

@yukiisbored yukiisbored force-pushed the linkcheck-gh-fail branch 2 times, most recently from 3d232b7 to bed572b Compare October 28, 2022 17:31
@yukiisbored yukiisbored changed the title Disable linkcheck anchors for GitHub links Rewrite GitHub anchor links to user-content- Oct 28, 2022
@zupo
Copy link
Contributor

zupo commented Oct 28, 2022

I'd very much like to keep anchor links checks. It's really discouraging to newcomers when docs point to bad URLs.

There is a third way too: fix/patch/monkeypatch or even fork Sphinx so link checking works for us.

Due to an Sphinx issue[1], linkcheck fails on GitHub anchors for
GitHub rendered documents.

This change rewrites the anchors to the ones that can be verified by
linkcheck.

[1]: sphinx-doc/sphinx#9016
Copy link
Contributor

@zupo zupo left a comment

Choose a reason for hiding this comment

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

Awesome find 👍 👍

@fricklerhandwerk fricklerhandwerk merged commit 2d989f4 into NixOS:master Nov 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

4 participants