Added support for multiple feeds (i.e. generating both Atom and RSS) #2477
Conversation
|
Ah I just realized I need to run |
|
Forgot rustfmt 😅 |
|
I've tested this for my own website and generating two feeds seems to work without a problem, both at the root level of the website and for taxonomies. It should be ready to merge now unless there's some issues I couldn't find. |
@LunarEclipse363 Story of my life. |
|
For reference, this PR is ready to merge, the force-push was just a rebase where the only conflict was in the changelog. The CI failing now is unrelated to the PR, it looks like some dependency increased its MSRV: |
|
Can you rebase on the next branch? I've fixed the CI except for Windows |
components/config/src/config/mod.rs
Outdated
|
|
||
| #[derive(Debug, Clone, Deserialize, PartialEq, Eq)] | ||
| #[serde(untagged)] | ||
| pub(crate) enum MightBeSingle<T> { |
There was a problem hiding this comment.
If we do support that, we need to add a deprecation message when there is only one value. I'm not entirely we need to though, it's a pretty simple change to do from the user POV.
As for name... SingleOrVec?
There was a problem hiding this comment.
The problem I ran into while testing before adding the backwards-compatibility is that Zola doesn't error out if you leave the old value names in the config file, instead using default values, which is a very bad way to handle it. The user will think they have the configuration option set while it's not until they realize it's broken purely due to a single-letter rename.
I can change the name and add a deprecation message, sure.
There was a problem hiding this comment.
Update: adding a meaningful deprecation message seems to be much harder than I expected due to the constraints of how serde works
| @@ -68,8 +68,8 @@ pub struct SectionFrontMatter { | |||
| #[serde(skip_serializing)] | |||
| pub aliases: Vec<String>, | |||
| /// Whether to generate a feed for the current section | |||
| #[serde(skip_serializing)] | |||
| pub generate_feed: bool, | |||
| #[serde(skip_serializing, alias = "generate_feed")] | |||
There was a problem hiding this comment.
you don't need it here i think
There was a problem hiding this comment.
Zola will ignore unknown fields here as well though which again ends up with the "suddenly an option stopped working without an error" problem
Which I'm not sure is solvable within serde
| }; | ||
| let mut feeds = Vec::new(); | ||
| for feed_filename in &site.config.feed_filenames { | ||
| let mut context = context.clone(); |
There was a problem hiding this comment.
That's pretty wasteful.
Can you render them one by one and just updating context.insert("feed_url", &feed_url); in the loop? I can't remember what additional_context_fn(context); does though
There was a problem hiding this comment.
render_template() takes ownership of the context so I need to clone it on every loop regardless
- Fixed language config merge bug found by a test - Adjusted two existing tests to fully check stuff related to multiple feeds - Added a new test for backwards-compatibility of the changes - Fixed bugs found by the newly added test
Introduction
As requested many times before, for example:
Potential improvements
I don't know if/how Zola handles deprecating config options, if there's anything I should add to let the user know the feed related options changed please let me know how to do it.Implemented backwards-compatibility as suggested by @selfisekai
Formalities
Sanity check:
Code changes
nextbranch?If the change is a new feature or adding to/changing an existing one: