-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Implement period_archives common context variable #3148
Implement period_archives common context variable #3148
Conversation
Also, set default patterns for time-period *_ARCHIVE_URL settings.
So that we don't contribute to further clutter in generate_context.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good.
Should you add an entry to the changelog for this, or will @justinmayer do that on release?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a few nits, but I don't see anything blocking.
Since I am new to the codebase, I could be missing something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many thanks to @djramones for the enhancement and to @boxydog, @offbyone, and @avaris for reviewing. 💯
Motivation
As discussed in #2773 (specifically this comment), a context variable that lists the generated period archives along with their URLs should be useful for those who'd like to have an index to those archives. Before this, users had to manually write links to period archives in content or templates, or even expect site visitors to explore editing URLs on their browser address bar.
Implementation
I moved code from
ArticlesGenerator.generate_period_archives()
togenerate_context()
. Initially, the new context variable only needed the list ofperiod
/period_num
tuples and the corresponding URLs, but in order to retain thegenerate_period_archives()
functionality, I decided to include the generation ofsave_as
and filteredarticles
/dates
collections.Also, I decided it appropriate to set default patterns for time-period
*_ARCHIVE_URL
settings, so that anyone using the new context var just by setting*_ARCHIVE_SAVE_AS
would get sensible URLs by default too.I thought about providing a default template implementing the new variable (perhaps named
period_archives_index.html
), but this is probably more of a website- or theme-level detail, so I just put in a sample HTML snippet in the docs.Pull Request Checklist
Resolves: #2773