Cumulus / Syndic

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

Make Xml possible in the parse tree + misc improvements #24

Closed dinosaure closed 10 years ago

dinosaure commented 10 years ago

See #21 and #22.

Chris00 commented 10 years ago

See https://github.com/Cumulus/Syndic/pull/22 that I rebased on master. It does not have https://github.com/Cumulus/Syndic/commit/28ce2e0285accff08a953e098e223e21c29ddac5 though. I am a bit lost on what you want to do.

dinosaure commented 10 years ago

I use exception Ignore_namespace for this line. The exception is local to a function generate_catcher and we should not manage without.

Another problem is namespace. Indeed, eeb85ef adds a namespace to all the generate_catcher (partial application does not to work). I think the history of namespaces (namespace worth "") is specific RSS 2.0.

I think change generate_catcher for that the list of tags optionally contain namespace more specifically (RSS 1.0 contains two namespaces for example, RSS 1.0 and RDF).

dinosaure commented 10 years ago

Ready to merge or another point (speciallement on namespaces) ?

Chris00 commented 10 years ago

Your solution is working but maybe the logic couls be clearer? What about soluting along the lines

    let in_namespace ((prefix, _), _) = List.mem prefix namespaces in

(on purpose not restricted to a specific type) and then

 let rec catch_datas acc = function
      | Node (tag, datas) :: r ->
         if in_namespace tag then
           match get_tag_name tag data_producer with
           | Some f -> catch_datas ((f acc (tag, datas)) :: acc) r
           | None -> catch_datas acc r
         else
           catch_datas acc r
dinosaure commented 10 years ago

Indeed, working on master now, thanks !