Skip to content

Loading…

Fix for Lighthouse ticket # 6334 : to_xml should render valid xml or raise an error all the time #445

Merged
merged 1 commit into from

3 participants

@prakashmurthy

Raise a RuntimeException with custom message when hash key contains space. Old behavior was { "New York"=>33, :Versailles => 3231 }.to_xml created an xml which was invalid.

@josevalim
Ruby on Rails member

Shouldn't we just dasherize the keys or something like that?

@tenderlove
Ruby on Rails member

@josevalim I agree. @prakashmurthy, can you change this to dasherize keys, and I'll apply?

@prakashmurthy

Since dasherize method only replaces '_' with '-', and does not do anything to spaces, we are thinking of using gsub(' ','-').Is this OK?

@prakashmurthy

Used gsub instead of _dasherize method as there is another issue with _dasherize method being broken. #450

@josevalim
Ruby on Rails member

Thanks @prakashmurthy! I have fixed the _dasherize method, do you think your pull request could be refactored then? As the default is to dasherize, I don't even think we need to do anything, just add your tests. Could you please check?

@prakashmurthy

Thanks @josevalim. Refactored the code; now the change includes only the tests. Let me know if anything else needs to be done.

@tenderlove tenderlove merged commit a45f300 into rails:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on May 8, 2011
  1. @prakashmurthy
Showing with 10 additions and 0 deletions.
  1. +10 −0 activesupport/test/xml_mini_test.rb
View
10 activesupport/test/xml_mini_test.rb
@@ -87,6 +87,16 @@ def to_xml(options) options[:builder].yo(options[:root].to_s) end
@xml.to_tag(:b, "Howdy", @options)
assert_xml "<b>Howdy</b>"
end
+
+ test "#to_tag should dasherize the space when passed a string with spaces as a key" do
+ @xml.to_tag("New York", 33, @options)
+ assert_xml "<New---York type=\"integer\">33</New---York>"
+ end
+
+ test "#to_tag should dasherize the space when passed a symbol with spaces as a key" do
+ @xml.to_tag(:"New York", 33, @options)
+ assert_xml "<New---York type=\"integer\">33</New---York>"
+ end
# TODO: test the remaining functions hidden in #to_tag.
end
end
Something went wrong with that request. Please try again.