Attribute values > to_phlex_attribute_value, to_str, to_s

[joelmoss]

Fixes #569

Attribute values can be any object that responds to to_phlex_attribute_value or to_str, falling back to to_s if neither is defined.

[joeldrapper]

This looks good to me. I’m happy with to_str coming first.

[joeldrapper]

This will need to be updated because of a change I made.

Suggested change

	buffer << " " << name << '="' << ERB::Escape.html_escape(value) << '"'
	buffer << " " << name << '="' << Phlex::Escape.html_escape(value) << '"'

[joeldrapper]

Looks good @joelmoss. Unless anyone else has any comments, I’m happy to go with this.

[willcosgrove]

This looks good! Thanks for doing this! I left one note on the tests to ensure that they're testing for precedence

[willcosgrove]

So that we can test for method precedence as well, what do you think about making these test classes build on each other like this:

Suggested change

	class ToPhlexAttributeValueable
	def to_phlex_attribute_value
	"to_phlex_attribute_value"
	end
	end
	class ToSable
	def to_s
	"to_s"
	end
	end
	class ToPhlexAttributeValueable < ToSable
	def to_phlex_attribute_value
	"to_phlex_attribute_value"
	end
	end
	class ToSable < ToStrable
	def to_s
	"to_s"
	end
	end

[joelmoss]

Sure, but not sure it's necessary. The code is hugely simple, so I think testing much more than the existence of the methods has very little benefit.

Happy to add if that's the consensus.

[joeldrapper]

Oh, good point re tests. I’ll open an issue for that since I merged this already.