format_interval doesn't format monthly intervals correctly
tassoman - April 26, 2009 - 15:17
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | base system |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs work |
| Issue tags: | DrupalWTF |
Description
By suggestion of merlinofchaos:
(16:52:57) merlinofchaos:
tassoman: You should submit a patch, it doesn't make sense that 'months' isn't an interval. People don't think in weeks > 4.
I'm posting the patch to use to solve that issue. One more value in array is needed. I've chosen a month to be a 30x24hrs interval arbitrary. That could be the only issue before committing the patch. But it's also a common tradition used to count months by humans...
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| format_interval-patch.txt | 450 bytes | Ignored | None | None |
#1
#2
Makes sense, but D6 is feature frozen, bumping to D7.
#3
Here for D7.
#4
This worth a WTF tag.
#5
Committed to CVS HEAD. Changing version to Drupal 6 for Gabor.
#6
Patch here
#7
I don't understand why 2592000 (30x24) makes sense. It makes more sense to me to use 2628000 (year 31536000 / 12)... I guess it's a moot point now. Oh well. :)
For instance, on admin/settings/statistics, we now get the following odd intervals that will have to be changed with an upgrade path:
1 month, 3 weeks
3 months, 3 weeks
I'll have to search for all our uses of format_interval to see where we now have "odd" intervals.
#8
I didn't said that 30x24hrs would make sense, but there's enough time to investigate and decide which interval could be better.
IMHO an interval is 2y, 11mo, 3w, 6d, 23h, 59m, 59s maybe interesting also approximated as: «about 3yrs ago».
#9
For D7, I think we should use calendar approximations for months and years, because, for everyone:
*
May 30, 2009is one month ago when compared toApril 30, 2009*
April 30, 2008is one year ago when compared toApril 30, 2009Returning to D7.
#10
Given that the length of a month can vary by ~10%, it seems like this is going to be tricky to do? I guess that's why months weren't in there in the first place...
For a simple solution, I think Dave Reid's suggestion makes the most sense. At least that way you avoid having format_interval() tell you things like 364 days = "12 months 4 days", but then one day later, 365 days = "1 year". That's probably a much bigger WTF than the code that it replaced :)
#11
The best behaviour would be having a configurable interval approximation... so 364 days maybe "less than one year ago" or "11 months 3 weeks and 6days ago" by granularity configuration.
#12
Changing this to a bug report, as per #7.