-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Hide "Edit on Github" link if page is a auto-created page #1643
Conversation
That applies for source file views, search page etc.
We should move this to the sphinx_rtd_theme repo, I have a PR I'm putting together now that blows away our vendorized theme :) Additionally, for the sphinx-autoapi, we'll want to preserve this behavior i believe. That is, with compiled API reference pages, there is no underlying document and we will be linking to the Github source as determined by the language's documentation tooling. Will this affect that? |
Probably it will affect this, yes. That's sad but we need to find another way though to see how we can distinguish between to-github-linkable pages and others. And doh!! I completely missed that I modified a vendored file. My fault. I'll leave this open for now until I get around next week to further investigate the issue. |
@agjohnson I tried to have a new look at this, but I don't know much yet about sphinx-autoapi. Will the pagename variable be the name of the sourcefile then? So it's |
@gregmuellegger we have a working .NET example, but it does require the full build chain to execute. Here's an example of the intermediate YAML output: .NET tooling gives us all the info we need to correctly link to Github. We generate a page name for our purposes, but that won't correlate to the source file name at all. For other tools, we might only have the relative path however and maybe we would be passing that in, relying on the information that RTD passes in to override the Github link? I'm not sure how this could be performed best though. Maybe sphinx-autoapi should, in the case of generated output, set/override the docname in javascript per-document? This might be something that the footer api fetch needs to respect. I'm not sure if this would get us any closer to removing the github link on the original issue though. Open to any other thoughts here. |
The autoapi pages will come from real RST files on the disk, so it will consider them "existing" in terms of Sphinx, but they won't exist on GH -- so this logic will likely break there -- but this is a good thing to do in general, and we can probably work around this in other ways in the autoapi to properly tell Sphinx these docs are "autogen'd" |
It might be worth opening a ticket with Sphinx about adding a way to track autogen'd docs inside Sphinx, so we can do this explicitly, rather than with a heuristic. |
I didn't find a good example in the autoapi docs, but I assume it would be used like:
So where does the autogen pages come in? The line above is handwritten in a API reference documentation and that is a RST file in the repo. So the link on the page will link to this RST. Is there any mechanic that can generate a set of documentation pages that are not initiated by such a statement? @ericholscher Indeed that's a good idea that this should be tackled somehow in Sphinx and might be useful in more places than just the GitHub link. |
the auto api is actually generating a full rst document at build time, there is no manual directive creation required. we do plan to support that flow next though. so, currently, documents from the autoapi will not have a linkable rst source file, but would instead link to the source code that generated the intermediary rst. |
Closing this, but we should add this logic to the theme in some way. readthedocs/sphinx_rtd_theme#252 |
We can use the hasdoc function to detect if the current page is a "physical", i.e. real page or if it's autocreated.
I tested this on the Tornado project with the "search.html" page and the auto inlined source file pages, like http://www.tornadoweb.org/en/stable/_modules/tornado/stack_context.html
Related #1637