[PATCH] Round-down years in "years+months" relative date view

[PATCH] Round-down years in "years+months" relative date view

[David Reiss]

Previously, a commit from 1 year and 7 months ago would display as
"2 years, 7 months ago".

Signed-off-by: David Reiss <dreiss@facebook.com>
---

Here's my test script.  Let me know if you'd rather have it as part
of the test suite.


#!/bin/sh
set -e
REPO="git-relative-dates-test"
rm -rf "$REPO"
mkdir "$REPO"
cd "$REPO"
git init
NOW=`date +%s`
env GIT_AUTHOR_DATE=`expr $NOW - \( 365 + 220 \) \* 24 \* 60 \* 60` git commit --allow-empty -m old-commit
git log --date=relative


 date.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/date.c b/date.c
index 1de1845..e848d96 100644
--- a/date.c
+++ b/date.c
@@ -137,7 +137,7 @@ const char *show_date(unsigned long time, int tz, enum date_mode mode)
 		}
 		/* Give years and months for 5 years or so */
 		if (diff < 1825) {
-			unsigned long years = (diff + 183) / 365;
+			unsigned long years = diff / 365;
 			unsigned long months = (diff % 365 + 15) / 30;
 			int n;
 			n = snprintf(timebuf, sizeof(timebuf), "%lu year%s",
-- 
1.6.0.4

Re: [PATCH] Round-down years in "years+months" relative date view

[Jeff King]

On Thu, Aug 27, 2009 at 04:39:38PM -0700, David Reiss wrote:

> Previously, a commit from 1 year and 7 months ago would display as
> "2 years, 7 months ago".

Wow, embarrassing.

Acked-by: Jeff King <peff@peff.net>

> Here's my test script.  Let me know if you'd rather have it as part
> of the test suite.

I couldn't find any tests related to relative date processing, so it
would be really nice to have some. But I'm not sure of the best way to
do it without dealing with race conditions. Annoyingly, show_date calls
gettimeofday at a pretty low level, so there isn't a way of
instrumenting it short of LD_PRELOAD trickery (which is probably not
very portable).

But maybe a patch like this is worth doing, which would allow us to test
in a repeatable fashion:

---
diff --git a/date.c b/date.c
index e848d96..db2f831 100644
--- a/date.c
+++ b/date.c
@@ -86,6 +86,33 @@ static int local_tzoffset(unsigned long time)
 	return offset * eastwest;
 }
 
+static int current_time(struct timeval *now)
+{
+	static struct timeval fake_time;
+	static int use_fake_time = -1;
+
+	if (use_fake_time == -1) {
+		const char *x = getenv("GIT_FAKE_TIME");
+		if (x) {
+			char buf[50];
+			if (parse_date(x, buf, sizeof(buf)) <= 0)
+				die("unable to parse GIT_FAKE_TIME");
+			fake_time.tv_sec = strtoul(buf, NULL, 10);
+			fake_time.tv_usec = 0;
+			use_fake_time = 1;
+		}
+		else
+			use_fake_time = 0;
+	}
+
+	if (use_fake_time == 1) {
+		memcpy(now, &fake_time, sizeof(*now));
+		return 0;
+	}
+
+	return gettimeofday(now, NULL);
+}
+
 const char *show_date(unsigned long time, int tz, enum date_mode mode)
 {
 	struct tm *tm;
@@ -99,7 +126,7 @@ const char *show_date(unsigned long time, int tz, enum date_mode mode)
 	if (mode == DATE_RELATIVE) {
 		unsigned long diff;
 		struct timeval now;
-		gettimeofday(&now, NULL);
+		current_time(&now);
 		if (now.tv_sec < time)
 			return "in the future";
 		diff = now.tv_sec - time;
@@ -929,7 +956,7 @@ unsigned long approxidate(const char *date)
 	if (parse_date(date, buffer, sizeof(buffer)) > 0)
 		return strtoul(buffer, NULL, 10);
 
-	gettimeofday(&tv, NULL);
+	current_time(&tv);
 	time_sec = tv.tv_sec;
 	localtime_r(&time_sec, &tm);
 	now = tm;

Re: [PATCH] Round-down years in "years+months" relative date view

[Alex Riesen]

On Fri, Aug 28, 2009 at 08:05, Jeff King<peff@peff.net> wrote:
> On Thu, Aug 27, 2009 at 04:39:38PM -0700, David Reiss wrote:
>
>> Previously, a commit from 1 year and 7 months ago would display as
>> "2 years, 7 months ago".
>
> Wow, embarrassing.
>
> Acked-by: Jeff King <peff@peff.net>
>
>> Here's my test script.  Let me know if you'd rather have it as part
>> of the test suite.
>
> I couldn't find any tests related to relative date processing, so it
> would be really nice to have some. But I'm not sure of the best way to
> do it without dealing with race conditions. Annoyingly, show_date calls
> gettimeofday at a pretty low level, so there isn't a way of
> instrumenting it short of LD_PRELOAD trickery (which is probably not
> very portable).

Maybe better prepare the _test_ so that it uses current time and time
arithmetics then put yet another cludge in operational code? Especially
when we already have a greate number of GIT_ environment variables,
documented nowhere, with effects not immediately obvious:

  $ git grep -n '"GIT_'| perl -ne '/"(GIT_\w+)/ && print "$1\n"' |
sort |uniq | wc -l
  49

  $ git grep -n '"GIT_'|grep ^Documentation
  $

GIT_FLUSH? GIT_SEND_EMAIL_NOTTY?! GIT_CHERRY_PICK_HELP?!!

Re: [PATCH] Round-down years in "years+months" relative date view

[Jeff King]

On Fri, Aug 28, 2009 at 09:58:27AM +0200, Alex Riesen wrote:

> > I couldn't find any tests related to relative date processing, so it
> > would be really nice to have some. But I'm not sure of the best way to
> > do it without dealing with race conditions. Annoyingly, show_date calls
> > gettimeofday at a pretty low level, so there isn't a way of
> > instrumenting it short of LD_PRELOAD trickery (which is probably not
> > very portable).
> 
> Maybe better prepare the _test_ so that it uses current time and time
> arithmetics then put yet another cludge in operational code? Especially
> when we already have a greate number of GIT_ environment variables,
> documented nowhere, with effects not immediately obvious:

But that's the point: you can't do that without a race condition. Your
test gets a sense of the current time, then runs git, which checks the
current time again. How many seconds elapsed between the two checks?

I guess it is good enough for testing large time spans, but I was hoping
for a comprehensive time test.

-Peff

Re: [PATCH] Round-down years in "years+months" relative date view

[Alex Riesen]

On Fri, Aug 28, 2009 at 17:02, Jeff King<peff@peff.net> wrote:
> But that's the point: you can't do that without a race condition. Your
> test gets a sense of the current time, then runs git, which checks the
> current time again. How many seconds elapsed between the two checks?

How _many_ do you need?

Re: [PATCH] Round-down years in "years+months" relative date view

[Jeff King]

On Fri, Aug 28, 2009 at 07:00:59PM +0200, Alex Riesen wrote:

> On Fri, Aug 28, 2009 at 17:02, Jeff King<peff@peff.net> wrote:
> > But that's the point: you can't do that without a race condition. Your
> > test gets a sense of the current time, then runs git, which checks the
> > current time again. How many seconds elapsed between the two checks?
> 
> How _many_ do you need?

I don't understand what you're trying to say. My point is that if you
are checking results to a one-second precision, you need to know whether
zero seconds elapsed, or one second, or two seconds, or whatever to get
a consistent result.

-Peff

Re: [PATCH] Round-down years in "years+months" relative date view

[Alex Riesen]

On Fri, Aug 28, 2009 at 19:15, Jeff King<peff@peff.net> wrote:
> On Fri, Aug 28, 2009 at 07:00:59PM +0200, Alex Riesen wrote:
>
>> On Fri, Aug 28, 2009 at 17:02, Jeff King<peff@peff.net> wrote:
>> > But that's the point: you can't do that without a race condition. Your
>> > test gets a sense of the current time, then runs git, which checks the
>> > current time again. How many seconds elapsed between the two checks?
>>
>> How _many_ do you need?
>
> I don't understand what you're trying to say. My point is that if you
> are checking results to a one-second precision, you need to know whether
> zero seconds elapsed, or one second, or two seconds, or whatever to get
> a consistent result.

Taking this particular case as an example, can't we just set the time
of the _commit_ back in time? We can.
And we don't need to know the difference precisely, it can be
something like /[0-9]+ ago/, can't it?
Ok, it is possible, that something goes terribly wrong and the test suite
freezes for an extended period of time, so the pattern above does
not apply anymore. In this case, wont you prefer the test suite to
break? Ok, maybe not, if the freeze was an Ctrl-Z pressed at
unlucky moment. Which involves an operator online and looking,
and action and reaction will be both visible.

So, yes, it is not absolutely safe, but this approach has no side effects
on the working code. And very low probability of something go wrong.

Re: [PATCH] Round-down years in "years+months" relative date view

[A Large Angry SCM]

Jeff King wrote:
> On Fri, Aug 28, 2009 at 07:00:59PM +0200, Alex Riesen wrote:
> 
>> On Fri, Aug 28, 2009 at 17:02, Jeff King<peff@peff.net> wrote:
>>> But that's the point: you can't do that without a race condition. Your
>>> test gets a sense of the current time, then runs git, which checks the
>>> current time again. How many seconds elapsed between the two checks?
>> How _many_ do you need?
> 
> I don't understand what you're trying to say. My point is that if you
> are checking results to a one-second precision, you need to know whether
> zero seconds elapsed, or one second, or two seconds, or whatever to get
> a consistent result.

To no-one in particular, Gitzilla mumbles "To do this right(tm) would 
probably require LD_PRELOAD magic with a environment variable invocation.

Re: [PATCH] Round-down years in "years+months" relative date view

[Nicolas Pitre]

On Fri, 28 Aug 2009, Jeff King wrote:

> On Fri, Aug 28, 2009 at 09:58:27AM +0200, Alex Riesen wrote:
> 
> > > I couldn't find any tests related to relative date processing, so it
> > > would be really nice to have some. But I'm not sure of the best way to
> > > do it without dealing with race conditions. Annoyingly, show_date calls
> > > gettimeofday at a pretty low level, so there isn't a way of
> > > instrumenting it short of LD_PRELOAD trickery (which is probably not
> > > very portable).
> > 
> > Maybe better prepare the _test_ so that it uses current time and time
> > arithmetics then put yet another cludge in operational code? Especially
> > when we already have a greate number of GIT_ environment variables,
> > documented nowhere, with effects not immediately obvious:
> 
> But that's the point: you can't do that without a race condition. Your
> test gets a sense of the current time, then runs git, which checks the
> current time again. How many seconds elapsed between the two checks?
> 
> I guess it is good enough for testing large time spans, but I was hoping
> for a comprehensive time test.

I agree with your concern.  This is why I created the --index-version 
switch to pack-objects.

However I was hoping for a current time trickery solution that could 
live in test-date.c instead of interfering with the main code in such a 
way.

Did a quick test to override the library version:

diff --git a/test-date.c b/test-date.c
index 62e8f23..0bcd0c9 100644
--- a/test-date.c
+++ b/test-date.c
@@ -1,5 +1,10 @@
 #include "cache.h"
 
+int gettimeofday(struct timeval *tv, struct timezone *tz)
+{
+	return 0;
+}
+
 int main(int argc, char **argv)
 {
 	int i;

Result:

$ ./test-date now
now -> bad -> Wed Dec 31 19:00:00 1969
now -> Tue Jan 22 10:48:24 10199

So this seems to work.  ;-)


Nicolas

Re: [PATCH] Round-down years in "years+months" relative date view

[Jeff King]

On Fri, Aug 28, 2009 at 01:28:34PM -0400, Nicolas Pitre wrote:

> However I was hoping for a current time trickery solution that could 
> live in test-date.c instead of interfering with the main code in such a 
> way.
> 
> Did a quick test to override the library version:

Thanks, that is a much better solution. And I don't know offhand of any
portability problems in overriding the library at link time.

-Peff

Re: [PATCH] Round-down years in "years+months" relative date view

[Alex Riesen]

On Fri, Aug 28, 2009 at 20:01, Jeff King<peff@peff.net> wrote:
> On Fri, Aug 28, 2009 at 01:28:34PM -0400, Nicolas Pitre wrote:
>
>> However I was hoping for a current time trickery solution that could
>> live in test-date.c instead of interfering with the main code in such a
>> way.
>>
>> Did a quick test to override the library version:
>
> Thanks, that is a much better solution. And I don't know offhand of any
> portability problems in overriding the library at link time.
>

Microsoft's compiler and libraries? MacOSX?

Re: [PATCH] Round-down years in "years+months" relative date view

[Jeff King]

On Fri, Aug 28, 2009 at 08:27:06PM +0200, Alex Riesen wrote:

> > Thanks, that is a much better solution. And I don't know offhand of any
> > portability problems in overriding the library at link time.
> 
> Microsoft's compiler and libraries? MacOSX?

Are you saying you know those to be platforms with problems, or are you
asking whether those platforms will have problems?

-Peff

Re: [PATCH] Round-down years in "years+months" relative date view

[Alex Riesen]

On Fri, Aug 28, 2009 at 20:39, Jeff King<peff@peff.net> wrote:
> On Fri, Aug 28, 2009 at 08:27:06PM +0200, Alex Riesen wrote:
>
>> > Thanks, that is a much better solution. And I don't know offhand of any
>> > portability problems in overriding the library at link time.
>>
>> Microsoft's compiler and libraries? MacOSX?
>
> Are you saying you know those to be platforms with problems, or are you
> asking whether those platforms will have problems?

Both: MS never had weak/vague linkage, but I don't know about MacOSX.
I suspect them to have the same deficiency, but I'd be glad to be wrong.

Re: [PATCH] Round-down years in "years+months" relative date view

[Alex Riesen]

On Fri, Aug 28, 2009 at 20:42, Alex Riesen<raa.lkml@gmail.com> wrote:
> On Fri, Aug 28, 2009 at 20:39, Jeff King<peff@peff.net> wrote:
>> On Fri, Aug 28, 2009 at 08:27:06PM +0200, Alex Riesen wrote:
>>
>>> > Thanks, that is a much better solution. And I don't know offhand of any
>>> > portability problems in overriding the library at link time.
>>>

Hm, how about supplying show_date and approxidate with current time?
Like this:

/* exported */
const char *show_date_rel(unsigned long time, int tz, struct timeval *now)
{
... the DATE_RELATIVE path of show_date
}

const char *show_date(unsigned long time, int tz, enum date_mode mode)
{
  struct timeval now;
  if (mode == DATE_RELATIVE) {
    gettimeofday(&now, NULL);
    return show_date_rel(time, tz, &now);
  }
  ... other paths of old show_date
}

Still affects the performance, but much less, and no side effects.
And you can test show_date_rel path in test-date.c

Re: [PATCH] Round-down years in "years+months" relative date view

[Nicolas Pitre]

On Fri, 28 Aug 2009, Alex Riesen wrote:

> On Fri, Aug 28, 2009 at 20:39, Jeff King<peff@peff.net> wrote:
> > On Fri, Aug 28, 2009 at 08:27:06PM +0200, Alex Riesen wrote:
> >
> >> > Thanks, that is a much better solution. And I don't know offhand of any
> >> > portability problems in overriding the library at link time.
> >>
> >> Microsoft's compiler and libraries? MacOSX?
> >
> > Are you saying you know those to be platforms with problems, or are you
> > asking whether those platforms will have problems?
> 
> Both: MS never had weak/vague linkage, but I don't know about MacOSX.

This is not about weak or vague linkage.  This is plain basic linker 
feature where no library object needs to be linked if there is no symbol 
to resolve.

> I suspect them to have the same deficiency, but I'd be glad to be wrong.

Are you able to test it?


Nicolas

Re: [PATCH] Round-down years in "years+months" relative date view

[Alex Riesen]

On Fri, Aug 28, 2009 at 21:00, Nicolas Pitre<nico@cam.org> wrote:
>> >> Microsoft's compiler and libraries? MacOSX?
>> >
>> > Are you saying you know those to be platforms with problems, or are you
>> > asking whether those platforms will have problems?
>>
>> Both: MS never had weak/vague linkage, but I don't know about MacOSX.
>
> This is not about weak or vague linkage.  This is plain basic linker
> feature where no library object needs to be linked if there is no symbol
> to resolve.

Maybe I missed something, but wasn't the idea to overwrite gettimeofday
with a public gettimeofday, defined in one of the object files?
And shouldn't a linker complain regarding duplicated symbols, unless
the other (library) symbol is defined as a weak symbol, allowing
overriding it with another symbol of stronger linkage?

>> I suspect them to have the same deficiency, but I'd be glad to be wrong.
>
> Are you able to test it?
>

Only the MS, but it is not interesting.

Re: [PATCH] Round-down years in "years+months" relative date view

[Nicolas Pitre]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1618 bytes --]

On Fri, 28 Aug 2009, Alex Riesen wrote:

> On Fri, Aug 28, 2009 at 21:00, Nicolas Pitre<nico@cam.org> wrote:
> >> >> Microsoft's compiler and libraries? MacOSX?
> >> >
> >> > Are you saying you know those to be platforms with problems, or are you
> >> > asking whether those platforms will have problems?
> >>
> >> Both: MS never had weak/vague linkage, but I don't know about MacOSX.
> >
> > This is not about weak or vague linkage.  This is plain basic linker
> > feature where no library object needs to be linked if there is no symbol
> > to resolve.
> 
> Maybe I missed something, but wasn't the idea to overwrite gettimeofday
> with a public gettimeofday, defined in one of the object files?

Yes, in test-date.o.

> And shouldn't a linker complain regarding duplicated symbols, unless
> the other (library) symbol is defined as a weak symbol, allowing
> overriding it with another symbol of stronger linkage?

Normally a linker would search for new objects to link only when there 
are still symbols to resolve.  If the library is well architected (mind 
you I don't know if that is the case on Windows or OS X) you should find 
many small object files in a library, so to have only related functions 
together in a single object for only the needed code to be linked in the 
final binary. Hence the printf symbol should be in a separate object 
file than gettimeofday, etc.

Only if the library's object file containing gettimeofday also contains 
another symbol pulled by the linker will you see a duplicated symbol 
error.  But this is still a possibility.  So your proposal is probably 
cleaner.


Nicolas

Re: [PATCH] Round-down years in "years+months" relative date view

[Alex Riesen]

Nicolas Pitre, Fri, Aug 28, 2009 21:27:53 +0200:
> On Fri, 28 Aug 2009, Alex Riesen wrote:
> > And shouldn't a linker complain regarding duplicated symbols, unless
> > the other (library) symbol is defined as a weak symbol, allowing
> > overriding it with another symbol of stronger linkage?
> 
> Normally a linker would search for new objects to link only when there 
> are still symbols to resolve.  If the library is well architected (mind 
> you I don't know if that is the case on Windows or OS X) you should find 
> many small object files in a library, so to have only related functions 
> together in a single object for only the needed code to be linked in the 
> final binary. Hence the printf symbol should be in a separate object 
> file than gettimeofday, etc.
> 
> Only if the library's object file containing gettimeofday also contains 
> another symbol pulled by the linker will you see a duplicated symbol 
> error.  But this is still a possibility.  So your proposal is probably 
> cleaner.

Is it so for dynamic linking as well? Like in libc.so?

Re: [PATCH] Round-down years in "years+months" relative date view

[Nicolas Pitre]

On Fri, 28 Aug 2009, Alex Riesen wrote:

> Nicolas Pitre, Fri, Aug 28, 2009 21:27:53 +0200:
> > On Fri, 28 Aug 2009, Alex Riesen wrote:
> > > And shouldn't a linker complain regarding duplicated symbols, unless
> > > the other (library) symbol is defined as a weak symbol, allowing
> > > overriding it with another symbol of stronger linkage?
> > 
> > Normally a linker would search for new objects to link only when there 
> > are still symbols to resolve.  If the library is well architected (mind 
> > you I don't know if that is the case on Windows or OS X) you should find 
> > many small object files in a library, so to have only related functions 
> > together in a single object for only the needed code to be linked in the 
> > final binary. Hence the printf symbol should be in a separate object 
> > file than gettimeofday, etc.
> > 
> > Only if the library's object file containing gettimeofday also contains 
> > another symbol pulled by the linker will you see a duplicated symbol 
> > error.  But this is still a possibility.  So your proposal is probably 
> > cleaner.
> 
> Is it so for dynamic linking as well? Like in libc.so?

Yes.  The linker still links against stubs in that case.


Nicolas

Re: [PATCH] Round-down years in "years+months" relative date view

[Alex Riesen]

>From b51bc56816490c71cd37f52be73a06cef6b9bf14 Mon Sep 17 00:00:00 2001
From: Alex Riesen <raa.lkml@gmail.com>
Date: Fri, 28 Aug 2009 20:59:59 +0200
Subject: [PATCH] Add date formatting functions with current time explicitely formatted

It should allow safe testing of this part of the code.
---
Nicolas Pitre, Fri, Aug 28, 2009 19:28:34 +0200:
> On Fri, 28 Aug 2009, Jeff King wrote:
> > On Fri, Aug 28, 2009 at 09:58:27AM +0200, Alex Riesen wrote:
> > 
> > > > I couldn't find any tests related to relative date processing, so it
> > > > would be really nice to have some. But I'm not sure of the best way to
> > > > do it without dealing with race conditions. Annoyingly, show_date calls
> > > > gettimeofday at a pretty low level, so there isn't a way of
> > > > instrumenting it short of LD_PRELOAD trickery (which is probably not
> > > > very portable).
> > > 
> > > Maybe better prepare the _test_ so that it uses current time and time
> > > arithmetics then put yet another cludge in operational code? Especially
> > > when we already have a greate number of GIT_ environment variables,
> > > documented nowhere, with effects not immediately obvious:
> > 
> > But that's the point: you can't do that without a race condition. Your
> > test gets a sense of the current time, then runs git, which checks the
> > current time again. How many seconds elapsed between the two checks?
> > 
> > I guess it is good enough for testing large time spans, but I was hoping
> > for a comprehensive time test.
> 
> I agree with your concern.  This is why I created the --index-version 
> switch to pack-objects.
> 

This is what I mean with "supplying current time":

 cache.h |    2 +
 date.c  |  130 ++++++++++++++++++++++++++++++++++----------------------------
 2 files changed, 73 insertions(+), 59 deletions(-)

diff --git a/cache.h b/cache.h
index dd7f71e..3fb0166 100644
--- a/cache.h
+++ b/cache.h
@@ -731,9 +731,11 @@ enum date_mode {
 };
 
 const char *show_date(unsigned long time, int timezone, enum date_mode mode);
+const char *show_date_relative(unsigned long time, int tz, const struct timeval *now);
 int parse_date(const char *date, char *buf, int bufsize);
 void datestamp(char *buf, int bufsize);
 unsigned long approxidate(const char *);
+unsigned long approxidate_relative(const char *date, const struct timeval *now);
 enum date_mode parse_date_format(const char *format);
 
 #define IDENT_WARN_ON_NO_NAME  1
diff --git a/date.c b/date.c
index 409a17d..08b4b49 100644
--- a/date.c
+++ b/date.c
@@ -84,6 +84,67 @@ static int local_tzoffset(unsigned long time)
 	return offset * eastwest;
 }
 
+const char *show_date_relative(unsigned long time, int tz, const struct timeval *now)
+{
+	static char timebuf[100 /* TODO: can be optimized */];
+	unsigned long diff;
+	if (now->tv_sec < time)
+		return "in the future";
+	diff = now->tv_sec - time;
+	if (diff < 90) {
+		snprintf(timebuf, sizeof(timebuf), "%lu seconds ago", diff);
+		return timebuf;
+	}
+	/* Turn it into minutes */
+	diff = (diff + 30) / 60;
+	if (diff < 90) {
+		snprintf(timebuf, sizeof(timebuf), "%lu minutes ago", diff);
+		return timebuf;
+	}
+	/* Turn it into hours */
+	diff = (diff + 30) / 60;
+	if (diff < 36) {
+		snprintf(timebuf, sizeof(timebuf), "%lu hours ago", diff);
+		return timebuf;
+	}
+	/* We deal with number of days from here on */
+	diff = (diff + 12) / 24;
+	if (diff < 14) {
+		snprintf(timebuf, sizeof(timebuf), "%lu days ago", diff);
+		return timebuf;
+	}
+	/* Say weeks for the past 10 weeks or so */
+	if (diff < 70) {
+		snprintf(timebuf, sizeof(timebuf), "%lu weeks ago", (diff + 3) / 7);
+		return timebuf;
+	}
+	/* Say months for the past 12 months or so */
+	if (diff < 360) {
+		snprintf(timebuf, sizeof(timebuf), "%lu months ago", (diff + 15) / 30);
+		return timebuf;
+	}
+	/* Give years and months for 5 years or so */
+	if (diff < 1825) {
+		unsigned long years = (diff + 183) / 365;
+		unsigned long months = (diff % 365 + 15) / 30;
+		int n;
+		n = snprintf(timebuf, sizeof(timebuf), "%lu year%s",
+			     years, (years > 1 ? "s" : ""));
+		if (months)
+			snprintf(timebuf + n, sizeof(timebuf) - n,
+				 ", %lu month%s ago",
+				 months, (months > 1 ? "s" : ""));
+		else
+			snprintf(timebuf + n, sizeof(timebuf) - n,
+				 " ago");
+		return timebuf;
+	}
+	/* Otherwise, just years. Centuries is probably overkill. */
+	snprintf(timebuf, sizeof(timebuf), "%lu years ago", (diff + 183) / 365);
+	return timebuf;
+
+}
+
 const char *show_date(unsigned long time, int tz, enum date_mode mode)
 {
 	struct tm *tm;
@@ -95,63 +156,9 @@ const char *show_date(unsigned long time, int tz, enum date_mode mode)
 	}
 
 	if (mode == DATE_RELATIVE) {
-		unsigned long diff;
 		struct timeval now;
 		gettimeofday(&now, NULL);
-		if (now.tv_sec < time)
-			return "in the future";
-		diff = now.tv_sec - time;
-		if (diff < 90) {
-			snprintf(timebuf, sizeof(timebuf), "%lu seconds ago", diff);
-			return timebuf;
-		}
-		/* Turn it into minutes */
-		diff = (diff + 30) / 60;
-		if (diff < 90) {
-			snprintf(timebuf, sizeof(timebuf), "%lu minutes ago", diff);
-			return timebuf;
-		}
-		/* Turn it into hours */
-		diff = (diff + 30) / 60;
-		if (diff < 36) {
-			snprintf(timebuf, sizeof(timebuf), "%lu hours ago", diff);
-			return timebuf;
-		}
-		/* We deal with number of days from here on */
-		diff = (diff + 12) / 24;
-		if (diff < 14) {
-			snprintf(timebuf, sizeof(timebuf), "%lu days ago", diff);
-			return timebuf;
-		}
-		/* Say weeks for the past 10 weeks or so */
-		if (diff < 70) {
-			snprintf(timebuf, sizeof(timebuf), "%lu weeks ago", (diff + 3) / 7);
-			return timebuf;
-		}
-		/* Say months for the past 12 months or so */
-		if (diff < 360) {
-			snprintf(timebuf, sizeof(timebuf), "%lu months ago", (diff + 15) / 30);
-			return timebuf;
-		}
-		/* Give years and months for 5 years or so */
-		if (diff < 1825) {
-			unsigned long years = (diff + 183) / 365;
-			unsigned long months = (diff % 365 + 15) / 30;
-			int n;
-			n = snprintf(timebuf, sizeof(timebuf), "%lu year%s",
-					years, (years > 1 ? "s" : ""));
-			if (months)
-				snprintf(timebuf + n, sizeof(timebuf) - n,
-					", %lu month%s ago",
-					months, (months > 1 ? "s" : ""));
-			else
-				snprintf(timebuf + n, sizeof(timebuf) - n,
-					" ago");
-			return timebuf;
-		}
-		/* Otherwise, just years. Centuries is probably overkill. */
-		snprintf(timebuf, sizeof(timebuf), "%lu years ago", (diff + 183) / 365);
-		return timebuf;
+		return show_date_relative(time, tz, &now);
 	}
 
 	if (mode == DATE_LOCAL)
@@ -866,19 +873,17 @@ static const char *approxidate_digit(const char *date, struct tm *tm, int *num)
 	return end;
 }
 
-unsigned long approxidate(const char *date)
+unsigned long approxidate_relative(const char *date, const struct timeval *tv)
 {
 	int number = 0;
 	struct tm tm, now;
-	struct timeval tv;
 	time_t time_sec;
 	char buffer[50];
 
 	if (parse_date(date, buffer, sizeof(buffer)) > 0)
 		return strtoul(buffer, NULL, 10);
 
-	gettimeofday(&tv, NULL);
-	time_sec = tv.tv_sec;
+	time_sec = tv->tv_sec;
 	localtime_r(&time_sec, &tm);
 	now = tm;
 	for (;;) {
@@ -899,3 +904,10 @@ unsigned long approxidate(const char *date)
 		tm.tm_year--;
 	return mktime(&tm);
 }
+
+unsigned long approxidate(const char *date)
+{
+	struct timeval tv;
+	gettimeofday(&tv, NULL);
+	return approxidate_relative(date, &tv);
+}
-- 
1.6.4.1.261.gf9874

Re: [PATCH] Round-down years in "years+months" relative date view

[Jeff King]

On Fri, Aug 28, 2009 at 09:03:19PM +0200, Alex Riesen wrote:

> +unsigned long approxidate(const char *date)
> +{
> +	struct timeval tv;
> +	gettimeofday(&tv, NULL);
> +	return approxidate_relative(date, &tv);
> +}

This now always calls gettimeofday, whereas the original approxidate
only did if parse_date failed.

I think you could also make this patch much smaller by just wrapping the
whole function and using a '0' sentinel for "you need to fill in the
time." Like:

---
diff --git a/date.c b/date.c
index 409a17d..b084d19 100644
--- a/date.c
+++ b/date.c
@@ -86,6 +86,14 @@ static int local_tzoffset(unsigned long time)
 
 const char *show_date(unsigned long time, int tz, enum date_mode mode)
 {
+	struct timeval now;
+	now.tv_sec = 0;
+	show_date_at_time(time, tz, mode, &now);
+}
+
+const char *show_date_at_time(unsigned long time, int tz, enum date_mode mode,
+		struct timeval now)
+{
 	struct tm *tm;
 	static char timebuf[200];
 
@@ -96,8 +104,8 @@ const char *show_date(unsigned long time, int tz, enum date_mode mode)
 
 	if (mode == DATE_RELATIVE) {
 		unsigned long diff;
-		struct timeval now;
-		gettimeofday(&now, NULL);
+		if (!now.tv_sec)
+			gettimeofday(&now, NULL);
 		if (now.tv_sec < time)
 			return "in the future";
 		diff = now.tv_sec - time;

On the other hand, refactoring the relative date code into its own
function is probably a good thing in the long run.

-Peff

Re: [PATCH] Round-down years in "years+months" relative date view

[Alex Riesen]

On Fri, Aug 28, 2009 at 21:15, Jeff King<peff@peff.net> wrote:
> On Fri, Aug 28, 2009 at 09:03:19PM +0200, Alex Riesen wrote:
>
>> +unsigned long approxidate(const char *date)
>> +{
>> +     struct timeval tv;
>> +     gettimeofday(&tv, NULL);
>> +     return approxidate_relative(date, &tv);
>> +}
>
> This now always calls gettimeofday, whereas the original approxidate
> only did if parse_date failed.

Oh, bugger...

> On the other hand, refactoring the relative date code into its own
> function is probably a good thing in the long run.

Exactly.

Re: [PATCH] Round-down years in "years+months" relative date view

[Alex Riesen]

From fe67532bdf095dc9ebc0c7dd67be384e807a197c Mon Sep 17 00:00:00 2001
From: Alex Riesen <raa.lkml@gmail.com>
Date: Fri, 28 Aug 2009 20:59:59 +0200
Subject: [PATCH] Add date formatting functions with current time explicitely formatted

It should allow safe testing of this part of the code.
---
Alex Riesen, Fri, Aug 28, 2009 21:20:50 +0200:
> On Fri, Aug 28, 2009 at 21:15, Jeff King<peff@peff.net> wrote:
> > On Fri, Aug 28, 2009 at 09:03:19PM +0200, Alex Riesen wrote:
> >
> >> +unsigned long approxidate(const char *date)
> >> +{
> >> +     struct timeval tv;
> >> +     gettimeofday(&tv, NULL);
> >> +     return approxidate_relative(date, &tv);
> >> +}
> >
> > This now always calls gettimeofday, whereas the original approxidate
> > only did if parse_date failed.
> 
> Oh, bugger...
> 

Rather like this, I mean :)

 cache.h |    2 +
 date.c  |  150 ++++++++++++++++++++++++++++++++++++--------------------------
 2 files changed, 89 insertions(+), 63 deletions(-)

diff --git a/cache.h b/cache.h
index dd7f71e..3fb0166 100644
--- a/cache.h
+++ b/cache.h
@@ -731,9 +731,11 @@ enum date_mode {
 };
 
 const char *show_date(unsigned long time, int timezone, enum date_mode mode);
+const char *show_date_relative(unsigned long time, int tz, const struct timeval *now);
 int parse_date(const char *date, char *buf, int bufsize);
 void datestamp(char *buf, int bufsize);
 unsigned long approxidate(const char *);
+unsigned long approxidate_relative(const char *date, const struct timeval *now);
 enum date_mode parse_date_format(const char *format);
 
 #define IDENT_WARN_ON_NO_NAME  1
diff --git a/date.c b/date.c
index 409a17d..171e68f 100644
--- a/date.c
+++ b/date.c
@@ -84,6 +84,67 @@ static int local_tzoffset(unsigned long time)
 	return offset * eastwest;
 }
 
+const char *show_date_relative(unsigned long time, int tz, const struct timeval *now)
+{
+	static char timebuf[100 /* TODO: can be optimized */];
+	unsigned long diff;
+	if (now->tv_sec < time)
+		return "in the future";
+	diff = now->tv_sec - time;
+	if (diff < 90) {
+		snprintf(timebuf, sizeof(timebuf), "%lu seconds ago", diff);
+		return timebuf;
+	}
+	/* Turn it into minutes */
+	diff = (diff + 30) / 60;
+	if (diff < 90) {
+		snprintf(timebuf, sizeof(timebuf), "%lu minutes ago", diff);
+		return timebuf;
+	}
+	/* Turn it into hours */
+	diff = (diff + 30) / 60;
+	if (diff < 36) {
+		snprintf(timebuf, sizeof(timebuf), "%lu hours ago", diff);
+		return timebuf;
+	}
+	/* We deal with number of days from here on */
+	diff = (diff + 12) / 24;
+	if (diff < 14) {
+		snprintf(timebuf, sizeof(timebuf), "%lu days ago", diff);
+		return timebuf;
+	}
+	/* Say weeks for the past 10 weeks or so */
+	if (diff < 70) {
+		snprintf(timebuf, sizeof(timebuf), "%lu weeks ago", (diff + 3) / 7);
+		return timebuf;
+	}
+	/* Say months for the past 12 months or so */
+	if (diff < 360) {
+		snprintf(timebuf, sizeof(timebuf), "%lu months ago", (diff + 15) / 30);
+		return timebuf;
+	}
+	/* Give years and months for 5 years or so */
+	if (diff < 1825) {
+		unsigned long years = (diff + 183) / 365;
+		unsigned long months = (diff % 365 + 15) / 30;
+		int n;
+		n = snprintf(timebuf, sizeof(timebuf), "%lu year%s",
+			     years, (years > 1 ? "s" : ""));
+		if (months)
+			snprintf(timebuf + n, sizeof(timebuf) - n,
+				 ", %lu month%s ago",
+				 months, (months > 1 ? "s" : ""));
+		else
+			snprintf(timebuf + n, sizeof(timebuf) - n,
+				 " ago");
+		return timebuf;
+	}
+	/* Otherwise, just years. Centuries is probably overkill. */
+	snprintf(timebuf, sizeof(timebuf), "%lu years ago", (diff + 183) / 365);
+	return timebuf;
+
+}
+
 const char *show_date(unsigned long time, int tz, enum date_mode mode)
 {
 	struct tm *tm;
@@ -95,63 +156,9 @@ const char *show_date(unsigned long time, int tz, enum date_mode mode)
 	}
 
 	if (mode == DATE_RELATIVE) {
-		unsigned long diff;
 		struct timeval now;
 		gettimeofday(&now, NULL);
-		if (now.tv_sec < time)
-			return "in the future";
-		diff = now.tv_sec - time;
-		if (diff < 90) {
-			snprintf(timebuf, sizeof(timebuf), "%lu seconds ago", diff);
-			return timebuf;
-		}
-		/* Turn it into minutes */
-		diff = (diff + 30) / 60;
-		if (diff < 90) {
-			snprintf(timebuf, sizeof(timebuf), "%lu minutes ago", diff);
-			return timebuf;
-		}
-		/* Turn it into hours */
-		diff = (diff + 30) / 60;
-		if (diff < 36) {
-			snprintf(timebuf, sizeof(timebuf), "%lu hours ago", diff);
-			return timebuf;
-		}
-		/* We deal with number of days from here on */
-		diff = (diff + 12) / 24;
-		if (diff < 14) {
-			snprintf(timebuf, sizeof(timebuf), "%lu days ago", diff);
-			return timebuf;
-		}
-		/* Say weeks for the past 10 weeks or so */
-		if (diff < 70) {
-			snprintf(timebuf, sizeof(timebuf), "%lu weeks ago", (diff + 3) / 7);
-			return timebuf;
-		}
-		/* Say months for the past 12 months or so */
-		if (diff < 360) {
-			snprintf(timebuf, sizeof(timebuf), "%lu months ago", (diff + 15) / 30);
-			return timebuf;
-		}
-		/* Give years and months for 5 years or so */
-		if (diff < 1825) {
-			unsigned long years = (diff + 183) / 365;
-			unsigned long months = (diff % 365 + 15) / 30;
-			int n;
-			n = snprintf(timebuf, sizeof(timebuf), "%lu year%s",
-					years, (years > 1 ? "s" : ""));
-			if (months)
-				snprintf(timebuf + n, sizeof(timebuf) - n,
-					", %lu month%s ago",
-					months, (months > 1 ? "s" : ""));
-			else
-				snprintf(timebuf + n, sizeof(timebuf) - n,
-					" ago");
-			return timebuf;
-		}
-		/* Otherwise, just years. Centuries is probably overkill. */
-		snprintf(timebuf, sizeof(timebuf), "%lu years ago", (diff + 183) / 365);
-		return timebuf;
+		return show_date_relative(time, tz, &now);
 	}
 
 	if (mode == DATE_LOCAL)
@@ -866,19 +873,13 @@ static const char *approxidate_digit(const char *date, struct tm *tm, int *num)
 	return end;
 }
 
-unsigned long approxidate(const char *date)
+static unsigned long approximation(const char *date, const struct timeval *tv)
 {
 	int number = 0;
 	struct tm tm, now;
-	struct timeval tv;
 	time_t time_sec;
-	char buffer[50];
 
-	if (parse_date(date, buffer, sizeof(buffer)) > 0)
-		return strtoul(buffer, NULL, 10);
-
-	gettimeofday(&tv, NULL);
-	time_sec = tv.tv_sec;
+	time_sec = tv->tv_sec;
 	localtime_r(&time_sec, &tm);
 	now = tm;
 	for (;;) {
@@ -899,3 +900,26 @@ unsigned long approxidate(const char *date)
 		tm.tm_year--;
 	return mktime(&tm);
 }
+
+unsigned long approxidate_relative(const char *date, const struct timeval *tv)
+{
+	char buffer[50];
+
+	if (parse_date(date, buffer, sizeof(buffer)) > 0)
+		return strtoul(buffer, NULL, 10);
+
+	return approximation(date, tv);
+}
+
+unsigned long approxidate(const char *date)
+{
+	struct timeval tv;
+	char buffer[50];
+
+	if (parse_date(date, buffer, sizeof(buffer)) > 0)
+		return strtoul(buffer, NULL, 10);
+
+	gettimeofday(&tv, NULL);
+	return approximation(date, &tv);
+}
+
-- 
1.6.4.1.261.gf9874

[PATCH] Allow testing of _relative family of time formatting and parsing functions

[Alex Riesen]

To complement the testability of approxidate.
---
Alex Riesen, Fri, Aug 28, 2009 21:33:02 +0200:
> 
> It should allow safe testing of this part of the code.

And this should really allow testing of it:

    $ ./test-date '10.days.ago'
    10.days.ago -> bad -> Thu Jan  1 01:00:00 1970
    10.days.ago -> Tue Aug 18 22:50:20 2009

    relative: 10.days.ago -> Fri Dec 22 12:00:00 1989

    relative: 10 days ago, out of Fri Dec 22 12:00:00 1989

    $

According to Wikipedia, absolutely nothing of note happened
at the day 10 January, 1990.

 test-date.c |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/test-date.c b/test-date.c
index 62e8f23..dcc7973 100644
--- a/test-date.c
+++ b/test-date.c
@@ -4,6 +4,17 @@ int main(int argc, char **argv)
 {
 	int i;
 
+	struct tm tm;
+	struct timeval when = {0, 0};
+	tm.tm_sec  = 0;
+	tm.tm_min  = 0;
+	tm.tm_hour = 12;
+	tm.tm_mday = 1;
+	tm.tm_mon  = 0  /* January */;
+	tm.tm_year = 90 /* 1990 */ ;
+	tm.tm_isdst = -1;
+	when.tv_sec = mktime(&tm);
+
 	for (i = 1; i < argc; i++) {
 		char result[100];
 		time_t t;
@@ -15,6 +26,12 @@ int main(int argc, char **argv)
 
 		t = approxidate(argv[i]);
 		printf("%s -> %s\n", argv[i], ctime(&t));
+
+		t = approxidate_relative(argv[i], &when);
+		printf("relative: %s -> %s\n", argv[i], ctime(&t));
+
+		printf("relative: %s, out of %s\n",
+		       show_date_relative(t, 0, &when), ctime(&t));
 	}
 	return 0;
 }
-- 
1.6.4.1.263.g468a

Re: [PATCH] Allow testing of _relative family of time formatting and parsing functions

[Alex Riesen]

Alex Riesen, Fri, Aug 28, 2009 22:52:32 +0200:
> According to Wikipedia, absolutely nothing of note happened
> at the day 10 January, 1990.

1 Jan. 1990, not 10.

Re: [PATCH] Allow testing of _relative family of time formatting and parsing functions

[Junio C Hamano]

Alex Riesen <raa.lkml@gmail.com> writes:

> To complement the testability of approxidate.
> ---
> Alex Riesen, Fri, Aug 28, 2009 21:33:02 +0200:
>> 
>> It should allow safe testing of this part of the code.
>
> And this should really allow testing of it:
>
>     $ ./test-date '10.days.ago'
>     10.days.ago -> bad -> Thu Jan  1 01:00:00 1970
>     10.days.ago -> Tue Aug 18 22:50:20 2009
>
>     relative: 10.days.ago -> Fri Dec 22 12:00:00 1989
>
>     relative: 10 days ago, out of Fri Dec 22 12:00:00 1989
>
>     $

What are these blank lines for?  Is this intended as a serious submission
for inclusion?  I am having a hrad time to guess, especially you did not
sign this off, nor Cc'ed me.

Re: [PATCH] Allow testing of _relative family of time formatting and parsing functions

[Alex Riesen]

On Sat, Aug 29, 2009 at 23:46, Junio C Hamano<gitster@pobox.com> wrote:
> Alex Riesen <raa.lkml@gmail.com> writes:
>> To complement the testability of approxidate.
>> ---
>> Alex Riesen, Fri, Aug 28, 2009 21:33:02 +0200:
>>>
>>> It should allow safe testing of this part of the code.
>>
>> And this should really allow testing of it:
>>
>>     $ ./test-date '10.days.ago'
>>     10.days.ago -> bad -> Thu Jan  1 01:00:00 1970
>>     10.days.ago -> Tue Aug 18 22:50:20 2009
>>
>>     relative: 10.days.ago -> Fri Dec 22 12:00:00 1989
>>
>>     relative: 10 days ago, out of Fri Dec 22 12:00:00 1989
>>
>>     $
>
> What are these blank lines for?

ctime(3) artifact (it adds a \n in the output buffer), which I missed.

> Is this intended as a serious submission for inclusion?

Not yet. AFAICS, test-date is never used in our test suite.

> I am having a hrad time to guess, especially you did not
> sign this off, nor Cc'ed me.

Right, that's because I'm not sure myself. Frankly, I'm not
convinced we have to test every single thing. In my experience,
the bigger a test suite, the less are people inclined to use it
(including setting up automatic test runs).

Jeff, Nicolas? Is this test enough? Are there any other code
paths you want to include in the test?

And sorry for having you missed in Cc:, that wasn't intended.

Re: [PATCH] Allow testing of _relative family of time formatting and parsing functions

[Jeff King]

On Sun, Aug 30, 2009 at 09:25:11AM +0200, Alex Riesen wrote:

> > Is this intended as a serious submission for inclusion?
> 
> Not yet. AFAICS, test-date is never used in our test suite.

No, it isn't, but I think the point of this is to change that.
So it is useless without an extra patch to the test suite. I'll try to
put something together.

> Right, that's because I'm not sure myself. Frankly, I'm not
> convinced we have to test every single thing. In my experience,
> the bigger a test suite, the less are people inclined to use it
> (including setting up automatic test runs).
> 
> Jeff, Nicolas? Is this test enough? Are there any other code
> paths you want to include in the test?

I think this is a useful addition to the test suite. The bug David fixed
was obvious, but it sat for a year because of poor test coverage. Linus
fixed several approxidate bugs recently. The approxidate code is
notoriously temperamental, so it is a good thing to be checking for
regressions.

And I don't think our test suite is nearly big enough to start worrying
about getting people not to use it. Without CVS and SVN tests, I can run
it on 3-year-old hardware in less than a minute. Either you bother to
run it or not, but I doubt that adding one new test script is going to
break the bank.

-Peff

Re: [PATCH] Allow testing of _relative family of time formatting and parsing functions

[Alex Riesen]

On Sun, Aug 30, 2009 at 09:51, Jeff King<peff@peff.net> wrote:
> On Sun, Aug 30, 2009 at 09:25:11AM +0200, Alex Riesen wrote:
>
>> > Is this intended as a serious submission for inclusion?
>>
>> Not yet. AFAICS, test-date is never used in our test suite.
>
> No, it isn't, but I think the point of this is to change that.
> So it is useless without an extra patch to the test suite. I'll try to
> put something together.

Thanks :)

[PATCH 1/2] Add date formatting and parsing functions relative to a given time

[Alex Riesen]

The main purpose is to allow predictable testing of the code.

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---

Have show_date_relative supplied the output buffer. As it is a new
interface, it can as well be a little bit more generic than its sole
caller. test-date.c is updated and shall follow in a moment.

And, after a while thinking, I am convinced that Jeff has a point
and used a more "internal" name for approxidate's recent "bottom half".

 cache.h |    5 ++
 date.c  |  152 +++++++++++++++++++++++++++++++++++++--------------------------
 2 files changed, 94 insertions(+), 63 deletions(-)

diff --git a/cache.h b/cache.h
index dd7f71e..1586f33 100644
--- a/cache.h
+++ b/cache.h
@@ -731,9 +731,14 @@ enum date_mode {
 };
 
 const char *show_date(unsigned long time, int timezone, enum date_mode mode);
+const char *show_date_relative(unsigned long time, int tz,
+			       const struct timeval *now,
+			       char *timebuf,
+			       size_t timebuf_size);
 int parse_date(const char *date, char *buf, int bufsize);
 void datestamp(char *buf, int bufsize);
 unsigned long approxidate(const char *);
+unsigned long approxidate_relative(const char *date, const struct timeval *now);
 enum date_mode parse_date_format(const char *format);
 
 #define IDENT_WARN_ON_NO_NAME  1
diff --git a/date.c b/date.c
index f011692..0b0f7a7 100644
--- a/date.c
+++ b/date.c
@@ -84,6 +84,68 @@ static int local_tzoffset(unsigned long time)
 	return offset * eastwest;
 }
 
+const char *show_date_relative(unsigned long time, int tz,
+			       const struct timeval *now,
+			       char *timebuf,
+			       size_t timebuf_size)
+{
+	unsigned long diff;
+	if (now->tv_sec < time)
+		return "in the future";
+	diff = now->tv_sec - time;
+	if (diff < 90) {
+		snprintf(timebuf, timebuf_size, "%lu seconds ago", diff);
+		return timebuf;
+	}
+	/* Turn it into minutes */
+	diff = (diff + 30) / 60;
+	if (diff < 90) {
+		snprintf(timebuf, timebuf_size, "%lu minutes ago", diff);
+		return timebuf;
+	}
+	/* Turn it into hours */
+	diff = (diff + 30) / 60;
+	if (diff < 36) {
+		snprintf(timebuf, timebuf_size, "%lu hours ago", diff);
+		return timebuf;
+	}
+	/* We deal with number of days from here on */
+	diff = (diff + 12) / 24;
+	if (diff < 14) {
+		snprintf(timebuf, timebuf_size, "%lu days ago", diff);
+		return timebuf;
+	}
+	/* Say weeks for the past 10 weeks or so */
+	if (diff < 70) {
+		snprintf(timebuf, timebuf_size, "%lu weeks ago", (diff + 3) / 7);
+		return timebuf;
+	}
+	/* Say months for the past 12 months or so */
+	if (diff < 360) {
+		snprintf(timebuf, timebuf_size, "%lu months ago", (diff + 15) / 30);
+		return timebuf;
+	}
+	/* Give years and months for 5 years or so */
+	if (diff < 1825) {
+		unsigned long years = diff / 365;
+		unsigned long months = (diff % 365 + 15) / 30;
+		int n;
+		n = snprintf(timebuf, timebuf_size, "%lu year%s",
+			     years, (years > 1 ? "s" : ""));
+		if (months)
+			snprintf(timebuf + n, timebuf_size - n,
+				 ", %lu month%s ago",
+				 months, (months > 1 ? "s" : ""));
+		else
+			snprintf(timebuf + n, timebuf_size - n,
+				 " ago");
+		return timebuf;
+	}
+	/* Otherwise, just years. Centuries is probably overkill. */
+	snprintf(timebuf, timebuf_size, "%lu years ago", (diff + 183) / 365);
+	return timebuf;
+}
+
 const char *show_date(unsigned long time, int tz, enum date_mode mode)
 {
 	struct tm *tm;
@@ -95,63 +157,10 @@ const char *show_date(unsigned long time, int tz, enum date_mode mode)
 	}
 
 	if (mode == DATE_RELATIVE) {
-		unsigned long diff;
 		struct timeval now;
 		gettimeofday(&now, NULL);
-		if (now.tv_sec < time)
-			return "in the future";
-		diff = now.tv_sec - time;
-		if (diff < 90) {
-			snprintf(timebuf, sizeof(timebuf), "%lu seconds ago", diff);
-			return timebuf;
-		}
-		/* Turn it into minutes */
-		diff = (diff + 30) / 60;
-		if (diff < 90) {
-			snprintf(timebuf, sizeof(timebuf), "%lu minutes ago", diff);
-			return timebuf;
-		}
-		/* Turn it into hours */
-		diff = (diff + 30) / 60;
-		if (diff < 36) {
-			snprintf(timebuf, sizeof(timebuf), "%lu hours ago", diff);
-			return timebuf;
-		}
-		/* We deal with number of days from here on */
-		diff = (diff + 12) / 24;
-		if (diff < 14) {
-			snprintf(timebuf, sizeof(timebuf), "%lu days ago", diff);
-			return timebuf;
-		}
-		/* Say weeks for the past 10 weeks or so */
-		if (diff < 70) {
-			snprintf(timebuf, sizeof(timebuf), "%lu weeks ago", (diff + 3) / 7);
-			return timebuf;
-		}
-		/* Say months for the past 12 months or so */
-		if (diff < 360) {
-			snprintf(timebuf, sizeof(timebuf), "%lu months ago", (diff + 15) / 30);
-			return timebuf;
-		}
-		/* Give years and months for 5 years or so */
-		if (diff < 1825) {
-			unsigned long years = diff / 365;
-			unsigned long months = (diff % 365 + 15) / 30;
-			int n;
-			n = snprintf(timebuf, sizeof(timebuf), "%lu year%s",
-					years, (years > 1 ? "s" : ""));
-			if (months)
-				snprintf(timebuf + n, sizeof(timebuf) - n,
-					", %lu month%s ago",
-					months, (months > 1 ? "s" : ""));
-			else
-				snprintf(timebuf + n, sizeof(timebuf) - n,
-					" ago");
-			return timebuf;
-		}
-		/* Otherwise, just years. Centuries is probably overkill. */
-		snprintf(timebuf, sizeof(timebuf), "%lu years ago", (diff + 183) / 365);
-		return timebuf;
+		return show_date_relative(time, tz, &now,
+					  timebuf, sizeof(timebuf));
 	}
 
 	if (mode == DATE_LOCAL)
@@ -866,19 +875,13 @@ static const char *approxidate_digit(const char *date, struct tm *tm, int *num)
 	return end;
 }
 
-unsigned long approxidate(const char *date)
+static unsigned long approxidate_str(const char *date, const struct timeval *tv)
 {
 	int number = 0;
 	struct tm tm, now;
-	struct timeval tv;
 	time_t time_sec;
-	char buffer[50];
 
-	if (parse_date(date, buffer, sizeof(buffer)) > 0)
-		return strtoul(buffer, NULL, 10);
-
-	gettimeofday(&tv, NULL);
-	time_sec = tv.tv_sec;
+	time_sec = tv->tv_sec;
 	localtime_r(&time_sec, &tm);
 	now = tm;
 	for (;;) {
@@ -899,3 +902,26 @@ unsigned long approxidate(const char *date)
 		tm.tm_year--;
 	return mktime(&tm);
 }
+
+unsigned long approxidate_relative(const char *date, const struct timeval *tv)
+{
+	char buffer[50];
+
+	if (parse_date(date, buffer, sizeof(buffer)) > 0)
+		return strtoul(buffer, NULL, 10);
+
+	return approxidate_str(date, tv);
+}
+
+unsigned long approxidate(const char *date)
+{
+	struct timeval tv;
+	char buffer[50];
+
+	if (parse_date(date, buffer, sizeof(buffer)) > 0)
+		return strtoul(buffer, NULL, 10);
+
+	gettimeofday(&tv, NULL);
+	return approxidate_str(date, &tv);
+}
+
-- 
1.6.4.1.294.g16262

[PATCH 2/2] Allow testing of _relative family of time formatting and parsing functions

[Alex Riesen]

To complement the testability of approxidate.

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---

Alex Riesen, Sun, Aug 30, 2009 11:13:46 +0200:
> test-date.c is updated and shall follow in a moment.

here it goes.

 test-date.c |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/test-date.c b/test-date.c
index 62e8f23..4cb146d 100644
--- a/test-date.c
+++ b/test-date.c
@@ -4,6 +4,19 @@ int main(int argc, char **argv)
 {
 	int i;
 
+	/* see date.c, function show_date_relative */
+	char timebuf[(sizeof(long) * 5 / 2 + sizeof(" minutes ago,")) * 2];
+	struct tm tm;
+	struct timeval when = {0, 0};
+	tm.tm_sec  = 0;
+	tm.tm_min  = 0;
+	tm.tm_hour = 12;
+	tm.tm_mday = 1;
+	tm.tm_mon  = 0  /* January */;
+	tm.tm_year = 90 /* 1990 */ ;
+	tm.tm_isdst = -1;
+	when.tv_sec = mktime(&tm);
+
 	for (i = 1; i < argc; i++) {
 		char result[100];
 		time_t t;
@@ -15,6 +28,13 @@ int main(int argc, char **argv)
 
 		t = approxidate(argv[i]);
 		printf("%s -> %s\n", argv[i], ctime(&t));
+
+		t = approxidate_relative(argv[i], &when);
+		printf("relative: %s -> %s", argv[i], ctime(&t));
+
+		printf("relative: %s, out of %s",
+		       show_date_relative(t, 0, &when, timebuf,sizeof(timebuf)),
+		       ctime(&t));
 	}
 	return 0;
 }
-- 
1.6.4.1.294.g16262

Re: [PATCH 1/2] Add date formatting and parsing functions relative to a given time

[Jeff King]

On Sun, Aug 30, 2009 at 11:13:46AM +0200, Alex Riesen wrote:

> Have show_date_relative supplied the output buffer. As it is a new
> interface, it can as well be a little bit more generic than its sole
> caller. test-date.c is updated and shall follow in a moment.

FYI, I am munging test-date to match the test script I am writing, so
don't bother with that patch.

-Peff

Re: [PATCH 1/2] Add date formatting and parsing functions relative to a given time

[Jeff King]

On Sun, Aug 30, 2009 at 05:15:57AM -0400, Jeff King wrote:

> FYI, I am munging test-date to match the test script I am writing, so
> don't bother with that patch.

Here is what my patch is looking like. Please give any comments, and
then I will resubmit in a form that will be simpler for Junio, which
should be a series with:

  - your patch to refactor date.c
  - this patch (this version uses the original interface to
    show_relative; I will tweak to match the new patch you just sent)
  - another patch to go on top of lt/approxidate to test recent fixes
    from Linus

---
diff --git a/t/t0006-date.sh b/t/t0006-date.sh
new file mode 100755
index 0000000..4beb44b
--- /dev/null
+++ b/t/t0006-date.sh
@@ -0,0 +1,71 @@
+#!/bin/sh
+
+test_description='test date parsing and printing'
+. ./test-lib.sh
+
+# arbitrary reference time: 2009-08-30 19:20:00
+TEST_DATE_NOW=1251660000; export TEST_DATE_NOW
+
+check_show() {
+	t=$(($TEST_DATE_NOW - $1))
+	echo "$t -> $2" >expect
+	test_expect_success "relative date ($2)" "
+	test-date show $t >actual &&
+	test_cmp expect actual
+	"
+}
+
+check_show 5 '5 seconds ago'
+check_show 300 '5 minutes ago'
+check_show 18000 '5 hours ago'
+check_show 432000 '5 days ago'
+check_show 1728000 '3 weeks ago'
+check_show 13000000 '5 months ago'
+check_show 37500000 '1 year, 2 months ago'
+check_show 55188000 '1 year, 9 months ago'
+check_show 630000000 '20 years ago'
+
+check_parse() {
+	echo "$1 -> $2" >expect
+	test_expect_success "parse date ($1)" "
+	test-date parse '$1' >actual &&
+	test_cmp expect actual
+	"
+}
+
+check_parse 2008 bad
+check_parse 2008-02 bad
+check_parse 2008-02-14 '2008-02-14 00:00:00 +0000'
+check_parse '2008-02-14 20:30:45' '2008-02-14 20:30:45 +0000'
+
+check_approxidate() {
+	echo "$1 -> $2 +0000" >expect
+	test_expect_success "parse approxidate ($1)" "
+	test-date approxidate '$1' >actual &&
+	test_cmp expect actual
+	"
+}
+
+check_approxidate now '2009-08-30 19:20:00'
+check_approxidate '5 seconds ago' '2009-08-30 19:19:55'
+check_approxidate 5.seconds.ago '2009-08-30 19:19:55'
+check_approxidate 10.minutes.ago '2009-08-30 19:10:00'
+check_approxidate yesterday '2009-08-29 19:20:00'
+check_approxidate 3.days.ago '2009-08-27 19:20:00'
+check_approxidate 3.weeks.ago '2009-08-09 19:20:00'
+check_approxidate 3.months.ago '2009-05-30 19:20:00'
+check_approxidate 2.years.3.months.ago '2007-05-30 19:20:00'
+
+check_approxidate '6am yesterday' '2009-08-29 06:00:00'
+check_approxidate '6pm yesterday' '2009-08-29 18:00:00'
+check_approxidate '3:00' '2009-08-30 03:00:00'
+check_approxidate '15:00' '2009-08-30 15:00:00'
+check_approxidate 'noon today' '2009-08-30 12:00:00'
+check_approxidate 'noon yesterday' '2009-08-29 12:00:00'
+
+check_approxidate 'last tuesday' '2009-08-25 19:20:00'
+check_approxidate 'July 5th' '2009-07-05 19:20:00'
+check_approxidate '06/05/2009' '2009-06-05 00:00:00'
+check_approxidate '06.05.2009' '2009-05-06 00:00:00'
+
+test_done
diff --git a/test-date.c b/test-date.c
index 62e8f23..8d263a3 100644
--- a/test-date.c
+++ b/test-date.c
@@ -1,20 +1,63 @@
 #include "cache.h"
 
-int main(int argc, char **argv)
+static const char *usage_msg = "\n"
+"  test-date show [time_t]...\n"
+"  test-date parse [date]...\n"
+"  test-date approxidate [date]...\n";
+
+static void show_dates(char **argv, struct timeval *now)
 {
-	int i;
+	for (; *argv; argv++) {
+		time_t t = atoi(*argv);
+		printf("%s -> %s\n", *argv, show_date_relative(t, 0, now));
+	}
+}
 
-	for (i = 1; i < argc; i++) {
+static void parse_dates(char **argv, struct timeval *now)
+{
+	for (; *argv; argv++) {
 		char result[100];
 		time_t t;
 
-		memcpy(result, "bad", 4);
-		parse_date(argv[i], result, sizeof(result));
+		parse_date(*argv, result, sizeof(result));
 		t = strtoul(result, NULL, 0);
-		printf("%s -> %s -> %s", argv[i], result, ctime(&t));
+		printf("%s -> %s\n", *argv,
+			t ? show_date(t, 0, DATE_ISO8601) : "bad");
+	}
+}
+
+static void parse_approxidate(char **argv, struct timeval *now)
+{
+	for (; *argv; argv++) {
+		time_t t;
+		t = approxidate_relative(*argv, now);
+		printf("%s -> %s\n", *argv, show_date(t, 0, DATE_ISO8601));
+	}
+}
 
-		t = approxidate(argv[i]);
-		printf("%s -> %s\n", argv[i], ctime(&t));
+int main(int argc, char **argv)
+{
+	struct timeval now;
+	const char *x;
+
+	x = getenv("TEST_DATE_NOW");
+	if (x) {
+		now.tv_sec = atoi(x);
+		now.tv_usec = 0;
 	}
+	else
+		gettimeofday(&now, NULL);
+
+	argv++;
+	if (!*argv)
+		usage(usage_msg);
+	if (!strcmp(*argv, "show"))
+		show_dates(argv+1, &now);
+	else if (!strcmp(*argv, "parse"))
+		parse_dates(argv+1, &now);
+	else if (!strcmp(*argv, "approxidate"))
+		parse_approxidate(argv+1, &now);
+	else
+		usage(usage_msg);
 	return 0;
 }

Re: [PATCH 1/2] Add date formatting and parsing functions relative to a given time

[Alex Riesen]

On Sun, Aug 30, 2009 at 11:36, Jeff King<peff@peff.net> wrote:
> On Sun, Aug 30, 2009 at 05:15:57AM -0400, Jeff King wrote:
>
>> FYI, I am munging test-date to match the test script I am writing, so
>> don't bother with that patch.
>
> Here is what my patch is looking like. Please give any comments, and
> then I will resubmit in a form that will be simpler for Junio, which
> should be a series with:
>
>  - your patch to refactor date.c
>  - this patch (this version uses the original interface to
>    show_relative; I will tweak to match the new patch you just sent)

Yes, I think this is the only comment I can make.

> +# arbitrary reference time: 2009-08-30 19:20:00

The world changed since 1980 :) There is already three things
happened at the day (http://en.wikipedia.org/wiki/August_2009),
and it is not evening yet (well, here in Europe)

> +check_show 5 '5 seconds ago'
> +check_show 300 '5 minutes ago'
> +check_show 18000 '5 hours ago'
> +check_show 432000 '5 days ago'
> +check_show 1728000 '3 weeks ago'
> +check_show 13000000 '5 months ago'
> +check_show 37500000 '1 year, 2 months ago'
> +check_show 55188000 '1 year, 9 months ago'
> +check_show 630000000 '20 years ago'

check_show 630000000 '20.years.ago'?
(Arbitrary, non-whitespace delimiters, which was an
advertised feature, to make shell's life easier)

Re: [PATCH 1/2] Add date formatting and parsing functions relative to a given time

[Jeff King]

On Sun, Aug 30, 2009 at 11:56:37AM +0200, Alex Riesen wrote:

> > +check_show 5 '5 seconds ago'
> > +check_show 300 '5 minutes ago'
> > +check_show 18000 '5 hours ago'
> > +check_show 432000 '5 days ago'
> > +check_show 1728000 '3 weeks ago'
> > +check_show 13000000 '5 months ago'
> > +check_show 37500000 '1 year, 2 months ago'
> > +check_show 55188000 '1 year, 9 months ago'
> > +check_show 630000000 '20 years ago'
> 
> check_show 630000000 '20.years.ago'?
> (Arbitrary, non-whitespace delimiters, which was an
> advertised feature, to make shell's life easier)

This part is about checking what show_date produces (the first number is
an offset from now in seconds, and the second is what we expect), so it
always has spaces.

See the check_approxidate section further down for an example of parsing
what you are talking about.

-Peff

Re: [PATCH 1/2] Add date formatting and parsing functions relative to a given time

[Alex Riesen]

On Sun, Aug 30, 2009 at 12:08, Jeff King<peff@peff.net> wrote:
> On Sun, Aug 30, 2009 at 11:56:37AM +0200, Alex Riesen wrote:
>>
>> check_show 630000000 '20.years.ago'?
>> (Arbitrary, non-whitespace delimiters, which was an
>> advertised feature, to make shell's life easier)
>
> This part is about checking what show_date produces (the first number is
> an offset from now in seconds, and the second is what we expect), so it
> always has spaces.
>
> See the check_approxidate section further down for an example of parsing
> what you are talking about.

Ah, I see

[PATCH 1/3] Add date formatting and parsing functions relative to a given time

[Jeff King]

From: Alex Riesen <raa.lkml@gmail.com>

The main purpose is to allow predictable testing of the code.

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
---
 cache.h |    5 ++
 date.c  |  152 +++++++++++++++++++++++++++++++++++++--------------------------
 2 files changed, 94 insertions(+), 63 deletions(-)

diff --git a/cache.h b/cache.h
index 808daba..5fad24c 100644
--- a/cache.h
+++ b/cache.h
@@ -731,9 +731,14 @@ enum date_mode {
 };
 
 const char *show_date(unsigned long time, int timezone, enum date_mode mode);
+const char *show_date_relative(unsigned long time, int tz,
+			       const struct timeval *now,
+			       char *timebuf,
+			       size_t timebuf_size);
 int parse_date(const char *date, char *buf, int bufsize);
 void datestamp(char *buf, int bufsize);
 unsigned long approxidate(const char *);
+unsigned long approxidate_relative(const char *date, const struct timeval *now);
 enum date_mode parse_date_format(const char *format);
 
 #define IDENT_WARN_ON_NO_NAME  1
diff --git a/date.c b/date.c
index f011692..0b0f7a7 100644
--- a/date.c
+++ b/date.c
@@ -84,6 +84,68 @@ static int local_tzoffset(unsigned long time)
 	return offset * eastwest;
 }
 
+const char *show_date_relative(unsigned long time, int tz,
+			       const struct timeval *now,
+			       char *timebuf,
+			       size_t timebuf_size)
+{
+	unsigned long diff;
+	if (now->tv_sec < time)
+		return "in the future";
+	diff = now->tv_sec - time;
+	if (diff < 90) {
+		snprintf(timebuf, timebuf_size, "%lu seconds ago", diff);
+		return timebuf;
+	}
+	/* Turn it into minutes */
+	diff = (diff + 30) / 60;
+	if (diff < 90) {
+		snprintf(timebuf, timebuf_size, "%lu minutes ago", diff);
+		return timebuf;
+	}
+	/* Turn it into hours */
+	diff = (diff + 30) / 60;
+	if (diff < 36) {
+		snprintf(timebuf, timebuf_size, "%lu hours ago", diff);
+		return timebuf;
+	}
+	/* We deal with number of days from here on */
+	diff = (diff + 12) / 24;
+	if (diff < 14) {
+		snprintf(timebuf, timebuf_size, "%lu days ago", diff);
+		return timebuf;
+	}
+	/* Say weeks for the past 10 weeks or so */
+	if (diff < 70) {
+		snprintf(timebuf, timebuf_size, "%lu weeks ago", (diff + 3) / 7);
+		return timebuf;
+	}
+	/* Say months for the past 12 months or so */
+	if (diff < 360) {
+		snprintf(timebuf, timebuf_size, "%lu months ago", (diff + 15) / 30);
+		return timebuf;
+	}
+	/* Give years and months for 5 years or so */
+	if (diff < 1825) {
+		unsigned long years = diff / 365;
+		unsigned long months = (diff % 365 + 15) / 30;
+		int n;
+		n = snprintf(timebuf, timebuf_size, "%lu year%s",
+			     years, (years > 1 ? "s" : ""));
+		if (months)
+			snprintf(timebuf + n, timebuf_size - n,
+				 ", %lu month%s ago",
+				 months, (months > 1 ? "s" : ""));
+		else
+			snprintf(timebuf + n, timebuf_size - n,
+				 " ago");
+		return timebuf;
+	}
+	/* Otherwise, just years. Centuries is probably overkill. */
+	snprintf(timebuf, timebuf_size, "%lu years ago", (diff + 183) / 365);
+	return timebuf;
+}
+
 const char *show_date(unsigned long time, int tz, enum date_mode mode)
 {
 	struct tm *tm;
@@ -95,63 +157,10 @@ const char *show_date(unsigned long time, int tz, enum date_mode mode)
 	}
 
 	if (mode == DATE_RELATIVE) {
-		unsigned long diff;
 		struct timeval now;
 		gettimeofday(&now, NULL);
-		if (now.tv_sec < time)
-			return "in the future";
-		diff = now.tv_sec - time;
-		if (diff < 90) {
-			snprintf(timebuf, sizeof(timebuf), "%lu seconds ago", diff);
-			return timebuf;
-		}
-		/* Turn it into minutes */
-		diff = (diff + 30) / 60;
-		if (diff < 90) {
-			snprintf(timebuf, sizeof(timebuf), "%lu minutes ago", diff);
-			return timebuf;
-		}
-		/* Turn it into hours */
-		diff = (diff + 30) / 60;
-		if (diff < 36) {
-			snprintf(timebuf, sizeof(timebuf), "%lu hours ago", diff);
-			return timebuf;
-		}
-		/* We deal with number of days from here on */
-		diff = (diff + 12) / 24;
-		if (diff < 14) {
-			snprintf(timebuf, sizeof(timebuf), "%lu days ago", diff);
-			return timebuf;
-		}
-		/* Say weeks for the past 10 weeks or so */
-		if (diff < 70) {
-			snprintf(timebuf, sizeof(timebuf), "%lu weeks ago", (diff + 3) / 7);
-			return timebuf;
-		}
-		/* Say months for the past 12 months or so */
-		if (diff < 360) {
-			snprintf(timebuf, sizeof(timebuf), "%lu months ago", (diff + 15) / 30);
-			return timebuf;
-		}
-		/* Give years and months for 5 years or so */
-		if (diff < 1825) {
-			unsigned long years = diff / 365;
-			unsigned long months = (diff % 365 + 15) / 30;
-			int n;
-			n = snprintf(timebuf, sizeof(timebuf), "%lu year%s",
-					years, (years > 1 ? "s" : ""));
-			if (months)
-				snprintf(timebuf + n, sizeof(timebuf) - n,
-					", %lu month%s ago",
-					months, (months > 1 ? "s" : ""));
-			else
-				snprintf(timebuf + n, sizeof(timebuf) - n,
-					" ago");
-			return timebuf;
-		}
-		/* Otherwise, just years. Centuries is probably overkill. */
-		snprintf(timebuf, sizeof(timebuf), "%lu years ago", (diff + 183) / 365);
-		return timebuf;
+		return show_date_relative(time, tz, &now,
+					  timebuf, sizeof(timebuf));
 	}
 
 	if (mode == DATE_LOCAL)
@@ -866,19 +875,13 @@ static const char *approxidate_digit(const char *date, struct tm *tm, int *num)
 	return end;
 }
 
-unsigned long approxidate(const char *date)
+static unsigned long approxidate_str(const char *date, const struct timeval *tv)
 {
 	int number = 0;
 	struct tm tm, now;
-	struct timeval tv;
 	time_t time_sec;
-	char buffer[50];
 
-	if (parse_date(date, buffer, sizeof(buffer)) > 0)
-		return strtoul(buffer, NULL, 10);
-
-	gettimeofday(&tv, NULL);
-	time_sec = tv.tv_sec;
+	time_sec = tv->tv_sec;
 	localtime_r(&time_sec, &tm);
 	now = tm;
 	for (;;) {
@@ -899,3 +902,26 @@ unsigned long approxidate(const char *date)
 		tm.tm_year--;
 	return mktime(&tm);
 }
+
+unsigned long approxidate_relative(const char *date, const struct timeval *tv)
+{
+	char buffer[50];
+
+	if (parse_date(date, buffer, sizeof(buffer)) > 0)
+		return strtoul(buffer, NULL, 10);
+
+	return approxidate_str(date, tv);
+}
+
+unsigned long approxidate(const char *date)
+{
+	struct timeval tv;
+	char buffer[50];
+
+	if (parse_date(date, buffer, sizeof(buffer)) > 0)
+		return strtoul(buffer, NULL, 10);
+
+	gettimeofday(&tv, NULL);
+	return approxidate_str(date, &tv);
+}
+
-- 
1.6.4.2.375.g73938

Re: [PATCH 1/3] Add date formatting and parsing functions relative to a given time

[Jeff King]

On Sun, Aug 30, 2009 at 05:43:09PM -0400, Jeff King wrote:

> From: Alex Riesen <raa.lkml@gmail.com>
> 
> The main purpose is to allow predictable testing of the code.
> 
> Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
> Signed-off-by: Jeff King <peff@peff.net>
> ---

Bleh. I just started working on a 4/3 that would test Linus' recent
approxidate changes, but then I realized that this massive date.c
refactoring conflicts with his changes.

I think the most sane thing is to rebase the whole series on top of
lt/approxidate. Let me see what I can do.

-Peff

Re: [PATCH 1/3] Add date formatting and parsing functions relative to a given time

[Jeff King]

On Sun, Aug 30, 2009 at 05:51:27PM -0400, Jeff King wrote:

> I think the most sane thing is to rebase the whole series on top of
> lt/approxidate. Let me see what I can do.

And here it is. It was a little more complex than a simple rebase
because lt/approxidate actually introduced new bugs. :) Hopefully this
will be the last re-roll required.

The new series applies on top of lt/approxidate, and contains:

  [1/4]: Add date formatting and parsing functions relative to a given time
  [2/4]: refactor test-date interface
  [3/4]: tests: add date printing and parsing tests
  [4/4]: fix approxidate parsing of relative months and years

-Peff

[PATCH v2 1/4] Add date formatting and parsing functions relative to a given time

[Jeff King]

From: Alex Riesen <raa.lkml@gmail.com>

The main purpose is to allow predictable testing of the code.

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
---
Same as previous 1/3, but rebased onto lt/approxidate topic. The merge
ended up as quite a mess because of textual differences, so I had to
fix up a fair bit by hand; Alex, please confirm that I didn't screw
anything up too badly right before putting your name at the top. ;)

 cache.h |    5 ++
 date.c  |  150 ++++++++++++++++++++++++++++++++++++--------------------------
 2 files changed, 92 insertions(+), 63 deletions(-)

diff --git a/cache.h b/cache.h
index 808daba..5fad24c 100644
--- a/cache.h
+++ b/cache.h
@@ -731,9 +731,14 @@ enum date_mode {
 };
 
 const char *show_date(unsigned long time, int timezone, enum date_mode mode);
+const char *show_date_relative(unsigned long time, int tz,
+			       const struct timeval *now,
+			       char *timebuf,
+			       size_t timebuf_size);
 int parse_date(const char *date, char *buf, int bufsize);
 void datestamp(char *buf, int bufsize);
 unsigned long approxidate(const char *);
+unsigned long approxidate_relative(const char *date, const struct timeval *now);
 enum date_mode parse_date_format(const char *format);
 
 #define IDENT_WARN_ON_NO_NAME  1
diff --git a/date.c b/date.c
index e848d96..8e57e5e 100644
--- a/date.c
+++ b/date.c
@@ -86,6 +86,67 @@ static int local_tzoffset(unsigned long time)
 	return offset * eastwest;
 }
 
+const char *show_date_relative(unsigned long time, int tz,
+			       const struct timeval *now,
+			       char *timebuf,
+			       size_t timebuf_size)
+{
+	unsigned long diff;
+	if (now->tv_sec < time)
+		return "in the future";
+	diff = now->tv_sec - time;
+	if (diff < 90) {
+		snprintf(timebuf, timebuf_size, "%lu seconds ago", diff);
+		return timebuf;
+	}
+	/* Turn it into minutes */
+	diff = (diff + 30) / 60;
+	if (diff < 90) {
+		snprintf(timebuf, timebuf_size, "%lu minutes ago", diff);
+		return timebuf;
+	}
+	/* Turn it into hours */
+	diff = (diff + 30) / 60;
+	if (diff < 36) {
+		snprintf(timebuf, timebuf_size, "%lu hours ago", diff);
+		return timebuf;
+	}
+	/* We deal with number of days from here on */
+	diff = (diff + 12) / 24;
+	if (diff < 14) {
+		snprintf(timebuf, timebuf_size, "%lu days ago", diff);
+		return timebuf;
+	}
+	/* Say weeks for the past 10 weeks or so */
+	if (diff < 70) {
+		snprintf(timebuf, timebuf_size, "%lu weeks ago", (diff + 3) / 7);
+		return timebuf;
+	}
+	/* Say months for the past 12 months or so */
+	if (diff < 360) {
+		snprintf(timebuf, timebuf_size, "%lu months ago", (diff + 15) / 30);
+		return timebuf;
+	}
+	/* Give years and months for 5 years or so */
+	if (diff < 1825) {
+		unsigned long years = diff / 365;
+		unsigned long months = (diff % 365 + 15) / 30;
+		int n;
+		n = snprintf(timebuf, timebuf_size, "%lu year%s",
+				years, (years > 1 ? "s" : ""));
+		if (months)
+			snprintf(timebuf + n, timebuf_size - n,
+					", %lu month%s ago",
+					months, (months > 1 ? "s" : ""));
+		else
+			snprintf(timebuf + n, timebuf_size - n, " ago");
+		return timebuf;
+	}
+	/* Otherwise, just years. Centuries is probably overkill. */
+	snprintf(timebuf, timebuf_size, "%lu years ago", (diff + 183) / 365);
+	return timebuf;
+}
+
 const char *show_date(unsigned long time, int tz, enum date_mode mode)
 {
 	struct tm *tm;
@@ -97,63 +158,10 @@ const char *show_date(unsigned long time, int tz, enum date_mode mode)
 	}
 
 	if (mode == DATE_RELATIVE) {
-		unsigned long diff;
 		struct timeval now;
 		gettimeofday(&now, NULL);
-		if (now.tv_sec < time)
-			return "in the future";
-		diff = now.tv_sec - time;
-		if (diff < 90) {
-			snprintf(timebuf, sizeof(timebuf), "%lu seconds ago", diff);
-			return timebuf;
-		}
-		/* Turn it into minutes */
-		diff = (diff + 30) / 60;
-		if (diff < 90) {
-			snprintf(timebuf, sizeof(timebuf), "%lu minutes ago", diff);
-			return timebuf;
-		}
-		/* Turn it into hours */
-		diff = (diff + 30) / 60;
-		if (diff < 36) {
-			snprintf(timebuf, sizeof(timebuf), "%lu hours ago", diff);
-			return timebuf;
-		}
-		/* We deal with number of days from here on */
-		diff = (diff + 12) / 24;
-		if (diff < 14) {
-			snprintf(timebuf, sizeof(timebuf), "%lu days ago", diff);
-			return timebuf;
-		}
-		/* Say weeks for the past 10 weeks or so */
-		if (diff < 70) {
-			snprintf(timebuf, sizeof(timebuf), "%lu weeks ago", (diff + 3) / 7);
-			return timebuf;
-		}
-		/* Say months for the past 12 months or so */
-		if (diff < 360) {
-			snprintf(timebuf, sizeof(timebuf), "%lu months ago", (diff + 15) / 30);
-			return timebuf;
-		}
-		/* Give years and months for 5 years or so */
-		if (diff < 1825) {
-			unsigned long years = diff / 365;
-			unsigned long months = (diff % 365 + 15) / 30;
-			int n;
-			n = snprintf(timebuf, sizeof(timebuf), "%lu year%s",
-					years, (years > 1 ? "s" : ""));
-			if (months)
-				snprintf(timebuf + n, sizeof(timebuf) - n,
-					", %lu month%s ago",
-					months, (months > 1 ? "s" : ""));
-			else
-				snprintf(timebuf + n, sizeof(timebuf) - n,
-					" ago");
-			return timebuf;
-		}
-		/* Otherwise, just years. Centuries is probably overkill. */
-		snprintf(timebuf, sizeof(timebuf), "%lu years ago", (diff + 183) / 365);
-		return timebuf;
+		return show_date_relative(time, tz, &now,
+					  timebuf, sizeof(timebuf));
 	}
 
 	if (mode == DATE_LOCAL)
@@ -918,19 +926,13 @@ static void pending_number(struct tm *tm, int *num)
 	}
 }
 
-unsigned long approxidate(const char *date)
+static unsigned long approxidate_str(const char *date, const struct timeval *tv)
 {
 	int number = 0;
 	struct tm tm, now;
-	struct timeval tv;
 	time_t time_sec;
-	char buffer[50];
 
-	if (parse_date(date, buffer, sizeof(buffer)) > 0)
-		return strtoul(buffer, NULL, 10);
-
-	gettimeofday(&tv, NULL);
-	time_sec = tv.tv_sec;
+	time_sec = tv->tv_sec;
 	localtime_r(&time_sec, &tm);
 	now = tm;
 
@@ -954,3 +956,25 @@ unsigned long approxidate(const char *date)
 	pending_number(&tm, &number);
 	return update_tm(&tm, &now, 0);
 }
+
+unsigned long approxidate_relative(const char *date, const struct timeval *tv)
+{
+	char buffer[50];
+
+	if (parse_date(date, buffer, sizeof(buffer)) > 0)
+		return strtoul(buffer, NULL, 0);
+
+	return approxidate_str(date, tv);
+}
+
+unsigned long approxidate(const char *date)
+{
+	struct timeval tv;
+	char buffer[50];
+
+	if (parse_date(date, buffer, sizeof(buffer)) > 0)
+		return strtoul(buffer, NULL, 0);
+
+	gettimeofday(&tv, NULL);
+	return approxidate_str(date, &tv);
+}
-- 
1.6.4.2.373.g5881fd

