Add to_s

[mperham]

I've used Builder for several projects over the last few years and every time I make the same API mistake: I use #to_s to finalize the built document.

xml = Builder::XmlMarkup.new
xml.foo do
  xml.bar
end
return xml.to_s

Only to find that I've injected an extra, unwanted <to_s/> element. A naive approach might try this:

class Builder::XmlMarkup
  def to_s
    @target.to_s
  end
end

But I know you just require the :<< method, thus allowing direct IO, which can't convert to a String. Perhaps something like this:

class Builder::XmlMarkup
  def to_s
    raise ArgumentError, "Cannot coerce #{@target.class.name} to String" unless @target.is_a?(String)
    @target
  end
end

WDYT?

[jimweirich]

I try to avoid all normal method names on the MarkUp object to avoid clashes with XML tag names. Will someone have an XML tag named "to_s"? Probably not, but I'm not in fond of setting a precedent.

[mperham]

What about alias_method :to_s!, :target!? At least that will trigger recognition in the mind of the developer.

[mperham]

Or maybe not an alias but the method I gave above.

[jimweirich]

I'm ok with the to_s! method, (provided it fails for non-string targets). But I'm not seeing how it actually solves the problem of forgetting that to_s doesn't actually exist.

[mperham]

Because it jogs the memory when reading the documentation. When looking through the methods on the object, target! looks like another XML declaration whereas to_s! would make me investigate that method because that's really what the developer is looking for: "how do I convert this thing to a String?"

[jc00ke]

I think this causes an issue with pry and irb too, unless I'm misunderstanding it. Builder::XmlMarkup#inspect is being treated like a tag, which makes it hard to explore in a console.

irb(main):024:0> builder = Builder::XmlMarkup.new
=> <inspect/>
irb(main):025:0> builder
=> <inspect/><inspect/>
irb(main):026:0> builder
=> <inspect/><inspect/><inspect/>

It seems like a few methods, #to_s, #inspect and #object_id for example, should not modify the instance.