Filters no longer parsed for unresolved variables

11 views
Skip to first unread message

SmileyChris

unread,
Aug 31, 2006, 6:17:08 PM8/31/06
to Django developers
Changeset [3268] made the following change for the resolve method in
django/template/__init__.py:
- obj = settings.TEMPLATE_STRING_IF_INVALID
+ if ignore_failures:
+ return None
+ else:
+ return settings.TEMPLATE_STRING_IF_INVALID

Because the TEMPLATE_STRING_IF_INVALID is returned now rather than
being set in obj (which is then passed to any filters), the filters are
not parsed on an unresolved variable which changes the behaviour of the
default and default_if_none filters.

I believe the last line of that patch should have been:
+ obj = settings.TEMPLATE_STRING_IF_INVALID

but I just wanted to check that the functionality change isn't intended
first.

SmileyChris

unread,
Aug 31, 2006, 7:04:20 PM8/31/06
to Django developers
Guess I should just make a ticket anyway:
http://code.djangoproject.com/ticket/2637

Russell Keith-Magee

unread,
Sep 1, 2006, 10:25:42 AM9/1/06
to django-d...@googlegroups.com
On 9/1/06, SmileyChris <smile...@gmail.com> wrote:

I believe the last line of that patch should have been:
+                obj = settings.TEMPLATE_STRING_IF_INVALID

but I just wanted to check that the functionality change isn't intended
first.

This was intentional; the original problem was that {% if var %} would evaluate as true if var was undefined. This was having immediate problems in the admin view; if you had TEMPLATE_STRING_IF_INVALID set, the admin view had some major rendering problems (because blocks were getting included or ignored inappropriately).

The fix you describe would exhibit a different variety of the same problem. If you use the default value for TEMPLATE_STRING_IF_INVALID (which is ''), then your patch would be fine; an invalid value would get passed to filters like default and be processed as expected. However, if you set TEMPLATE_STRING_IF_INVALID to be non-empty, ( e.g., '--INVALID--'), the default filter will think that the variable has a value ('--INVALID--) and do nothing. Similar problems would exist for other filters.

I think a better solution would be something like:

def resolve(self, context, ignore_failures=False):
    try:
        obj = resolve_variable(self.var, context)
    except VariableDoesNotExist:
        if ignore_failures:
            obj = None
        else:
            if settings.TEMPLATE_STRING_IF_INVALID:
                 return settings.TEMPLATE_STRING_IF_INVALID
            else:
                 obj = settings.TEMPLATE_STRING_IF_INVALID
    for func, args in self.filters:
        ....

This way,
- If/For nodes can ignore any setting for TEMPLATE_STRING_IF_INVALID and pass a value of None straight to the filters

- For other nodes, if you have an empty TEMPLATE_STRING_IF_INVALID, the empty value will be passed to the filter, which can be processed in a meaningful way

- If you actually have a TEMPLATE_STRING_IF_INVALID setting, you shortcut the filtering process (since there is no point passing '--INVALID--' to a filter).

Does this make sense? Have I missed anything here?

Yours,
Russ Magee %-)

SmileyChris

unread,
Sep 3, 2006, 10:00:00 PM9/3/06
to Django developers
Hi Russ,

Thanks, that explains the problem with my patch perfectly :)
Your solution looks great: it still solves the original problem while
bringing back the correct functionality if TEMPLATE_STRING_IF_INVALID
is not set.

I can't see that you've missed anything (but then again, I didn't
manage to get it right on my attempt so don't trust me) ;)

This should be documented under the settings page to explain that if
you set TEMPLATE_STRING_IF_INVALID to be non-empty, filters will be not
be processed on variable tags where the variable is not found in the
template context.

Russell Keith-Magee

unread,
Sep 4, 2006, 10:04:27 AM9/4/06
to django-d...@googlegroups.com
On 9/4/06, SmileyChris <smile...@gmail.com> wrote:

I can't see that you've missed anything (but then again, I didn't
manage to get it right on my attempt so don't trust me) ;)

I've just committed the fix (along with some extra unit tests to make sure it stays fixed) as r3714. Thanks for the help in nailing this one.

Yours
Russ Magee %-)

Reply all
Reply to author
Forward
0 new messages