Introduce Jinja2 filters for converting links in additional fields

[mosra]

Reworked to reflect the review comment, see below for the original proposal.

Until now, the link replacing worked only on article and page contents or summaries. With this patch, if one needes to replace links in custom fields, there are two new Jinja2 filters that can do that. For fields that are referenced in the FORMATTED_FIELDS setting, one can use the expand_links Jinja2 filter in the template, passing the field name as a parameter:

{{ article|expand_links('legal') }}

If the custom field consists of just one link (for example a link to article cover image for a social meta tag), one can use the expand_link Jinja2 filter:

{{ article|expand_link('cover') }}

With the above being in a template and FORMATTED_FIELDS setting containing the 'legal' field, a RST article making use of both fields could look like this:

An article
##########

:date: 2017-06-22
:legal: This article is released under `CC0 {filename}/license.rst`.
:cover: {filename}/img/article-cover.jpg

Disclaimer: I'm a Python noob and pretty new to Pelican as well, so it's possible I did something terribly wrong here. I am happy to incorporate changes based on your reviews. Things I'm not sure about:

• I added an entry to the changelog and to the contents docs as well. Hope I did that correctly :)
• I was not able to create any tests for this, because in order to test this there needs to be a template that makes use of the additional metadata. This is probably the case for FORMATTED_FIELDS as well -- however I was not able to find any test covering this setting.

Fixes #1169 (which is closed now).

---

Introduce INTERNAL_LINK_FIELDS setting (obsoleted by the above)

Metadata fields with links to internal content listed in this array will be properly converted to absolute URLs the same way as they do in page/article content and summary. This allows themes to specify custom metadata fields to be used for linking to internal content such as social media tags, page background etc. For example, a theme can define this for an article page:

<meta property="og:title" content="{{ article.title }}" />
<meta property="og:image" content="{{ article.cover }}" />

And then a reST article can include an image as an additional metadata field:

A cool article
##############

:date: 2017-06-06
:cover: {filename}/img/cool-photo.jpg

With the following in the settings:

SITEURL = "http://cool.site/"
INTERNAL_LINK_FIELDS = ['cover']

The generated article page will have this:

<meta property="og:title" content="A cool article" />
<meta property="og:image" content="http://cool.site/img/cool-photo.jpg" />

[avaris]

I'm very wary of __getattribute__ overrides and would like to avoid them if possible. As an alternative, what if we provide a function to the Jinja that can do this? e.g.

{{ article|expand_link('cover') }}

But I have to note that, this has the caveat of having to modify templates, whereas yours do not.

[mosra]

I did this to be consistent with the FORMATTED_FIELDS setting, where the modification went to the setting rather than to the template. (As an aside, internal links in the additional FORMATTED_FIELDS aren't expanded as well.)

I like your proposal -- I'll look into how Jinja could be extended and modify the patch accordingly.

But I have to note that, this has the caveat of having to modify templates, whereas yours do not.

Actually, this would make it nicely self-contained in the template, as one doesn't need to put any additional stuff into the settings. Now, what about doing something similar for FORMATTED_FIELDS as well, including the link expansion?

[mosra]

I reworked the code based on your proposal, liking it much better now. The original commits are still there for archival purposes, I can remove them after.

[justinmayer]

Any follow-up comments from @getpelican/reviewers ?

[iKevinY]

It would be good to have a some tests for these filters. You should be able to copy the format of the tests in TestDateFormatter, which basically just creates a dummy template containing the filter, calls the generator, and checks the output directly.

[iKevinY]

I might be missing something obvious, but what's the difference between expand_link and expand_links? Could they be consolidated into a single filter? Also, it might be worth pulling the logic for these filters out of generators.py and turning them into classes in utils.py, similar to how the strftime filter uses DateFormatter.

[mosra]

I moved the logic into classes in utils.py, also rebased on current master to prevent conflicts and removed the old commits.

The difference is the following and I don't think these two filters could be consolidated:

• expand_link treats a whole field as a link, it doesn't extract links from HTML. Using expand_links on such field would do nothing, because it would not detect any <a> or other tags containing links. I needed this for stuff in the <meta> tags (as shown above).
• expand_links treats the field as HTML and expands links inside the HTML tags. That's equivalent to what's done implicitly on article/page contents and summaries.

The tests are unfortunately not just about creating a dummy template (and that's why I didn't do them, as I mentioned in the PR description) -- the filter is accessing internals of a particular Content instance (article or page), in particular the _link_replacer(), so I think I would need to create a whole theme, some input file and unleash the generator on that. I'm not quite sure how to do that, help welcome.

[iKevinY]

Ah, thank you for the clarification, and much appreciate the refactoring as well. Regarding the tests, perhaps some other @getpelican/reviewers have some ideas on what to do here.

[avaris]

@mosra All you need is a custom jinja Environment with, say, a DictLoader containing your custom template and the extra filters (expand_link(s)), then some Content objects with appropriate metadata and context. Rest can be simply env.get_template('templatename').render(article=the_content) and checking the output.

[mosra]

Great, thank you, that was actually quite easy to do. Tests are done now.

(Sorry for the delay, was on vacation the past weeks.)

[iKevinY]

Thank you for making these changes (and to @avaris for the assistance)!

[iKevinY]

I momentarily had the thought that the filters would be more intuitive if the usage looked something like {{ article.cover | expand_link }}, but then I realized you need the entire context of the article object to do the expansion, so that's unfortunately not possible. 😞

[mosra]

Is there anything left on my side preventing this to be merged? :)

