rust-syndication / rss

Library for serializing the RSS web content syndication format
https://crates.io/crates/rss
Apache License 2.0
419 stars 52 forks source link

Return partial RSS feeds, rss_loose feature, resolves #22 #23

Closed msjyoo closed 8 years ago

msjyoo commented 8 years ago

Hello,

Here is the pull request for the partial RSS feeds feature.

One thing I'm concerned about is I've had to use the feature flag #![feature(stmt_expr_attributes)] to be able to get enough control over the compiler to implement these changes, so currently this patch only works for the nightly branch until that feature is un-gated. So it might be a better idea to cook this patch until then.

Still, others might find this useful regardless by leaving it here.

Please review and comment / merge.

Thanks!

msjyoo commented 8 years ago

22

frewsxcv commented 8 years ago

I'm not planning on merging anything that breaks compatibility with stable or beta.

That said, I think you should be able to replace most of your stmt_expr_attributes usages (which is unstable) with cfg! (which is stable) without too much effort.

Thanks for the pull request!

frewsxcv commented 8 years ago

Another resource: https://doc.rust-lang.org/std/macro.cfg!.html

msjyoo commented 8 years ago

@frewsxcv I've tried that of course ;) But, using if cfg!() is still indicating to the compiler that I want both if / else sections compiled, and of course since this is a type changing conditional compilation, it produces an error (since both cases has to be compiled, before one being optimised away by the true / false constant if). Took me a while to figure out that one :P

msjyoo commented 8 years ago

@frewsxcv I understand not accepting patches that break stable compatibility, but I'd like to leave this open in case anyone else comes across the same problem as mine (and willing to use the nightly) and also for the time when the stmt_expr_attributes becomes stable.

msjyoo commented 8 years ago

Tracking issue here: https://github.com/rust-lang/rust/issues/15701

jameshurst commented 8 years ago

This has been resolved by #24.