Chris Morgan’s whole bunch of miscellaneous work for landing #994
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Whitespace is not allowed between the `]` and `(`.
It doesn’t have a field named slug. It does have a field named lang.
The HTML spec doesn’t require it, and I prefer to omit it. This has been bothering me for ages, but I hadn’t gotten round to fixing it yet. This can cause nominally invalid HTML to be emitted, if `</body>` was omitted but `</html>` was present, but that’s unlikely to happen, and this is for development purposes only, and the right thing will happen anyway in all environments (per browser behaviour and spec). I don’t think this warrants a changelog entry.
Two main purposes of changes here: - To make the formatting and indentation of the raw output prettier; - To simplify the HTML yielded by dropping unnecessary bits. The 404 changes are a tad more extensive, altering the actual wording to match conventional stub 404 pages a little more.
This includes several breaking changes, but they’re easy to adjust for. Atom 1.0 is superior to RSS 2.0 in a number of ways, both technical and legal, though information from the last decade is hard to find. http://www.intertwingly.net/wiki/pie/Rss20AndAtom10Compared has some info which is probably still mostly correct. How do RSS and Atom compare in terms of implementation support? The impression I get is that proper Atom support in normal content websites has been universal for over twelve years, but that support in podcasts was not quite so good, but getting there, over twelve years ago. I have no more recent facts or figures; no one talks about this stuff these days. I remember investigating this stuff back in 2011–2013 and coming to the same conclusion. At that time, I went with Atom on websites and RSS in podcasts. Now I’d just go full Atom and hang any podcast tools that don’t support Atom, because Atom’s semantics truly are much better. In light of all this, I make the bold recommendation to default to Atom. Nonetheless, for compatibility for existing users, and for those that have Opinions, I’ve retained the RSS template, so that you can escape the breaking change easily. I personally prefer to give feeds a basename that doesn’t mention “Atom” or “RSS”, e.g. “feed.xml”. I’ll be doing that myself, as I’ll be using my own template with more Atom features anyway, like author information, taxonomies and making the title field HTML. Some notes about the Atom feed template: - I went with atom.xml rather than something like feed.atom (the .atom file format being registered for this purpose by RFC4287) due to lack of confidence that it’ll be served with the right MIME type. .xml is a safer default. - It might be nice to get Zola’s version number into the <generator> tag. Not for any particularly good reason, y’know. Just picture it: <generator uri="https://www.getzola.org/" version="0.10.0"> Zola </generator> - I’d like to get taxonomies into the feed, but this requires exposing a little more info than is currently exposed. I think it’d require `TaxonomyConfig` to preferably have a new member `permalink` added (which should be equivalent to something like `config.base_url ~ "/" ~ taxonomy.slug ~ "/"`), and for the feed to get all the taxonomies passed into it (`taxonomies: HashMap<String, TaxonomyTerm>`). Then, the template could be like this, inside the entry: {% for taxonomy, terms in page.taxonomies %} {% for term in terms %} <category scheme="{{ taxonomies[taxonomy].permalink }}" term="{{ term.slug }}" label="{{ term.name }}" /> {% endfor %} {% endfor %} Other remarks: - I have added a date field `extra.updated` to my posts and include that in the feed; I’ve observed others with a similar field. I believe this should be included as an official field. I’m inclined to add author to at least config.toml, too, for feeds. - We need to have a link from the docs to the source of the built-in templates, to help people that wish to alter it.
The variable name matched the RSS tag it ended up in, but was misleading about what it actually was—because if you actually want “last build date”, you should use `now()`. (Due to the potential for edits, I think that either there should be an official `updated` field on pages, or that these templates should use `now()`.)
Also change a few other things to use it, as noted in CHANGELOG.md. TODO: - Write a couple of tests: updated field, last_updated template variable One slight open questions: should `updated` default to the value of `date` rather than to None? Then pages with `date` could safely assume `updated`.
Also a FIXME on the rebuilding part, because it’s presently very wrong.
|
Test failures are not my fault: tests are failing on the next branch. |
Keats
reviewed
Apr 14, 2020
| - Add `updated` front-matter field for pages, which sitemap templates will use for the `SitemapEntry.date` field instead of the `date` front-matter field, and which the default Atom feed template will use | ||
| - Add `lang` to the feed template context | ||
| - Add `taxonomy` and `term` to the feed template context for taxonomy feeds | ||
|
|
||
| ## 0.10.2 (unreleased) |
There was a problem hiding this comment.
can you move the 0.10.2 changes to the 0.11 part?
| filename if filename == site.config.feed_filename => { | ||
| // FIXME: this is insufficient; for multilingual sites, it’s rendering the wrong | ||
| // content into the root feed, and it’s not regenerating any of the other feeds (other | ||
| // languages or taxonomies with feed enabled). |
There was a problem hiding this comment.
the whole rebuild subcrate needs to be rewritten :(
| <id>{{ feed_url | safe }}</id> | ||
| {%- for page in pages %} | ||
| <entry | ||
| {%- if page.lang != lang %} xml:lang="{{ page.lang }}"{% endif -%} |
There was a problem hiding this comment.
what's wrong with always setting the lang?
| @@ -13,7 +13,7 @@ A taxonomy has five variables: | |||
| - `paginate_by`: if this is set to a number, each term page will be paginated by this much. | |||
| - `paginate_path`: if set, this path will be used by the paginated page and the page number will be appended after it. | |||
| For example the default would be page/1. | |||
| - `rss`: if set to `true`, an RSS feed will be generated for each term. | |||
| - `feed`: if set to `true`, a feed will be generated for each term. | |||
There was a problem hiding this comment.
can we add it's an atom feed by default there?
|
I've fixed the tests on next if you want to rebase. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This contains the requested parts of #986. That is, excluding the author metadata field and my replace_all filter.
Miscellaneous documentation fixes, template normalisation, Atom feeds and related variable changes and additions, a metadata field for when the page was last updated.
Open questions/remaining work: (per commit messages)
updatedneeds testsupdatedbe treated as defaulting todaterather thanNone? (Mostly affects templates, but also affects data model conceptually.)