From d9c1e142c73b5756c8e6339db12d0cb06243b1c3 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Sun, 27 Oct 2024 21:52:17 +1300 Subject: [PATCH] Simplify design of `Markup.append`. --- ext/xrb/escape.c | 44 ++++++++-------------------------- ext/xrb/escape.h | 2 +- ext/xrb/tag.c | 4 ++-- ext/xrb/xrb.c | 1 + lib/xrb/builder.rb | 27 ++++++++++++--------- lib/xrb/markup.rb | 39 ++++++++++++------------------ lib/xrb/tag.rb | 6 ++--- test/xrb/.template/element.xrb | 16 ++++--------- test/xrb/markup.rb | 24 +++++-------------- test/xrb/markup_parser.rb | 8 +++---- test/xrb/markup_string.rb | 2 +- 11 files changed, 63 insertions(+), 110 deletions(-) diff --git a/ext/xrb/escape.c b/ext/xrb/escape.c index 3fe4c72..afaf003 100644 --- a/ext/xrb/escape.c +++ b/ext/xrb/escape.c @@ -2,18 +2,6 @@ #include "escape.h" #include -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); diff --git a/ext/xrb/escape.h b/ext/xrb/escape.h index a14895e..b75564b 100644 --- a/ext/xrb/escape.h +++ b/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); diff --git a/ext/xrb/tag.c b/ext/xrb/tag.c index 38de53d..9555183 100644 --- a/ext/xrb/tag.c +++ b/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, "`, `&`, 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,6 +49,10 @@ def self.raw(string) def html_safe? true end + + def append_markup(output) + output << self + end end module Script @@ -61,12 +60,4 @@ 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 diff --git a/lib/xrb/tag.rb b/lib/xrb/tag.rb index 0a02d0f..6fb4c6a 100644 --- a/lib/xrb/tag.rb +++ b/lib/xrb/tag.rb @@ -8,8 +8,6 @@ module XRB # This represents an individual SGML tag, e.g. , or , 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 << '' 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 diff --git a/test/xrb/.template/element.xrb b/test/xrb/.template/element.xrb index dae8341..d69946e 100644 --- a/test/xrb/.template/element.xrb +++ b/test/xrb/.template/element.xrb @@ -1,19 +1,13 @@ #{MyElement.new} \ No newline at end of file diff --git a/test/xrb/markup.rb b/test/xrb/markup.rb index a81aadd..a714ead 100644 --- a/test/xrb/markup.rb +++ b/test/xrb/markup.rb @@ -52,23 +52,11 @@ def json_generate(events) end end - with '.escape_string' do - it 'escapes special characters' do - expect(XRB::Markup.escape_string("

Hello World

")).to be == "<h1>Hello World</h1>" - 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 @@ -76,7 +64,7 @@ def json_generate(events) it 'appends escaped string to buffer' do buffer = String.new - XRB::Markup.append(buffer, "

Hello World

") + "

Hello World

".append_markup(buffer) expect(buffer).to be == "<h1>Hello World</h1>" end @@ -84,7 +72,7 @@ def json_generate(events) it 'appends nil to buffer' do buffer = String.new - XRB::Markup.append(buffer, nil) + nil.append_markup(buffer) expect(buffer).to be == "" end @@ -92,7 +80,7 @@ def json_generate(events) it 'appends MarkupString to buffer' do buffer = String.new - XRB::Markup.append(buffer, XRB::MarkupString.raw("

Hello World

")) + XRB::MarkupString.raw("

Hello World

").append_markup(buffer) expect(buffer).to be == "

Hello World

" 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 =~ /<") + "".append_markup(buffer) expect(buffer).to be == ["<Hello>"] end diff --git a/test/xrb/markup_parser.rb b/test/xrb/markup_parser.rb index fcbbd3d..eb08de1 100755 --- a/test/xrb/markup_parser.rb +++ b/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 diff --git a/test/xrb/markup_string.rb b/test/xrb/markup_string.rb index 0b2e804..746ae37 100644 --- a/test/xrb/markup_string.rb +++ b/test/xrb/markup_string.rb @@ -32,7 +32,7 @@ it "should convert nil to empty string" do markup_string = XRB::MarkupString.new - XRB::Markup.append(markup_string, nil) + nil.append_markup(markup_string) expect(markup_string).to be(:empty?) end