Strip wrapper directory from generated doc URLs#49
Open
danceratopz wants to merge 3 commits intoSamWilsn:masterfrom
Open
Strip wrapper directory from generated doc URLs#49danceratopz wants to merge 3 commits intoSamWilsn:masterfrom
danceratopz wants to merge 3 commits intoSamWilsn:masterfrom
Conversation
Auto-detect whether each discovery root is itself a Python package: if the root has no __init__.py it's treated as a wrapper (e.g. ``src/``) and dropped from output_path so generated URLs start at the top-level package, not the source-tree directory. Roots that *are* packages keep their name. The listing plugin's hierarchy lookups now key off output_path (via a small _hierarchy_path helper) so navigation matches the URL tree once the wrapper is stripped, and listing entries use a separate _display_path so a file-backed index like ``__init__.py`` still shows under its directory rather than the source-tree name.
An `__init__.py` source's hierarchy position is the directory it indexes, so `_hierarchy_path(...).parent` pointed one level too high and `siblings()` returned the parent listing instead of the package's own members. Treat an index source as a member of the directory it indexes when computing siblings, matching what regular files in the same package see.
Drop the per-discover strip cache: detect each root's wrapper status inline in `discover()` and pass `strip_root` directly to the source factory. Collapse `output_path`, `_hierarchy_path`, `_display_path`, and `Listing.siblings` to their conditional one-liners and reuse `descendants()` for index-source siblings. Pure simplification — no behavior change.
SamWilsn
reviewed
May 1, 2026
Owner
There was a problem hiding this comment.
Two alternate proposals we could consider:
- We use
os.path.commonpathon all of the paths fromtool.docc.plugins."docc.python.discover".paths, and just strip that from everything. For our case of justsrc, it works. If someone else were to havesrc1andsrc2, it wouldn't smush everything into one listing. - Instead of stripping prefixes, we do what GitHub does and shortcut any listing with only one child. 1 Advantage here is that this isn't python-specific, and would work for any listing. For us, this would mean [
./diffs/,./src/] would become [./diffs/,./src/ethereum/]. Even bigger advantage would be cleaning up the diffs listing.
Footnotes
Comment on lines
+215
to
+222
|
|
||
| An index source like ``__init__.py`` is treated as a member of | ||
| the directory it indexes, so its siblings are that directory's | ||
| entries rather than entries one level higher in the tree. | ||
| """ | ||
| source_path = source.relative_path or source.output_path | ||
| return self.sources[source_path.parent] | ||
| if Listable._index_dir(source) is not None: | ||
| return self.descendants(source) | ||
| return self.sources[_hierarchy_path(source).parent] |
Owner
There was a problem hiding this comment.
In add_source, if we're still adding the index page in two places, why do we need to change this?
Comment on lines
+424
to
+425
| display = _display_path(source) | ||
| path = display.name if node.leaf else str(display) |
Owner
There was a problem hiding this comment.
Suggested change
| display = _display_path(source) | |
| path = display.name if node.leaf else str(display) | |
| path = _display_path(source).name if node.leaf else str(display) |
Tiny micro optimization :P
Comment on lines
+95
to
+96
| *, | ||
| strip_root: bool = False, |
Owner
There was a problem hiding this comment.
Would you mind also bumping the major version in src/docc/__init__.py? I believe this is a breaking change.
Comment on lines
+197
to
+202
| base = ( | ||
| self.absolute_path.relative_to(self.root_path) | ||
| if self._strip_root | ||
| else self._relative_path | ||
| ) | ||
| return base.with_name("index") if self._is_init() else base |
Owner
There was a problem hiding this comment.
Suggested change
| base = ( | |
| self.absolute_path.relative_to(self.root_path) | |
| if self._strip_root | |
| else self._relative_path | |
| ) | |
| return base.with_name("index") if self._is_init() else base | |
| if self._strip_root: | |
| base = self.absolute_path.relative_to(self.root_path) | |
| else: | |
| base = self._relative_path | |
| return base.with_name("index") if self._is_init() else base |
Comment on lines
+26
to
+32
| return PluginSettings( | ||
| Settings( | ||
| root, | ||
| {"tool": {"docc": {"plugins": {"docc.python.discover": {}}}}}, | ||
| ), | ||
| {"paths": list(paths)}, | ||
| ) |
Owner
There was a problem hiding this comment.
A bit more repetitive I guess, but also more correct:
Suggested change
| return PluginSettings( | |
| Settings( | |
| root, | |
| {"tool": {"docc": {"plugins": {"docc.python.discover": {}}}}}, | |
| ), | |
| {"paths": list(paths)}, | |
| ) | |
| plugin = "docc.python.discover" | |
| return Settings( | |
| root, | |
| {"tool": {"docc": {"plugins": {plugin: {"paths": list(paths)}}}}} | |
| ).for_plugin(plugin) |
| ] | ||
|
|
||
|
|
||
| def test_package_root_is_kept(tmp_path: Path) -> None: |
Owner
There was a problem hiding this comment.
Why do we preserve the directory if it contains an __init__.py?
At first I was like, yeah, that makes sense. Now I'm kinda questioning it.
| ), | ||
| ) | ||
|
|
||
| discover = PythonDiscover(_settings(tmp_path, ("src", "lib"))) |
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
2 participants
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.
What
When discovery is configured with a wrapper directory like
paths = ["src"], drop that prefix from every generated URL. URLs becomedocc/build.py.htmlinstead ofsrc/docc/build.py.html— the source-tree directory belongs to the repo, not the package being documented.Approach
Auto-detect at discovery time, no new config:
pathsentry__init__.py?src/pkg/....mypkg/mypkg/...PythonSource.relative_pathis unchanged (still project-rooted, used for source-link display); onlyoutput_pathshifts.Listing module
Once
output_pathandrelative_pathcan disagree, the listing's hierarchy lookups need a single source of truth. Two small helpers do this:_hierarchy_path(source)— navigation-tree position; for an index source (__init__.pyorListingSource) it is the directory indexed, otherwiseoutput_path._display_path(source)— file-style display path; rejoins the original filename to the URL-relative directory so a stripped__init__.pystill shows under its package name.Listing.add_source/descendants/siblingsand the listing sort keys all key off_hierarchy_path.siblings()of an index source returns its own descendants — a__init__.pyis a member of the package it defines, not a sibling of the parent directory's entries.Tests
Added
tests/test_python_discover.pyandtests/test_listing.pycovering:_hierarchy_path/_display_pathfor__init__.py,ListingSource, regular modulesListing.add_source/descendants/siblingsinvariants, including the__init__.py-is-its-own-package caseVerification
Built this repo's docs locally: tree starts at
docc/,docs/docc/index.htmllists all package members with correct links, nosrc/leaks anywhere in the navigation.