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

pelican is not compatible with jinja2 #2442

Closed
amane-katagiri opened this issue Nov 14, 2018 · 23 comments
Closed

pelican is not compatible with jinja2 #2442

amane-katagiri opened this issue Nov 14, 2018 · 23 comments

Comments

@amane-katagiri
Copy link

Pelican 4.0.0 replaces %s in feed settings to {slug} (on #2432), but format filter in Jinja2 2.10 cannot process the formatted representation ({slug}). How can I fix it?

show versions

$ pip freeze
blinker==1.4
docutils==0.14
feedgenerator==1.9
Jinja2==2.10
Markdown==3.0.1
MarkupSafe==1.1.0
pelican==4.0.0
pkg-resources==0.0.0
Pygments==2.2.0
python-dateutil==2.7.5
pytz==2018.7
six==1.11.0
Unidecode==1.0.22

make project

$ pelican-quickstart
Welcome to pelican-quickstart v4.0.0.

This script will help you create a new Pelican-based website.

Please answer the following questions so this script can generate the files
needed by Pelican.


> Where do you want to create your new web site? [.]
> What will be the title of this web site? HOGE
> Who will be the author of this web site? HOGE
> What will be the default language of this web site? [en] ja
> Do you want to specify a URL prefix? e.g., https://example.com   (Y/n) y
> What is your URL prefix? (see above example; no trailing slash) http://example.com
> Do you want to enable article pagination? (Y/n) n
> What is your time zone? [Europe/Paris] Asia/Tokyo
> Do you want to generate a tasks.py/Makefile to automate generation and publishing? (Y/n) y
> Do you want to upload your website using FTP? (y/N) n
> Do you want to upload your website using SSH? (y/N) n
> Do you want to upload your website using Dropbox? (y/N) n
> Do you want to upload your website using S3? (y/N) n
> Do you want to upload your website using Rackspace Cloud Files? (y/N) n
> Do you want to upload your website using GitHub Pages? (y/N) n
Done. Your new project is available at /dev/shm/hoge
$ echo "THEME = 'simple'" >> pelicanconf.py
$ cat > content/hoge.md <<EOS
> Title: HOGE
> Category: hoge
> Date: 2018-11-14 11:00
> Slug: hoge
>
> HOGE
> EOS

make site

$ make publish
pelican /dev/shm/hoge/content -o /dev/shm/hoge/output -s /dev/shm/hoge/publishconf.py -D
(* snip *)
CRITICAL: TypeError: not all arguments converted during string formatting
Traceback (most recent call last):
(* snip *)
  File "/usr/local/lib/python3.6/dist-packages/pelican/themes/simple/templates/base.html", line 20, in block "head"
    <link href="{{ FEED_DOMAIN }}/{% if CATEGORY_FEED_ATOM_URL %}{{ CATEGORY_FEED_ATOM_URL|format(category.slug) }}{% else %}{{ CATEGORY_FEED_ATOM|format(category.slug) }}{% endif %}" type="application/atom+xml" rel="alternate" title="{{ SITENAME }} Categories Atom Feed" />
  File "/usr/local/lib/python3.6/dist-packages/jinja2/filters.py", line 685, in do_format
    return soft_unicode(value) % (kwargs or args)
TypeError: not all arguments converted during string formatting
make: *** [Makefile:72: publish] Error 1
@amane-katagiri
Copy link
Author

My ad hoc change:

  • Add "format": lambda x, *y, **z: x.format(*y, **z) to JINJA_FILTERS.
  • Update FEED|format(slug) to FEED|format(slug=slug) in template.

@mosra
Copy link
Contributor

mosra commented Nov 14, 2018

Oh, dang, this slipped through during the release 🙈

I think replacing FEED|format(slug) with FEED|replace('{slug}', slug) everywhere could do it ... unless the placeholder contains weird things like {slug!s}, in which case it would break.

Or maybe "just" {{ FEED.format(slug) }}? Not sure if that works tho.

@amane-katagiri
Copy link
Author

{{ FEED.format(slug) }} fails with KeyError: 'slug', {{ FEED.format(slug=slug) }} will be OK.

Either way, we need to make changes to the template (Users using official themes have to wait until the upstream is changed ...).

@avaris
Copy link
Member

avaris commented Nov 14, 2018

Yeah, should be {{ FEED.format(slug=slug) }}. Odd that none of the tests picked this.

@amane-katagiri
Copy link
Author

I think #2432 should be reverted, or at least be changed to have compatibility with "%s".

Personally, I do not think that there is a big difference between "%s" and "{slug}" in function of simple string substitution. It is stupid that we lose compatibility due to a trivial improvement.

@avaris
Copy link
Member

avaris commented Nov 14, 2018

Honestly, I don't like templates doing these kind of stuff. It opens the possibility of breaking things for changes like this (which are supposedly internal). Instead, these should all be things like:

{{ category.feed_atom_url }}
{{ category.feed_rss_url }}
etc...

@rinze
Copy link

rinze commented Nov 14, 2018

Can we assume that a lot of templates are doing this anyway? I was testing yesterday and got the same error, using a slightly modified version of the bootstrap3 theme. I managed to debug all the way to the template but my jinja2 skills are nil.

@LouisJackman
Copy link
Contributor

I encountered something that looks similar on my own site, apologies if it's actually unrelated:

$ pelican content -o output -s publishconf.py
CRITICAL: TypeError: not all arguments converted during string formatting

My fix was to update these lines in one of my template files:

<link href="{{ FEED_DOMAIN }}/{{ CATEGORY_FEED_ATOM|format(category.slug) }}" type="application/atom+xml" rel="alternate" title="{{ SITENAME }} Categories Atom Feed" />
<link href="{{ FEED_DOMAIN }}/{{ CATEGORY_FEED_RSS|format(category.slug) }}" type="application/rss+xml" rel="alternate" title="{{ SITENAME }} Categories
RSS Feed" />

To these (note the slug= additions to the format invocation):

<link href="{{ FEED_DOMAIN }}/{{ CATEGORY_FEED_ATOM|format(slug=category.slug) }}" type="application/atom+xml" rel="alternate" title="{{ SITENAME }} Categories Atom Feed" />
<link href="{{ FEED_DOMAIN }}/{{ CATEGORY_FEED_RSS|format(slug=category.slug) }}" type="application/rss+xml" rel="alternate" title="{{ SITENAME }} Categories
RSS Feed" />

Am I right in understanding that some templates will need to be updated from positional to keyword arguments for slug?

Thanks a lot to all involved in the Pelican 4.0 release, by the way. It's a great static site generator!

amitsaha added a commit to amitsaha/pelican-svbhack that referenced this issue Nov 16, 2018
amitsaha added a commit to amitsaha/pelican-svbhack that referenced this issue Nov 16, 2018
@mirekdlugosz
Copy link
Contributor

Can we assume that a lot of templates are doing this anyway?

Out of 124 themes in getpelican/pelican-themes , 36 refers TAG_FEED_RSS or TAG_FEED_ATOM. That's about 30%. While I haven't investigate each and every one of them, it seems that common idiom was {{ TAG_FEED_RSS|format(tag.slug) }}.

I would consider this major breakage of backwards-compatibility. At the very least, we should add some note into current documentation / changelog, telling what is broken and how to recover from it.

@avaris
Copy link
Member

avaris commented Nov 17, 2018

In the spirit of backwards compatibility, we can ship a monkey-patched jinja filter format but eventually I'd rather have this "themes calculating core attributes" be deprecated and replaced with object attributes.

@justinmayer
Copy link
Member

@Mirzal said:

At the very least, we should add some note into current documentation / changelog, telling what is broken and how to recover from it.

@Mirzal: Would you like to propose some verbiage to that effect? I'd be happy to add your contributed text to the release post.

@mirekdlugosz
Copy link
Contributor

@justinmayer sure!

I would add this paragraph to release post. Personally I would put it right below "Upgrading from previous releases" header.

Some user-submitted themes use positional formatting on object-related feed URLs, which will cause site to fail to build with error TypeError: not all arguments converted during string formatting. Theme needs to be updated, e.g. substitute TAG_FEED_ATOM|format(tag.slug) with TAG_FEED_ATOM|format(slug=tag.slug). Affected variables are: CATEGORY_FEED_ATOM, CATEGORY_FEED_ATOM_URL, CATEGORY_FEED_RSS, CATEGORY_FEED_RSS_URL, AUTHOR_FEED_ATOM, AUTHOR_FEED_ATOM_URL, AUTHOR_FEED_RSS, AUTHOR_FEED_RSS_URL, TAG_FEED_ATOM, TAG_FEED_ATOM_URL and TAG_FEED_RSS.

On changelog (if we are OK with changing changelog without releasing new version), I would change point about {slug} to read:

All settings for slugs now use {slug} and/or {lang} rather than %s. If %s-style settings are encountered, Pelican will emit a warning and fallback to the default setting. Some user-submitted themes might try to format setting values and fail to build site due to TypeError. In such cases, theme needs to be updated - e.g. instead of TAG_FEED_ATOM|format(tag.slug), use TAG_FEED_ATOM|format(slug=tag.slug).

Raw md text for easier copying

@avaris
Copy link
Member

avaris commented Nov 20, 2018

Hmm, if TAG_FEED_ATOM|format(slug=tag.slug) works, then we would be able to do it backwards compatible by omitting names (i.e. {slug} -> {}). TAG_FEED_ATOM|format(tag.slug) should work again in that case. I'm OK, with this.

justinmayer added a commit that referenced this issue Nov 29, 2018
@justinmayer
Copy link
Member

@Mirzal: Many thanks! That's super helpful. The change log and release post are in reST format, so I converted your docs appropriately. I updated the change log via 49c77d7 and also appended the relevant notes to the release post. Once again, thank you. Much appreciated! 😄

I'll leave this issue open for now in case someone wants to implement the backwards-compatibility @avaris mentioned above.

@muellermartin
Copy link
Contributor

I noticed that while using the scheme TAG_FEED_ATOM|format(slug=tag.slug) does not result in the described error, it does not format the string as expected (the variable will be empty). What worked for me was additionally replacing the pipe symbol with a dot, e.g. TAG_FEED_ATOM.format(slug=tag.slug) as suggested above. I think this is because Jinja2 only supports the percentage-style string substitution.

@Kristinita
Copy link
Contributor

Status: ✔️ Fixed for me by @muellermartin comment.

  • Pelican 4.0.1

1. Non-expected behavior

If pipe:

<link href="{{ FEED_DOMAIN }}/{{ CATEGORY_FEED_ATOM|format(slug=category.slug) }}" rel="alternate" type="application/atom+xml">

I get, for example:

<link href="./feeds/{slug}.atom.xml" rel="alternate" type="application/atom+xml">

2. Expected behavior

Else dot:

<link href="{{ FEED_DOMAIN }}/{{ CATEGORY_FEED_ATOM.format(slug=category.slug) }}" rel="alternate" type="application/atom+xml">

I get, for example:

<link href="./feeds/kira-goddess.atom.xml" rel="alternate" type="application/atom+xml">

Thanks.

@jzrchilel
Copy link

Guys, this line of my publishconf.py is causing me the error

### CATEGORY_FEED_ATOM = 'feeds/{slug}.atom.xml'

I tried with:

CATEGORY_FEED_ATOM = 'feeds/${slug}.atom.xml'

but it doesn't worked. How can i solved this?
(I commented the line and make github and make publish worked fine)

here's my full publishconf.py:

#!/usr/bin/env python
# -*- coding: utf-8 -*- #
from __future__ import unicode_literals

# This file is only used if you use `make publish` or
# explicitly specify it as your config file.

import os
import sys
sys.path.append(os.curdir)
from pelicanconf import *

# If your site is available via HTTPS, make sure SITEURL begins with https://
SITEURL = ''
RELATIVE_URLS = False

FEED_ALL_ATOM = 'feeds/all.atom.xml'
CATEGORY_FEED_ATOM = 'feeds/{slug}.atom.xml'

DELETE_OUTPUT_DIRECTORY = True

# Following items are often useful when publishing

#DISQUS_SITENAME = ""
#GOOGLE_ANALYTICS = ""

@jorgesumle
Copy link
Contributor

@jzrchilel, see http://docs.getpelican.com/en/stable/contribute.html#how-to-get-help

@ardeidae
Copy link

Could anyone explain why format works with a dot and not with a pipe, please?
In the jinja documentation it is explicitly stated that filters must be used with pipes.

I am very interested to understand why.

@avaris
Copy link
Member

avaris commented Mar 21, 2019

@ardeidae because Jinja's format filter is a shortcut for (rather old) % style formatting and current implementation uses newer .format style formatting.

By doing, 'somestring'.format(...) inside template, you're actually running the whole thing, therefore using .format style.
And by doing Jinja filter way, 'somestring'|format(...), you're in turn doing 'somestring' % ....

@ardeidae
Copy link

@avaris Thanks for the explanation. So, I suppose the dot notation is accepted by jinja as python code?

@avaris
Copy link
Member

avaris commented Mar 22, 2019

@ardeidae Yes-ish. Jinja does some magic under the hood but essentially it boils down to getting the .format method of that formatting string and calling with the parameters passed.

@justinmayer
Copy link
Member

I get the impression that this issue can be closed since the relevant change has been documented in the release post and change-log. If that's not the case, please post a comment here and we can re-open this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests