nmdias / FeedKit

An RSS, Atom and JSON Feed parser written in Swift
MIT License
1.19k stars 174 forks source link

Support content field with XHTML specified #29

Closed Yasumoto closed 6 years ago

Yasumoto commented 7 years ago

What an amazing framework!

The content I'm parsing looks like it follows this format of including HTML within the content field, and this is a stab at parsing that.

Ran the tests, but totally open to feedback on improvements.

Thanks! 🙏

nmdias commented 7 years ago

@Yasumoto Hi! Thanks for contributing 😃

Care to discuss these with me?

  1. As I see it, innerXMLContentsAccumulator should not exist. It doesn't need to. Do you think it's possible for us to get rid of it? Here's my reasoning:

    • Characters mapping should be the done by the characters mapping extensions of the Model, to keep things consistent. As long as the self.currentXMLDOMPath is untouched with the correct path, calling mapCharacters should be able to keep appending everything you throw at it, in the right place.
  2. There is an assumption that the XHTML will start with a div element. The RFC doesn't specify this. XHTML can technically have a root element html.

    • Question: Do we need to validate if it's a div? By the time div is parsing, we already know it should be handled as XHTML. the enclosing element of type="xhtml" already tells us this.
  3. Maybe we should update the link reference of the RFC specification and not xml.com.

    • "bit cumbersome and verbose", "makes up for these shortcomings", " feels a bit" indicate opinion based statements. Which is fine, but I would rather refer first from the RFC.
  4. RSS handles HTML/XHTML enclosed within CDATA tags or with encoded entities. Just something to keep in mind, because if it's parsing an element with type xhtml in an RSS feed, it will incorrectly assume that it doesn't already handle this situation. I wonder if we should make sure this is only valid for Atom feeds.

Ok, so this is just from the top of my head. Can you give your input on these?

Thanks again for contributing!

Cheers

nmdias commented 6 years ago

Closing this for inactivity, for now.

Yasumoto commented 6 years ago

Thanks @nmdias, I’ll pick this back up in June after some other projects get wrapped up 👍