Cumulus / Syndic

RSS and Atom feed parsing
MIT License
34 stars 13 forks source link

+OPML #40

Closed Chris00 closed 9 years ago

Chris00 commented 10 years ago

One nice addition to Syndic would be OPML.

dinosaure commented 10 years ago

See #48 for OPML 1.0.

Chris00 commented 9 years ago

Thanks to @Eyyub a parser was added. A writer is still needed (alike to_xml and output in Syndic.Atom) so I propose that we leave this open until one is added.

I have removed trailing whitespace and wrapped the lines to 80 chars. In doing so, I have changed the comments where I saw fit. I propose that comments talk about the types as little as possible. Not only this unnecessarily duplicates information but also easily gets out of sync (such as “isComment is a string, either "true" or "false"”). I understand they were copied from the spec — I edited them. Some comments that are not clear to me are:

Also type_ is not very common, I think people usually use typ or prefix it.

Chris00 commented 9 years ago

I also wonder: why is text a string option instead of just a string (i.e. is the difference between None and Some "" significant?).

Chris00 commented 9 years ago

I made more element of head optional because that was required to parse, say, http://planet.ocaml.org/opml.xml

eyyub commented 9 years ago

Big thanks for your improvements @Chris00 ! Do you know how to handle other attributes in ? (e.g in http://planet.ocaml.org/opml.xml there is the xmlUrl which is not in the common attributes)

Chris00 commented 9 years ago

I added output functions. There is still the discussion on the above points and then this issue can be closed.

Chris00 commented 9 years ago

@Eyyub For additional attributes one can:

What do you think?

eyyub commented 9 years ago

I think it's the better way we can do, we have more safety in one side and more flexibility in the other, it so I'm in favor !

Chris00 commented 9 years ago

http://www.howtocreate.co.uk/tutorials/jsexamples/sampleOPML.xml uses both xmlUrl and htmlUrl. So maybe only adding an association list is better.

Chris00 commented 9 years ago

http://news.bbc.co.uk/rss/feeds.opml uses both again. Since the name seem fairly common, one should maybe add both fields xml_url and html_url (+ associative list).

Chris00 commented 9 years ago

I added support for what we discussed above: 28d04f3c994996c890dffe8bf9713cacaf887ebc Note that I could not use the generic generate_catcher. Instead I wrote outline_of_xml and outline_of_xml' by hand. They are both shorter and more efficient (O(n) instead of O(np) where n is the length of the list of attributes or sub-nodes and p is the number of interesting characteristics).

eyyub commented 9 years ago

Beautiful !

Chris00 commented 9 years ago

All the above points have been addressed, so closing.