Hi Christopher, Some feedback below.
> 2. This package has two kinds of global state. The first one are global > attributes defined at module level of template package. These are > base.builtins, base.libraries, loader.template_source_loaders and > base.templatetags_modules. The second one are dependencies on settings and > some other module-level attributes in template package like > base.invalid_var_format_string (this is cache; it's a flag which means "is > there '%s' in settings.TEMPLATE_STRING_IF_INVALID?") or > context._standard_context_processors, which is cache set in > context.get_standard_processors function which transform data from settings > in some way. I decided to focus on the first kind of global state at first. You've listed every cache that depends on a setting. It would be interesting to add a listener for the setting_changed signal to reset these caches when appropriate. This is only used in tests; one isn't supposed to change settings at runtime. If I understand correctly, once your work is complete, you could listen to changes to a bunch of settings, and just re-instanciate the default template engine when any of them is changed. > 5. The one issue is that the engine is needed everywhere. For example, > consider creating base.Template instance. At the time of creation the > template source is being compiled. The template may load a library, so the > compilation cannot be done without list of all libraries defined in the > engine. To sum up, we cannot just type Template(source), because this depends > on template engine. I solved this issue by passing engine instance to > Template constructor. In order not to break everything, I renamed Template > class to _Template and created function called Template that does "return > _Template(default_engine, **kwargs)". Maybe a stupid question — couldn't you add an optional 'default_engine' keyword argument to the Template class, and if it isn't provided, use the default template engine? > 9. Now, there is another issue. Changing API. It's obvious that changing > base.Parser signature is OK since it's an internal structure. It's also easy > to notice that base.Template is used everywhere so we cannot change API of > Template. But there are many places where it's unclear if a method or class > is public (it means that we promise we won't change its signature) or > internal. For example, I don't know how much we can change loaders. I don't > know if Django developers types "isinstance(obj, Template)" -- it won't work > know. I don't know if I can change signature of SsiNode as I did. By the way, > I think that we should make clear distinction between these two methods by > using underscore convention. As explained by Carl, documented APIs are public, other APIs are private, and Django avoids the underscore convention. Then there's the gray area of things that aren't formally documented but are known to be used in the wild. Fortunately the template engine is mostly preserved from this problem. > 11. A small issue is test > regressiontests.views.tests.debug.DebugViewTests.test_template_loader_postmortem. > The problem is in method > django.views.debug.ExceptionReporter.get_traceback_data. It uses > template_source_loaders global state which was removed. We can just use > default_engine. I wonder when a Django developer uses a custom template > engine instead of the global default one then they will get faulty error > message. Is it important or negligible? There's no such thing as a custom template engine right now. It's very important that the template engine's public APIs keep behave exactly like they do currently; you can deal with specific bits of Django that won't work with custom template engines later on. > 14. Is this plan good? What issues do you see? Could you look at my work and > review it (I wonder especially if I'm using a python feature not supported by > python version supported by Django?) You must write compatible code for Python 2 and 3. Quick tips: - have a look at https://docs.djangoproject.com/en/dev/topics/python3/ - make sure all files have `from __future__ import unicode_literals` at the top; add it (and run tests) if it's missing - don't put any u prefixes before your string literals The template engine works entirely in unicode, and it's sane. You should be all right :) The only tricky part is loading templates from the filesystem; keep the current code and tests, especially those with non-ASCII file names. -- Aymeric. -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. Visit this group at http://groups.google.com/group/django-developers?hl=en. For more options, visit https://groups.google.com/groups/opt_out.
