Simplify design of Markup.append.

ext/xrb/escape.c

@@ -2,18 +2,6 @@
 #include "escape.h"
 #include <assert.h>
 
-inline static int XRB_Markup_is_markup(VALUE value) {
-	if (RB_IMMEDIATE_P(value))
-		return 0;
-
-	// This is a short-cut:
-	if (rb_class_of(value) == rb_XRB_MarkupString) {
-		return 1;
-	}
-
-	return rb_funcall(value, id_is_a, 1, rb_XRB_Markup) == Qtrue;
-}
-
 VALUE XRB_MarkupString_raw(VALUE self, VALUE string) {
 	string = rb_str_dup(string);
 
@@ -91,7 +79,7 @@ static inline VALUE XRB_Markup_append_buffer(VALUE buffer, const char * s, const
 }
 
 // Escape and append a string to the output buffer.
-VALUE XRB_Markup_append_string(VALUE buffer, VALUE string) {
+static VALUE XRB_Markup_append_string(VALUE buffer, VALUE string) {
 	const char * begin = RSTRING_PTR(string);
 	const char * end = begin + RSTRING_LEN(string);
 
@@ -103,8 +91,11 @@ VALUE XRB_Markup_append_string(VALUE buffer, VALUE string) {
 	return XRB_Markup_append_buffer(buffer, s, p, end);
 }
 
-VALUE XRB_Markup_append(VALUE self, VALUE output, VALUE value) {
-	if (value == Qnil) return Qnil;
+// Append self to the output buffer efficiently, escaping special characters.
+VALUE XRB_Markup_append_markup(VALUE self, VALUE output) {
+	if (self == Qnil) return output;
+
+	VALUE value = self;
 
 	// Ensure value is a string:
 	if (rb_type(value) != T_STRING) {
@@ -115,19 +106,10 @@ VALUE XRB_Markup_append(VALUE self, VALUE output, VALUE value) {
 	if (RB_TYPE_P(output, T_STRING)) {
 		// Fast path:
 		rb_str_modify_expand(output, RSTRING_LEN(value));
-
-		// The output buffer is a string, so we can append directly:
-		if (XRB_Markup_is_markup(value)) {
-			rb_str_append(output, value);
-		} else {
-			XRB_Markup_append_string(output, value);
-		}
+		XRB_Markup_append_string(output, value);
 	} else {
 		// Slow path (generates temporary strings):
-		if (!XRB_Markup_is_markup(value)) {
-			value = XRB_Markup_escape_string(Qnil, value);
-		}
-
+		value = XRB_Markup_escape_string(Qnil, value);
 		rb_funcall(output, id_concat, 1, value);
 	}
 
@@ -155,14 +137,8 @@ VALUE XRB_Markup_escape_string(VALUE self, VALUE string) {
 }
 
 void Init_XRB_escape(void) {
-	rb_XRB_MarkupString = rb_define_class_under(rb_XRB, "MarkupString", rb_cString);
-	rb_include_module(rb_XRB_MarkupString, rb_XRB_Markup);
-
-	rb_undef_method(rb_class_of(rb_XRB_Markup), "escape_string");
-	rb_define_singleton_method(rb_XRB_Markup, "escape_string", XRB_Markup_escape_string, 1);
-
-	rb_undef_method(rb_class_of(rb_XRB_Markup), "append");
-	rb_define_singleton_method(rb_XRB_Markup, "append", XRB_Markup_append, 2);
+	rb_undef_method(rb_XRB_Markup, "append_markup");
+	rb_define_method(rb_XRB_Markup, "append_markup", XRB_Markup_append_markup, 1);
 
 	rb_undef_method(rb_class_of(rb_XRB_Markup), "raw");
 	rb_define_singleton_method(rb_XRB_Markup, "raw", XRB_MarkupString_raw, 1);

ext/xrb/escape.h

@@ -9,7 +9,7 @@ void Init_XRB_escape(void);
 VALUE XRB_MarkupString_raw(VALUE self, VALUE string);
 
 // Append any value to the output buffer efficiently, escaping entities as needed.
-VALUE XRB_Markup_append(VALUE self, VALUE buffer, VALUE value);
+VALUE XRB_Markup_append_markup(VALUE self, VALUE buffer);
 
 // Escape any entities in the given string. If no entities were found, might return the original string.
 VALUE XRB_Markup_escape_string(VALUE self, VALUE string);

ext/xrb/tag.c

@@ -67,7 +67,7 @@ static void XRB_Tag_append_tag_attribute(VALUE buffer, VALUE key, VALUE value, V
 
 		if (value != Qtrue) {
 			rb_str_cat_cstr(buffer, "=\"");
-			XRB_Markup_append(Qnil, buffer, value);
+			XRB_Markup_append_markup(value, buffer);
 			rb_str_cat_cstr(buffer, "\"");
 		}
 	}
@@ -160,7 +160,7 @@ VALUE XRB_Tag_append_tag_string(VALUE self, VALUE buffer, VALUE name, VALUE attr
 		rb_str_cat_cstr(buffer, ">");
 
 		if (content != Qtrue) {
-			XRB_Markup_append(self, buffer, content);
+			XRB_Markup_append_markup(content, buffer);
 		}
 
 		rb_str_cat_cstr(buffer, "</");

ext/xrb/xrb.c

@@ -51,6 +51,7 @@ void Init_XRB_Extension(void) {
 
 	rb_XRB = rb_define_module("XRB");
 	rb_XRB_Markup = rb_define_module_under(rb_XRB, "Markup");
+	rb_XRB_MarkupString = rb_define_class_under(rb_XRB, "MarkupString", rb_cString);
 	rb_XRB_Native = rb_define_module_under(rb_XRB, "Native");
 
 	Init_XRB_escape();

lib/xrb/builder.rb

@@ -9,8 +9,6 @@
 module XRB
 	# Build markup quickly and efficiently.
 	class Builder
-		include Markup
-
 		INDENT = "\t"
 
 		class Fragment
@@ -19,17 +17,20 @@ def initialize(block)
 				@builder = nil
 			end
 
-			def to_markup(builder)
+			def append_markup(output)
+				builder = Builder.new(output)
+
+				self.build_markup(builder)
+
+				return builder.output
+			end
+
+			def build_markup(builder)
 				@block.call(builder)
 			end
 
-
 			def to_s
-				builder = Builder.new
-
-				self.to_markup(builder)
-
-				return builder.to_s
+				self.append_markup(nil)
 			end
 
 			def == other
@@ -83,6 +84,10 @@ def initialize(output = nil, indent: true, encoding: Encoding::UTF_8)
 			@children = [0]
 		end
 
+		def build_markup(builder)
+			builder.append(@output)
+		end
+
 		attr :output
 
 		def encoding
@@ -148,7 +153,7 @@ def text(content)
 				@output << indentation
 			end
 
-			Markup.append(@output, content)
+			content.build_markup(self)
 
 			if @indent
 				@output << "\n"
@@ -160,7 +165,7 @@ def raw(content)
 		end
 
 		def <<(content)
-			content&.to_markup(self)
+			content&.build_markup(self)
 		end
 
 		# Append pre-existing markup:

lib/xrb/markup.rb

@@ -8,34 +8,29 @@
 module XRB
 	# A wrapper which indicates that `value` can be appended to the output buffer without any changes.
 	module Markup
-		# Converts special characters `<`, `>`, `&`, and `"` into their equivalent entities.
-		# @return [String] May return the original string if no changes were made.
-		def self.escape_string(string)
-			CGI.escape_html(string)
+		def self.raw(string)
+			MarkupString.raw(string)
 		end
 
-		# Appends a string to the output buffer, escaping if if necessary.
-		def self.append(buffer, value)
-			value = value.to_s
-
-			if value.is_a? Markup
-				buffer << value
-			elsif value
-				buffer << self.escape_string(value)
-			end
+		def append_markup(output)
+			output << ::CGI.escape_html(self.to_s)
+		end
+
+		def build_markup(builder)
+			append_markup(builder.output)
 		end
 	end
 
+	::Object.prepend(Markup)
+
 	# Initialized from text which is escaped to use HTML entities.
 	class MarkupString < String
-		include Markup
-
 		# @param string [String] the string value itself.
 		# @param escape [Boolean] whether or not to escape the string.
 		def initialize(string = nil, escape = true)
 			if string
 				if escape
-					string = Markup.escape_string(string)
+					string = ::CGI.escape_html(string)
 				end
 
 				super(string)
@@ -54,19 +49,15 @@ def self.raw(string)
 		def html_safe?
 			true
 		end
+
+		def append_markup(output)
+			output << self
+		end
 	end
 
 	module Script
 		def self.json(value)
 			MarkupString.new(JSON.dump(value), false)
 		end
 	end
-
-	module ToMarkup
-		def to_markup(builder)
-			::XRB::Markup.append(builder.output, self.to_s)
-		end
-	end
-
-	::Object.prepend(ToMarkup)
 end

lib/xrb/tag.rb

@@ -8,8 +8,6 @@
 module XRB
 	# This represents an individual SGML tag, e.g. <a>, </a> or <a />, with attributes. Attribute values must be escaped.
 	Tag = Struct.new(:name, :closed, :attributes) do
-		include XRB::Markup
-
 		def self.split(qualified_name)
 			if i = qualified_name.index(':')
 				return qualified_name.slice(0...i), qualified_name.slice(i+1..-1)
@@ -80,7 +78,7 @@ def self.append_tag(buffer, name, attributes, content)
 			else
 				buffer << '>'
 				unless content == true
-					Markup.append(buffer, content)
+					content.append_markup(buffer)
 				end
 				buffer << '</' << name.to_s << '>'
 			end
@@ -108,7 +106,7 @@ def self.append_attributes(buffer, attributes, prefix)
 					buffer << ' ' << attribute_key.to_s
 				else
 					buffer << ' ' << attribute_key.to_s << '="'
-					Markup.append(buffer, value)
+					value.append_markup(buffer)
 					buffer << '"'
 				end
 			end

test/xrb/.template/element.xrb

@@ -1,19 +1,13 @@
 <?r
 class MyElement
-	def to_html
-		XRB::Builder.fragment do |builder|
-			builder.inline(:div) do
-				builder.inline(:p) do
-					builder.text("Hello ")
-					builder.inline(:strong) { builder.text("World") }
-				end
+	def build_markup(builder)
+		builder.inline(:div) do
+			builder.inline(:p) do
+				builder.text("Hello ")
+				builder.inline(:strong) { builder.text("World") }
 			end
 		end
 	end
-
-	def to_s
-		to_html.to_s
-	end
 end
 ?>
 #{MyElement.new}

test/xrb/markup.rb

@@ -52,47 +52,35 @@ def json_generate(events)
 		end
 	end
 
-	with '.escape_string' do
-		it 'escapes special characters' do
-			expect(XRB::Markup.escape_string("<h1>Hello World</h1>")).to be == "&lt;h1&gt;Hello World&lt;/h1&gt;"
-		end
-
-		it 'fails to escape non-string' do
-			expect do
-				XRB::Markup.escape_string(Object.new)
-			end.to raise_exception(TypeError)
-		end
-	end
-
 	with '.append' do
 		it 'appends string to buffer' do
 			buffer = String.new
 
-			XRB::Markup.append(buffer, "Hello")
+			"Hello".append_markup(buffer)
 
 			expect(buffer).to be == "Hello"
 		end
 
 		it 'appends escaped string to buffer' do
 			buffer = String.new
 
-			XRB::Markup.append(buffer, "<h1>Hello World</h1>")
+			"<h1>Hello World</h1>".append_markup(buffer)
 
 			expect(buffer).to be == "&lt;h1&gt;Hello World&lt;/h1&gt;"
 		end
 
 		it 'appends nil to buffer' do
 			buffer = String.new
 
-			XRB::Markup.append(buffer, nil)
+			nil.append_markup(buffer)
 
 			expect(buffer).to be == ""
 		end
 
 		it 'appends MarkupString to buffer' do
 			buffer = String.new
 
-			XRB::Markup.append(buffer, XRB::MarkupString.raw("<h1>Hello World</h1>"))
+			XRB::MarkupString.raw("<h1>Hello World</h1>").append_markup(buffer)
 
 			expect(buffer).to be == "<h1>Hello World</h1>"
 		end
@@ -101,14 +89,14 @@ def json_generate(events)
 			buffer = Object.new
 
 			expect do
-				XRB::Markup.append(buffer, "Hello")
+				"Hello".append_markup(buffer)
 			end.to raise_exception(NoMethodError, message: be =~ /<</)
 		end
 
 		it 'falls back to appending with #<<' do
 			buffer = []
 
-			XRB::Markup.append(buffer, "<Hello>")
+			"<Hello>".append_markup(buffer)
 
 			expect(buffer).to be == ["&lt;Hello&gt;"]
 		end

test/xrb/markup_parser.rb

@@ -102,8 +102,8 @@
 	end
 
 	it "should track entities" do
-		expect(events[1][2]).to be_a XRB::Markup
-		expect(events[4][1]).to be_a XRB::Markup
+		expect(events[1][2]).to be_a XRB::MarkupString
+		expect(events[4][1]).to be_a XRB::MarkupString
 	end
 end
 
@@ -156,8 +156,8 @@
 	end
 
 	it "should track entities" do
-		expect(events[1][2]).not.to be_a XRB::Markup
-		expect(events[3][1]).not.to be_a XRB::Markup
+		expect(events[1][2]).not.to be_a XRB::MarkupString
+		expect(events[3][1]).not.to be_a XRB::MarkupString
 	end
 end