Opened 17 years ago

Closed 17 years ago

Last modified 17 years ago

#3838 closed (wontfix)

stringformat filter allows trailing non-format characters but not leading characters

Reported by: msimoens@… Owned by: nobody
Component: Template system Version: dev
Severity: Keywords: stringformat
Cc: msimoens@…, lakin.wecker@…, jm.bugtracking@… Triage Stage: Design decision needed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

This filter seems to provide an, intentionally simplified, implementation of python's format strings. I'm not sure what the intent was with this simplification, however it is inconsistent. Due to the requirement that the preceding '%' be left out, one cannot put leading constant characters into the format string (which may be intended). However, one _can_ put trailing constant characters into the format string and it will still work. For instance:

{# fails #}
{{ generic_key_type|stringformat:"course/%s.html" }} 

{# works #}
{{ generic_key_type|stringformat:"s/course/index.html" }} 


The first way will not work, however the second way will work, which is inconsistent. Either the filter should ensure that no trailing constant characters are used or it should allow both ways. If the intent is to simplify it in some way, then it should consistently not allow either trailing or leading characters. In this case, the name is very misleading as the filter is not equivalent to python's format strings, and I would argue that it should be renamed after the subset of functionality that it is intended to provide.

With its current name, I would argue that it should allow both trailing and leading characters, and the following is an implementation that allows this, and attempts to remain backwards compatible.

def stringformat(value, arg):
    """
    Formats the variable according to the argument, a string formatting specifier.
    This specifier uses Python string formating syntax with the exception that
    the leading "%" may be dropped.

    See http://docs.python.org/lib/typesseq-strings.html for documentation
    of Python string formatting
    """
    
    try:
        return ("%" + str(arg)) % value
    except (ValueError, TypeError):
        try: 
            return str(arg) % value
        except (ValueError, TypeError):
            return ""

Change History (7)

comment:1 by Chris Beaven, 17 years ago

Resolution: wontfix
Status: newclosed

This filter is really more for alternate types of formatting (eg |stringfilter:".2f") than string concatenation.

Your alternate method seems a bit too magic, and still isn't going to work with cases like |stringformat:"settings/%s.html".

comment:2 by lakin.wecker@…, 17 years ago

Resolution: wontfix
Status: closedreopened

I'm re-opening this report, as the resolution is not appropriate given the answer that was posted. I respectfully disagree with this being a "wontfix". If this filter

... is really more for alternate types of formatting (eg |stringfilter:".2f") than string concatenation.

Then there is a bug ... Because it does allow for string concatenation. Hence there is a bug to be fixed. Mark it as low priority if you'd like, but it's still a bug. More importantly, I agree with the original poster that if this is the intent, then the filter is mis-named and mis-documented. It needs attention.

The proposed method is no more "magic" than the original implementation, and it _will_ handle the case that you brought up:

|stringformat:"settings/%s.html"

comment:3 by Chris Beaven, 17 years ago

Ok, fair enough - it does handle the case I brought up because that raises a TypeError.

I'll defer the decision to a core developer (and thanks to lakin's correction of my interpretation, my opinion changes is +0)

comment:4 by Chris Beaven, 17 years ago

Triage Stage: UnreviewedDesign decision needed

comment:5 by anonymous, 17 years ago

Cc: jm.bugtracking@… added

comment:6 by James Bennett, 17 years ago

Resolution: wontfix
Status: reopenedclosed

Going back to wontfix for the reasons Chris gave originally; the fact that you can, in some situations, accidentally use something in a way that's undocumented and unsupported does not mean we should be fixing bugs for that use.

comment:7 by jm.bugtracking@…, 17 years ago

but you should fix it so that it can't be used in ways that are undocumented and unsupported. I still find the naming ambiguous and frankly think that a filter should does what it name says, not what the documentation says that it currently does, especially if both have a big overlap as it is now.

So... if stringformat can't be fixed so that it allows all possible uses (and adding documentation for those, as proposed here), then it probably should be changed in a way that raises an appropriate exception with a meaningful error message, or not?

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