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

No CDATA #27

Open
tmshv opened this issue Feb 9, 2014 · 12 comments
Open

No CDATA #27

tmshv opened this issue Feb 9, 2014 · 12 comments

Comments

@tmshv
Copy link

tmshv commented Feb 9, 2014

Is it possible to create XML without a CDATA?

@dylang
Copy link
Owner

dylang commented Feb 9, 2014

It should be, what does your code look like?

@tmshv
Copy link
Author

tmshv commented Feb 9, 2014

console.log(new RSS({
    title: "hello"
}).xml("\t"));
<?xml version="1.0" encoding="UTF-8"?>
<rss xmlns:dc="http://purl.org/dc/elements/1.1/" xmlns:content="http://purl.org/rss/1.0/modules/content/" xmlns:atom="http://www.w3.org/2005/Atom" version="2.0">
    <channel>
        <title><![CDATA[hello]]></title>
        <description><![CDATA[hello]]></description>
        <link>http://github.com/dylan/node-rss</link>
        <generator>RSS for Node</generator>
        <lastBuildDate>Sun, 09 Feb 2014 15:57:10 GMT</lastBuildDate>
    </channel>
</rss>

@tmshv
Copy link
Author

tmshv commented Feb 9, 2014

Pull request: #28

@dylang
Copy link
Owner

dylang commented Feb 9, 2014

Thank you for the pull request!

I replied quickly on my phone without looking carefully and thought you were asking about node-xml, not this one.

I put those in ![CDATA because I wasn't sure if the text would be valid for XML. Do you think there's a way to check the string and automatically wrap with ![CDATA only when needed?

@dylang
Copy link
Owner

dylang commented Feb 9, 2014

According to http://www.w3.org/TR/REC-xml/#syntax it looks like CDATA is only needed when the string has <, >, or &. What do you think checking for those strings and only using CDATA when one of them is found?

@tmshv
Copy link
Author

tmshv commented Feb 10, 2014

Actually xml module making this work

var xml = require("xml");
console.log(xml({hello:"lol"})); //<hello>lol</hello>
console.log(xml({hello:"<lol>"})); //<hello>&lt;lol&gt;</hello>

@dylang
Copy link
Owner

dylang commented Sep 26, 2014

Is this and #28 still needed?

@tmshv
Copy link
Author

tmshv commented Sep 27, 2014

Right now I'm not working on a project which used it, but why not?

@dylang
Copy link
Owner

dylang commented Sep 27, 2014

If nobody is having problems with the current code I'd rather not change the code which could potentially introduce new problems.

@tmshv
Copy link
Author

tmshv commented Sep 28, 2014

Sounds like a death sentence.

@ErisDS
Copy link
Collaborator

ErisDS commented Mar 22, 2015

I would be in favour of making this change, it seems unnecessary to have CDATA everywhere when it is only required if the string contains one of three characters, which is a rarity. It's definitely aesthetics rather than having a true technical grounding, but when you compare an RSS feed from Ghost to one from another platform ours look really messy and we are really keen to do much better in this area.

@cekvenich
Copy link

bump/update/eta?

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

4 participants