Jieiku / abridge

Fast & Lightweight Zola Theme
https://abridge.pages.dev/
MIT License
147 stars 41 forks source link

Limit section post listings to that section's pages #118

Closed wold5 closed 1 year ago

wold5 commented 1 year ago

Hi,

It seems that post lists for a section (_index.md) aren't currently limited to pages from that section. Shown instead are all posts (for all sections). This concerns _index.md using template = "posts.html".

The cause seems get_section() using the root _index.md like this:

{%- set section_item = get_section(path="_index.md") %}

It appears to need the parent paths as well: (after adidoks, here)

{%- set section_item = get_section(path= current_path ~ "_index.md" | trim_start_matches(pat="/")) %}

but doing so breaks paginating:

Error: Failed to render pager 3
Error: Reason: Failed to render 'posts.html'
Error: Reason: Function call 'get_section' failed
Error: Reason: Section `posts/page/3/_index.md` not found.

Or are there other ways?

Jieiku commented 1 year ago

Can you walk me through how this happened?

What would I need to change locally to see the same error?

If I can reproduce the error then I can probably fix it, or if you have managed to fix it locally, without breaking the posts page then I would be happy to review and commit a fix if you wanted to submit a pull request.

For instance, here is what I have done so far:

git clone https://github.com/Jieiku/abridge
cd abridge
git checkout 5c3cc8966834a0d140f82aa178ba26109f79dd8f
zola serve
Building site...
Checking all internal links with anchors.
> Successfully checked 1 internal link(s) with anchors.
-> Creating 11 pages (0 orphan) and 2 sections
Done in 433ms.

Listening for changes in /home/jieiku/.dev/abridge-sec/abridge/{config.toml,content,sass,static,templates}
Press Ctrl+C to stop

Web server is available at http://127.0.0.1:1111

The page seems to load OK for me using Zola 0.17.2 on the same commit:

2023-05-15_09-09-03

wold5 commented 1 year ago

I did find one workaround using custom templates, that suffices (more on that later).

But to clarify: next to the posts in /content/ I have several subdirectories, like:

/content/software/
/content/software/manuals/
/content/2022/
/content/2023/

Zola considers each subdirectory to be a section. Each has an _index.md file for generating an index.html.

The intent is that when opening, say, /content/software/, only content for that section is shown. This worked with a previous theme**, so I expected similar behaviour - my fault. When using Abridges posts.html template in a section's _index.md, it shows all posts for all sections.

The workaround mentioned, involves adding a customized copy of Abridge's posts.html template - one for each section. In it, the path used by get_section() line is modified like this: {%- set section_item = get_section(path="**software**/_index.md") %} for /content/software/, and so on. This template is then used in _index.md.

Still, it may be worthwhile to make a generalized version of posts.html. I can look into it later, or you may consider it for the re-factor.

** tale-zola, which seems to have been abandoned. Its index.html template for _index.md works in the intended way.

Jieiku commented 1 year ago

Ahhhh, yes, that makes sense... and it should help me to test for that I think. The refactor is more or less complete, just waiting for the next version of zola to drop.

I am going to try and reproduce your error if I can, now that I know what causes it.

Jieiku commented 1 year ago

I just fixed this: 135f61c95616f66fb79bf1c6d930956564e39e7b

I am assuming this is the functionality you were looking for.

This allows the normal Posts page to still work, or it can also work on an individual section.

Posts.html template from Posts page:

2023-05-16_10-00-40

Posts.html template from Software Section:

2023-05-16_10-01-59

Can you confirm this is the functionality you were looking for? If so this will be in the refactor.

Jieiku commented 1 year ago

I was able to generate your error, it happened when I tried to specify index.html as the template for sections.

I have extended index.html to work as the template for sections in addition to posts.html.

Now you can use index.html or posts.html as your template for your sections: https://github.com/Jieiku/abridge/commit/0e18bc6a59553dee3adf1ec36c591f9a4ee37c7c

These changes are in the refactor branch, they will go live once the next version of zola drops.

If you find any errors with this implementation then please report back. I don't use sections, so my testing is limited.

Thank you!

edit: After adding MORE content, I found another bug, let me look this over a bit more.