Re: [PATCH v2 1/4] Add date formatting and parsing functions relative to a given time

[Alex Riesen]

On Mon, Aug 31, 2009 at 04:26, Jeff King<peff@peff.net> wrote:
> From: Alex Riesen <raa.lkml@gmail.com>
>
> The main purpose is to allow predictable testing of the code.
>
> Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> Same as previous 1/3, but rebased onto lt/approxidate topic. The merge
> ended up as quite a mess because of textual differences, so I had to
> fix up a fair bit by hand; Alex, please confirm that I didn't screw
> anything up too badly right before putting your name at the top. ;)

Looks good.

[PATCH v2 2/4] refactor test-date interface

[Jeff King]

The test-date program goes back to the early days of git,
where it was presumably used to do manual sanity checks on
changes to the date code. However, it is not actually used
by the test suite to do any sort of automatic of systematic
tests.

This patch refactors the interface to the program to try to
make it more suitable for use by the test suite. There
should be no fallouts to changing the interface since it is
not actually installed and is not internally called by any
other programs.

The changes are:

  - add a "mode" parameter so the caller can specify which
    operation to test

  - add a mode to test relative date output from show_date

  - allow faking a fixed time via the TEST_DATE_NOW
    environment variable, which allows consistent automated
    testing

  - drop the use of ctime for showing dates in favor of our
    internal iso8601 printing routines. The ctime output is
    somewhat redundant (because of the day-of-week) which
    makes writing test cases more annoying.

