#23913 closed Cleanup/optimization (fixed)
= comparison does work in templates although it shouldn't
Reported by: | Philipp Metzler | Owned by: | Ola Sitarska |
---|---|---|---|
Component: | Template system | Version: | dev |
Severity: | Normal | Keywords: | 1.8-alpha |
Cc: | django@… | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
There's no = operator documented: https://docs.djangoproject.com/en/1.7/ref/templates/builtins/#operator
For comparisons we use the == operator as = does assign values in most languages. In JavaScript the = assignment for example always returns true here:
<script language="javascript">
a = 'foo';
b = 'bar';
if (a=b) {
alert('assignment always returns true here')
}
</script>
BUT in the Django templates = does work like a comparison operator:
{% if "a" = "a" %}TRUE but Django should throw an error because there's no = operator{% endif %}
{% if "a" = "b" %}Django doesn't return True here{% endif %}
I think this is highly misleading and confusing.
Change History (16)
comment:1 by , 10 years ago
Triage Stage: | Unreviewed → Accepted |
---|---|
Type: | Bug → Cleanup/optimization |
Version: | 1.7 → master |
comment:2 by , 10 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
follow-up: 6 comment:3 by , 10 years ago
I'm not convinced we should make this change. It looks like something that could break existing templates on stable websites.
I understand that the reporter was confused but I don't remember other similar reports. The advantages seem quite small to me in comparison with the drawbacks.
comment:4 by , 10 years ago
For the record, this feature was introduced with the "smart if" (2c2f5aee4d44836779fcd74c7782368914f9cfd1) which landed in 1.2. Before that, you had to use {% ifequal %}
.
I'll also note that this syntax is invalid in jinja2.
However, if we want to keep it around for stability's sake, we should at least have tests for it.
I remain doubtful about the need to document it though as I don't think we should encourage the practice.
comment:5 by , 10 years ago
BTW - I used this grep now to find occurrences in our code:
grep -E "if[[:space:]]+(\w|\.)*[[:space:]]*={1}[^=]" . -H -r -i -n -s -C 1 --include=*.{htm*,} --exclude-dir="node_modules"
comment:6 by , 10 years ago
Replying to aaugustin:
I'm not convinced we should make this change. It looks like something that could break existing templates on stable websites.
I understand that the reporter was confused but I don't remember other similar reports.
I believe a change is required:
- Deprecate and eventually pull out the functionality (earliest opportunity being 1.9) or
- Stabilise the parsing (
{% if 1 = 1 %}
vs{% if 1=1 %}
) test and document it, so that the principle of least surprise is minimised.
The fact that there may be code out there that might accidentally work because of the right combination of typos (one equals sign, enough spaces) seems a mis-step that could be rectified (or made authoritatively correct).
It's also a quirk that doesn't fit with equality vs assignment in Python (and thus Django).
comment:7 by , 10 years ago
Cc: | added |
---|
comment:9 by , 10 years ago
At work we use " = " a bunch in our templates for comparisons. I didn't realize it wasn't documented, but I always found it annoying that it worked both ways. Upgrading will be annoying, but deprecating it sounds like a good idea to me.
comment:10 by , 10 years ago
Patch needs improvement: | set |
---|
comment:12 by , 10 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
comment:13 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Hi,
I was surprised to see that the
=
operator does appear to work so I did some digging:{% if 1 = 1 %}
works but not{% if 1=1 %}
{% if %}
tag's docstring [1]Given all this, I'm in favor of removing that "feature".
We still should go through the deprecation policy though, just in case anyone might be relying on this undocumented and untested feature.
Thanks!
[1] https://github.com/django/django/blob/c1552af1fe5832011e3b1c3e5b40c20ff3dbe6f9/django/template/defaulttags.py#L988
[2] https://github.com/django/django/blob/c1552af1fe5832011e3b1c3e5b40c20ff3dbe6f9/django/template/smartif.py#L102