[Django] #24199: string_if_invalid doesn't provide information in many cases
Django
---------------------------------+--------------------
Reporter: adam-iris | Owner: nobody
Type: Uncategorized | Status: new
Component: Template system | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+--------------------
There's a check for "%s" and variable information is added, but only when
handling VariableDoesNotExist for simple variables:
{{{
except VariableDoesNotExist:
if ignore_failures:
obj = None
else:
string_if_invalid = context.engine.string_if_invalid
if string_if_invalid:
if '%s' in string_if_invalid:
return string_if_invalid % self.var
else:
return string_if_invalid
else:
obj = string_if_invalid
}}}
But string_if_invalid is also used in other cases, for example if a
TypeError is raised from a method:
{{{
if callable(current):
if getattr(current, 'do_not_call_in_templates',
False):
pass
elif getattr(current, 'alters_data', False):
current = context.engine.string_if_invalid
else:
try: # method call (assuming no args required)
current = current()
except TypeError:
try:
getcallargs(current)
except TypeError: # arguments *were* required
current = context.engine.string_if_invalid
# invalid method call
}}}
In those cases, there's no check for "%s" and nothing about the method or
the exception is included, making this largely useless for debugging.
--
Ticket URL: <https://code.djangoproject.com/ticket/24199>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: adam-iris | Owner: nobody
Type: Uncategorized | Status: new
Component: Template system | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
The documentation [https://docs.djangoproject.com/en/dev/ref/templates/api
/#how-invalid-variables-are-handled discourages the use of
string_if_invalid]. I'm not sure there's much value in trying to expand
its usefulness, but I'll leave the ticket for a second opinion.
--
Ticket URL: <https://code.djangoproject.com/ticket/24199#comment:1>
Django
Reporter: adam-iris | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Template system | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by aaugustin):
* type: Uncategorized => Cleanup/optimization
* stage: Unreviewed => Accepted
Comment:
Could we refactor this code into a method of Engine?
{{{
def invalid_value(self, value):
# ...
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24199#comment:2>
Django
Reporter: adam-iris | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Template system | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Daniel Hahler):
* cc: django@… (added)
Comment:
Just for information: pytest-django provides a helper (`--fail-on-
template-vars`) that enables this.
It goes through some hoops to get to the required information
(`template`): https://github.com/pytest-dev/pytest-
django/blob/ca3b5a2498ac2714f22305fded1c1518a66a07f6/pytest_django/plugin.py#L476.
I've looked at it when trying to skip it with the `default` filter, but
then created a PR for Django directly:
https://github.com/django/django/pull/8200.
--
Ticket URL: <https://code.djangoproject.com/ticket/24199#comment:3>