Signed-off-by: Jeff King <peff@peff.net>
---
Same as earlier 2/3.

 test-date.c |   86 +++++++++++++++++++++++++++++++++++++++++++++-------------
 1 files changed, 66 insertions(+), 20 deletions(-)
 rewrite test-date.c (63%)

diff --git a/test-date.c b/test-date.c
dissimilarity index 63%
index 62e8f23..5b0a220 100644
--- a/test-date.c
+++ b/test-date.c
@@ -1,20 +1,66 @@
-#include "cache.h"
-
-int main(int argc, char **argv)
-{
-	int i;
-
-	for (i = 1; i < argc; i++) {
-		char result[100];
-		time_t t;
-
-		memcpy(result, "bad", 4);
-		parse_date(argv[i], result, sizeof(result));
-		t = strtoul(result, NULL, 0);
-		printf("%s -> %s -> %s", argv[i], result, ctime(&t));
-
-		t = approxidate(argv[i]);
-		printf("%s -> %s\n", argv[i], ctime(&t));
-	}
-	return 0;
-}
+#include "cache.h"
+
+static const char *usage_msg = "\n"
+"  test-date show [time_t]...\n"
+"  test-date parse [date]...\n"
+"  test-date approxidate [date]...\n";
+
+static void show_dates(char **argv, struct timeval *now)
+{
+	char buf[128];
+
+	for (; *argv; argv++) {
+		time_t t = atoi(*argv);
+		show_date_relative(t, 0, now, buf, sizeof(buf));
+		printf("%s -> %s\n", *argv, buf);
+	}
+}
+
+static void parse_dates(char **argv, struct timeval *now)
+{
+	for (; *argv; argv++) {
+		char result[100];
+		time_t t;
+
+		parse_date(*argv, result, sizeof(result));
+		t = strtoul(result, NULL, 0);
+		printf("%s -> %s\n", *argv,
+			t ? show_date(t, 0, DATE_ISO8601) : "bad");
+	}
+}
+
+static void parse_approxidate(char **argv, struct timeval *now)
+{
+	for (; *argv; argv++) {
+		time_t t;
+		t = approxidate_relative(*argv, now);
+		printf("%s -> %s\n", *argv, show_date(t, 0, DATE_ISO8601));
+	}
+}
+
+int main(int argc, char **argv)
+{
+	struct timeval now;
+	const char *x;
+
+	x = getenv("TEST_DATE_NOW");
+	if (x) {
+		now.tv_sec = atoi(x);
+		now.tv_usec = 0;
+	}
+	else
+		gettimeofday(&now, NULL);
+
+	argv++;
+	if (!*argv)
+		usage(usage_msg);
+	if (!strcmp(*argv, "show"))
+		show_dates(argv+1, &now);
+	else if (!strcmp(*argv, "parse"))
+		parse_dates(argv+1, &now);
+	else if (!strcmp(*argv, "approxidate"))
+		parse_approxidate(argv+1, &now);
+	else
+		usage(usage_msg);
+	return 0;
+}
-- 
1.6.4.2.373.g5881fd

[PATCH v2 3/4] tests: add date printing and parsing tests

[Jeff King]

Until now, there was no coverage of relative date printing
or approxidate parsing routines (mainly because we had no
way of faking the "now" time for relative date calculations,
which made consistent testing impossible).

This new script tries to exercise the basic features of
show_date and approxidate. Most of the tests are just "this
obvious thing works" to prevent future regressions, with a
few exceptions:

  - We confirm the fix in 607a9e8 that relative year/month
    dates in the latter half of a year round correctly.

  - We confirm that the improvements in b5373e9 and 1bddb25
    work.

  - A few tests are marked to expect failure, which are
    regressions recently introduced by the two commits
    above.

Signed-off-by: Jeff King <peff@peff.net>
---
Similar to earlier 3/3, but improvements and regressions from
lt/approxidate included.

Linus, when you posted the approxidate fixes earlier, you mentioned you
might have some other corner cases. I think you were just referring to
the stuff you improved in the followup patch, but if you know of more
broken-ness, we should probably include it here.

 t/t0006-date.sh |   75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 75 insertions(+), 0 deletions(-)
 create mode 100755 t/t0006-date.sh

diff --git a/t/t0006-date.sh b/t/t0006-date.sh
new file mode 100755
index 0000000..02cd565
--- /dev/null
+++ b/t/t0006-date.sh
@@ -0,0 +1,75 @@
+#!/bin/sh
+
+test_description='test date parsing and printing'
+. ./test-lib.sh
+
+# arbitrary reference time: 2009-08-30 19:20:00
+TEST_DATE_NOW=1251660000; export TEST_DATE_NOW
+
+check_show() {
+	t=$(($TEST_DATE_NOW - $1))
+	echo "$t -> $2" >expect
+	test_expect_${3:-success} "relative date ($2)" "
+	test-date show $t >actual &&
+	test_cmp expect actual
+	"
+}
+
+check_show 5 '5 seconds ago'
+check_show 300 '5 minutes ago'
+check_show 18000 '5 hours ago'
+check_show 432000 '5 days ago'
+check_show 1728000 '3 weeks ago'
+check_show 13000000 '5 months ago'
+check_show 37500000 '1 year, 2 months ago'
+check_show 55188000 '1 year, 9 months ago'
+check_show 630000000 '20 years ago'
+
+check_parse() {
+	echo "$1 -> $2" >expect
+	test_expect_${3:-success} "parse date ($1)" "
+	test-date parse '$1' >actual &&
+	test_cmp expect actual
+	"
+}
+
+check_parse 2008 bad
+check_parse 2008-02 bad
+check_parse 2008-02-14 bad
+check_parse '2008-02-14 20:30:45' '2008-02-14 20:30:45 +0000'
+
+check_approxidate() {
+	echo "$1 -> $2 +0000" >expect
+	test_expect_${3:-success} "parse approxidate ($1)" "
+	test-date approxidate '$1' >actual &&
+	test_cmp expect actual
+	"
+}
+
+check_approxidate now '2009-08-30 19:20:00'
+check_approxidate '5 seconds ago' '2009-08-30 19:19:55'
+check_approxidate 5.seconds.ago '2009-08-30 19:19:55'
+check_approxidate 10.minutes.ago '2009-08-30 19:10:00'
+check_approxidate yesterday '2009-08-29 19:20:00'
+check_approxidate 3.days.ago '2009-08-27 19:20:00'
+check_approxidate 3.weeks.ago '2009-08-09 19:20:00'
+check_approxidate 3.months.ago '2009-05-30 19:20:00' failure
+check_approxidate 2.years.3.months.ago '2007-05-30 19:20:00' failure
+
+check_approxidate '6am yesterday' '2009-08-29 06:00:00'
+check_approxidate '6pm yesterday' '2009-08-29 18:00:00'
+check_approxidate '3:00' '2009-08-30 03:00:00'
+check_approxidate '15:00' '2009-08-30 15:00:00'
+check_approxidate 'noon today' '2009-08-30 12:00:00'
+check_approxidate 'noon yesterday' '2009-08-29 12:00:00'
+
+check_approxidate 'last tuesday' '2009-08-25 19:20:00'
+check_approxidate 'July 5th' '2009-07-05 19:20:00'
+check_approxidate '06/05/2009' '2009-06-05 19:20:00'
+check_approxidate '06.05.2009' '2009-05-06 19:20:00'
+
+check_approxidate 'Jun 6, 5AM' '2009-06-06 05:00:00'
+check_approxidate '5AM Jun 6' '2009-06-06 05:00:00'
+check_approxidate '6AM, June 7, 2009' '2009-06-07 06:00:00'
+
+test_done
-- 
1.6.4.2.373.g5881fd

Re: [PATCH v2 3/4] tests: add date printing and parsing tests

[Jeff King]

On Sun, Aug 30, 2009 at 10:30:15PM -0400, Jeff King wrote:

>   - We confirm that the improvements in b5373e9 and 1bddb25
>     work.

Ugh. I just realized (when explaining how awesome git resurrect was in
another mail) that I managed to bungle these commit hashes (and the one
mentioned in the following patch).

What happened is that I was building on the topic branch and lazily did
a "rebase -i origin" to fix up my patches. I left the first two patches
untouched, of course, but they still ended up with new committer
information.

As my patches are merged to 'next' already, I think it is too late to
fixup the commit message. But for posterity, the correct referenced
commits are 9029055 and 36e4986.

Caveat rebaser.

-Peff

[PATCH v2 4/4] fix approxidate parsing of relative months and years

[Jeff King]

These were broken by b5373e9. The problem is that the code
marks the month and year with "-1" for "we don't know it
yet", but the month and year code paths were not adjusted to
fill in the current time before doing their calculations
(whereas other units follow a different code path and are
fine).

Signed-off-by: Jeff King <peff@peff.net>
---
This one is new from the last series, as it fixes bugs only found in
lt/approxidate.

 date.c          |    5 ++++-
 t/t0006-date.sh |    4 ++--
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/date.c b/date.c
index 8e57e5e..e9ee4aa 100644
--- a/date.c
+++ b/date.c
@@ -857,7 +857,9 @@ static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm
 	}
 
 	if (match_string(date, "months") >= 5) {
-		int n = tm->tm_mon - *num;
+		int n;
+		update_tm(tm, now, 0); /* fill in date fields if needed */
+		n = tm->tm_mon - *num;
 		*num = 0;
 		while (n < 0) {
 			n += 12;
@@ -868,6 +870,7 @@ static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm
 	}
 
 	if (match_string(date, "years") >= 4) {
+		update_tm(tm, now, 0); /* fill in date fields if needed */
 		tm->tm_year -= *num;
 		*num = 0;
 		return end;
diff --git a/t/t0006-date.sh b/t/t0006-date.sh
index 02cd565..a4d8fa8 100755
--- a/t/t0006-date.sh
+++ b/t/t0006-date.sh
@@ -53,8 +53,8 @@ check_approxidate 10.minutes.ago '2009-08-30 19:10:00'
 check_approxidate yesterday '2009-08-29 19:20:00'
 check_approxidate 3.days.ago '2009-08-27 19:20:00'
 check_approxidate 3.weeks.ago '2009-08-09 19:20:00'
-check_approxidate 3.months.ago '2009-05-30 19:20:00' failure
-check_approxidate 2.years.3.months.ago '2007-05-30 19:20:00' failure
+check_approxidate 3.months.ago '2009-05-30 19:20:00'
+check_approxidate 2.years.3.months.ago '2007-05-30 19:20:00'
 
 check_approxidate '6am yesterday' '2009-08-29 06:00:00'
 check_approxidate '6pm yesterday' '2009-08-29 18:00:00'
-- 
1.6.4.2.373.g5881fd

[PATCH 2/3] refactor test-date interface

[Jeff King]

The test-date program goes back to the early days of git,
where it was presumably used to do manual sanity checks on
changes to the date code. However, it is not actually used
by the test suite to do any sort of automatic of systematic
tests.

This patch refactors the interface to the program to try to
make it more suitable for use by the test suite. There
should be no fallouts to changing the interface since it is
not actually installed and is not internally called by any
other programs.

The changes are:

  - add a "mode" parameter so the caller can specify which
    operation to test

  - add a mode to test relative date output from show_date

  - allow faking a fixed time via the TEST_DATE_NOW
    environment variable, which allows consistent automated
    testing

  - drop the use of ctime for showing dates in favor of our
    internal iso8601 printing routines. The ctime output is
    somewhat redundant (because of the day-of-week) which
    makes writing test cases more annoying.

Signed-off-by: Jeff King <peff@peff.net>
---
I mulled over replacing ctime for a bit, as we are testing git's date
code with other parts of git's date code. But it really is more
convenient for writing test cases to use iso8601, since you don't have
to calculate the day-of-week (and I also think it is a bit more
readable). And our iso8601 code is dead simple, so I am not too worried
about a bug in it hiding a bug elsewhere.

 test-date.c |   86 +++++++++++++++++++++++++++++++++++++++++++++-------------
 1 files changed, 66 insertions(+), 20 deletions(-)
 rewrite test-date.c (63%)

diff --git a/test-date.c b/test-date.c
dissimilarity index 63%
index 62e8f23..5b0a220 100644
--- a/test-date.c
+++ b/test-date.c
@@ -1,20 +1,66 @@
-#include "cache.h"
-
-int main(int argc, char **argv)
-{
-	int i;
-
-	for (i = 1; i < argc; i++) {
-		char result[100];
-		time_t t;
-
-		memcpy(result, "bad", 4);
-		parse_date(argv[i], result, sizeof(result));
-		t = strtoul(result, NULL, 0);
-		printf("%s -> %s -> %s", argv[i], result, ctime(&t));
-
-		t = approxidate(argv[i]);
-		printf("%s -> %s\n", argv[i], ctime(&t));
-	}
-	return 0;
-}
+#include "cache.h"
+
+static const char *usage_msg = "\n"
+"  test-date show [time_t]...\n"
+"  test-date parse [date]...\n"
+"  test-date approxidate [date]...\n";
+
+static void show_dates(char **argv, struct timeval *now)
+{
+	char buf[128];
+
+	for (; *argv; argv++) {
+		time_t t = atoi(*argv);
+		show_date_relative(t, 0, now, buf, sizeof(buf));
+		printf("%s -> %s\n", *argv, buf);
+	}
+}
+
+static void parse_dates(char **argv, struct timeval *now)
+{
+	for (; *argv; argv++) {
+		char result[100];
+		time_t t;
+
+		parse_date(*argv, result, sizeof(result));
+		t = strtoul(result, NULL, 0);
+		printf("%s -> %s\n", *argv,
+			t ? show_date(t, 0, DATE_ISO8601) : "bad");
+	}
+}
+
+static void parse_approxidate(char **argv, struct timeval *now)
+{
+	for (; *argv; argv++) {
+		time_t t;
+		t = approxidate_relative(*argv, now);
+		printf("%s -> %s\n", *argv, show_date(t, 0, DATE_ISO8601));
+	}
+}
+
+int main(int argc, char **argv)
+{
+	struct timeval now;
+	const char *x;
+
+	x = getenv("TEST_DATE_NOW");
+	if (x) {
+		now.tv_sec = atoi(x);
+		now.tv_usec = 0;
+	}
+	else
+		gettimeofday(&now, NULL);
+
+	argv++;
+	if (!*argv)
+		usage(usage_msg);
+	if (!strcmp(*argv, "show"))
+		show_dates(argv+1, &now);
+	else if (!strcmp(*argv, "parse"))
+		parse_dates(argv+1, &now);
+	else if (!strcmp(*argv, "approxidate"))
+		parse_approxidate(argv+1, &now);
+	else
+		usage(usage_msg);
+	return 0;
+}
-- 
1.6.4.2.375.g73938

[PATCH 3/3] tests: add date printing and parsing tests

[Jeff King]

Until now, there was no coverage of relative date printing
or approxidate parsing routines (mainly because we had no
way of faking the "now" time for relative date calculations,
which made consistent testing impossible).

This new script tries to exercise the basic features of
show_date and approxidate. The only specific problem case
tested is showing relative year/month dates in the latter
half of a year, as fixed by 607a9e8.

Signed-off-by: Jeff King <peff@peff.net>
---
Like I said, this is really just to exercise the basic code paths.
But now that the infrastructure is there, we can add any corner cases
or verify new features or bug fixes as they come up. Patches welcome. :)

 t/t0006-date.sh |   71 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 71 insertions(+), 0 deletions(-)
 create mode 100755 t/t0006-date.sh

diff --git a/t/t0006-date.sh b/t/t0006-date.sh
new file mode 100755
index 0000000..4beb44b
--- /dev/null
+++ b/t/t0006-date.sh
@@ -0,0 +1,71 @@
+#!/bin/sh
+
+test_description='test date parsing and printing'
+. ./test-lib.sh
+
+# arbitrary reference time: 2009-08-30 19:20:00
+TEST_DATE_NOW=1251660000; export TEST_DATE_NOW
+
+check_show() {
+	t=$(($TEST_DATE_NOW - $1))
+	echo "$t -> $2" >expect
+	test_expect_success "relative date ($2)" "
+	test-date show $t >actual &&
+	test_cmp expect actual
+	"
+}
+
+check_show 5 '5 seconds ago'
+check_show 300 '5 minutes ago'
+check_show 18000 '5 hours ago'
+check_show 432000 '5 days ago'
+check_show 1728000 '3 weeks ago'
+check_show 13000000 '5 months ago'
+check_show 37500000 '1 year, 2 months ago'
+check_show 55188000 '1 year, 9 months ago'
+check_show 630000000 '20 years ago'
+
+check_parse() {
+	echo "$1 -> $2" >expect
+	test_expect_success "parse date ($1)" "
+	test-date parse '$1' >actual &&
+	test_cmp expect actual
+	"
+}
+
+check_parse 2008 bad
+check_parse 2008-02 bad
+check_parse 2008-02-14 '2008-02-14 00:00:00 +0000'
+check_parse '2008-02-14 20:30:45' '2008-02-14 20:30:45 +0000'
+
+check_approxidate() {
+	echo "$1 -> $2 +0000" >expect
+	test_expect_success "parse approxidate ($1)" "
+	test-date approxidate '$1' >actual &&
+	test_cmp expect actual
+	"
+}
+
+check_approxidate now '2009-08-30 19:20:00'
+check_approxidate '5 seconds ago' '2009-08-30 19:19:55'
+check_approxidate 5.seconds.ago '2009-08-30 19:19:55'
+check_approxidate 10.minutes.ago '2009-08-30 19:10:00'
+check_approxidate yesterday '2009-08-29 19:20:00'
+check_approxidate 3.days.ago '2009-08-27 19:20:00'
+check_approxidate 3.weeks.ago '2009-08-09 19:20:00'
+check_approxidate 3.months.ago '2009-05-30 19:20:00'
+check_approxidate 2.years.3.months.ago '2007-05-30 19:20:00'
+
+check_approxidate '6am yesterday' '2009-08-29 06:00:00'
+check_approxidate '6pm yesterday' '2009-08-29 18:00:00'
+check_approxidate '3:00' '2009-08-30 03:00:00'
+check_approxidate '15:00' '2009-08-30 15:00:00'
+check_approxidate 'noon today' '2009-08-30 12:00:00'
+check_approxidate 'noon yesterday' '2009-08-29 12:00:00'
+
+check_approxidate 'last tuesday' '2009-08-25 19:20:00'
+check_approxidate 'July 5th' '2009-07-05 19:20:00'
+check_approxidate '06/05/2009' '2009-06-05 00:00:00'
+check_approxidate '06.05.2009' '2009-05-06 00:00:00'
+
+test_done
-- 
1.6.4.2.375.g73938

[PATCH 1/2] Add date formatting and parsing functions relative to a given time

[Alex Riesen]

The main purpose is to allow predictable testing of the code.
---

Rebased on current master.

 cache.h |    2 +
 date.c  |  150 ++++++++++++++++++++++++++++++++++++--------------------------
 2 files changed, 89 insertions(+), 63 deletions(-)

diff --git a/cache.h b/cache.h
index dd7f71e..3fb0166 100644
--- a/cache.h
+++ b/cache.h
@@ -731,9 +731,11 @@ enum date_mode {
 };
 
 const char *show_date(unsigned long time, int timezone, enum date_mode mode);
+const char *show_date_relative(unsigned long time, int tz, const struct timeval *now);
 int parse_date(const char *date, char *buf, int bufsize);
 void datestamp(char *buf, int bufsize);
 unsigned long approxidate(const char *);
+unsigned long approxidate_relative(const char *date, const struct timeval *now);
 enum date_mode parse_date_format(const char *format);
 
 #define IDENT_WARN_ON_NO_NAME  1
diff --git a/date.c b/date.c
index f011692..0840e77 100644
--- a/date.c
+++ b/date.c
@@ -84,6 +84,67 @@ static int local_tzoffset(unsigned long time)
 	return offset * eastwest;
 }
 
