node-xml
node-xml copied to clipboard
Implements an option to prevent the automatic encoding
I'm implementing an Atom feed generator and I'd like to be able to disable the automatic string encoding.
This change looks good :+1:
Would you mind dropping a bit more of an explanation, including references, for the use-case here? This will help with documentation and other people trying to do the same thing.
Sure!
This PR adds a global escape
option. When set to true
, special characters in strings passed to xml
won't be escaped to HTML-entities. This could be used for many things. One example is atom
-feed entries. They can have a type="html"
attribute. In most use cases you would get the HTML as a string and render the feed using node-xml
. That's when you'd want to disable escaping. Making it a global option is simply the easiest way to implement that.
Here's a usage example I used in the tests:
xml(
[ { x: '<a>test</a>' } ],
{ escape: false }
)
This would return
<x><a>test</a></x>
We'd probably also want a local way to do this. Similar to how _cdata
works:
[ { x: { _raw: '<a>test</a>' } } ]
I'm interested to understand fully the reason for doing this in atom - you say "[feed entries] can have a type="html" attribute.", I'm trying to understand the difference between "can" and "should", is there a reference in the atom docs or to something about best practice that you can link here?
There is a very related issue on the node-rss repo (which directly depends on node-xml): https://github.com/dylang/node-rss/issues/27 - node-rss is supposed to output atom-compatible feeds, and the discussion there suggests that escaping should happen only if there are chars which need escaping, E.g. an 'auto' mode rather than on/off.
Just trying to get an idea of what the best path forward is for both node-xml and node-rss in respect to this issue.
Sorry, I may have been a bit wage in my explanation. Text elements can have an attribute type="text|html|xhtml"
. So the feed reader would interpret the content as HTML.
If type="html", then this element contains entity escaped html.
Here's the relevant part on their Website
The example on that page isn't very clear. But as I interpret it, you could do
<content type="html">
<em>Yolö</em>
</content>
That <em>Yolö</em>
would in most cases come from some CMS or something. So you could simply use it as is inside the <content>
of an article inside the Atom-Feed.
As far as I understand the problem in dylang/node-rss#27 is only remotely related. The auto detection he's talking about isn't something that I would include in node-xml itself (except maybe as an option), since it would give the programmer less control.
I wanted to check up on the state of this issue, since the Atom feed on my project is invalid ATM.
Will this get merged?
I also have a project that would benefit from this additional functionality.
I'm sorry, for some reason all the notifications from this repo started going into my spam folder so I had no idea there'd been any followup!
This PR currently has a merge conflict. If we could get that fixed up, then I'll look at pushing this out.
This PR is based from a master branch, making it slightly tricker than normal to fixup, but these steps should work assuming your repo is called origin
and this one upstream
in your remotes config:
-
git fetch upstream
(fetch from this repo) -
git pull -r upstream master
(rebase in latest changes from this repo) - resolve any conflicts (I think it's just a newline at the end of xml.js)
-
git push origin master -f
(push the update to this PR)
Hopefully this gets us moving again!
Great! Thanks a lot. :)
I'll probably get to it today.
Thanks a lot @ErisDS for the instructions :)
Is there still something that has to be done, in order for this PR to be merged?
We need this too!
Bump! Looking forward to this
Will this PR ever be merged? @halbgut @dylang ..We need this!