[ingwinlu]

i can have a look at it over the weekend earliest. if someone else can do it earlier please do so @getpelican/reviewers

[ingwinlu]

would it make sense to obsolete how we access metadata right now to

• pack them all into a dictionary
• expand on access via the writers similar to how content does it

This would remove the need for filters or themes implementing them.

[mosra]

@ingwinlu This would mean there would need to be additional settings next to FORMATTED_FIELDS that would mark additional metadata fields for link expansion (akin to the expand_link filter from this PR).

Personally, I am more in favor of the theme implementing a filter for given field than requiring the user to manually add such fields to FORMATTED_FIELDS in the settings -- so I would vote for an opposite, obsoleting FORMATTED_FIELDS and let the templates do that themselves via a filter. The reason is: if the theme does it, it's more self-contained -- the user doesn't need to mess with settings just to get it to render properly.

[ingwinlu]

if you do have the same functionality in two different places (auto expansion of content and expansion of metadata) with different parts having responsibility (one time pelican-dore, one time theme) of handling the replacement i would hardly ever call that 'self-contained'.

my point it is probably a way better solution and less tangled if we could have one responsible party and one place where expansion happens. either everything in themes, or everything in pelican-core.

The way todo both of those things is either:

• don't expand anything in pelican and only provide filters for themes (hard to do with compatibility in mind as it would break themes not expecting this change) or
• provide expansion of the complete metadata dictionary in content automatically and tell people to use that instead of the current article.metadatakey, resulting in expansion the same way for metadata as it is for content while also removing the need for additional filters

[mosra]

i would hardly ever call that 'self-contained'.

True :) I meant more like self-contained from user PoV -- "I install a theme and it just works, yay!".

don't expand anything in pelican and only provide filters for themes (hard to do with compatibility in mind as it would break themes not expecting this change) or

Yup, this is probably a no-go if we want to preserve backwards compatibility.

provide expansion of the complete metadata dictionary in content automatically and tell people to use that instead of the current article.metadatakey, resulting in expansion the same way for metadata as it is for content while also removing the need for additional filters

This would be nice. How would the dictionary approach look, then? I see one potentially problematic thing here, by having the expansion implicit for all fields, how one makes sure that it doesn't do unwanted things? For example:

• The expand_link filter expands fields containing just a link (see the :cover: field in my example above). How would one make sure that the implicit expansion gets triggered only on fields that are meant to be expanded as links?
• I assume this would replace the FORMATTED_FIELDS as well to make the "self-containedness" a reality, right? Or not? How would one tell Pelican which fields to format and which not?

[mosra]

Ookay. I see that this cuts deeper into the design than I originally thought, so I came up with a stripped-down idea that could be integrated and released in 3.8 without much hassle (and without providing any new user-visible functionality).

In short -- those Jinja2 filters can be easily implemented as a plugin that doesn't affect Pelican core at all, it just needs to have access to the _link_replacer() function, which is now a local function.

So what about this: I strip down the PR to just extracting the replacer() function to the _link_replacer() and remove everything else. Then it's possible for me to add the link expanding functionality via a plugin and it's also possible to implement this in Pelican core in any way you desire, after 3.8 is released.

Would this be better for now?

EDIT: Well, I just got a need for a third filter which takes an arbitrary string and expands it via the page / article object, so that triggered me to rethinking this feature as belonging rather in a plugin instead of being in core :)

[mosra]

Any chance of having at least the "link replacer" function made public (and discarding the rest of this PR)? I have a few plugins/patches that need to call it from outside.

Thank you.

[mosra]

In order to reduce confusion, I'm closing this in favor of 1f30306 that's contained in #2196 and #2260. I personally make use of just that part and moved away from having the Jinja filters in Pelican core (providing them via my own plugin instead).

One less PR in your backlog :)