>> Variable and FilterExpression will be deprecated as they require the
>> caller to know the length of the expression in advance.
>
> I'm not sure I follow this assertion. I have no problem believing
> FilterExpression can be made cleaner, but Variable takes an expression
> like 'article.section', plus a context, and resolves it into a string.
> In itself, it seems like a well built and stable piece of API.
To get this expression string in the first place you have to split
Token.contents somehow. smart_split() does that well for space
delimited expressions.
But if you try to parse something like {% url %} you have to split its
results again: "foo.bar,baz|filter:arg,42" won't do it. The current {%
url %} parser now resorts to `str.split(",")` and
{% url viewname list|join:"," %}
will break it. Of course that's a corner case and could be fixed with
something like `smart_split(delim=",")`. But it would be much more
elegant if you could just read an expression from a stream of tokens.
That's what I meant when I said "the {% url %} parser is broken by
design". I didn't mean to sound harsh - {% url %}'s parser is a
reasonable tradeoff between simplicity and correctness.
> As best as I can make out, you are correct. FilterExpression appears
> to be an internal, so isn't covered by our backwards compatibility
> guarantees. However, Variable is documented API.
Variable and FilterExpression would still be around - compatible
versions live in `django.template.compat` - and import paths remain
functional.
>> 1.) Currently `Variable("unresolvable")` and
>> `FilterExpression("unresolvable", p)` resolve differently: Variable
>> raises VariableDoesNotExist and FilterExpression returns
>> TEMPLATE_STRING_IF_INVALID. Except when `ignore_failures=True` is
>> given. But `FilterExpression("foo|cut:unresolvable", p)` will again
>> raise VariableDoesNotExist - regardless of `ignore_failures=True`.
>>
>> My Expression implementation unifies these behaviours: If any part of
>> an expression is unresolvable a LookupError will be raised.
>> `ignore_failures` will be deprecated but there's a resolve_safe()
>> method on the Expression base class that reads:
>>
>> def resolve_safe(self, context, default=None):
>> try:
>> return self.resolve(context)
>> except LookupError:
>> return default
>>
>> This would be a small backwards incompatible change. I have one
>> failing test (exception02) because the ExtendsNode implementation
>> depends on the current FilterExpression behaviour.
>
> It's either backwards compatible or it isn't. Backwards compatibility
> is a requirement, not an optional extra.
If FilterExpression is not part of the public API this is a non-issue.
The problem was that unresolvable variables were treated differently
from unresolvable filter arguments.
>> 3.) Numeric literals ending with "." are currently treated as
>> Variables. The following test (ifequal-numeric07) succeds because
>> `ignore_failures=True` in IfEqualNode suppresses the inevitable
>> VariableDoesNotExist exception and then compares None to 2: ('{%
>> ifequal x 2. %}yes{% endifequal %}', {'x': 2}, ''). My current
>> implementation raises a TemplateSyntaxError as soon as it
>> encounters a
>> numeric literal with a trailing dot. This could easily be made
>> backwards compatible if it's considered worth it.
>
> Sorry, but this one can't change. "2." is a tested format for handling
> floats; the fact that this is inconvenient to your parser is
> irrelevant.
The problem is probably in bug in Variable:
>>> from django.template import Variable, Context
>>> Variable("2.").resolve(Context())
Traceback (most recent call last):
...
raise VariableDoesNotExist("Failed lookup for key [%s] in %r", (bit,
current)) # missing attribute
>>> Variable("2.").resolve(Context({"2": {"": "foo"}}))
'foo'
I originally wanted to parse "2." as a numeric literal. But there's
currently code that explicitly attempts to reject such literals:
http://code.djangoproject.com/browser/django/trunk/django/template/__init__.py#L663
And here's how I fixed it in my branch (as mentioned in my second post
in this thread):
http://github.com/emulbreh/django/commit/73bc93b4dbf251968e062825439033de33597b08
>> * {% url ... %}, {% ssi ... %} currently accept unquoted literal
>> strings. This will continue to work but the preferred syntax will use
>> properly quoted strings. This may one day allow for viewnames and ssi
>> paths that come from expressions.
>
> At this point, this sort of fix can't happen until v2.0. It's would be
> a major change at this point. It's nice to know that the template
> parsing tools will handle it when the time comes, but I would expect
> that to be true of _any_ rework of the parser.
The proposed change is fully backwards-compatible. Both tags only gain
a new syntax variant. By documenting the unquoted variant as
deprecated means that newly written templates will already be
compatible when (in some distant future) expression will be allowed as
viewnames.
>> * {% if not %} would fail with my current implementation. This is
>> fixable. But I'm not sure it's worth it: not/and/or should be
>> keywords
>> in the context of boolean expressions.
>
> I'm unclear what you're proposing here. {% if not %} is a well
> documented API, so it can't be deprecated.
The problem was that "{% if not %}" (without any further expression;
the variable is called "not") failed. However, I fixed this already
(as mentioned in my followup post).
> I don't want to sound dismissive of the work you're done here, but
> your comments in this post haven't inspired me to go looking deeper.
> You've been a little cavalier with the term "backwards incompatible"
> on a couple of occasions, which is something that simply isn't
> acceptable in something as prominent and well documented as Django's
> template language.
Criticism is welcome. If FilterExpression is not public API ,
everything should be backwards compatible now. That was the goal from
the beginning.
The {% if not %} and Variable("2.") fixes are ugly but compact.
All tests pass (with two minor corrections: {% foo | bar %} is now
valid and FilterExpression internals have changed).
> There are bugs in the template system. Many of these are caused by
> inconsistent handling of parsing, especially of variables. If you can
> point at something that you think is a bug (such as the handlng of
> foo._attrib), then it's no longer covered by backwards compatibility.
> However, you don't get that accommodation simply because something
> doesn't fit into your nice new parsing scheme.
>
> Now, some of this might just be a misunderstanding on my part -
> especially with regards to what is partially implemented, what you
> intend to fix, and what you think can't (or shouldn't) be fixed.
> You've spent a lot of time describing your new parsing API, but not a
> lot of time describing why the new API is required. To that end, I'm
> not entirely sure how much of this patch is intended to cover the "fix
> the inconsistencies in the template system" problem, and how much is
> intended to fix the "make template tags easier to write" problem. If
> you're proposing a major rework of internals with the goal of fixing
> bugs, explaining exactly what is broken and how it is broken is an
> important part of your proposal. If the goal is to improve the
> usability of the public API, explaining the use cases you're trying to
> make easier is important.
It's both: making the API friendlier and fixing bugs (by using the
higher level API).
I thought that motivation and scope were mostly clear and approved.
Let me refer you to the two previous threads for now:
http://groups.google.com/group/django-developers/browse_thread/thread/fb246422fa4e23d8
http://groups.google.com/group/django-developers/browse_thread/thread/4f5b01719e497325
> I'm also intrigued to know how this proposal intersects with Mike
> Malone's patch to introduce template caching (#6262).
There will be merge conflicts but I'm neither touching loader.py nor
context.py so that should be managable.
> Please don't let this discourage you - I am keen to see improvements
> in the template system. However, any widespread changes are going to
> get _very_ thorough investigation, and before I invest the time
> tearing down the code, I want to make sure the fundamentals
> underpinning the patch are well understood and well founded.
__
Johannes
--~--~---------~--~----~------------~-------~--~----~
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
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en
-~----------~----~----~----~------~----~------~--~---
