Add XML.build and XML::Writer #3806

asterite · 2016-12-30T19:00:38Z

Right now it's impossible to generate correct XML in an easy way. This PR fixes that. It wraps a libxml2 writer. Writing to the writer directly writes to the underlying IO.

Example:

require "xml"

string = XML.write(indent: "  ") do |xml|
  xml.element("person", id: 1) do
    xml.element("firstname") { xml.text "Jane" }
    xml.element("lastname") { xml.text "Doe" }
  end
end

puts string

Output:

<?xml version="1.0"?>
<person id="1">
  <firstname>Jane</firstname>
  <lastname>Doe</lastname>
</person>

As you can see, there's no "magic" in the writer. You have to call element to emit elements. You also have attribute, text and other methods to emit these individually.

I thought about either adding a separate XML::Builder class that uses method_missing and is similar to Nokogiri::XML::Builder, or either adding that functionality to XML::Writer, but I'm not sure about that. For example in Nokogiri you have to use type_ or object_id_ (with an ending underscore) if you need these names so they don't conflict with existing methods. Also you basically have to learn a new mini-language for emitting stuff, and it gets a bit more complex when namespaces and attributes are involved. So I prefer to keep things simple and straight-forward here (though a builder using method_missing is definitely possible (in master)).

luislavena · 2016-12-30T19:12:42Z

src/xml/writer.cr

+      call SetQuoteChar, char.ord
+    end
+
+    # TODO: mark as private


With #3747 merged and part of 0.20.3, cannot this macro be marked as private already?

Last thing I knew, the binaries that travis uses for linux weren't up to date. I'll try to change this and see if it's already fixed :-)

It failed... jhass used to maintain these, I guess we should take that over

ysbaddaden · 2016-12-30T21:58:33Z

Pedantic: I'm not sure about the XML.write that returns a String; it sounds like it writes to a file. What about XML.build to reflect on String.build?

ysbaddaden

I noted some typos. Code looks good, and feature is 👍

ysbaddaden · 2016-12-30T22:01:54Z

src/xml/writer.cr

+    end
+
+    # Emits the start attribute of an attribute with namespace info.
+    def start_attribute(prefix : String?, name : String, namesapce_uri : String?)


Typo: namespace_uri

ysbaddaden · 2016-12-30T22:02:58Z

src/xml/writer.cr

+      end_element
+    end
+
+    # Emits the start attribute of an attribute.


The "start attribute of an attribute"? Did you mean the "start of an attribute"? Same below.

ysbaddaden · 2016-12-30T22:04:17Z

src/xml/writer.cr

+    end
+
+    # Emits the start of the document, invokes the block,
+    # and the emits the end of the document.


"then emits"

ysbaddaden · 2016-12-30T22:06:18Z

src/xml/writer.cr

@@ -0,0 +1,312 @@
+module XML
+  # Returns the resulting of writing XML to the yielded `XML::Writer`.


"returns the resulting String"?

ysbaddaden · 2016-12-30T22:08:10Z

src/xml/writer.cr

+    end
+
+    # Emits the start of a comment, invokes the block
+    # and the emits the end of the comment.


"then emits"

ysbaddaden · 2016-12-30T22:09:23Z

src/xml/writer.cr

+    end
+
+    # Emits the start of a CDATA section, invokes the block
+    # and then emits the enf of the CDATA section.


"emits the end" or maybe "closes the CDATA section"? Same for other occurrences.

ysbaddaden · 2016-12-30T22:10:29Z

src/xml/writer.cr

+
+    # Emits text content.
+    #
+    # Text content can happen inside of an element, attribute value, cdata, dtd, etc.


Maybe have backtics and hash to link to mentioned method?

splattael · 2016-12-31T15:54:35Z

Shouldn't the class XML::Writer be called XML::Builder similar to String::Builder?

luislavena · 2016-12-31T17:15:17Z

@splattael we could make that extensive to JSON::Builder, CSV::Builder, HTML::Builder (now separate project).

@asterite, since this was already merged, think will accept a PR with that rename? 😄

asterite · 2016-12-31T19:16:43Z

Sounds good. Also YAML. I wasn't sure because it doesn't build xml nodes, it writes an xml string to an IO, but I don't think we'll have a separate builder for something else, so having consistent names is good.

asterite · 2017-01-01T15:53:51Z

@luislavena Don't worry, I already started this refactor. I decided to do it myself because there's no JSON::Builder right now and I'm thinking of making to_json accept a builder, so it's similar to to_yaml, and you can configure its indent, etc.

luislavena reviewed Dec 30, 2016

View reviewed changes

asterite force-pushed the feature/xml_writer branch 2 times, most recently from e40f382 to a6d9323 Compare December 30, 2016 19:40

asterite added kind:feature topic:stdlib labels Dec 30, 2016

ysbaddaden reviewed Dec 30, 2016

View reviewed changes

asterite force-pushed the feature/xml_writer branch from a6d9323 to 8c96b62 Compare December 30, 2016 22:33

Add XML.build and XML::Writer

6bc552d

asterite force-pushed the feature/xml_writer branch from 8c96b62 to 6bc552d Compare December 30, 2016 22:36

asterite changed the title ~~Add XML.write and XML::Writer~~ Add XML.build and XML::Writer Dec 31, 2016

asterite added this to the 0.20.4 milestone Dec 31, 2016

asterite merged commit 3582156 into master Dec 31, 2016

asterite deleted the feature/xml_writer branch December 31, 2016 13:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add XML.build and XML::Writer #3806

Add XML.build and XML::Writer #3806

asterite commented Dec 30, 2016

luislavena Dec 30, 2016

asterite Dec 30, 2016

asterite Dec 30, 2016

ysbaddaden commented Dec 30, 2016

ysbaddaden left a comment

ysbaddaden Dec 30, 2016

ysbaddaden Dec 30, 2016

ysbaddaden Dec 30, 2016

ysbaddaden Dec 30, 2016

ysbaddaden Dec 30, 2016

ysbaddaden Dec 30, 2016

ysbaddaden Dec 30, 2016

splattael commented Dec 31, 2016

luislavena commented Dec 31, 2016

asterite commented Dec 31, 2016

asterite commented Jan 1, 2017

		@@ -0,0 +1,312 @@
		module XML
		# Returns the resulting of writing XML to the yielded `XML::Writer`.

Add XML.build and XML::Writer #3806

Add XML.build and XML::Writer #3806

Conversation

asterite commented Dec 30, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ysbaddaden commented Dec 30, 2016

ysbaddaden left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

splattael commented Dec 31, 2016

luislavena commented Dec 31, 2016

asterite commented Dec 31, 2016

asterite commented Jan 1, 2017