From 62ac11ac73dafdc4c749aa2c1be58bfcdc233584 Mon Sep 17 00:00:00 2001 From: Kohei Suzuki Date: Sun, 14 Jun 2015 17:02:18 +0900 Subject: [PATCH] Skip falsey attribute rendering only if it's boolean attribute This is an optimization idea of Hamlit. --- ext/attribute_builder/attribute_builder.c | 86 +++++++-- .../spec/render/attribute_spec.md | 174 +++++++++++++++--- lib/faml/compiler.rb | 4 +- lib/faml/html.rb | 35 ++-- spec/render/attribute_spec.rb | 34 +++- 5 files changed, 273 insertions(+), 60 deletions(-) diff --git a/ext/attribute_builder/attribute_builder.c b/ext/attribute_builder/attribute_builder.c index c88a286..cba77c0 100644 --- a/ext/attribute_builder/attribute_builder.c +++ b/ext/attribute_builder/attribute_builder.c @@ -10,6 +10,52 @@ VALUE rb_mAttributeBuilder; static ID id_keys, id_sort_bang, id_uniq_bang, id_merge_bang, id_temple, id_utils, id_escape_html, id_gsub, id_to_s; + /* borrowed from https://github.com/rails/rails/blob/4-2-stable/actionview/lib/action_view/helpers/tag_helper.rb#L14-L19 */ +static const char *boolean_attributes[] = { + "disabled", + "readonly", + "multiple", + "checked", + "autobuffer", + "autoplay", + "controls", + "loop", + "selected", + "hidden", + "scoped", + "async", + "defer", + "reversed", + "ismap", + "seamless", + "muted", + "required", + "autofocus", + "novalidate", + "formnovalidate", + "open", + "pubdate", + "itemscope", + "allowfullscreen", + "default", + "inert", + "sortable", + "truespeed", + "typemustmatch", +}; + +static int +boolean_attribute_p(const char *key) +{ + unsigned i; + + for (i = 0; i < sizeof(boolean_attributes)/sizeof(boolean_attributes[0]); i++) { + if (strcmp(boolean_attributes[i], key) == 0) { + return 1; + } + } + return 0; +} static void concat_array_attribute(VALUE attributes, VALUE hash, VALUE key) @@ -198,19 +244,25 @@ build_attribute(VALUE attr_quote, int is_html, VALUE key, VALUE value) } return put_attribute(attr_quote, key, rb_ary_join(ary, rb_str_new_cstr("_"))); } - } else if (RB_TYPE_P(value, T_TRUE)) { - if (is_html) { - VALUE attr = rb_str_buf_new(1 + RSTRING_LEN(key)); - rb_str_buf_cat(attr, " ", 1); - rb_str_buf_append(attr, key); - return attr; + } else { + if (boolean_attribute_p(key_cstr)) { + if (RB_TYPE_P(value, T_TRUE)) { + if (is_html) { + VALUE attr = rb_str_buf_new(1 + RSTRING_LEN(key)); + rb_str_buf_cat(attr, " ", 1); + rb_str_buf_append(attr, key); + return attr; + } else { + return put_attribute(attr_quote, key, key); + } + } else if (RB_TYPE_P(value, T_FALSE) || NIL_P(value)) { + return Qnil; + } else { + return put_attribute(attr_quote, key, value); + } } else { - return put_attribute(attr_quote, key, key); + return put_attribute(attr_quote, key, value); } - } else if (RB_TYPE_P(value, T_FALSE) || NIL_P(value)) { - return Qnil; - } else { - return put_attribute(attr_quote, key, value); } return value; } @@ -248,6 +300,17 @@ m_normalize_data(RB_UNUSED_VAR(VALUE self), VALUE data) return normalize_data(data); } +static VALUE +m_boolean_attribute_p(RB_UNUSED_VAR(VALUE self), VALUE key) +{ + const char *key_cstr = StringValueCStr(key); + if (boolean_attribute_p(key_cstr)) { + return Qtrue; + } else { + return Qfalse; + } +} + void Init_attribute_builder(void) { @@ -255,6 +318,7 @@ Init_attribute_builder(void) rb_mAttributeBuilder = rb_define_module_under(mFaml, "AttributeBuilder"); rb_define_singleton_method(rb_mAttributeBuilder, "build", RUBY_METHOD_FUNC(m_build), -1); rb_define_singleton_method(rb_mAttributeBuilder, "normalize_data", RUBY_METHOD_FUNC(m_normalize_data), 1); + rb_define_singleton_method(rb_mAttributeBuilder, "boolean_attribute?", RUBY_METHOD_FUNC(m_boolean_attribute_p), 1); id_keys = rb_intern("keys"); id_sort_bang = rb_intern("sort!"); diff --git a/incompatibilities/spec/render/attribute_spec.md b/incompatibilities/spec/render/attribute_spec.md index 3fc18b3..8718960 100644 --- a/incompatibilities/spec/render/attribute_spec.md +++ b/incompatibilities/spec/render/attribute_spec.md @@ -125,33 +125,149 @@ Hamlit::SyntaxError ``` -# [./spec/render/attribute_spec.rb:92](../../../spec/render/attribute_spec.rb#L92) +# [./spec/render/attribute_spec.rb:84](../../../spec/render/attribute_spec.rb#L84) +## Input +```haml +%span{disabled: true, bar: 1} hello +``` + +## Faml, Haml +```html +hello + +``` + +## Hamlit +```html +hello + +``` + +# [./spec/render/attribute_spec.rb:90](../../../spec/render/attribute_spec.rb#L90) +## Input +```haml +%span{foo: true, bar: 1} hello +``` + +## Faml +```html +hello + +``` + +## Haml, Hamlit +```html +hello + +``` + +# [./spec/render/attribute_spec.rb:101](../../../spec/render/attribute_spec.rb#L101) ## Input (with options={:format=>:xhtml}) ```haml -- foo = true -%span{foo: foo, bar: 1} hello +%span{disabled: true, bar: 1} hello ``` ## Faml, Haml ```html -hello +hello ``` ## Hamlit ```html +hello + +``` + +# [./spec/render/attribute_spec.rb:101](../../../spec/render/attribute_spec.rb#L101) +## Input (with options={:format=>:xhtml}) +```haml +- disabled = true +%span{disabled: disabled, bar: 1} hello +``` + +## Faml, Haml +```html +hello + +``` + +## Hamlit +```html +hello + +``` + +# [./spec/render/attribute_spec.rb:101](../../../spec/render/attribute_spec.rb#L101) +## Input (with options={:format=>:xhtml}) +```haml +- h = {disabled: true, bar: 1} +%span{h} hello +``` + +## Faml, Haml +```html +hello + +``` + +## Hamlit +```html +hello + +``` + +# [./spec/render/attribute_spec.rb:109](../../../spec/render/attribute_spec.rb#L109) +## Input (with options={:format=>:xhtml}) +```haml +%span{foo: true, bar: 1} hello +``` + +## Faml +```html +hello + +``` + +## Haml, Hamlit +```html +hello + +``` + +# [./spec/render/attribute_spec.rb:109](../../../spec/render/attribute_spec.rb#L109) +## Input (with options={:format=>:xhtml}) +```haml +- foo = true +%span{foo: foo, bar: 1} hello +``` + +## Faml, Hamlit +```html hello ``` -# [./spec/render/attribute_spec.rb:92](../../../spec/render/attribute_spec.rb#L92) +## Haml +```html +hello + +``` + +# [./spec/render/attribute_spec.rb:109](../../../spec/render/attribute_spec.rb#L109) ## Input (with options={:format=>:xhtml}) ```haml - h = {foo: true, bar: 1} %span{h} hello ``` -## Faml, Haml +## Faml +```html +hello + +``` + +## Haml ```html hello @@ -163,7 +279,7 @@ Hamlit::SyntaxError ``` -# [./spec/render/attribute_spec.rb:99](../../../spec/render/attribute_spec.rb#L99) +# [./spec/render/attribute_spec.rb:117](../../../spec/render/attribute_spec.rb#L117) ## Input ```haml %span{foo: {bar: 1+2}} hello @@ -181,7 +297,7 @@ Hamlit::SyntaxError ``` -# [./spec/render/attribute_spec.rb:103](../../../spec/render/attribute_spec.rb#L103) +# [./spec/render/attribute_spec.rb:121](../../../spec/render/attribute_spec.rb#L121) ## Input ```haml - attrs = { foo: 1, bar: { hoge: :fuga }, baz: true } @@ -191,7 +307,7 @@ Hamlit::SyntaxError ## Faml ```html -hello +hello ``` @@ -207,7 +323,7 @@ Hamlit::SyntaxError ``` -# [./spec/render/attribute_spec.rb:117](../../../spec/render/attribute_spec.rb#L117) +# [./spec/render/attribute_spec.rb:135](../../../spec/render/attribute_spec.rb#L135) ## Input ```haml - data = { foo: 1 } @@ -227,7 +343,7 @@ Hamlit::SyntaxError ``` -# [./spec/render/attribute_spec.rb:124](../../../spec/render/attribute_spec.rb#L124) +# [./spec/render/attribute_spec.rb:142](../../../spec/render/attribute_spec.rb#L142) ## Input ```haml %span{foo: {bar: 1+2}} hello @@ -245,7 +361,7 @@ Hamlit::SyntaxError ``` -# [./spec/render/attribute_spec.rb:157](../../../spec/render/attribute_spec.rb#L157) +# [./spec/render/attribute_spec.rb:175](../../../spec/render/attribute_spec.rb#L175) ## Input ```haml %span{foo: 1 @@ -269,7 +385,7 @@ Unbalanced brackets. ``` -# [./spec/render/attribute_spec.rb:166](../../../spec/render/attribute_spec.rb#L166) +# [./spec/render/attribute_spec.rb:184](../../../spec/render/attribute_spec.rb#L184) ## Input ```haml %span{data: {foo: 1, bar: 'baz', :hoge => :fuga, k1: { k2: 'v3' }}} hello @@ -287,7 +403,7 @@ Unbalanced brackets. ``` -# [./spec/render/attribute_spec.rb:174](../../../spec/render/attribute_spec.rb#L174) +# [./spec/render/attribute_spec.rb:192](../../../spec/render/attribute_spec.rb#L192) ## Input ```haml %span{data: {foo: 1, bar: 2+3}} hello @@ -305,7 +421,7 @@ Unbalanced brackets. ``` -# [./spec/render/attribute_spec.rb:178](../../../spec/render/attribute_spec.rb#L178) +# [./spec/render/attribute_spec.rb:196](../../../spec/render/attribute_spec.rb#L196) ## Input ```haml - data = { foo: 1, bar: 2 } @@ -325,7 +441,7 @@ Unbalanced brackets. ``` -# [./spec/render/attribute_spec.rb:186](../../../spec/render/attribute_spec.rb#L186) +# [./spec/render/attribute_spec.rb:204](../../../spec/render/attribute_spec.rb#L204) ## Input ```haml %span{b: __LINE__, @@ -345,7 +461,7 @@ Unbalanced brackets. ``` -# [./spec/render/attribute_spec.rb:212](../../../spec/render/attribute_spec.rb#L212) +# [./spec/render/attribute_spec.rb:230](../../../spec/render/attribute_spec.rb#L230) ## Input ```haml %span{data: {foo: 1, @@ -370,7 +486,7 @@ Unbalanced brackets. ``` -# [./spec/render/attribute_spec.rb:220](../../../spec/render/attribute_spec.rb#L220) +# [./spec/render/attribute_spec.rb:238](../../../spec/render/attribute_spec.rb#L238) ## Input ```haml %span(foo=1 @@ -391,13 +507,19 @@ bar=3) hello ``` -# [./spec/render/attribute_spec.rb:237](../../../spec/render/attribute_spec.rb#L237) +# [./spec/render/attribute_spec.rb:255](../../../spec/render/attribute_spec.rb#L255) ## Input ```haml %span(foo bar=1) hello ``` -## Faml, Haml +## Faml +```html +hello + +``` + +## Haml ```html hello @@ -409,7 +531,7 @@ bar=3) hello ``` -# [./spec/render/attribute_spec.rb:241](../../../spec/render/attribute_spec.rb#L241) +# [./spec/render/attribute_spec.rb:259](../../../spec/render/attribute_spec.rb#L259) ## Input ```haml %span(foo=1 bar='baz#{1 + 2}') hello @@ -427,7 +549,7 @@ bar=3) hello ``` -# [./spec/render/attribute_spec.rb:246](../../../spec/render/attribute_spec.rb#L246) +# [./spec/render/attribute_spec.rb:264](../../../spec/render/attribute_spec.rb#L264) ## Input ```haml %span(foo=1 bar="ba\"z") hello @@ -445,7 +567,7 @@ bar=3) hello ``` -# [./spec/render/attribute_spec.rb:246](../../../spec/render/attribute_spec.rb#L246) +# [./spec/render/attribute_spec.rb:264](../../../spec/render/attribute_spec.rb#L264) ## Input ```haml %span(foo=1 bar='ba\'z') hello @@ -463,7 +585,7 @@ bar=3) hello ``` -# [./spec/render/attribute_spec.rb:255](../../../spec/render/attribute_spec.rb#L255) +# [./spec/render/attribute_spec.rb:273](../../../spec/render/attribute_spec.rb#L273) ## Input ```haml %span(foo=1 3.14=3) hello @@ -485,7 +607,7 @@ Invalid attribute list: "(foo=1 3.14=3)". ``` -# [./spec/render/attribute_spec.rb:259](../../../spec/render/attribute_spec.rb#L259) +# [./spec/render/attribute_spec.rb:277](../../../spec/render/attribute_spec.rb#L277) ## Input ```haml %span(foo=1 bar=) hello @@ -507,7 +629,7 @@ Invalid attribute list: "(foo=1 bar=)". ``` -# [./spec/render/attribute_spec.rb:271](../../../spec/render/attribute_spec.rb#L271) +# [./spec/render/attribute_spec.rb:289](../../../spec/render/attribute_spec.rb#L289) ## Input ```haml %span(b=__LINE__ diff --git a/lib/faml/compiler.rb b/lib/faml/compiler.rb index 3f74585..400695e 100644 --- a/lib/faml/compiler.rb +++ b/lib/faml/compiler.rb @@ -347,9 +347,9 @@ def build_optimized_dynamic_attributes(parser, static_attributes) def compile_static_attribute(key, value) case - when value == true + when AttributeBuilder.boolean_attribute?(key) && value == true [[:haml, :attr, key, [:multi]]] - when value == false || value == nil + when AttributeBuilder.boolean_attribute?(key) && !value [[:multi]] when value.is_a?(Hash) && key == 'data' data = AttributeBuilder.normalize_data(value) diff --git a/lib/faml/html.rb b/lib/faml/html.rb index 95b1931..052c87a 100644 --- a/lib/faml/html.rb +++ b/lib/faml/html.rb @@ -24,19 +24,19 @@ def on_haml_attr(name, value) if empty_exp?(value) true_attribute(name) elsif value[0] == :dvalue - sym = unique_name - [:multi, - [:code, "#{sym} = (#{value[1]})"], - [:case, sym, - ['true', true_attribute(name)], - ['false, nil', [:multi]], - [:else, [:multi, - [:static, " #{name}=#{options[:attr_quote]}"], - [:escape, true, [:dynamic, sym]], - [:static, options[:attr_quote]], - ]], - ], - ] + if AttributeBuilder.boolean_attribute?(name) + sym = unique_name + [:multi, + [:code, "#{sym} = (#{value[1]})"], + [:case, sym, + ['true', true_attribute(name)], + ['false, nil', [:multi]], + [:else, dynamic_attribute(name, sym)], + ], + ] + else + dynamic_attribute(name, value[1]) + end else [:multi, [:static, " #{name}=#{options[:attr_quote]}"], @@ -64,5 +64,14 @@ def true_attribute(name) [:static, " #{name}=#{options[:attr_quote]}#{name}#{options[:attr_quote]}"] end end + + def dynamic_attribute(name, value) + [ + :multi, + [:static, " #{name}=#{options[:attr_quote]}"], + [:escape, true, [:dynamic, value]], + [:static, options[:attr_quote]], + ] + end end end diff --git a/spec/render/attribute_spec.rb b/spec/render/attribute_spec.rb index d36afbb..c41cc0c 100644 --- a/spec/render/attribute_spec.rb +++ b/spec/render/attribute_spec.rb @@ -80,8 +80,16 @@ expect(render_string('%span{foo: 1}{bar: 2}')).to eq("{bar: 2}\n") end - it 'renders only name if value is true' do - expect(render_string(%q|%span{foo: true, bar: 1} hello|)).to eq(%Q{hello\n}) + context 'with boolean attribute' do + it 'renders only name if value is true' do + expect(render_string(%q|%span{disabled: true, bar: 1} hello|)).to eq(%Q{hello\n}) + end + end + + context 'with non-boolean attribute' do + it 'renders true value' do + expect(render_string(%q|%span{foo: true, bar: 1} hello|)).to eq(%Q{hello\n}) + end end it 'raises error when unparsable Ruby code is given' do @@ -89,10 +97,20 @@ end context 'with xhtml format' do - it 'renders name="name" if value is true' do - expect(render_string(%q|%span{foo: true, bar: 1} hello|, format: :xhtml)).to eq(%Q{hello\n}) - expect(render_string(%Q|- foo = true\n%span{foo: foo, bar: 1} hello|, format: :xhtml)).to eq(%Q{hello\n}) - expect(render_string(%Q|- h = {foo: true, bar: 1}\n%span{h} hello|, format: :xhtml)).to eq(%Q{hello\n}) + context 'with boolean attribute' do + it 'renders name="name" if value is true' do + expect(render_string(%q|%span{disabled: true, bar: 1} hello|, format: :xhtml)).to eq(%Q{hello\n}) + expect(render_string(%Q|- disabled = true\n%span{disabled: disabled, bar: 1} hello|, format: :xhtml)).to eq(%Q{hello\n}) + expect(render_string(%Q|- h = {disabled: true, bar: 1}\n%span{h} hello|, format: :xhtml)).to eq(%Q{hello\n}) + end + end + + context 'with non-boolean attribute' do + it 'renders name="true" if value is true' do + expect(render_string(%q|%span{foo: true, bar: 1} hello|, format: :xhtml)).to eq(%Q{hello\n}) + expect(render_string(%Q|- foo = true\n%span{foo: foo, bar: 1} hello|, format: :xhtml)).to eq(%Q{hello\n}) + expect(render_string(%Q|- h = {foo: true, bar: 1}\n%span{h} hello|, format: :xhtml)).to eq(%Q{hello\n}) + end end end @@ -101,7 +119,7 @@ end it 'renders code attributes' do - expect(render_string(<hello\n|) + expect(render_string(<hello\n|) - attrs = { foo: 1, bar: { hoge: :fuga }, baz: true } %span{attrs} hello HAML @@ -235,7 +253,7 @@ end it 'parses key-only attribute' do - expect(render_string('%span(foo bar=1) hello')).to eq("hello\n") + expect(render_string('%span(foo bar=1) hello')).to eq("hello\n") end it 'renders string interpolation' do