DM-55634: Fix the Edit on GitHub link's document path - #352
Merged
Conversation
Computes a path's POSIX-style location within the Git working tree, normalizing the two cases that matter for building a URL from the result: the repository root yields an empty string rather than ".", and both sides are resolved so a symlinked path still compares as inside the tree.
pydata-sphinx-theme joins doc_path with a source-directory-relative file name, so doc_path must be the srcdir's path in the working tree. It was derived instead from the current working directory at conf.py exec time, which is the *config* directory. Those differ under 'sphinx-build -c . docs ...', where the resulting URLs 404, and a srcdir at the repository root produced a stray './' in the URL. The srcdir isn't knowable in conf.py, so the computation moves to a new config-inited handler. That event is both the earliest point where the srcdir is known and the last at which the button can still be disabled, since builder.init() resolves the theme options before builder-inited. Dropping the conf-time Git access also means a build outside a Git checkout no longer raises InvalidGitRepositoryError; the button is omitted instead, and only at info level so that '-W' builds still pass. The test roots no longer need show_github_edit_link = false to avoid that hard failure, so they now cover the degradation path.
Adds a sphinx-extensions page covering the URL template, why doc_path must be srcdir-relative, and the non-git degradation, plus a note in the documenteer.toml reference for show_github_edit_link.
The config-inited handler runs after conf.py, so unconditionally setting html_context['doc_path'] made the value impossible to override -- a regression against the previous conf-time behavior for anyone who had already worked around the wrong-path bug by hand. A configured path now short-circuits auto-detection entirely, so it also keeps the button working outside a Git checkout: supplying the path answers the question the working tree would have been consulted for. This is the escape hatch for layouts a working-tree path can't describe, such as a generated source directory.
The existing cases drive a hand-written conf.py, so removing the extension from the guide preset's extensions list, or breaking set_edit_on_github's enable path, passed the whole suite. This case builds documenteer.conf.guide itself in the dp2 layout; verified that it fails when the extension is unregistered. The sources are committed to Git because the preset auto-loads sphinx-last-updated-by-git, which strips 'sourcename' for untracked files -- and pydata's edit-this-page component is gated on it, so an uncommitted page renders no button regardless of doc_path.
Relaxing the five test roots removed the only execution of the show_github_edit_link = false early return, and the missing-github_url ConfigError had no coverage either. Both guards survived the move to the extension, so pin them directly, along with the fact that this method no longer sets doc_path.
The doc_path-is-None branch warns rather than logging at info, on the grounds that a repository whose working tree excludes the srcdir is a real misconfiguration. It had no coverage, so pin it with a mock. The warning-stream filter matched any 'is already registered' line. Anchor it on the re-setup of a built-in sphinx.* extension instead, which is the actual cross-application artifact -- note that this noise covers re-registered directives and roles too, not just node classes, so a narrower match on the node message alone does not hold.
The claim that config-inited is the last event at which the button can be disabled is too strong: on Sphinx 8.2 and 9.1 the HTML builder's get_theme_config returns html_theme_options itself, so builder-inited would still work there. The real reason is that a theme *may* copy the options when the builder initializes -- pydata's own get_theme_options_dict documents that ambiguity -- so settling it at config-inited avoids depending on the version's behavior. Also corrects 'silently omitted' in the TOML reference, which described the pre-review behavior; the omission is logged at info level.
jonathansick
force-pushed
the
tickets/DM-55634
branch
from
July 27, 2026 17:58
41e4721 to
c8fddea
Compare
Verified empirically: outside a Git checkout the guide build now exits 0 and omits the button, but sphinx-last-updated-by-git -- auto-loaded via sphinx-sitemap -- still warns 'Error getting data from Git [git.subprocess_error]', so a -W build still fails. Rubin projects build with -n -W, so the unqualified 'the build proceeds' was misleading exactly where it mattered. None of that extension's own config values gate the warning; the fix available to a project is suppress_warnings, which is now documented rather than claimed away.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
conf.py. pydata-sphinx-theme joinsdoc_pathwith a source-relative file name, butdoc_pathwas derived from the current working directory atconf.pyexec time — the config directory. Projects that build withsphinx-build -c . docs _build/htmltherefore emitted URLs that 404: https://dp2.lsst.io/ links to.../edit/main/./index.rstinstead of.../edit/main/docs/index.rst. A source directory at the repository root also produced a stray./in the URL.conf.py, so the computation moves into a newdocumenteer.ext.githubeditlinkextension that runs atconfig-inited— the earliest event wheresrcdiris known, and the safest one for disabling the button (a theme may copyhtml_theme_optionswhen the builder initializes, so a later change may not reach the template).doc_pathset explicitly inhtml_contextis honored and short-circuits auto-detection, so the button also keeps working outside a Git checkout when the path is supplied by hand..git) raisedInvalidGitRepositoryError. The button is now omitted instead, logged at info rather than warning level so that-Wbuilds still pass. Five test roots no longer needshow_github_edit_link = falseto work around that, so they now cover the degradation path.Validation steps
Built the real failing project against this branch and confirmed both the URL and that
-Wstill passes:A/B against released 2.4.0 on the same checkout: all 100 edit URLs change from
/edit/main/./Xto/edit/main/docs/X. Resolving each against the working tree, 2.4.0 gets 0/100 real files and this branch 100/100; GitHub returns 404 for the old form and 200 for the new. Button coverage (100 of 103 pages) and the warning-free-n -Wbuild are unchanged.Confirmed Documenteer's own docs (where confdir == srcdir) are unchanged —
nox -s docs, then checkdocs/_build/html/guides/toml-reference.htmlstill links to.../edit/main/docs/guides/toml-reference.rst.Review the new
docs/sphinx-extensions/github-edit-link.rstpage and theshow_github_edit_linknote in thedocumenteer.tomlreference.References