[skip ci] Doc for Node#add_child#3453
Conversation
|
@flavorjones, in the event, I'm less convinced about the extended-form "Related" I've shown here. See what you think. |
flavorjones
left a comment
flavorjones
left a comment
There was a problem hiding this comment.
In general, I like this addition to the documentation. I made a few small notes.
| # returns +object+: | ||
| # | ||
| # bookstore_doc = Nokogiri::XML::Document.parse(BOOKSTORE_XML) | ||
| # book_nodeset = bookstore_doc.search('//book') |
There was a problem hiding this comment.
I'd prefer if we did not use search in examples. I've been trying to discourage the use of that method for many years. Let's please use either xpath or css.
There was a problem hiding this comment.
@flavorjones, going forward, do have a preference between the two?
There was a problem hiding this comment.
I don't have a preference, honestly. Depending on the task, an xpath query or a css selector might be easier ... Or if we want to cater to the reader, XML examples might use XPath, and HTML examples might use CSS?
There was a problem hiding this comment.
Interesting, thanks.
| # doc = Nokogiri::XML::Document.parse(xml) | ||
| # src_child_node = doc.at_css('src_child') | ||
| # dst_parent_node = doc.at_css('dst_parent') | ||
| # dst_parent_node.add_child(src_child_node) |
There was a problem hiding this comment.
I'd like these examples to also illustrate the final state of the document, after the changes were made. Maybe this could be as simple as something like this:
# doc.to_xml
# # => '<root>...</root>'
| # doc = Nokogiri::XML::Document.parse('<root/>') | ||
| # doc.root.add_child('<foo/><bar/>') | ||
| # | ||
| # [Related] |
There was a problem hiding this comment.
I agree this doesn't feel particularly high-value. I've already got methods bucketed by general function, e.g.:
# :section: Manipulating Document Structure
|
|
||
| ### | ||
| # Add +node_or_tags+ as a child of this Node. | ||
| 0 |
|
@flavorjones, I have re-worked the examples to show before/after states. |
|
@BurdetteLamar This looks great! Merging. Thank you! 🙇 |
3 participants
No description provided.