Opened 8 years ago

Closed 8 years ago

#3670 closed (fixed)

ifequal and ifnotequal failing with negative numbers

Reported by: bram.dejong+django@… Owned by: adrian
Component: Template system Version: master
Severity: Keywords: ifequal ifnotequal negative
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

{% ifequal -1 -20 %}
hello
{% endifequal %}

prints "hello"

{% ifnotequal -1 -1 %}
hello
{% endifnotequal %}

doesn't print anything

this because in resolve_variable( ) in django.template.init uses:

if path[0].isdigit():

which could be:

if path[0].isdigit() or path[0] == "-":

Attachments (3)

templateif_patch.patch (436 bytes) - added by bram.dejong+django@… 8 years ago.
numeric.patch (2.8 KB) - added by SmileyChris 8 years ago.
numeric_with_negative.patch (3.1 KB) - added by SmileyChris 8 years ago.

Download all attachments as: .zip

Change History (10)

Changed 8 years ago by bram.dejong+django@…

comment:1 Changed 8 years ago by bram.dejong+django@…

that should have been:

{% ifnotequal -1 -5 %}
hello
{% endifnotequal %}

prints nothing

comment:2 Changed 8 years ago by mtredinnick

  • Needs tests set
  • Patch needs improvement set

This patch doesn't handle the case of {% ifequal foo -elephant %} sensibly. It will try to process "-elephant" as if it were a number. I realise that is an error in any case, but we shouldn't confuse things further.

comment:3 Changed 8 years ago by mtredinnick

  • Triage Stage changed from Unreviewed to Accepted

Changed 8 years ago by SmileyChris

Changed 8 years ago by SmileyChris

comment:4 Changed 8 years ago by SmileyChris

  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Accepted to Ready for checkin

Straightforward enough.

comment:5 Changed 8 years ago by mtredinnick

We run this piece of code a *lot* (for every argument in every template tag), so speed is important. I'm going to hold off committing whilst I do some timing tests on different approaches. No need to rewrite the patch in any way -- it's correct on a functional level. I just want to try out a few options whilst we're playing in this area.

comment:6 Changed 8 years ago by mtredinnick

The reg-exp seems to be fastest here. I tried a couple of ways of removing the double scan over the string in the case of numbers (we do a reg-exp pass and then a pass for '.'), but it added more code than speed-ups and wasn't worth it.

comment:7 Changed 8 years ago by mtredinnick

  • Resolution set to fixed
  • Status changed from new to closed

(In [4690]) Fixed #3670 -- Fixed template argument parsing so that it understands negative
floats and integers as numeric types. Patch from SmileyChris.

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