+const char *show_date_relative(unsigned long time, int tz, const struct timeval *now)
+{
+	static char timebuf[100 /* TODO: can be optimized */];
+	unsigned long diff;
+	if (now->tv_sec < time)
+		return "in the future";
+	diff = now->tv_sec - time;
+	if (diff < 90) {
+		snprintf(timebuf, sizeof(timebuf), "%lu seconds ago", diff);
+		return timebuf;
+	}
+	/* Turn it into minutes */
+	diff = (diff + 30) / 60;
+	if (diff < 90) {
+		snprintf(timebuf, sizeof(timebuf), "%lu minutes ago", diff);
+		return timebuf;
+	}
+	/* Turn it into hours */
+	diff = (diff + 30) / 60;
+	if (diff < 36) {
+		snprintf(timebuf, sizeof(timebuf), "%lu hours ago", diff);
+		return timebuf;
+	}
+	/* We deal with number of days from here on */
+	diff = (diff + 12) / 24;
+	if (diff < 14) {
+		snprintf(timebuf, sizeof(timebuf), "%lu days ago", diff);
+		return timebuf;
+	}
+	/* Say weeks for the past 10 weeks or so */
+	if (diff < 70) {
+		snprintf(timebuf, sizeof(timebuf), "%lu weeks ago", (diff + 3) / 7);
+		return timebuf;
+	}
+	/* Say months for the past 12 months or so */
+	if (diff < 360) {
+		snprintf(timebuf, sizeof(timebuf), "%lu months ago", (diff + 15) / 30);
+		return timebuf;
+	}
+	/* Give years and months for 5 years or so */
+	if (diff < 1825) {
+		unsigned long years = diff / 365;
+		unsigned long months = (diff % 365 + 15) / 30;
+		int n;
+		n = snprintf(timebuf, sizeof(timebuf), "%lu year%s",
+			     years, (years > 1 ? "s" : ""));
+		if (months)
+			snprintf(timebuf + n, sizeof(timebuf) - n,
+				 ", %lu month%s ago",
+				 months, (months > 1 ? "s" : ""));
+		else
+			snprintf(timebuf + n, sizeof(timebuf) - n,
+				 " ago");
+		return timebuf;
+	}
+	/* Otherwise, just years. Centuries is probably overkill. */
+	snprintf(timebuf, sizeof(timebuf), "%lu years ago", (diff + 183) / 365);
+	return timebuf;
+
+}
+
 const char *show_date(unsigned long time, int tz, enum date_mode mode)
 {
 	struct tm *tm;
@@ -95,63 +156,9 @@ const char *show_date(unsigned long time, int tz, enum date_mode mode)
 	}
 
 	if (mode == DATE_RELATIVE) {
-		unsigned long diff;
 		struct timeval now;
 		gettimeofday(&now, NULL);
-		if (now.tv_sec < time)
-			return "in the future";
-		diff = now.tv_sec - time;
-		if (diff < 90) {
-			snprintf(timebuf, sizeof(timebuf), "%lu seconds ago", diff);
-			return timebuf;
-		}
-		/* Turn it into minutes */
-		diff = (diff + 30) / 60;
-		if (diff < 90) {
-			snprintf(timebuf, sizeof(timebuf), "%lu minutes ago", diff);
-			return timebuf;
-		}
-		/* Turn it into hours */
-		diff = (diff + 30) / 60;
-		if (diff < 36) {
-			snprintf(timebuf, sizeof(timebuf), "%lu hours ago", diff);
-			return timebuf;
-		}
-		/* We deal with number of days from here on */
-		diff = (diff + 12) / 24;
-		if (diff < 14) {
-			snprintf(timebuf, sizeof(timebuf), "%lu days ago", diff);
-			return timebuf;
-		}
-		/* Say weeks for the past 10 weeks or so */
-		if (diff < 70) {
-			snprintf(timebuf, sizeof(timebuf), "%lu weeks ago", (diff + 3) / 7);
-			return timebuf;
-		}
-		/* Say months for the past 12 months or so */
-		if (diff < 360) {
-			snprintf(timebuf, sizeof(timebuf), "%lu months ago", (diff + 15) / 30);
-			return timebuf;
-		}
-		/* Give years and months for 5 years or so */
-		if (diff < 1825) {
-			unsigned long years = diff / 365;
-			unsigned long months = (diff % 365 + 15) / 30;
-			int n;
-			n = snprintf(timebuf, sizeof(timebuf), "%lu year%s",
-					years, (years > 1 ? "s" : ""));
-			if (months)
-				snprintf(timebuf + n, sizeof(timebuf) - n,
-					", %lu month%s ago",
-					months, (months > 1 ? "s" : ""));
-			else
-				snprintf(timebuf + n, sizeof(timebuf) - n,
-					" ago");
-			return timebuf;
-		}
-		/* Otherwise, just years. Centuries is probably overkill. */
-		snprintf(timebuf, sizeof(timebuf), "%lu years ago", (diff + 183) / 365);
-		return timebuf;
+		return show_date_relative(time, tz, &now);
 	}
 
 	if (mode == DATE_LOCAL)
