Git development
 help / color / mirror / Atom feed
* [PATCH] show_date(): fix relative dates
@ 2007-01-20 21:21 Johannes Schindelin
  2007-01-21  0:59 ` Junio C Hamano
  0 siblings, 1 reply; 4+ messages in thread
From: Johannes Schindelin @ 2007-01-20 21:21 UTC (permalink / raw)
  To: git, junkio


We pass a timestamp (i.e. number of seconds elapsed since Jan 1 1970,
00:00:00 GMT) to the function. So there is no need to "fix" the
timestamp according to the timezone.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---

	Just compile & run this little test program to see what I mean:

	#include "cache.h"

	int main(int argc, char **argv)
	{
		time_t t = time(NULL);
		printf("%s\n", show_rfc2822_date(t, 100));
		printf("%s\n", show_date(t, 100, 0));
		printf("%s\n", show_date(t, 0, 0));
		printf("%s\n", show_date(t, 100, 1));
		printf("%s\n", show_date(t, 0, 1));
		return 0;
	}

	Here, it outputs this:

	Sat, 20 Jan 2007 22:20:42 +0100
	Sat Jan 20 22:20:42 2007 +0100
	Sat Jan 20 21:20:42 2007 +0000
	in the future
	0 seconds ago

	Noticed that "in the future"?

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

diff --git a/date.c b/date.c
index 7acb8cb..542c004 100644
--- a/date.c
+++ b/date.c
@@ -62,12 +62,11 @@ const char *show_date(unsigned long time, int tz, int relative)
 
 	if (relative) {
 		unsigned long diff;
-		time_t t = gm_time_t(time, tz);
 		struct timeval now;
 		gettimeofday(&now, NULL);
-		if (now.tv_sec < t)
+		if (now.tv_sec < time)
 			return "in the future";
-		diff = now.tv_sec - t;
+		diff = now.tv_sec - time;
 		if (diff < 90) {
 			snprintf(timebuf, sizeof(timebuf), "%lu seconds ago", diff);
 			return timebuf;
-- 
1.5.0.rc1.g956c1-dirty

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] show_date(): fix relative dates
  2007-01-20 21:21 [PATCH] show_date(): fix relative dates Johannes Schindelin
@ 2007-01-21  0:59 ` Junio C Hamano
  2007-01-21  1:34   ` Johannes Schindelin
  0 siblings, 1 reply; 4+ messages in thread
From: Junio C Hamano @ 2007-01-21  0:59 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, junkio

I noticed this and have a different solution in the show-branch
--reflog code (it does not pass tz).  I haven't thought about
which solution is the correct one (and do not have time to think
about it now while writing this message, sorry -- will do the
thinking later unless you beat me to it).

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] show_date(): fix relative dates
  2007-01-21  0:59 ` Junio C Hamano
@ 2007-01-21  1:34   ` Johannes Schindelin
  2007-01-21  2:48     ` Junio C Hamano
  0 siblings, 1 reply; 4+ messages in thread
From: Johannes Schindelin @ 2007-01-21  1:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Hi,

On Sat, 20 Jan 2007, Junio C Hamano wrote:

> I noticed this and have a different solution in the show-branch
> --reflog code (it does not pass tz).

Actually, you do pass tz, but with a fixed value of 0. But this is only a 
workaround.

The thing is, if you pass the same timestamp with a different timezone, 
absolute (non-relative) date will show the same time, i.e. 22:24 +0000 
is the same as 23:24 +0100.

Now I expect the same of relative mode, only that the timezone does not 
matter for relative mode _at all_.

As you can see in my patch for --walk-reflogs (to honour --relative-date), 
it can make sense to pass make the choice whether to show a relative or an 
absolute date a runtime option (where you don't want to set tz to 0 
depending on relativa_date).

I guess nobody realized that "git log --relative-date" does not work 
correctly (for all commits which were not committed in tz +0000), is due 
to its narrowspread use, or that it is not all that obviously wrong.

However, I was very surprised when "git log --walk-reflogs 
--relative-date" showed me "in the future" for my last commits.

If you don't want to think it through, please just verify my reasoning by 
compiling the script I gave with the patch both before and after applying 
the patch. The absolute dates _and_ the relative dates should be all the 
same, but the relative date with non-zero timezone is displayed as "in the 
future" withouth the patch.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] show_date(): fix relative dates
  2007-01-21  1:34   ` Johannes Schindelin
@ 2007-01-21  2:48     ` Junio C Hamano
  0 siblings, 0 replies; 4+ messages in thread
From: Junio C Hamano @ 2007-01-21  2:48 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Now I expect the same of relative mode, only that the timezone does not 
> matter for relative mode _at all_.
> ...
> However, I was very surprised when "git log --walk-reflogs 
> --relative-date" showed me "in the future" for my last commits.

Exactly.

You are absolutely right.  I should have done that when I did
the show-branch --reflog workaround to pass 0.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2007-01-21  2:48 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-01-20 21:21 [PATCH] show_date(): fix relative dates Johannes Schindelin
2007-01-21  0:59 ` Junio C Hamano
2007-01-21  1:34   ` Johannes Schindelin
2007-01-21  2:48     ` Junio C Hamano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox