Template-04's scope
Johannes Dollinger
what's in scope for 1.1 and what you believe doesn't belong in django.
Why the patch still does that/too much:
* The code shuffling is purely cosmetic, it just feels wrong to stuff
everthing in __init__.py. It's already too big for my taste. And I
honestly don't know how to cleanly split shuffling and functionality
in separate patches.
* I always felt the API for custom tags is too verbose. There's almost
no abstraction which makes parsing something like {% url %}
unnecessarily hard - even django's builtin parser doesn't get it right
(split(' ') and split(',')). Yes, that could be seen as a feature
request. But builtin tags would also benefit from abstraction. Just
comparing code length (compactness is desirable, although by no means
the only quality measure): #7806's defaulttags contains 20% less LOC
(not counting blank lines, comments, and docstrings).
* The bugs, obviously. As Malcolm pointed out, most are symptoms of
the same underlying problems.
Anyway, here's a list of the tickets I tried to fix in #7806 (although
some of them were merely side effects):
Bugs
====
#5756: Any template tag that uses Variable instead of
parser.compile_filter does not handle filters.
http://code.djangoproject.com/ticket/5756
#5862: Filters are not applied in an include tag
Wontfixed, but it's really an instance of #5756.
http://code.djangoproject.com/ticket/5862
#5971: django.template.TokenParser inconsistent with parsing filters
http://code.djangoproject.com/ticket/5971
#6271: filter arguments with spaces break several template tags
http://code.djangoproject.com/ticket/6271
#6296: ifequal, filter and hardcoded-string doesn't work
http://code.djangoproject.com/ticket/5756
#6510: get_nodes_by_type refactor for easier extensibility (fixes an
underlying bug for IfEqualNode)
http://code.djangoproject.com/ticket/6510
#6535: parser.compile_filter() does not support negative numbers
http://code.djangoproject.com/ticket/6535
#7295: quotes, escaping and translation of string literals handled
inconsistently in templates
http://code.djangoproject.com/ticket/7295
#7377: "extends" and "block" tags are not available when constructing
template from scratch
http://code.djangoproject.com/ticket/7377
#9315: Keyword arguments with spaces and the url tag
http://code.djangoproject.com/ticket/9315
Features
=======
#4746: Allow whitespace before and after filter separator
The patch goes further: whitespace is insignificant anywhere between
tokens. That means
`{% url view foo , kw = bar | filter : arg %}`
would be valid.
http://code.djangoproject.com/ticket/4746
Threads
=======
There are a couple more I'm aware of and probably and probably dozens
I haven't found yet - as a starting point:
http://groups.google.com/group/django-developers/browse_thread/thread/4f5b01719e497325
http://groups.google.com/group/django-developers/browse_thread/thread/ba0644b835e3ec40
http://groups.google.com/group/django-developers/browse_thread/thread/c90b6e29d20724ca
Related to the code shuffling in #7806 (although my intention was
merely readability):
http://groups.google.com/group/django-developers/browse_thread/thread/35aae0e8da804bd9
Jacob Kaplan-Moss
<johannes....@einfallsreich.net> wrote:
> I'm aware that #7806 propably does too much at once. I'd like to hear
> what's in scope for 1.1 and what you believe doesn't belong in django.
I'm actually pretty happy with the general idea you've taken here --
django.template needs some cleanup, and a holistic approach that
addresses the multiple issues at once is going to produce better code
than multiple patches all stacked up.
It seems most of the core team agrees -- it got 7 +0 votes. I think
the only reason it failed to get any +1s is that it's just a big
amount of work to review all at once. There's also the obvious
question of weather we can pull off this refactoring without breaking
backwards-compatibility.
I think the best thing to do with this is to take it to a branch so we
can easily evaluate it against the trunk templates. If you're down for
working on this off on a branch, I'm willing to help out and also
review your code.
> * The code shuffling is purely cosmetic, it just feels wrong to stuff
> everthing in __init__.py. It's already too big for my taste. And I
> honestly don't know how to cleanly split shuffling and functionality
> in separate patches.
Yeah, this is the big reason a branch would be better -- we can
separate the big deck-chair-shuffling from the refactoring and be able
to review better.
> * I always felt the API for custom tags is too verbose.
There's almost universal agreement on this point :)
It's unfortunately pretty tricky to do in a backwards-compatible way:
we've sorta pasted ourselves into a corner here by letting tags do
anything they want in terms of parsing. This lead to an inconsistency
in syntax that we (unfortunately) have to continue to support.
However, that's no reason not to have a better API on top that
third-party tags can use -- as long as internal tags don't break.
So: what do you say we make a template-cleanup branch and start
working on this over there?
Jacob
Steve Holden
> On Tue, Nov 25, 2008 at 10:11 AM, Johannes Dollinger
>> * I always felt the API for custom tags is too verbose.
>>
>
> There's almost universal agreement on this point :)
>
> It's unfortunately pretty tricky to do in a backwards-compatible way:
> we've sorta pasted ourselves into a corner here by letting tags do
> anything they want in terms of parsing. This lead to an inconsistency
> in syntax that we (unfortunately) have to continue to support.
>
> However, that's no reason not to have a better API on top that
> third-party tags can use -- as long as internal tags don't break.
>
processor (falling back to the original if necessary), and therefore no
need for a fully backwards-compatible approach?
regards
Steve
Johannes Dollinger
Am 25.11.2008 um 17:28 schrieb Jacob Kaplan-Moss:
> I'm actually pretty happy with the general idea you've taken here --
> django.template needs some cleanup, and a holistic approach that
> addresses the multiple issues at once is going to produce better code
> than multiple patches all stacked up.
>
> It seems most of the core team agrees -- it got 7 +0 votes. I think
> the only reason it failed to get any +1s is that it's just a big
> amount of work to review all at once. There's also the obvious
> question of weather we can pull off this refactoring without breaking
> backwards-compatibility.
I'm glad you like it, more on backwards-compatibility below.
> I think the best thing to do with this is to take it to a branch so we
> can easily evaluate it against the trunk templates. If you're down for
> working on this off on a branch, I'm willing to help out and also
> review your code.
>
>> * The code shuffling is purely cosmetic, it just feels wrong to stuff
>> everthing in __init__.py. It's already too big for my taste. And I
>> honestly don't know how to cleanly split shuffling and functionality
>> in separate patches.
>
> Yeah, this is the big reason a branch would be better -- we can
> separate the big deck-chair-shuffling from the refactoring and be able
> to review better.
>
>> * I always felt the API for custom tags is too verbose.
>
> There's almost universal agreement on this point :)
>
> It's unfortunately pretty tricky to do in a backwards-compatible way:
> we've sorta pasted ourselves into a corner here by letting tags do
> anything they want in terms of parsing. This lead to an inconsistency
> in syntax that we (unfortunately) have to continue to support.
>
> However, that's no reason not to have a better API on top that
> third-party tags can use -- as long as internal tags don't break.
That should be entirely possible, using either an additional decorator
for parser functions `@$new_api` or `@register.tag($new_api=True)`, or
a new method `@register.$new_api` (for some reasonable value of
$new_api).
It might even be possible to fold `TokenStream` into `Token`, but that
would introduce state in `Token` - haven't thought this through.
The refactored version of defaulttags currently passes most tests with
the following exceptions
* {% if not not %} raises TemplateSyntaxError (fixable, but I'm not
shure it's worth it)
* integer literals with a trailing colon raise TemplateSyntaxError
* whitespace is insignificant, therefore {{ foo | filter }} passes.
The only real bc problem were unquoted unicode viewnames in {% url %}.
That's fixed in my latest patch at the expense of regexp beauty.
The current version accepts a quoted string as well, so one could
deprecate unquoted viewnames and fix that in 2.x.
That's been rejected in http://groups.google.com/group/django-developers/browse_thread/thread/fd30cb20f80c6c79
.
> So: what do you say we make a template-cleanup branch and start
> working on this over there?
If it has to be a branch, so be it. I entered myself as implementor
into the feature table.
Johannes Dollinger
That hopefully won't be necessary. However, the possibility to
configure those token boundaries at runtime might be a nice feature. I
missed it for LaTeX templates.
Malcolm Tredinnick
On Tue, 2008-11-25 at 10:28 -0600, Jacob Kaplan-Moss wrote:
> On Tue, Nov 25, 2008 at 10:11 AM, Johannes Dollinger
> <johannes....@einfallsreich.net> wrote:
> > I'm aware that #7806 propably does too much at once. I'd like to hear
> > what's in scope for 1.1 and what you believe doesn't belong in django.
>
> I'm actually pretty happy with the general idea you've taken here --
> django.template needs some cleanup, and a holistic approach that
> addresses the multiple issues at once is going to produce better code
> than multiple patches all stacked up.
>
> It seems most of the core team agrees -- it got 7 +0 votes. I think
> the only reason it failed to get any +1s is that it's just a big
> amount of work to review all at once. There's also the obvious
> question of weather we can pull off this refactoring without breaking
> backwards-compatibility.
>
> I think the best thing to do with this is to take it to a branch so we
> can easily evaluate it against the trunk templates. If you're down for
> working on this off on a branch, I'm willing to help out and also
> review your code.
I agree with the mini-branch approach (particularly since Jacob's
volunteered to shepherd it). However, one thing I'm cautious about and
that I'd like to keep split out: a bunch of the bugs such as filtering
in ifequal tags, etc, can be fixed fairly simply and they're legitimate
bug fixes that can be applied to 1.0.X as well as trunk. So I've always
been thinking of this as a two-phase process: fixing most of the bugs
and then the refactoring separately. The first part can then easily be
applied to the 1.0.X branch as well.
Whether phase one is done on a branch or just a bunch of small patches
(and most of them will be relatively small) is a separate issue. If
Russell ever gets his recently announced +4 Fountain of Free Time
installed in his backyard, I'm flying out to Perth to have a drink from
it and then fix the bug portion, but right at the moment I'm not holding
my breath. My TODO list looks scary enough that if other people are
doing this, I'll just step back and kibitz.
Regards,
Malcolm
Johannes Dollinger
>> However, that's no reason not to have a better API on top that
>> third-party tags can use -- as long as internal tags don't break.
>
> That should be entirely possible, using either an additional decorator
> for parser functions `@$new_api` or `@register.tag($new_api=True)`, or
> a new method `@register.$new_api` (for some reasonable value of
> $new_api).
Even better, we could use class based parsers: http://dpaste.com/96291/
That would make reusing parsers much simpler.