Skip to content

Commit

Permalink
close a tag if value is nil or empty string
Browse files Browse the repository at this point in the history
  • Loading branch information
@jimweirich
jimweirich committed Sep 6, 2012
1 parent 409c4a0 commit 7c82499
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 1 deletion.
2 changes: 1 addition & 1 deletion lib/builder/xmlbase.rb
View file
Expand Up @@ -64,7 +64,7 @@ def tag!(sym, *args, &block)
_end_tag(sym)
_newline
end
elsif text.nil?
elsif text.nil? || text.empty?

This comment has been minimized.

Sign in to view

Copy link
@rubys

rubys Oct 15, 2012

Until this change, it was fairly easy to produce HTML using builder. There are a number of tags (for example: script) for which an empty body is needed. Script is the canonical example. The following used to produce both an open and close tag, all on the same line:

builder.script '', :src => 'jquery-min.js'

This comment has been minimized.

Sign in to view

Copy link
@rubys

rubys Oct 15, 2012

Example of code that this breaks:
https://github.com/rubys/wunderbar/blob/master/lib/wunderbar/html-methods.rb#L89

Since you are doing the following earlier in the method, this is difficult to work around:
text ||= ''
text << arg.to_s

This comment has been minimized.

Sign in to view

Copy link
@rubys

rubys Oct 15, 2012

Workaround: rubys/wunderbar@04c5d09

This comment has been minimized.

Sign in to view

Copy link
@jimweirich

jimweirich Oct 16, 2012

Author Owner

Would the following behavior work for you?

def test_empty_value
  @xml.value("")
  assert_equal "<value></value>", @xml.target!
end

def test_nil_value
  @xml.value(nil)
  assert_equal "<value/>", @xml.target!
end

def test_no_value
  @xml.value()
  assert_equal "<value/>", @xml.target!
end

This comment has been minimized.

Sign in to view

Copy link
@rubys

rubys Oct 16, 2012

No, that would not work for me.

What I need is a way to emit markup like the following:

<script src="jquery-min.js"></script>
<div style="clear: both"></div>
<textarea></textarea>
<br/>

Prior to builder 3.1.3, I could do that with the following:

@xml.script '', :src => 'jquery-min.js'
@xml.div '', :style => 'clear: both'
@xml.textarea ''
@xml.br

For this to work, the behavior I desire is the following:

def test_empty_value
  @xml.value("")
  assert_equal "<value></value>", @xml.target!
end

def test_no_value
  @xml.value()
  assert_equal "<value/>", @xml.target!
end

I have no problem with the following:

def test_nil_value
  @xml.value(nil)
  assert_equal "<value/>", @xml.target!
end

Or with the previous behavior:

def test_nil_value
  @xml.value(nil)
  assert_equal "<value></value>", @xml.target!
end

This comment has been minimized.

Sign in to view

Copy link
@rubys

rubys Oct 16, 2012

I've issued a pull request for a potential fix:

#29

This comment has been minimized.

Sign in to view

Copy link
@jimweirich

jimweirich Oct 16, 2012

Author Owner

Ooops, markup swallowed my comparison values. Yes, what you replied with was what I intended. Great.
_indent
_start_tag(sym, attrs, true)
_newline
Expand Down
10 changes: 10 additions & 0 deletions test/test_markupbuilder.rb
View file
Expand Up @@ -34,6 +34,16 @@ def test_value
assert_equal "<value>hi</value>", @xml.target!
end

def test_empty_value
@xml.value("")
assert_equal "<value/>", @xml.target!
end

def test_nil_value
@xml.value(nil)
assert_equal "<value/>", @xml.target!
end

def test_nested
@xml.outer { |x| x.inner("x") }
assert_equal "<outer><inner>x</inner></outer>", @xml.target!
Expand Down

0 comments on commit 7c82499

Please sign in to comment.