Jieiku commented 1 year ago

Adidoks seems to hard code the section when they use pagination: https://github.com/aaranxu/adidoks/blob/5c698271c460046034605b743a15196b12e32887/templates/docs/section.html#L9

I have made it so that they will be dynamic, allowing to set index.html or posts.html as your template for any section without the need to explicitly specify what section it is.

The amount of Tera functions that I stringed together to accomplish this is a bit like a magic spell... so it's possible something could break this in the future.

posts: https://github.com/Jieiku/abridge/blob/refactor/templates/posts.html

index: https://github.com/Jieiku/abridge/blob/8f9d3e1c449ed28b2014c78519caf733457cca8b/templates/index.html#L62-L90

Please let me know if you run into any issues, thanks!

wold5 commented 1 year ago

Great. https://github.com/Jieiku/abridge/commit/135f61c95616f66fb79bf1c6d930956564e39e7b already made me happy. index.html adds pagination.

You've made the regex more robust in https://github.com/Jieiku/abridge/commit/8f9d3e1c449ed28b2014c78519caf733457cca8b A crude alternative was to match on page instead (with containing()) like:

{%- if current_path is containing("/page/") %}

making /posts/2022/ filter to just 2022. And preventing the earlier pagination error. Both are fixed anyway. An alternative would be checking some pagination indicator in the content, if it exists.

The amount of Tera functions that I stringed together to accomplish this is a bit like a magic spell... so it's possible something could break this in the future.

For now it works. :)

For the sec = lines, can't the trim_start_matches pattern be concatenated? Like:

      {%- if lang == config.default_language %}
        set languagepath = ""
      {% else %}
        set languagepath = lang ~ "/"
      {%- endif %}

      {%- set sec = current_path | trim_start_matches(pat="/" ~ languagepath) | split(pat="/page/") | slice(end=1) | join(sep="") | trim_end_matches(pat="/") %}
        {%- set sec = sec ~ "/" %}
      {%- endif %}

languagepath should then fold if empty. But there may other considerations to take into account.

wold5 commented 1 year ago

Let me (ab)use the thread for sharing two slightly related changes made to posts.html, in case you're interested (no fancy github diffs yet, sorry.).

Include the _index.md title if set

    <div>
        {% if section_item.title %}
            <h2>{{section_item.title}}</h2>
        {%- endif %}

and for hide any dates, conditional on there being a title - a page.extra variable could be used as well. This is for the aforementioned /software/ index.

    {%- set hide_dates = section_item.title %}
    <div>

    (...)

      {% if not hide_dates %}
        <h2>{{ year }}</h2>
      {%- endif %}
      {%- for post in posts %}
        <p><a href="{{ post.permalink | safe }}{%- if config.extra.uglyurls %}index.html{%- endif %}">{{ post.title }}</a>{% if not hide_dates %}- <time datetime="{{ post.date }}">{{ post.date | date(format="%F") }}</time>{% endif %}</p>
      {%- endfor %}
Jieiku commented 1 year ago

languagepath should then fold if empty.

great idea, I implemented it just now. https://github.com/Jieiku/abridge/commit/fdb7ef80980bfae7d070be550b89102568f06834

and for hide any dates, conditional on there being a title

I am not sure I like the idea of hiding the dates when there is a title, because other people may actually want the dates. I did actually implement a twist on this though, I show the title at the top if it exists, then use a smaller heading for the year.

I could also add a config.toml value to hide dates further though...

wold5 commented 1 year ago

and for hide any dates, conditional on there being a title

I am not sure I like the idea of hiding the dates when there is a title, because other people may actually want the dates. I did actually implement a twist on this though, I show the title at the top if it exists, then use a smaller heading for the year.

I could also add a config.toml value to hide dates further though...

I won't say no to that, but note the dates are needed on the homepage (using index.html): just not in the software list. A toggle in _index.md would be great, but it will probably break if _index.md is unavailable (like the earlier errors).

Anyway, for now a local template will work as well, and save you some work - and BTW, thank you for the changes!

Jieiku commented 1 year ago

Aye, thanks for the suggestion, makes abridge useful to a wider range of people.