@@ -866,19 +873,13 @@ static const char *approxidate_digit(const char *date, struct tm *tm, int *num)
 	return end;
 }
 
-unsigned long approxidate(const char *date)
+static unsigned long approximation(const char *date, const struct timeval *tv)
 {
 	int number = 0;
 	struct tm tm, now;
-	struct timeval tv;
 	time_t time_sec;
-	char buffer[50];
 
-	if (parse_date(date, buffer, sizeof(buffer)) > 0)
-		return strtoul(buffer, NULL, 10);
-
-	gettimeofday(&tv, NULL);
-	time_sec = tv.tv_sec;
+	time_sec = tv->tv_sec;
 	localtime_r(&time_sec, &tm);
 	now = tm;
 	for (;;) {
@@ -899,3 +900,26 @@ unsigned long approxidate(const char *date)
 		tm.tm_year--;
 	return mktime(&tm);
 }
+
+unsigned long approxidate_relative(const char *date, const struct timeval *tv)
+{
+	char buffer[50];
+
+	if (parse_date(date, buffer, sizeof(buffer)) > 0)
+		return strtoul(buffer, NULL, 10);
+
+	return approximation(date, tv);
+}
+
+unsigned long approxidate(const char *date)
+{
+	struct timeval tv;
+	char buffer[50];
+
+	if (parse_date(date, buffer, sizeof(buffer)) > 0)
+		return strtoul(buffer, NULL, 10);
+
+	gettimeofday(&tv, NULL);
+	return approximation(date, &tv);
+}
+
-- 
1.6.4.1.263.g468a

Re: [PATCH 1/2] Add date formatting and parsing functions relative to a given time

[Jeff King]

On Fri, Aug 28, 2009 at 11:04:04PM +0200, Alex Riesen wrote:

> +const char *show_date_relative(unsigned long time, int tz, const struct timeval *now)
> +{
> +	static char timebuf[100 /* TODO: can be optimized */];

This was 200 in the original version. I doubt that it makes a
difference, but I think in a refactoring patch I think it is best to
simply reorganize and make no other changes.

> +static unsigned long approximation(const char *date, const struct timeval *tv)

I know it's static, but this is a terribly undescriptive function name.
Approximation of what?  Can we call it approxidate_internal or
something?

-Peff

Re: [PATCH 1/2] Add date formatting and parsing functions relative to a given time

[Alex Riesen]

On Sun, Aug 30, 2009 at 09:36, Jeff King<peff@peff.net> wrote:
> On Fri, Aug 28, 2009 at 11:04:04PM +0200, Alex Riesen wrote:
>
>> +const char *show_date_relative(unsigned long time, int tz, const struct timeval *now)
>> +{
>> +     static char timebuf[100 /* TODO: can be optimized */];
>
> This was 200 in the original version. I doubt that it makes a
> difference, but I think in a refactoring patch I think it is best to
> simply reorganize and make no other changes.

Yes, I just noticed that 200 was much too much, made the note
to fix it sometime and forgot the note in the final submission.

>> +static unsigned long approximation(const char *date, const struct timeval *tv)
>
> I know it's static, but this is a terribly undescriptive function name.
> Approximation of what?  Can we call it approxidate_internal or
> something?

Been there, tried that. Didn't like it, because it didn't feel enough
approxidate (the original) anymore. Not even internally, because
of missing parse_date. My other attempts were guessdate and
approxidate_bottom_half (but only very shortly).

The "approximation", if you consider the functions arguments,
seems to me the closest to what the function _is_. OTOH,
maybe I should have used a verb...