Code

Opened 6 years ago

Closed 6 years ago

Last modified 3 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: UI/UX:

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)"

Attachments (0)

Change History (8)

comment:1 Changed 6 years ago by mir

  • Needs documentation unset
  • Needs tests set
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 6 years ago by jheasly

  • Owner changed from nobody to jheasly
  • Status changed from new to assigned

comment:3 Changed 6 years ago by kevin

  • Resolution set to invalid
  • Status changed from assigned to closed

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 Changed 6 years ago by ashearer

  • Resolution invalid deleted
  • Status changed from closed to reopened

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 Changed 6 years ago by ubernostrum

  • Resolution set to duplicate
  • Status changed from reopened to closed

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

comment:6 Changed 6 years ago by ashearer

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 Changed 6 years ago by carlou

  • Cc ouyhui@… added

comment:8 Changed 3 years ago by jacob

  • milestone 1.0 deleted

Milestone 1.0 deleted

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.