Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#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 Baptiste Mispelon, 9 years ago

Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization
Version: 1.7master

Hi,

I was surprised to see that the = operator does appear to work so I did some digging:

  • It only works when surrounded by spaces (eg {% if 1 = 1 %} works but not {% if 1=1 %}
  • It's sortof documented in the {% if %} tag's docstring [1]
  • it's not actually documented publicly (as you mentionned)
  • It's not tested (removing the parsing code for it [2] doesn't break any test)

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

comment:2 by Ola Sitarska, 9 years ago

Owner: changed from nobody to Ola Sitarska
Status: newassigned

comment:3 by Aymeric Augustin, 9 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 Baptiste Mispelon, 9 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 Philipp Metzler, 9 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"
Last edited 9 years ago by Philipp Metzler (previous) (diff)

in reply to:  3 comment:6 by Keryn Knight, 9 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 Keryn Knight, 9 years ago

Cc: django@… added

comment:9 by Collin Anderson, 9 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.

Last edited 9 years ago by Collin Anderson (previous) (diff)

comment:10 by Tim Graham, 9 years ago

Patch needs improvement: set

comment:11 by Tim Graham, 9 years ago

Keywords: 1.8-alpha added
Patch needs improvement: unset

comment:12 by Berker Peksag, 9 years ago

Triage Stage: AcceptedReady for checkin

comment:13 by Tim Graham <timograham@…>, 9 years ago

Resolution: fixed
Status: assignedclosed

In d563e3be68369694a3bac1efd7779d8e03bb6a51:

Fixed #23913 -- Deprecated the = comparison in if template tag.

comment:14 by Tim Graham <timograham@…>, 9 years ago

In 9f51d0c86d9348349d921c393d6ffcbbfd521192:

Fixed test from refs #23913 when running tests in reverse.

comment:16 by Tim Graham <timograham@…>, 9 years ago

In 2ccfac1a:

Refs #23913 -- Removed support for a single equals sign in {% if %} tag.

Per deprecation timeline.

Note: See TracTickets for help on using tickets.
Back to Top