Skip to content
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

Missing node types (comment and processing-instruction) #48

Merged
merged 6 commits into from
Oct 14, 2013

Conversation

jbowtie
Copy link
Contributor

@jbowtie jbowtie commented Oct 6, 2013

This pull request adds support for the missing node types comment and processing-instruction, allowing them to be created, recognized during parsing, and used for type switching.

Adds the missing node types needed for outputting complete XML documents
(namely, comment and processing-instruction nodes).

All seven node types defined in the XML spec can now be read, created,
and serialized using gokogiri.
@@ -194,6 +196,17 @@ func (document *XmlDocument) Root() (element *ElementNode) {
return
}

func (document *XmlDocument) NodeById(id string) (element *ElementNode) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a test case for this method?

@mdayaram
Copy link
Contributor

mdayaram commented Oct 7, 2013

Everything looks really good! Thanks again for the contribution! Once you add a test case for the NodeByID method, we'll go ahead and merge it in ^.^

@jbowtie
Copy link
Contributor Author

jbowtie commented Oct 14, 2013

I've added a documentation string and a test case.

@mdayaram
Copy link
Contributor

Looks good! Merging in! Thanks again for the contribution, we really appreciate it ^.^

mdayaram added a commit that referenced this pull request Oct 14, 2013
Missing node types (comment and processing-instruction)
@mdayaram mdayaram merged commit e99e1e9 into moovweb:master Oct 14, 2013
@jbowtie jbowtie deleted the missing-node-types branch April 20, 2016 01:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants