-
-
Notifications
You must be signed in to change notification settings - Fork 34.5k
gh-128508: Add some docstrings to xml.dom.minidom #128477
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
base: main
Are you sure you want to change the base?
Changes from 24 commits
21bb374
f16761b
eb0208b
f4f6334
70617ea
90b344f
34c317a
5f7ba82
23d11be
e6a8d38
1592145
1a8ee69
61c3e8a
69f8e4b
7f767cd
2f8b072
539b638
a4c5f4f
c612744
b0a9129
3f71f1f
8b7ff8e
afa51ef
14bb77e
a4840f3
38be045
cca0fda
8df6795
727af86
2da171f
ecb4a54
8b63730
49a12e6
298a20f
b97d812
19d245c
7b1666d
c68cb4d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,6 +32,7 @@ | |
|
|
||
|
|
||
| class Node(xml.dom.Node): | ||
| """Define properties accessible on a DOM node.""" | ||
| namespaceURI = None # this is non-null only for elements and attributes | ||
| parentNode = None | ||
| ownerDocument = None | ||
|
|
@@ -44,6 +45,7 @@ def __bool__(self): | |
| return True | ||
|
|
||
| def toxml(self, encoding=None, standalone=None): | ||
| """Generate a string representation of a DOM.""" | ||
| return self.toprettyxml("", "", encoding, standalone) | ||
|
|
||
| def toprettyxml(self, indent="\t", newl="\n", encoding=None, | ||
|
|
@@ -80,10 +82,17 @@ def _get_lastChild(self): | |
| return self.childNodes[-1] | ||
|
|
||
| def insertBefore(self, newChild, refChild): | ||
| """Insert a new DOM Node before an existing Node. | ||
|
srinivasreddy marked this conversation as resolved.
|
||
|
|
||
| https://dom.spec.whatwg.org/#dom-node-insertbefore | ||
| The insertBefore(node, child) method, when invoked, | ||
| must return the result of pre-inserting node into | ||
| this before child. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the docstring should document what the method does, not quote parts of a spec. I would suggest simpler wording, but I don’t understand the existing one («the result of pre-inserting node into this before child» ?!)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in b97d812 |
||
| See also https://dom.spec.whatwg.org/#concept-node-pre-insert | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would have links like this in the reST docs, not docstrings.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in b97d812 |
||
| """ | ||
| if newChild.nodeType == self.DOCUMENT_FRAGMENT_NODE: | ||
| for c in tuple(newChild.childNodes): | ||
| self.insertBefore(c, refChild) | ||
| ### The DOM does not clearly specify what to return in this case | ||
|
erlend-aasland marked this conversation as resolved.
|
||
| return newChild | ||
| if newChild.nodeType not in self._child_node_types: | ||
| raise xml.dom.HierarchyRequestErr( | ||
|
|
@@ -112,6 +121,7 @@ def insertBefore(self, newChild, refChild): | |
| return newChild | ||
|
|
||
| def appendChild(self, node): | ||
| """Append a child node to an existing node.""" | ||
| if node.nodeType == self.DOCUMENT_FRAGMENT_NODE: | ||
| for c in tuple(node.childNodes): | ||
| self.appendChild(c) | ||
|
|
@@ -129,6 +139,7 @@ def appendChild(self, node): | |
| return node | ||
|
|
||
| def replaceChild(self, newChild, oldChild): | ||
| """Replace child node *oldChild* with *newChild*.""" | ||
| if newChild.nodeType == self.DOCUMENT_FRAGMENT_NODE: | ||
| refChild = oldChild.nextSibling | ||
| self.removeChild(oldChild) | ||
|
|
@@ -161,6 +172,7 @@ def replaceChild(self, newChild, oldChild): | |
| return oldChild | ||
|
|
||
| def removeChild(self, oldChild): | ||
| """Remove an existing child.""" | ||
| try: | ||
| self.childNodes.remove(oldChild) | ||
| except ValueError: | ||
|
|
@@ -172,11 +184,16 @@ def removeChild(self, oldChild): | |
| oldChild.nextSibling = oldChild.previousSibling = None | ||
| if oldChild.nodeType in _nodeTypes_with_children: | ||
| _clear_id_cache(self) | ||
|
|
||
| oldChild.parentNode = None | ||
| return oldChild | ||
|
|
||
| def normalize(self): | ||
| """Transform a node into its normalized form. | ||
|
srinivasreddy marked this conversation as resolved.
Outdated
|
||
|
|
||
| Removes empty exclusive Text nodes and concatenates the data of | ||
|
erlend-aasland marked this conversation as resolved.
Outdated
|
||
| remaining contiguous exclusive Text nodes into the first of | ||
| their nodes. | ||
| """ | ||
| L = [] | ||
| for child in self.childNodes: | ||
| if child.nodeType == Node.TEXT_NODE: | ||
|
|
@@ -204,6 +221,12 @@ def normalize(self): | |
| self.childNodes[:] = L | ||
|
|
||
| def cloneNode(self, deep): | ||
| """Create and return a duplicate of this node. | ||
|
|
||
| Args: | ||
| deep (bool): If True, recursively clone this node's descendants. | ||
| If False, clone only this node. | ||
|
erlend-aasland marked this conversation as resolved.
Outdated
|
||
| """ | ||
| return _clone_node(self, deep, self.ownerDocument or self) | ||
|
|
||
| def isSupported(self, feature, version): | ||
|
|
@@ -1341,6 +1364,12 @@ def _get_internalSubset(self): | |
| return self.internalSubset | ||
|
|
||
| def cloneNode(self, deep): | ||
| """Create and return a duplicate of this node. | ||
|
|
||
| Args: | ||
| deep (bool): If True, recursively clone this node's descendants. | ||
| If False, clone only this node. | ||
| """ | ||
| if self.ownerDocument is None: | ||
| # it's ok | ||
| clone = DocumentType(None) | ||
|
|
@@ -1903,9 +1932,16 @@ def renameNode(self, n, namespaceURI, name): | |
|
|
||
|
|
||
| def _clone_node(node, deep, newOwnerDocument): | ||
| """ | ||
| Clone a node and give it the new owner document. | ||
| Called by Node.cloneNode and Document.importNode | ||
| """Create and return a clone of a DOM node. | ||
|
|
||
| Args: | ||
| node: The DOM node to clone | ||
| deep (bool): If True, recursively clone the node's descendants. | ||
| If False, only clone the node itself. | ||
| newOwnerDocument: The document that will own the cloned node | ||
|
|
||
| Returns: | ||
| The cloned node | ||
| """ | ||
| if node.ownerDocument.isSameNode(newOwnerDocument): | ||
| operation = xml.dom.UserDataHandler.NODE_CLONED | ||
|
|
||
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 strange to have a class docstring start with a verb; it makes sense for a method (that does something), but a class usually represents a thing (noun), it does not do something.
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.
Addressed in b97d812