Template improvements #3760

aknrdureegaesr · 2024-02-20T13:59:52Z

Pull Request Checklist

I’ve read the guidelines for contributing.
I updated AUTHORS.txt and CHANGES.txt (if the change is non-trivial) and documentation (if applicable).
I tested my changes.

Description

New features:

Trace template usage if env variable NIKOLA_TEMPLATES_TRACE is set.
Enable Jinja2 extensions via theme configuration.
Clarify template documentation: A parent template is not strictly needed.

Additional code improvements
helpful for those developing templates who actually read the code:

Added several typing hints and improved others.
A few code comments were added or improved.

Details

The code has three commits:

The first has no change, but the commit comment is the above notice, which I propose as the commit message of the entire commit once this is squashed on merge.
The second has only the changes to implement template usage logging.
The third implements ability to use Jinja2 extensions via theme configuration.

New features: - Trace template usage if env variable NIKOLA_TEMPLATES_TRACE is set. - Enable Jinja2 extensions via theme configuration. - Clarify template documentation: A parent template is not strictly needed. Additional code improvements helpful for those developing templates who actually read the code: - Added several typing hints and improved others. - A few code comments were added or improved.

…guration. Currently only implemented configuration is for the Jinja templating engine, it is now possible to configure Jinja extensions. Also some improvements to make the template handling code better and more readable; mostly typing.

Kwpolska · 2024-02-20T22:00:28Z

docs/theming.rst

@@ -130,8 +134,9 @@ The following keys are currently supported:

    The parent is so you don’t have to create a full theme each time: just
    create an empty theme, set the parent, and add the bits you want modified.
-    You **must** define a parent, otherwise many features won’t work due to
-    missing templates, messages, and assets.
+    It is strongly recommended you define a parent.  If you don't, many features


Nope. Every theme must inherit from at least base/base-jinja, we explicitly refuse to support any other scenario (even if the code does not prevent this, we would prefer not to admit that it is a possibility, and we reserve the right to break it at any time.)

I have a humble nice little blog in mind. For that, I absolutely want my HTML generated without any <div> in it. I want to prove that semantic HTML can be done. I also think that my little web site should work nicely without any Javascript (ROCA-style). This is not theory, this has really been my intention when I came to Nikola in the first place. If something in the templating changes, I'm willing to accept that my site will break on Nikola update and I'll have to fix.

I am willing to accept that I'll have to do a lot of theme customization to get that. As a platform as it stands now, Nikola would technically let me do this.

You are saying that Nikola strategically does not support my use case and I should go look elsewhere?

See https://www.famsik.de/opinions for background. In my opinion, Nikola could serve nicely for people who want to do experiments with stuff like that. But accepting that as a valid Nikola use case is a project-political decision.

Thinking about it a bit further: From my point of view, it would be useful if Nikola would simply officially state that

it is possible to go without a parent template

things might break on any Nikola version update, as new template files become required from the code.

As for support, that would mean for Nokia a new template requirement needs to be announced in the change file.

Would that be something you'd be willing to do if asked nicely?

Here’s some example wording that might work:

While it is possible to create a theme without a parent, it is strongly discouraged and not officially supported. The base and base-jinja themes provide assets, messages, and generic templates that Nikola expects to be able to use in all sites. That said, if you are making something very custom, Nikola will not prevent the creation of a theme without base, but you will need to manually determine which templates and messages are required in your theme.

It’s unlikely we’ll ever introduce a new template, but I guess it might be mentioned in the changelog if that happens.

As far as the changelog is concerned: Anything in the templates can be user-changed. So I think Nikola has to mention any change in the template arena in the changelog anyway. I see that as independent of the question "with base or not".

Some stance like the one you suggest would answer my concerns. Thanks!

For some detail fine-tuning: I would prefer to warn people of consequences, yes, but otherwise treat them as adults who are able to make grown-up decisions. So I would think it nicer if we left out the "strongly discouraged" part. Maybe warn people that growing their own theme is harder than they might think at first sight.

As for the "not officially supported": Before people file a bug, we should require them to add the pertinent parent. If that makes the bug go away, it must not be filed.

Anything else the "not officially supported" entails?

“Strongly discouraged” and “not officially supported” are meant to suggest “don’t do this unless you really know what you’re doing”. For user-facing software, phrases like this are a good idea IMO to clearly show the limitations.

“Not officially supported” means basically what you said, i.e. we won’t help with issues caused by a parentless theme, and we don’t guarantee it will always work.

Via the polyfill discussion, a solid reason surfaced why Nikola might rethink its policy of not officially supporting templates without parent.

Where GDPR rules, one needs a certain written agreement with any company before sending web traffic their way. This is a consequence of GDPR art 28, in particular section 2.. (I've personally been one of many developers who ported all references to Google fonts from external to locally hosted, in the web presence of a big Germany company (some 300.000+ employed).)

I believe small, private web sites are inside the target group of Nikola. People responsible for such web sites might not have the time and legal power to establish any such agreements legally needed. So they may choose to use no external resources whatsoever, but self-host everything their web site needs.

Nikola makes not promises regarding self-hosting presently. Indeed, build-in Nikola templates freely use external resources. Any Nikola version update might conceivably introduce or change external references in the HTML produced. Wanting to play it safe is a very solid reason to use a template without parent.

Generally there's the option use_cdn, but as #3763 shows this isn't always respected, like for polyfill.io.

I'm personally also using a template without a parent for my sites. (More precisely I have my own base template, and some templates deriving from my base.)

Kwpolska · 2024-02-20T22:01:40Z

docs/theming.rst

@@ -163,6 +168,13 @@ The following keys are currently supported:
  * ``ignored_assets`` — comma-separated list of assets to ignore (relative to
    the ``assets/`` directory, eg. ``css/theme.css``)

+* ``jinja`` - This section is ignored unless your theme's engine is ``jinja``.
+
+  * ``extensions`` - comma-separated list of


I’m OK with this, but perhaps it would be more useful to do this in conf.py instead?

I first thought the opportune place for template configuration is the theme configuration file. It is there I tell Nikola which engine to use in the first place.

On the other hand, conf.py would allow real python code, as opposed to mere strings. It also has the global context and other such goodies, that may interact with the template. Since presenting this PR, I've also encountered the desire to have my templates bark at undefined variables as opposed to just printing them as an empty string.

So I'll agree doing something like this in conf.py is the better idea, and doing it in a way that allows any configuration change (via Python), as opposed to just allowing extension activation and nothing else.

Kwpolska · 2024-02-20T22:01:53Z

docs/theming.rst

@@ -184,8 +196,16 @@ so ``post.tmpl`` only define the content, and the layout is inherited from ``bas

 Another concept is theme inheritance. You do not need to duplicate all the
 default templates in your theme — you can just override the ones you want
-changed, and the rest will come from the parent theme. (Every theme needs a
-parent.)
+changed, and the rest will come from the parent theme.  If your theme does not


Again, unsupported and should not be documented.

Kwpolska · 2024-02-20T22:02:45Z

nikola/__init__.py

 __version__ = '8.3.0'
+# A flag whether logging should emmit debug information:


Suggested change

# A flag whether logging should emmit debug information:

# A flag whether logging should emit debug information:

Kwpolska · 2024-02-20T22:03:55Z

nikola/__init__.py

+# When this flag is set, fewer exceptions are handled internally;
+# instead they are left unhandled for the run time system to deal with them,
+# which typically leads to the stack traces being exposed.


Suggested change

# When this flag is set, fewer exceptions are handled internally;

# instead they are left unhandled for the run time system to deal with them,

# which typically leads to the stack traces being exposed.

# A flag to show tracebacks of unhandled exceptions.

# An alternative to the DEBUG flag with less noise.

(NIKOLA_DEBUG used to be very spammy with yapsy, it’s still not great to have it always on; export NIKOLA_SHOW_TRACEBACKS=1 is in my .zshrc though.)

Kwpolska · 2024-02-20T22:13:03Z

nikola/plugins/template/jinja.py

    dependency_cache = {}
    per_file_cache = {}
+    _user_configured_jina_extensions: List[str] = []


Typo: jina → jinja

Kwpolska · 2024-02-20T22:14:00Z

nikola/plugins/template/jinja.py

+    if jinja2 is None:
+        lookup = None
+    else:
+        lookup: Optional[jinja2.Environment] = None


Consider using a string annotation:

Suggested change

if jinja2 is None:

lookup = None

else:

lookup: Optional[jinja2.Environment] = None

lookup: 'Optional[jinja2.Environment]' = None

Kwpolska · 2024-02-20T22:18:10Z

nikola/utils.py

-    if isinstance(string_or_iterable, (str, bytes)):
-        return string_or_iterable
-    elif isinstance(string_or_iterable, Iterable):
+    if isinstance(string_or_iterable, Iterable):


This won’t work, please undo:

>>> isinstance("a", typing.Iterable) True

Kwpolska · 2024-02-20T22:18:45Z

nikola/utils.py


    >>> smartjoin('; ', 'foo, bar')
    'foo, bar'
    >>> smartjoin('; ', ['foo', 'bar'])
    'foo; bar'
    >>> smartjoin(' to ', ['count', 42])
    'count to 42'
+
+    The treatment of bytes (calling str(string_or_iterable)) is somewhat dubious. Is this needed?


Might be worth checking if bytes is ever passed here.

Kwpolska · 2024-02-20T22:28:00Z

nikola/__init__.py

 DEBUG = bool(os.getenv('NIKOLA_DEBUG'))
+# A flag whether special templates trace logging should be generated:


Suggested change

# A flag whether special templates trace logging should be generated:

# A flag whether special templates trace logging should be generated:

Kwpolska · 2024-02-20T22:33:10Z

nikola/log.py

+
+
+if TEMPLATES_TRACE:
+    init_template_trace_logging("templates_log.txt")


Consider using the .log extension, to avoid confusion with things like reST posts (which can also end with .txt).

Suggested change

init_template_trace_logging("templates_log.txt")

init_template_trace_logging("templates_trace.log")

aknrdureegaesr added 5 commits February 20, 2024 14:27

Templates trace logging introduced.

fc7a75f

Adding a docstring as required by the automated test.

074f59d

Backporting to Python 3.8.

b46b121

aknrdureegaesr force-pushed the template_improvements branch from 0f3c362 to b46b121 Compare February 20, 2024 15:34

Kwpolska requested changes Feb 20, 2024

View reviewed changes

Kwpolska reviewed Feb 20, 2024

View reviewed changes

aknrdureegaesr marked this pull request as draft February 25, 2024 10:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Template improvements #3760

Template improvements #3760

aknrdureegaesr commented Feb 20, 2024

Kwpolska Feb 20, 2024

aknrdureegaesr Feb 22, 2024

aknrdureegaesr Feb 22, 2024

aknrdureegaesr Feb 23, 2024 •

edited

Kwpolska Feb 23, 2024

aknrdureegaesr Feb 24, 2024

aknrdureegaesr Feb 24, 2024 •

edited

Kwpolska Feb 24, 2024

aknrdureegaesr Mar 2, 2024 •

edited

felixfontein Mar 2, 2024

Kwpolska Feb 20, 2024

aknrdureegaesr Feb 25, 2024 •

edited

Kwpolska Feb 20, 2024

Kwpolska Feb 20, 2024

Kwpolska Feb 20, 2024

Kwpolska Feb 20, 2024

Kwpolska Feb 20, 2024

Kwpolska Feb 20, 2024

Kwpolska Feb 20, 2024

Kwpolska Feb 20, 2024

Kwpolska Feb 20, 2024

		__version__ = '8.3.0'
		# A flag whether logging should emmit debug information:

	# A flag whether logging should emmit debug information:
	# A flag whether logging should emit debug information:

		DEBUG = bool(os.getenv('NIKOLA_DEBUG'))
		# A flag whether special templates trace logging should be generated:



		if TEMPLATES_TRACE:
		init_template_trace_logging("templates_log.txt")

	init_template_trace_logging("templates_log.txt")
	init_template_trace_logging("templates_trace.log")

Template improvements #3760

Are you sure you want to change the base?

Template improvements #3760

Conversation

aknrdureegaesr commented Feb 20, 2024

Pull Request Checklist

Description

Details

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aknrdureegaesr Feb 23, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aknrdureegaesr Feb 24, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aknrdureegaesr Mar 2, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aknrdureegaesr Feb 25, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aknrdureegaesr Feb 23, 2024 •

edited

aknrdureegaesr Feb 24, 2024 •

edited

aknrdureegaesr Mar 2, 2024 •

edited

aknrdureegaesr Feb 25, 2024 •

edited