sphinx
sphinx copied to clipboard
sphinx-doc
Fix singlehtml target uris to be same-document references
Subject: Fix singlehtml target uris to be same-document references
Feature or Bugfix
- Feature (Output quality improvement)
Purpose
Before this change, target_uris would be generated as
index.html#foo. This makes it difficult to rename the
generated document and can require reloading the document
when following a link, which can cause problems with JupyterLab
hosted documentation.
After this change, they are just #foo, which avoids the above
problems.
Detail
I looked into the history of this code and there doesn't seem to be any reason to put the name of the generated document into target_uris. Support for same document references (with only a fragment identifier) should be universal; documented in https://www.ietf.org/rfc/rfc2396.txt section 4.2.
Relates
n/a
This RFC is actually outdated. Could you quickly check that this is still compliant with RFC 3986?
Also, I don't know if extensions may rely on the existence of that page name ... I don't think so but if you can quickly do a github search for that one, it'd be great (and maybe add a test to check that the URIs are correctly generated).
Sorry for being nitpicky, but by experience, those changes that seem "fine" sometimes raise unexpected issues...
Could you quickly check that this is still compliant with RFC 3986?
My reading of RFC 3986 affirms that #foo (a fragment-only reference) is still compliant, as it would be resolved against the base URI, as explained in Section 5.1
According to two external reviews, it appears that it will be compliant wih the current standard. Thank you everyone for your time.
@eanorige Could you add a CHANGES entry please? (no need for a test I think).
I've added the CHANGES entry, crediting you with your github nickname. Thank you for your contribution! I'll merge once the tests passed.
I just want to double-check this, since its a pretty significant change; in particular I know there are tools that build PDFs from these documents, and I want to check this won't break them 😅
in particular I know there are tools that build PDFs from these documents, and I want to check this won't break them 🤔
Oh ! I wasn't aware of this (I would have thought they would be using LaTeX). Good catch then (though I would say, if we are RFC compliant, should we actually worry about it; I'd expect the tools to be compliant as well but maybe it's a bit too much, or maybe it's because singlehtml may be understood from a browser point of view or a standalone HTML, no browser needed, point of view).
I would have thought they would be using LaTeX
Basically its a "light-weight" alternative to running the whole horrible latex toolchain lol (naturally with some tradeoffs)
cc @danwos and @ubmarco, maybe you want to have a quick check of this, in particular if it would affect: https://github.com/useblocks/sphinx-simplepdf/blob/3a37cce56e5c4c019959621853c7b92e10f8a4ad/sphinx_simplepdf/builders/simplepdf.py#L26
FYI, I think it makes sense in general, I just want to check whether it would affect anyone "relying" on the current HTML output. Was trying to track down where the current behaviour was implemented, its at least in https://github.com/sphinx-doc/sphinx/commit/b08a7c4757faf174ab9559526ae295b4da1afb02, ...
I'll let you merge this one then, unless there is an issue, in which case I'd like you to @ me.
Was trying to track down where the current behaviour was implemented
yeh goes all the way back to its creation: https://github.com/sphinx-doc/sphinx/commit/744a519c9234910eacd638abed7c0a947344f464
so thats good, that it was not added later to fix a particular bug or anything 👍
Thanks for taking sphinx-extensions into account :)
I have made a quick test with Sphinx-SimplePDF and this PR.
I used the official Sphinx documentation as test object and it looks good:
The internal links are still working.
So, the change looks fine to me :+1: Thanks for it!
