GeezerVison is showing github.com — Add class directives for ``xml.dom`` by AA-Turner · Pull Request #138419 · python/cpython
Conversation
AA-Turner
commented
Sep 3, 2025
•
edited by github-actions
Bot
Loading
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
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 have great doubts about these changes. Because all these classes are not defined in the xml.dom module. This is not description of concrete classes, this is description of ephemeral interfaces. Concrete classes are defined in the minidom submodule and in third-party modules. Adding class directives here will create wrong index entries and add wrong user visible information.
Try to add :no-typesetting: and :no-index-entry: options. Not sure about :no-contents-entry:. Will this be of any benefit?
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This is not description of concrete classes, this is description of ephemeral interfaces.
Sphinx doesn't have a .. py:interface:: or .. py:protocol:: directive, so class is the best thing we can do. Would you be happier with an admonition on each directive reminding the reader that they're protocol classes?
I think it would be useful to add them, but I'm happy to revert to just adding the targets if you still have reservations.
A
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
User should not see class Element here, nor the entry "Element (xml.dom class)", because xml.dom.Element does not exist.
It would be nice to make some class references referring to corresponding section in this file, for example, :class:`Document` in the xml.dom.minidom.parse() description to Document Objects. But not for all (for example NodeList does not exist, it is just a list). Sphinx is bad in using nested markup, so we cannot make a link from Document formatted as class to arbitrary anchor. Can we? If this is possible, it would be an ideal variant -- make all class references inert and add links manually.
Before we find a satisfying solution, it is better to leave unresolved references as a reminder.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Ok, I will close this for now & we can revisit it later, I will make a smaller change to #138418.
A
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
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.
|