Skip to content
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

Merged
merged 5 commits into from
Oct 28, 2023

Conversation

djramones
Copy link
Contributor

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() to generate_context(). Initially, the new context variable only needed the list of period/period_num tuples and the corresponding URLs, but in order to retain the generate_period_archives() functionality, I decided to include the generation of save_as and filtered articles/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

  • Ensured tests pass and (if applicable) updated functional test output
  • Conformed to code style guidelines by running appropriate linting tools
  • Added tests for changed code
  • Updated documentation for changed code

Also, set default patterns for time-period *_ARCHIVE_URL settings.
@djramones djramones mentioned this pull request Jun 18, 2023
2 tasks
@justinmayer justinmayer requested a review from avaris June 19, 2023 04:47
@justinmayer justinmayer requested a review from a team July 27, 2023 20:22
pelican/generators.py Outdated Show resolved Hide resolved
pelican/settings.py Outdated Show resolved Hide resolved
@avaris avaris self-assigned this Oct 28, 2023
Copy link
Contributor

@offbyone offbyone left a 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?

Copy link
Contributor

@boxydog boxydog left a 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.

pelican/generators.py Outdated Show resolved Hide resolved
pelican/tests/test_generators.py Outdated Show resolved Hide resolved
pelican/tests/test_generators.py Outdated Show resolved Hide resolved
pelican/tests/test_generators.py Outdated Show resolved Hide resolved
@justinmayer
Copy link
Member

@offbyone: Normally we handle change-log entries via individual RELEASE.md files that should be included with PRs, but since we are merging a bunch of PRs into a single Pelican 4.9 release, I will handle this particular change-log bundle manually. You can see my collection so far in: #3219

Copy link
Member

@justinmayer justinmayer left a 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. 💯

@justinmayer justinmayer merged commit 85bf982 into getpelican:master Oct 28, 2023
10 checks passed
@djramones djramones deleted the period-archives-context branch October 29, 2023 00:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

Period Archive Quirk
5 participants