Opened 16 years ago

Closed 16 years ago

Last modified 12 years ago

#8453 closed (duplicate)

Error in filter "timesince"

Reported by: carlou Owned by: jheasly
Component: Template system Version: 1.0-beta
Severity: Keywords: timesince
Cc: ouyhui@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

def timesince(value, arg=None):
    """Formats a date as the time since that date (i.e. "4 days, 6 hours")."""
    from django.utils.timesince import timesince
    if not value:
        return u''
    if arg:
        return timesince(arg, value)    
    return timesince(value)
timesince.is_safe = False

It should be "return timesince(value, arg)"

Change History (8)

comment:1 by Michael Radziej, 16 years ago

Needs tests: set
Triage Stage: UnreviewedAccepted

comment:2 by jheasly, 16 years ago

Owner: changed from nobody to jheasly
Status: newassigned

comment:3 by Kevin McConnell, 16 years ago

Resolution: invalid
Status: assignedclosed

I discussed this ticket with Malcolm. The current behavior could indeed be considered to be somewhat surprising, in that value is typically expected to be older than now() in the default case, yet is expected to be the newer of the two times where a reference point other than now() is supplied. In effect, the meaning of arg and value are reversed when a reference point is supplied.

That said, the current situation works correctly as documented, and existing code may depend on its behavior. It's not considered to be a bug.

comment:4 by ashearer, 16 years ago

Resolution: invalid
Status: closedreopened

I'd agree that the current behavior is surprising, but I'd also say that it contradicts the documentation. Or two out of its three relevant sentences. See #7443:comment:5 for details.

Am I reading incorrectly? I can't believe I've misread it this many times in a row.

Here's the current behavior (where 'earlier' is a week behind 'now' and 'later' is a week ahead):

  • earlier|timesince: 1 week
  • earlier|timesince:now: 0 minutes
  • earlier|timeuntil: 0 minutes
  • earlier|timeuntil:now: 0 minutes
  • later|timesince: 0 minutes
  • later|timesince:now: 1 week
  • later|timeuntil: 1 week
  • later|timeuntil:now: 1 week

The first full sentence of the documentation says timesince "[t]akes an optional argument that is a variable containing the date to use as the comparison point (without the argument, the comparison point is now)." However, passing now explicitly as the optional argument leads to results opposite to allowing it to be the default.

The last sentence says '“0 minutes” will be returned for any date that is in the future relative to the comparison point'. According to the first sentence, that corresponds to the 'later|timesince:now' test, which returns '1 week' instead.

The middle sentence, giving an example, seems to contradict the other two.

If this apparent conflict is resolved in favor of having mydate|timesince:now act like mydate|timesince (obviously the approach I favor), 2/3 of the documentation and the analogy with timeuntil already agree. The only hangup would be existing code. But I'd wonder how widespread that is for the two-argument version. Anyone surprised by the behavior or who realized the documentation didn't entirely match the results could have steered clear by swapping arguments and using two-argument timeuntil instead, which is currently an exact synonym of timesince, but matches its own English meaning and its documentation. In any case, if it's going to change to a less surprising behavior someday, the backward compatibility argument points to getting it in by 1.0.

More discussion, a patch, and test cases in #7443.

comment:5 by James Bennett, 16 years ago

Resolution: duplicate
Status: reopenedclosed

Reopening a ticket to point out that you're arguing about the same thing in another ticket is bad, mmkay?

comment:6 by ashearer, 16 years ago

Sorry, my mistake. I had reopened it in order to reset its resolution to 'duplicate', but for some reason thought the option didn't become available. Turns out I was mistaken. Thanks for doing it for me.

comment:7 by carlou, 16 years ago

Cc: ouyhui@… added

comment:8 by Jacob, 12 years ago

milestone: 1.0

Milestone 1.0 deleted

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