Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTML.escape and XML.escape #5045

Closed
straight-shoota opened this issue Sep 27, 2017 · 3 comments
Closed

HTML.escape and XML.escape #5045

straight-shoota opened this issue Sep 27, 2017 · 3 comments

Comments

@straight-shoota
Copy link
Member

With #5012 being merged (thanks @asterite and @akzhan) we now have both HTML.escape and XML.escape which are mostly identical. The only difference is that HTML escapes ' as ' and XML as '. This is because ' is an entity in XML, XHTML and HTML5, but not in legacy HTML. Though you could also escape XML with ' and almost all HTML appliances would be fine with '.

So I'm not 100% sure about it, but I think it makes sense to keep both implementations with this little difference.

However, I'd suggest to use the same API for both, which means implementing XML.escape(String, IO) similar to it's HTML counterpart.
And the documentation for the HTML parts should be copied and adapted to XML. It would also be nice to reference each from the other.

@asterite
Copy link
Member

I think we can safely remove XML.escape because for creating XML one can use an XML::Builder. Plus no such thing exists in, for example, Nokogiri, which is a library that also wraps libxml.

@straight-shoota
Copy link
Member Author

True, there shouldn't be too many actual use-cases.
In the case that someone wants to have a plain string escaped (maybe for an XML template instead of a builder), it's safe to use HTML.escape which is also valid for XML.

@straight-shoota
Copy link
Member Author

To keep the reference: XML.escape was introduced by #2455 to provide a JUnit formatter for specs, though that has been refactored to use a builder.

@asterite asterite added this to the Next milestone Sep 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants