Giter Site home page Giter Site logo

Comments (18)

mmerickel avatar mmerickel commented on June 3, 2024

Yes, due to an inherent issue in Jinja2's design it is not possible to fallback to the search path without some very horrible hacks. The 1.x series always used the search path for inheritance and you could not inherit from a template relative to yourself at all. See specifically PARENT_RELATIVE_DELIM in 69fea30 that actually does work but was decided to have some potentially unknown side effects.

The current behavior in 2.x is intentional that a package should use either path-based or caller-relative lookups. With this in mind I'm happy to talk about the use cases you're trying to solve and possible options.

from pyramid_jinja2.

peterhudec avatar peterhudec commented on June 3, 2024

Thanks for clarification. We just stick to version 1.10 for a while.

from pyramid_jinja2.

Epithumia avatar Epithumia commented on June 3, 2024

How does one set pyramid up to always use path-based lookups?
I have a line that includes the content of a css file, and in 2.1, I get the TemplateNotFound error no matter what I use in my .ini file and/or my init.py in my app. It works fine in 1.1.

<head>
<title>Bulletin - {{ nom }} {{ prenom }}</title>
<style>{% include 'static/css/bulletin.css' %}</style>
</head>

And it always prefixes that with the path to the jinja2 file that has that code.

from pyramid_jinja2.

dairiki avatar dairiki commented on June 3, 2024

Try

<style>{% include '/static/css/bulletin.css' %}</style>

(I.e. I think if the path starts with slash (looks like an "absolute" path) then the template search path is used, otherwise the path is interpreted relative to that of the invoking template.)

from pyramid_jinja2.

Epithumia avatar Epithumia commented on June 3, 2024

That seems to have done the trick. Thanks!

from pyramid_jinja2.

mmerickel avatar mmerickel commented on June 3, 2024

The / prefix is actually intended to be an absolute path, rather than relative-to-search-path. However, this is a neat trick (unanticipated) trick that perhaps we could formalize. Currently the template is actually searched for on the filesystem as /static/css/bulletin.css (which could lead to subtle differences between machines if you have templates lying around). This means it may be worthwhile to default to the search path before searching the filesystem.

In general, if you would like search-path based lookup everywhere then you should add all directories with templates to the search path and then use only filenames. This is the only way to guarantee a solid search-path based lookup that allows people to override templates, etc. For example, instead of templates/foo.jinja2, you should add the templates folder to the search path and use foo.jinja2. If everything is like this, the search path should be properly used.

Can I get some thoughts from you guys? It's super weird/ill-advised to me to try to mix caller-relative and search-path based lookups which is why I didn't feel bad breaking it in the first place, so I'd love some counter arguments!

from pyramid_jinja2.

Epithumia avatar Epithumia commented on June 3, 2024

When I have time this afternoon/tonight to redo my attempts from last night, I'll show you what errors I got, what I expected and thus why it was puzzling me.

from pyramid_jinja2.

mmerickel avatar mmerickel commented on June 3, 2024

@Epithumia great, thanks for your patience!

from pyramid_jinja2.

Epithumia avatar Epithumia commented on June 3, 2024

Version <style>{% include 'static/css/bulletin.css' %}</style>
-> TemplateNotFound: brasserie:/templates/2013/1/static/css/bulletin.css; asset=brasserie:/templates/2013/1/static/css/bulletin.css; searchpath=['/home/endless/test2/brasserie/brasserie']

Version <style>{% include '/static/css/bulletin.css' %}</style>
-> Works but can be confused with unix path

And the next two baffle me since when I tried them last night, they weren't working. -_-

Version <style>{% include 'brasserie:static/css/bulletin.css' %}</style>
-> Works, makes sense

Version <style>{% include 'brasserie:/static/css/bulletin.css' %}</style>
-> Same

from pyramid_jinja2.

mmerickel avatar mmerickel commented on June 3, 2024

My tentative plan is to make the following change/distinction:

There are 2 types of lookup.

  1. Asset spec-based (which includes caller-relative).
  2. Search path-based.

The scenarios are independent and defined below.

  • Use template-relative lookups when using asset specs.
    • When renderer="templates/foo.jinja2" is found via caller-relative, and foo.jinja2 includes another template via {% include 'bar.jinja2' %}, then bar.jinja2 will be found relative to the templates folder, i.e. templates/bar.jinja2.
    • In the 1.x series, bar.jinja2 was actually found via the search path (wrong).
    • This behavior will also affect absolute paths since they do not have a search path to fallback (open for discussion if anyone cares).
  • Use absolute lookups when using search path.
    • When renderer="templates/foo.jinja2" is found via search path, and foo.jinja2 includes another template via {% include 'bar.jinja2' %}, then bar.jinja2 will be found relative to the search path.
    • As of 2.1, templates/bar.jinja2 would be found, which is a regression from the 1.x series where bar.jinja2 was found on the search path.

Finally, the initial lookup order is to find a template on the search path before finding a template via caller-relative. This is because renderer="templates/foo.jinja2" looks like a search path to most users, and if templates/foo.jinja2 actually exists relative, then subsequent includes may be broken. If a caller-relative is actually wanted here, an asset spec can be used to override the collision.

If there are no objections I'll make this distinction and release a fix tonight.

from pyramid_jinja2.

mmerickel avatar mmerickel commented on June 3, 2024

A tentative patch (if you'd like to test it on your project) is here:

diff --git a/pyramid_jinja2/__init__.py b/pyramid_jinja2/__init__.py
index ba43790..3236099 100644
--- a/pyramid_jinja2/__init__.py
+++ b/pyramid_jinja2/__init__.py
@@ -45,8 +45,8 @@ class Environment(_Jinja2Environment):
             reluri = posixpath.join(posixpath.dirname(ppath), uri)
             return '{0}:{1}'.format(ppkg, reluri)

-        # parent is just a normal path
-        return posixpath.join(posixpath.dirname(parent), uri)
+        # parent is from the search path, fallback
+        return uri


 class FileInfo(object):
@@ -190,16 +190,17 @@ class Jinja2RendererFactory(object):
         name, package = info.name, info.package

         def template_loader():
-            # first try a caller-relative template if possible
             try:
+                # try search path first to avoid mangling the name of the
+                # template for later includes
+                return self.environment.get_template(name)
+            except TemplateNotFound:
+                # fallback to a caller-relative template if possible
                 if ':' not in name and package is not None:
                     name_with_package = '%s:%s' % (package.__name__, name)
                     return self.environment.get_template(name_with_package)
-            except TemplateNotFound:
-                pass
-
-            # fallback to search path
-            return self.environment.get_template(name)
+                else:
+                    raise

         return Jinja2TemplateRenderer(template_loader)

from pyramid_jinja2.

Epithumia avatar Epithumia commented on June 3, 2024

With the patch, all 4 versions in my previous post now work.
For reference, the template is used in the following code:

def make_bulletin(request)
[...]
result = render('/templates/{0}/{1}/bulletin.jinja2'.format(annee,semestre),
                    {'data':modules, 'libelle':lib, 'nom': nom, 'prenom': prenom,
                     'appr' : appreciation, 'grp' : groupe, 'datenaiss': datenaiss,
                    'num': id, 'rang': rang, 'taillePromo': taillePromo, 'logo_url':logo_url},
                    request=request)
return(result)

from pyramid_jinja2.

mmerickel avatar mmerickel commented on June 3, 2024

With the patch, all 4 versions in my previous post now work.

How about without the slash prefix? None of what I've said expects that hack to be used.

from pyramid_jinja2.

Epithumia avatar Epithumia commented on June 3, 2024

<style>{% include 'static/css/bulletin.css' %}</style> works.

And when I put an unexisting path on purpose, it raises an exception and doesn't show the path appended to the template's path anymore.

Edit: if you mean in the code call, let me check.
Edit2: with result = render('templates/{0}/{1}[...], all four work.

from pyramid_jinja2.

dairiki avatar dairiki commented on June 3, 2024

If there are no objections I'll make this distinction and release a fix tonight.

I think I object. At least without further research, I object. I apologize that the following is not that well thought out, but I'm not thinking very well ATM. It sounds like you're moving quickly on this, so I wanted to get this in...

Here's my use case: I'm using pyramid_layout along with pyramid_jinja2, in a project that was started with pyramid_jinja2 1.x. With pyramid_layout, each page template begins:

{% extends main_template %}

The way my project is currently laid out, the layout templates are found via the search path, while (most of) my page templates are found via asset-spec/relative paths. I think (untested) that by declaring the layout templates using asset-specs, I can fix this so it will work under your proposal.
But I still have a few page templates (lingering from 1.x days) which are found via search path;
that fix will break these, I think.

Obviously the clean fix is, in any particular project, to use either search-path based or relative lookup, not both. This can be tricky however in large projects (which have evolved from 1.x when relative paths didn't really work), and in project composed of smaller packages which invoke templates in different ways.

Anyhow, I think it would probably be good to have a way to mix the two lookup schemes within a single template invocation...

from pyramid_jinja2.

mmerickel avatar mmerickel commented on June 3, 2024

@dairiki a couple comments:

Mixing of different lookups is already broken in the 2.x series. The fix I proposed is to make the distinction more clear and solve some broken behavior when using the search path.

Secondly I actually did make a fix which uses the search path as a fallback but it required a really nasty string-mangling hack to get around jinja2's api. It just wasn't going to be a good longterm solution. 69fea30 for reference.

It's always possible to use asset specs to include other templates. The distinction is at a per-project level on how that project wants to find its top-level templates. Any projects that want to override those templates will need to (and this distinction allows) them to use the appropriate mechanism. That is to say that there is a path to override a projects templates in either scenario but it may not be the same way.

If a project is using a search path then you can just add your own override folder to the search path. If a project is using asset specs then you can override them with config.override_asset.

At the end of the day this seems like a sane compromise to me.

from pyramid_jinja2.

sontek avatar sontek commented on June 3, 2024

+1 To this.

The reason I like this solution is because it makes things more consistent with pyramid itself. For example if you define the following:

@view_config(renderer='subfolder/root.jinja2')
def my_cool_view(request):
    return {}

Inside root.jinja2, prior to this fix this would break:

{% include "subfolder/menu.jinja" %}

Because it was using the path relative to root.jinja2 for lookup, which is inconsistent from how pyramid itself does for renderer lookups.

from pyramid_jinja2.

mmerickel avatar mmerickel commented on June 3, 2024

Alright guys, as I went through the tests to bring them up to date I struggled to get them to work because the default search path kept turning templates that I expected to be caller-relative into search-path templates. This ambiguity convinced me to re-add the hack from 69fea30. 2.2 is released with this fix and any templates not found relative to the parent are now found on the search path. Hopefully we can consider this issue closed.

from pyramid_jinja2.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.