The 'add' filter should stringify value or argument if the other value is a string

[dtrebbien]

The 1.3 documentation for the add filter states:

This filter will first try to coerce both values to integers. If this fails, it'll attempt to add the values together anyway. This will work on some data types (strings, list, etc.) and fail on others. If it fails, the result will be an empty string.

When the value and argument represent the second case ("If this fails, it'll attempt to add the values together anyway.") and one is a string while the other is a number, the result of the add filter is simply the value.

Similar to the fix for ticket #393, I think that for this second case and if either value or argument to the filter is a string, the other value should be stringified.

Test cases

{{ 'test'|add:2 }}

Result is: test
Expected result: test2

{{ 2|add:'test' }}

Result is: 2
Expected result: 2test

Workarounds

For the first test case, one workaround is to use a {% with %} tag:

{% with 2|stringformat:'d' as arg_as_str %}
{{ 'test'|add:arg_as_str }}
{% endwith %}

For the second test case, the stringformat filter can be used to convert the number to a string before add is applied:

{{ 2|stringformat:'d'|add:'test' }}

Related tickets

• Ticket #393 [patch] Filters don't take the str() value of a var
• Ticket #8088 Template system improvement: "cat" filter, "include" tag with filters
• Ticket #11687 The 'add' template filter only works for integers, and can fail noisily

[Aymeric Augustin]

In Python it's an error to add a string and an int:

>>> 'a' + 1
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: cannot concatenate 'str' and 'int' objects
>>> 1 + 'a'
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: unsupported operand type(s) for +: 'int' and 'str'

In this case, the proper output is an empty string, as the docs says. I could confirm that Django returns the first value. It's a bug.

---

However, |add doesn't really make sense with strings. + on strings is just concatenation, and concatenation is natively provided by the template engine.

Instead of {{ strval|add:intval }}, just write {{ strval }}{{ intval }}.

For this reason, I don't like the idea of converting both values to strings.

---

I'm attaching a patch with tests (actually, the tests existed, but they contradicted the documentation; I think the documentation has precedence). I also fixed a naked except -- we never want to catch BaseException.

[Chris Beaven]

Although the documentation has stated (for quite some time) that it returns an empty string, the current behavior of returning the original input is an acceptable response to an error. The docs say:

In case of error, they should return either the original input or an empty string -- whichever makes more sense.

There could be minor elements of backwards incompatibility if we change this. Perhaps just changing the docs to match the behavior would be alright to do here?

[Aymeric Augustin]

In theory, I find it slightly better to display nothing that to display a wrong result. For instance, if someone adds total price and shipping cost with this filter (an horrible idea for sure, and a strong argument against the existence of the add filter), and accidentally has one of these values represented as a string, the resulting price would be wrong.

In practice, I understand the value of preserving backwards compatibility.

I'm attaching an alternative patch that changes the docs to match the code. I'm still slightly in favor of the initial patch. But what really matters is to resolve the inconsistency between the code and the docs, one way or another.

[Aymeric Augustin]

alternative solution

[dtrebbien]

I prefer Aymeric's first patch. Preserving backward compatibility with behavior that contradicts the docs does not seem preferable to making the behavior conform to current wording. Also, I think that returning an empty string is more consistent with the template system's tendency to return nothing if there was an error (e.g.: attempting to print an undefined variable).

[Julien Phalip]

An approach would be to return settings.TEMPLATE_STRING_IF_INVALID (which defaults to an empty string). However, I'd be happy enough with the doc being modified to reflect the current behaviour.

[Jacob]

The fix here is to change the template to return the empty string as the documentation says. Returning the left-hand-side on a failure is weird. The auto-stringifying-behavior suggested doesn't sound like it matches Django very wel, so let's not do that.

[Aymeric Augustin]

The initial patch was reviewed and marked as RFC by Russell. Since you just confirmed that the solution it uses is correct, I'm marking this ticket as RFC again (even though I'm the author).

The patch that must be committed is the first one, "16676.patch".

[Julien Phalip]

Question: should settings.TEMPLATE_STRING_IF_INVALID be returned instead of a systematic hardcoded empty string, or should that setting be reserved for invalid variables?

[Jacob]

No, TEMPLATE_STRING_IF_INVALID is for variables; there's no precedent in using it for filters, and I don't think we should expand it at all. It already causes enough problems!

[Julien Phalip]

In [16851]:

Fixed #16676 -- Corrected the behavior of the 'add' template filter to return an empty string instead of the given value unchanged in the case of an invalid use. Thanks to dtrebbien for the report and to Aymeric Augustin for the patch.