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
#9413 Declare py namespace on document root to generate valid xml #9416
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with nits.
@@ -71,6 +71,7 @@ def write_doc(self, docname: str, doctree: Node) -> None: | |||
# work around multiple string % tuple issues in docutils; | |||
# replace tuples in attribute values with lists | |||
doctree = doctree.deepcopy() | |||
doctree.document.attributes["xmlns:py"] = "https://github.com/sphinx-doc/sphinx/issues/9413" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's not good to set the URL of #9413. I'd like to use a better one for this. What kind of URL should we use instead? document? project page (GitHub)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can point to the documentation of the domains?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can point only the URL of the "bundled" domains: https://www.sphinx-doc.org/en/master/usage/restructuredtext/domains.html
But this document does not contains the 3rd party domains. For example, sphinx-http-domain: https://pypi.org/project/sphinx-http-domain/.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can keep it as an attribute in the domain class with default pointing to the "bundled" domains doc?
@@ -71,6 +71,7 @@ def write_doc(self, docname: str, doctree: Node) -> None: | |||
# work around multiple string % tuple issues in docutils; | |||
# replace tuples in attribute values with lists | |||
doctree = doctree.deepcopy() | |||
doctree.document.attributes["xmlns:py"] = "https://github.com/sphinx-doc/sphinx/issues/9413" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "py" namespace comes from the domains mark-up.
https://www.sphinx-doc.org/en/master/usage/restructuredtext/domains.html
And we have many kinds of domains (ex. C, C++, etc.). And developers can add them via API. So it would be better to add it with such code:
doctree.document.attributes["xmlns:py"] = "https://github.com/sphinx-doc/sphinx/issues/9413" | |
for domain in self.env.domains.values(): | |
xmlns = "xmlns:" + domain.name | |
doctree.document.attributes[xmlns] = "https://URL/to/somewhere" |
Now I merged #10100 instead. Thank you for your contribution! |
Subject: Declare py namespace in XML writer.
Feature or Bugfix
Purpose
py:class
,py:module
, etc as attributes for nodes. This causes errors as the generated xml does not have a namespace forpy
declared. This PR fixes this by declaring axmlns:py
at the document level.Relates
Please recommend what URI to use for the declaration of the
py
xml namespace