Opened 11 years ago
Last modified 9 years ago
#24199 new Cleanup/optimization
string_if_invalid doesn't provide information in many cases
| Reported by: | adam-iris | Owned by: | nobody |
|---|---|---|---|
| Component: | Template system | Version: | dev |
| Severity: | Normal | Keywords: | |
| Cc: | django@… | Triage Stage: | Accepted |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
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.
Change History (3)
comment:1 by , 11 years ago
comment:2 by , 11 years ago
| Triage Stage: | Unreviewed → Accepted |
|---|---|
| Type: | Uncategorized → Cleanup/optimization |
Could we refactor this code into a method of Engine?
def invalid_value(self, value):
# ...
comment:3 by , 9 years ago
| Cc: | added |
|---|
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.
The documentation 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.