wold5 commented 1 year ago

A toggle in _index.md would be great, but it will probably break if _index.md is unavailable (like the earlier errors).

Well, this requires a variable under [extra]. However, if one page lacks [extra], this breaks. Forcing users to add [extra] won't do.

There are also {% block %} template overrides. It would allow overriding just the iterator (or inner loop) with a template. Would you be willing to add in a block iterator like below?

     {%- block content_iterator %} <----

        {%- for year, posts in section_item.pages | group_by(attribute="year") %}
        (...)
      {%- endfor %}
      {%- endfor %}

     {%- endblock content_iterator %} <---
Jieiku commented 1 year ago

No, its easier than you think, you just check for extra first.

{%- if page.extra %}
{%- if page.extra.hidedates %}
{%- if page.extra.hidedates != true %}
  - <time datetime="{{ post.date }}">{{ post.date | date(format="%F") }}</time>
{%- endif %}
{%- endif %}
{%- endif %}

Also this is assuming you want to set it on a per page basis, the config value could also just be placed into the config.toml if you want it this way for all sections. (config.extra.hide_section_dates)

or if we wanted to do it on a per section basis:

{%- if sec != "" %}
{%- if section_item.extra %}
{%- if section_item.extra.hidedates %}
{%- if section_item.extra.hidedates != true %}
  - <time datetime="{{ post.date }}">{{ post.date | date(format="%F") }}</time>
{%- endif %}
{%- endif %}
{%- endif %}
{%- endif %}

I think setting the value in the config.toml or the section _index.md will be the least amount of work for end users that want to use the feature... (so that they dont have to edit every single post.) If the goal is to do this for all sections then config.toml will be the easiest because you would only have to set it in one place.

wold5 commented 1 year ago

Also this is assuming you want to set it on a per page basis, the config value could also just be placed into the config.toml if you want it this way for all sections. (config.extra.hide_section_dates)

I think setting the value in the config.toml or the section _index.md will be the least amount of work for end users that want to use the feature... (so that they dont have to edit every single post.) If the goal is to do this for all sections then config.toml will be the easiest because you would only have to set it in one place.

My vote would be for the section, as it suits my situation. Both section and config.toml can be done (preferring flexibility, to a degree).

No, its easier than you think, you just check for extra first.

{%- if page.extra %}
{%- if page.extra.hidedates %}
{%- if page.extra.hidedates != true %}
  - <time datetime="{{ post.date }}">{{ post.date | date(format="%F") }}</time>
{%- endif %}
{%- endif %}
{%- endif %}

Nice. Like checking pointers :) The conditions can be chained and undefined variables default to false (see link):

{% set hide_section_dates = false %}
{% if sec %}
    {# the section always overrides the config #}
    {%- if section_item.extra and 'hide_section_dates' in section_item.extra %}
        {% set hide_section_dates = section_item.extra.hide_section_dates %}
    {%- elif config.extra.hide_section_dates %}
        {% set hide_section_dates = true %}
    {% endif %}
{% endif %}

Not sure about if sec. Python considers "" to be false (Jinja/Tera/... largely follow Python).

During testing, toggling the config value the 2nd time required me to restart Zola. Doesn't seem to involve the logic (occurs when using hide_section_dates in config.extra variant).

Jieiku commented 1 year ago

ah your right, I wrote most of that in a couple minutes without testing any of it, simply using if sec is better than evaluating it against something.

I will go ahead and implement it with both sections and config then.

Jieiku commented 1 year ago

Done: https://github.com/Jieiku/abridge/blob/refactor/templates/posts.html

wold5 commented 1 year ago

ah your right, I wrote most of that in a couple minutes without testing any of it, simply using if sec is better than evaluating it against something.

I will go ahead and implement it with both sections and config then.

Ah it's ok. I had fun fixing it, got carried away a bit perhaps ;)

Jieiku commented 1 year ago

interestingly an empty string does not evaluate as false in Tera... maybe I subconsciously remembered this...

so I ended up comparing it after all:

https://github.com/Jieiku/abridge/blob/def54c8ceaadb13248bd93891dca1366003b74c6/templates/posts.html#LL42C5-L42C24