Added support for multiple feeds (i.e. generating both Atom and RSS)

[LunarEclipse363]

Introduction

As requested many times before, for example:

• https://zola.discourse.group/t/generate-both-rss-and-atom-feeds/441
• #2083

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:

• [x] Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Code changes

• [x] Are you doing the PR on the next branch?

If the change is a new feature or adding to/changing an existing one:

• [x] Have you created/updated the relevant documentation page(s)?

[LunarEclipse363]

Ah I just realized I need to run cargo test --all for all the tests to run locally, gonna look into what's wrong.

[LunarEclipse363]

Forgot rustfmt :sweat_smile:

[LunarEclipse363]

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.

[jamwil]

Forgot rustfmt 😅

@LunarEclipse363 Story of my life.

[LunarEclipse363]

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: https://dev.azure.com/getzola/zola/_build/results?buildId=3278&view=logs&jobId=bad3c584-618a-56e1-e8ff-fa893be575d7&j=bad3c584-618a-56e1-e8ff-fa893be575d7&t=15b64e0a-658a-5cf8-fb48-72caeb8060e1

error: package `clap_derive v4.5.4` cannot be built because it requires rustc 1.74 or newer, while the currently active rustc version is 1.71.1
Either upgrade to rustc 1.74 or newer, or use
cargo update -p [email protected] --precise ver
where `ver` is the latest version of `clap_derive` supporting rustc 1.71.1

[Keats]

Can you rebase on the next branch? I've fixed the CI except for Windows

[Keats]

I would be tempted to just do a breaking change if there's no easy way to indicate that the single name is deprecated.

[LunarEclipse363]

I would be tempted to just do a breaking change if there's no easy way to indicate that the single name is deprecated.

I would recommend against that. Unless there is a way to make it a hard error to use the old syntax, it will mean that suddenly Zola quietly stops generating feeds for existing projects, which would be terrible UX.

I actually ran into this while testing - Zola just ignores the old configuration keys without an error if there is no compatibility mechanism.

Would adding #[serde(deny_unknown_fields)] to the config struct work? This would make it a breaking change, but also a clear error, so users would know action is required instead of Zola silently failing to render feeds.

I'm not sure if that would allow us to get a good error message specific to the deprecated fields though, I would need to look into what kind of error the deserializer returns and if it allows checking for this sort of information.

[Keats]

Would adding #[serde(deny_unknown_fields)] to the config struct work?

It would be nice yes. The migration guide can be added to the changelog

[LunarEclipse363]

This is more complicated than it needs to purely because there is one test that apparently requires that you can put arbitrary keys into the front-matter instead of only into an [extra] section.

Anyway, this only adds a special deprecation message for the front matter (I figured out how to do it in the end, it's not pretty) and relies on #[serde(deny_unknown_fields)] for the config file to avoid adding like 5 unnecessary types into the parsing code or sharing them between modules based on "this has the same error text" rather than "this is the same thing".

Very much a breaking change now, and I imagine #[serde(deny_unknown_fields)] might have unforeseen consequences for some users.

Anyway, everything is in the changelog so I guess it's fine.

[Keats]

This is more complicated than it needs to purely because there is one test that apparently requires that you can put arbitrary keys into the front-matter instead of only into an [extra] section.

Which test is that?

[LunarEclipse363]

This is more complicated than it needs to purely because there is one test that apparently requires that you can put arbitrary keys into the front-matter instead of only into an [extra] section.

Which test is that?

I don't remember off the top of my head but if you add #[serde(deny_unknown_fields)] to the section front matter struct, it will fail.

[Keats]

This is more complicated than it needs to purely because there is one test that apparently requires that you can put arbitrary keys into the front-matter instead of only into an [extra] section.

That test was invalid, you can just do:

-        f.write_all(b"+++\nslug=\"hey\"\n+++\n").unwrap();
+        f.write_all(b"+++\n+++\n").unwrap();

#[serde(deny_unknown_fields)] does find a few other issues so that's nice to have

[apiraino]

@LunarEclipse363 while I appreciate the flexibility that this patch brings to Zola, I think that this might catch users by surprise:

1. A mention in the changelog does not make it very explicit that it is a breaking change
2. If a zola website does not have anything configured for feed_filenames, the new default (["atom.xml"]) will just break the site building (see #2561 and other commits issue linked to this patchset)
3. CIs could auto-update to the latest Zola version and website just stop building, because ...
4. ... some themes might use the old syntax to get the feed filename from the config

I think the correct way to handle this could have been:

1. make this retrocompatible (example: handle the old case of feed_filenames as a string and internally make it an array of 1 item)
2. announce the breaking change more prominently in the changelog and the github release so people with websites and themes authors are aware and can upgrade their configs without rush

To be clear: I appreciate the time you took to deliver this patch, just my .2 cents on what I think could be the fallout here

[LunarEclipse363]

I think the correct way to handle this could have been:

1. make this retrocompatible (example: handle the old case of feed_filenames as a string and internally make it an array of 1 item)

2. announce the breaking change more prominently in the changelog and the github release so people with websites and themes authors are aware and can upgrade their configs without rush

1. I originally took the time to use a custom deserializer and implement backwards-compatibility (with the help of @selfisekai), however @Keats asked me to rewrite this to be a breaking change. There was also a WIP version of this which had better error messages but the code was really ugly because serde isn't designed to accomodate deprecating a field. By ugly I mean like 20 lines of code repeated for every deprecated field, where the actual message was 1 line.

2. I have no influence over how Zola releases are handled but I agree

[LunarEclipse363]

Regarding earlier versions that explored other solutions:

I originally took the time to use a custom deserializer and implement backwards-compatibility

see commit 2dae1d1d07cda99e7c654508e6a02387461fce95

There was also a WIP version of this which had better error messages but the code was really ugly

see commit 56cb421080c9e759d0313bdc6e87c4a75c128035, more specifically here and there

[Keats]

I think in that case the breaking change was ok since old usage would error out. I'll try to add back the breaking section for the next releases (like in https://github.com/getzola/zola/blob/master/CHANGELOG.md#breaking)