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

Return partial RSS feeds #22

Closed
msjyoo opened this issue Jun 28, 2016 · 12 comments
Closed

Return partial RSS feeds #22

msjyoo opened this issue Jun 28, 2016 · 12 comments

Comments

@msjyoo
Copy link

msjyoo commented Jun 28, 2016

Hello,

You're using Option() for the RSS channel and item fields, but you fail the parse function when for example, the Channel Description is missing. I don't see why this should be necessary?

Why not simply set Option(None) and continue on with the parsing returning the partial results?

I've come across a site that this library refuses to work on (looking at you abc.net.au -_-) because I get a Err(ChannelMissingDescription).

Thanks!

@frewsxcv
Copy link
Member

The RSS specification says that description is required:

http://cyber.law.harvard.edu/rss/rss.html#requiredChannelElements

So, as per the specification, their RSS feed is not valid. What do you propose I do?

@frewsxcv
Copy link
Member

It's also required in the RSS 1.0 spec (the link above is the RSS 2.0 spec)

http://web.resource.org/rss/1.0/spec#s5.3.3

@msjyoo
Copy link
Author

msjyoo commented Jun 28, 2016

I understand that abc.net.au's feeds aren't RSS compliant, but if the library fails just because the given feed isn't strictly RSS compliant, it wouldn't be of much use in real life. Many websites are lazy and don't strictly follow the spec.

Since you are already using Some() on those fields, wouldn't it be as trivial as letting the value be None if the description or any other fields aren't detected?

@msjyoo
Copy link
Author

msjyoo commented Jun 28, 2016

Otherwise, what is the point of using the Result struct in the first place? It's only useful if the value can be omitted. Let me know what you think.

@frewsxcv
Copy link
Member

Otherwise, what is the point of using the Result struct in the first place?

Which Result struct are you referring to?

@msjyoo
Copy link
Author

msjyoo commented Jun 28, 2016

@frewsxcv
Copy link
Member

So if we want this to happen, there needs to be two parsers:

  • One (the existing one) that is strict that matches the spec.
  • The other one needs to be loose that has all Option fields.

Each of these parsers will result in a different struct. The strict parser should not have all Option fields.

What do you think?

@frewsxcv
Copy link
Member

If it is the case that sounds good, I'm going to disclaim that I'm pretty busy right now and probably won't get around to implementing it anytime soon. If you do though, I'll very happily merge in patches.

@msjyoo
Copy link
Author

msjyoo commented Jun 29, 2016

@frewsxcv I'll give it a go :)

@msjyoo
Copy link
Author

msjyoo commented Jul 2, 2016

@frewsxcv Would it be okay to use conditional compilation to implement this? A new feature flag of rss_loose compiles the loose version of rss? Which, of course, defaults to false.

It's just that creating a separate struct of RssLoose results in a lot of code duplication (at least from what Rust I know).

    #[cfg(not(feature = "rss_loose"))]
    pub title: String,
    #[cfg(not(feature = "rss_loose"))]
    pub link: String,
    #[cfg(not(feature = "rss_loose"))]
    pub description: String,

    #[cfg(feature = "rss_loose")]
    pub title: Option<String>,
    #[cfg(feature = "rss_loose")]
    pub link: Option<String>,
    #[cfg(feature = "rss_loose")]
    pub description: Option<String>,

Thanks :)

@frewsxcv
Copy link
Member

frewsxcv commented Jul 2, 2016

Great idea, sounds good

@jameshurst
Copy link
Member

This should be fixed by #24. Any missing fields thats are declared as "required" by the RSS 2.0 Specification will default to an empty string.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants