Hi Carl, Thank you very much for the thorough review!
Most of your comments are in line with my thinking and only require clarifications in the DEP. Tomorrow I'll proof-read and push what I've written tonight. I'm answering on a few items below. Everything else I agree with :-) 2014-11-03 19:47 GMT+01:00 Carl Meyer <[email protected]>: * The description for option 4 implies that ordering of template engines > is only useful if their directories overlap. This is not the case. I > would see ordering as perhaps most useful in cases where a project wants > to selectively (or wholly) override some templates provided by a > third-party app with replacements using a different engine. In this > case, there would be no overlapping directories in the configuration, > just the same template name provided in two different engines' directories. > In version you read, I didn't find that implication; my reasoning was: 1 - If different engines look for files in the same directory, and you aren't careful, one may attempt to render a file intended for another and bad things will happen. 2 - Within different directories, it will usually be easier to avoid conflicts, if only to make sure the developer can tell what template in rendered. In hindsight, this isn't good advice. The setup you're proposing is valid. It's the kind of idea I was describing as a fallback scheme ("try to find an override version, fall back to the app's default templates") in the last paragraph. I've revised it to remove idea 2 and be neutral about such setups. * In the current DTL configuration, the relative priority of > `TEMPLATE_DIRS` vs app directories can be controlled via ordering of the > `TEMPLATE_LOADERS` setting. In your proposed configuration scheme, how > can one control the relative priority of `DIRS` vs `APP_DIRS`? This can > be quite important for overriding templates provided by third-party > apps. If that's the primary/only use case, perhaps it's sufficient to > just have `DIRS` always take priority, since that's the more > explicitly-configured option? (I see that this is what your sample > `BaseEngine` implementation does - if that's your proposal, it should be > more explicit in the DEP.) > Yes, `DIRS` always takes priority. I don't see an use case for the opposite behavior (and there's an escape hatch described a few paragraphs below). I've made that explicit in the DEP. > * Similarly, since it's left up to the user to configure other loading > mechanisms that an engine might offer (presumably via OPTIONS), that > raises the question of how a user could control the priority of those > alternative loading mechanisms vs `DIRS` and `APP_DIRS`. Perhaps the > best choice here is to punt that issue to the individual template engine > and its OPTIONS. > Yes, that's my choice. I've also made that explicit. * Along a similar vein, I think it should be permissible (though not > encouraged) for a template engine to refuse to define an app-dirs > subdirectory name. In this case it would simply be an error to attempt > to configure that template engine with `APP_DIRS: True`. > I've chosen to make it mandatory for consistency across engines and because every engine should support loading templates from directories. Your suggestion would be easy to implement, but to me it looks like it's adding something that doesn't have a use case. Is there a good reason? > * Is there any meaning to the top-level keys in the `TEMPLATES` setting? > None is mentioned; the only one I can think of would be as a string that > one could use in the option-1 API to explicitly select a specific > engine. Yes, that's the use case. I've mentioned it. > I think the ability to configure template engine priority will > be important, and a dictionary is problematic for that. Importing > OrderedDict is ugly boilerplate. So I wonder if `TEMPLATES` should be a > list of dictionaries rather than a dictionary of dictionaries; perhaps > with a `NAME` key if we need a string identifier for each configured > engine? > I'm torn between consistency with DATABASES and CACHES on one side and the ugliness of OrderedDict on the other side... I chose the former because I value consistency a lot and because I discouraged relying on the order (but that's no longer true...) I find the idea of extracting a special key called "NAME" and using it as the identifier rather confusing. The datastructure is really an ordered mapping of engine identifier => engine configuration. There's no syntax for that in Python, so... `import collections`. > * Related to the above, I am glad to see that the proposed configuration > scheme would allow configuring more than one instance of the same > template engine. I don't have any use case for this off the top of my > head, but my instinct says it's a useful capability to maintain. (This > would imply that we shouldn't re-use the BACKEND string itself as an > identifier for a particular configured engine.) > Yes, it feels like a sane property even without a use case. You could use it to have APP_DIRS take precedence over DIRS. * I'm not convinced that rejecting the "switch to Babel" option for > makemessages actually reduces the scope / difficulty / disruptiveness of > this DEP. I think improving makemessages to handle multiple template > engines will likely involve an undesirable level of reimplementing > things Babel already does well (whereas since Babel already has support > for DTL syntax, makemessages' hacky handling of DTL could be removed > with a switch to Babel). There is already precedent in Django for > required dependencies for subsystems which are not enabled by default > and do not block the initial getting-started flow. I think a more > flexible makemessages will likely be best (and simplest) implemented as > a wrapper to Babel, perhaps settings some configuration defaults > according to what is known about the Django project structure. > I don't think I have enough information at this point to make a decision nor that a decision is absolutely required right now. I've changed the DEP to keep this option under consideration. > * It's not mentioned explicitly, but I presume the DTL backend's > `render` method will take an ordinary dict (not a `Context` instance) as > its `context` argument, and will automatically use `RequestContext` (and > thus context processors) if the `request` argument is provided? This > seems like the best approach to me (though for backwards-compatibility > it will also need to accept a `Context` instance, and even perhaps pull > the request out of a `RequestContext` instance if provided, at least for > a deprecation period). > Yes. > * That gets into the question of how the public API and implementation > of `django.shortcuts.render` and `render_to_response` will change. This > isn't fully outlined in the DEP; perhaps it should be, since for most > people it's the primary entry point for templates? > They don't change — well, almost :-) They get a new keyword argument, `engine`. I think I'll have to deprecate the current_app keyword argument and make it an attribute of the request instead. That's an implementation contingency for which I'll implement a deprecation path. > * I'm confused about how you plan to organize the Python namespaces for > Django's template support. I see use of both `django.template` and > `django.templates` in various code examples, but if there is a > consistency to the usage I'm missing it (and I don't think > differentiating modules by a single letter would be a good plan, > anyway). It's a typo. Read `django.template` everywhere. > Ideally it seems to me that the DTL (currently in > `django.template` and must probably stay there for > backwards-compatibility) and the new engine-configuration system would > reside in totally separate namespaces; having the latter reside in a > submodule of the former seems logically backward. Indeed. For lack of a better solution, I plan to let them live in the same namespace. That gives: django.template.backends.* - built-in backends django.template.utils - utilities for supporting backends django.template.* - implementation of the DTL I shall rewrite the docstring of django.template to explain that. > The issue is finding a > good name for the latter, since `django.template` would be the obvious > choice. Perhaps `django.templating`? Or maybe your apparent choice of > `django.template.backends` is the best option in practice, even if it > implies a relationship that is inverted from the reality > (`django.template` is one of the backends supported by > `django.template.backends`; `django.template.backends` is not a feature > of `django.template`). > To me the alternative would be to move the implementation of the DTL to e.g. django.template.engine. I judged that it would create gratuitous code churn and I chose not do to it but I can if you think it's useful. * You mention `SimpleTemplateResponse` and `TemplateResponse` in > Appendix A and note that they aren't really features of DTL (which I > agree with), but you don't provide anywhere a migration plan for them. I > would like to see them usable with any template engine, which shouldn't > be too hard. It does raise the question of whether they should be moved > to a different location. > My answer is the same as for `render` and `render_to_response`. Besides their private method `resolve_context` won't be needed any more since `Template.render` will accept a plain dict. Since template responses couple request handling and template rendering, they belong to django.http, django.template, or django.shortcuts. I don't believe the other places are enough of an improvement to warrant moving them. > * Re FAQ "Is it possible to use Django template filters or tags with > other engines?" - I'm not sure I agree with the strong assertion that > this "certainly will" be implemented as a third-party module. As you > mention in the previous FAQ answer, in general it will be much less work > and a better implementation for providers of custom DTL filters/tags to > provide the core functionality via simple Python functions (which can > then be trivially added to the context for most non-DTL template > languages, including Jinja2), and the DTL integration as thin wrappers > around the core function. > I've changed the language but I'm still convinced that someone will do it :-) -- Aymeric. -- You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at http://groups.google.com/group/django-developers. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CANE-7mW---ddxNhZ61Jzm-HemGKag14bsbzJp58oGAUwMu88pQ%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
