We're all going to find this easier if the right words are used.
Template class instances are mutable objects that hold their current
state. This is a design choice, not a bug. When something is done
intentionally and works as intended, that's a design choice, not a bug.
The distinction is important, as it leads to discussions being made as
to the differences in opinions about the decision, not an all-or-nothing
bug fix. It's for the same reason we don't refer to dictionaries, lists,
sets and other mutable Python objects as having thread bugs.
> Everywhere, where a tag render method
> somehow modifies the node object. Where does this happen? From what
> I've seen so far it's done in "block", "ifchanged", "cycle" and
> "extends". Now, of course the attribute assignments happen for a
> reason: they are necessary, at least for cycle and ifchanged. Block
> and extends look like accidents, they could be solved a lot cleaner
> (block would put a reference to a hypothetical SuperBlock into the
> context, rather than a reference to itself) and extends doesn't have
> to store the evaluated value on the node at all, a local variable is
> just fine.
That would change the rendering behaviour, however. Whether they were
accidents originally or not isn't really a concern right this moment.
Backwards compatibility is. The approach is certainly worth considering
(we have to do something like this for reset-ability of cycling, for
example), but the backwards-incompatible effects needs to be itemised
and addressed as well.
[...]
> This used in all core tags makes the `Template` object thread safe,
> but not the `Context` which is perfectly okay because it doesn't
> happen (or should not happen) that a context object is shared between
> threads. At least I don't see a situation where this is useful.
All that leaves is the thousands of third-party tags. Again, trying to
reach for some guarantee that instances can be shared between threads
has quite an extensive set of secondary effects. It might be preferable
to say that our current situation of being a mutable object holding
state is not that bad, rather than causing massive disruptions to the
userbase, since the current code still meets the "fast enough" criteria
for a very large group of uses.
> What's harder to fix is how the i18n integration translates filter
> arguments or gettext constants (those _("foo") thingies). Currently
> that happens very often at node/object creation rather than node/
> object evaluation. A good example for that problem is
> FilterExpression.__init__. The translation would have to be moved to
> the resolve method in that case. When would a language change between
> template compilation and rendering? If the language changes each
> request which is a very common case on multilingual websites.
That's actually a known. From memory, it's buried in a ticket talking
about something else, but I know it's been on my list of things to
address for a while now and pre-1.1 is probably the time frame where I'm
goign to be doing a lot of i18n work.
A quite reasonable goal is that template instances should be cacheable
right after they've been loaded (we already know that rendering changes
state, but right after loading they're always in the same state). That
means being pickle-able and, later, unpickle-able in a different view
with, as you note, a different active locale.
There are actually a whole swathe of problems with the various i18n
template tags that need to be cleaned up. They're in line behind sorting
some of the thing like making Variable() consistent and stuff like that
so that we can reuse the common support stuff. That will come out of
merging the work Johannes Dollinger is proposing with the existing code.
So there's an ordering of fixes going on there, but it's certainly on my
personal radar.
>
> Changing the parser to a more advanced system or making the template
> engine more consistent is not so important for the time being, but I
> want to raise a few situations I encountered where the behaviour is
> confusing:
>
> - cycle arguments are delimited by spaces and each item can be an
> expression (however if there is a comma involved somewhere, it
> seems like the tag is interpreted as comma separated list of
> strings
> which makes the 'cycle "foo", "bar", "baz"' yield unexpected
> results.
>
> - On the other hand arguments for the url tag are comma delimited,
> but whitespace after comma is not allowed.
>
> - The group-by part of the regroup tag is an expression, but
> everything
> else but a variable name returns unexpected results. Furthermore
> does this tag only work on dicts. By the group-by part I'm
> refering
> to the expression after the "by" keyword:
>
> {% regroupy foo by bar as blah %}
>
> bar is here treated as constant string, even though it's
> represented
> as variable in the regroup node and in the syntax.
Most of these are already parts of existing tickets. You might want to
make sure the remainder are filed as such if they can be fixed by
loosening the parsing slightly (whilst maintaining backwards
compatibility for the advertised, documented behaviour). I think you'll
also find that Johannes' patch(es) address a number of these.
> - Likewise ssi excepts one argument that looks like a variable, but
> is
> treated as constant string
Things like that have been around forever and really can't be changed
now. It's not that hard to get it right and backwards compatibility is
very important.
Regards,
Malcolm
My patch treats _(x) as syntactic sugar for x|gettext - where
`gettext = lambda x: x and ugettext(x) or u''`.
I agree, a convention for tuples and literal strings would be really
helpful. I had to work around the {% url %} viewname issue (that's
why TokenStream.parse_string() takes bare=True). It's solved
backwards compatible yet accepts properly quoted strings. Whitespace
may occur between tokens but is discarded. {% regroup %} uses
TokenStream.parse_expression(), so that should be fixed too. {% cycle
%} is problematic because it accepts the legacy {% cycle a,b as c %}
format but that should be curable.