[PATCH 2/2] --date=relative falls back to "short" format for commits older than a year

[PATCH 2/2] --date=relative falls back to "short" format for commits older than a year

[eletuchy]

From: Eugene Letuchy <eugene@facebook.com>

In the context of sizing the git blame time column, it doesn't make a
lot of sense to see "12 months ago" next to an exact timestamp +
timezone for something 13 months ago. This commit makes commits older
than 12 months display the date only, not the time.

Signed-off-by: Eugene Letuchy <eugene@facebook.com>
---
 builtin-blame.c |    5 ++---
 date.c          |    4 +++-
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/builtin-blame.c b/builtin-blame.c
index aa5c66c..48cedfd 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -2286,9 +2286,8 @@ parse_done:
 		blame_date_width = sizeof("2006-10-19");
 		break;
 	case DATE_RELATIVE:
-		/* unfortunately "normal" is the fallback for "relative" */
-		/* blame_date_width = sizeof("14 minutes ago"); */
-		/* break; */
+		blame_date_width = sizeof("14 minutes ago");
+		break;
 	case DATE_LOCAL:
 	case DATE_NORMAL:
 		blame_date_width = sizeof("Thu Oct 19 16:00:04 2006 -0700");
diff --git a/date.c b/date.c
index 950b88f..edb2078 100644
--- a/date.c
+++ b/date.c
@@ -128,7 +128,9 @@ const char *show_date(unsigned long time, int tz, enum date_mode mode)
 			snprintf(timebuf, sizeof(timebuf), "%lu months ago", (diff + 15) / 30);
 			return timebuf;
 		}
-		/* Else fall back on absolute format.. */
+
+		/* Else fall back to the short format */
+		mode = DATE_SHORT;
 	}
 
 	if (mode == DATE_LOCAL)
-- 
1.6.2.rc1.14.g07c3.dirty

Re: [PATCH 2/2] --date=relative falls back to "short" format for commits older than a year

[Linus Torvalds]

Subject: Support 'raw' date format

Talking about --date, one thing I wanted for the 1234567890 date was to 
get things in the raw format. Sure, you get them with --pretty=raw, but it 
felt a bit sad that you couldn't just ask for the date in raw format.

So here's a throw-away patch (meaning: I won't be re-sending it, because I 
really don't think it's a big deal) to add "--date=raw". It just prints 
out the internal raw git format - seconds since epoch plus timezone (put 
another way: 'date +"%s %z"' format)

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---

Not a whole lot of testing. But 

	git show --date=raw v2.6.29-rc5

works correctly.

 Documentation/rev-list-options.txt |    4 +++-
 cache.h                            |    3 ++-
 date.c                             |    7 +++++++
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git i/Documentation/rev-list-options.txt w/Documentation/rev-list-options.txt
index b9f6e4d..5076322 100644
--- i/Documentation/rev-list-options.txt
+++ w/Documentation/rev-list-options.txt
@@ -13,7 +13,7 @@ include::pretty-options.txt[]
 
 	Synonym for `--date=relative`.
 
---date={relative,local,default,iso,rfc,short}::
+--date={relative,local,default,iso,rfc,short,raw}::
 
 	Only takes effect for dates shown in human-readable format, such
 	as when using "--pretty". `log.date` config variable sets a default
@@ -31,6 +31,8 @@ format, often found in E-mail messages.
 +
 `--date=short` shows only date but not time, in `YYYY-MM-DD` format.
 +
+`--date=raw` shows the date in the internal raw git format `%s %z` format.
++
 `--date=default` shows timestamps in the original timezone
 (either committer's or author's).
 
diff --git i/cache.h w/cache.h
index 21a6310..189151d 100644
--- i/cache.h
+++ w/cache.h
@@ -696,7 +696,8 @@ enum date_mode {
 	DATE_SHORT,
 	DATE_LOCAL,
 	DATE_ISO8601,
-	DATE_RFC2822
+	DATE_RFC2822,
+	DATE_RAW
 };
 
 const char *show_date(unsigned long time, int timezone, enum date_mode mode);
diff --git i/date.c w/date.c
index 950b88f..d75dff4 100644
--- i/date.c
+++ w/date.c
@@ -89,6 +89,11 @@ const char *show_date(unsigned long time, int tz, enum date_mode mode)
 	struct tm *tm;
 	static char timebuf[200];
 
+	if (mode == DATE_RAW) {
+		snprintf(timebuf, sizeof(timebuf), "%lu %+05d", time, tz);
+		return timebuf;
+	}
+
 	if (mode == DATE_RELATIVE) {
 		unsigned long diff;
 		struct timeval now;
@@ -615,6 +620,8 @@ enum date_mode parse_date_format(const char *format)
 		return DATE_LOCAL;
 	else if (!strcmp(format, "default"))
 		return DATE_NORMAL;
+	else if (!strcmp(format, "raw"))
+		return DATE_RAW;
 	else
 		die("unknown date format %s", format);
 }

Re: [PATCH 2/2] --date=relative falls back to "short" format for commits older than a year

[Eugene Letuchy]

Cool. I think git blame would need to be tweaked a bit after my patch
(at least documentation wise), since it already has a "raw timestamp"
option (-t).

On Fri, Feb 20, 2009 at 2:15 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Subject: Support 'raw' date format
>
> Talking about --date, one thing I wanted for the 1234567890 date was to
> get things in the raw format. Sure, you get them with --pretty=raw, but it
> felt a bit sad that you couldn't just ask for the date in raw format.
>
> So here's a throw-away patch (meaning: I won't be re-sending it, because I
> really don't think it's a big deal) to add "--date=raw". It just prints
> out the internal raw git format - seconds since epoch plus timezone (put
> another way: 'date +"%s %z"' format)
>
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> ---
>
> Not a whole lot of testing. But
>
>        git show --date=raw v2.6.29-rc5
>
> works correctly.
>
>  Documentation/rev-list-options.txt |    4 +++-
>  cache.h                            |    3 ++-
>  date.c                             |    7 +++++++
>  3 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git i/Documentation/rev-list-options.txt w/Documentation/rev-list-options.txt
> index b9f6e4d..5076322 100644
> --- i/Documentation/rev-list-options.txt
> +++ w/Documentation/rev-list-options.txt
> @@ -13,7 +13,7 @@ include::pretty-options.txt[]
>
>        Synonym for `--date=relative`.
>
> ---date={relative,local,default,iso,rfc,short}::
> +--date={relative,local,default,iso,rfc,short,raw}::
>
>        Only takes effect for dates shown in human-readable format, such
>        as when using "--pretty". `log.date` config variable sets a default
> @@ -31,6 +31,8 @@ format, often found in E-mail messages.
>  +
>  `--date=short` shows only date but not time, in `YYYY-MM-DD` format.
>  +
> +`--date=raw` shows the date in the internal raw git format `%s %z` format.
> ++
>  `--date=default` shows timestamps in the original timezone
>  (either committer's or author's).
>
> diff --git i/cache.h w/cache.h
> index 21a6310..189151d 100644
> --- i/cache.h
> +++ w/cache.h
> @@ -696,7 +696,8 @@ enum date_mode {
>        DATE_SHORT,
>        DATE_LOCAL,
>        DATE_ISO8601,
> -       DATE_RFC2822
> +       DATE_RFC2822,
> +       DATE_RAW
>  };
>
>  const char *show_date(unsigned long time, int timezone, enum date_mode mode);
> diff --git i/date.c w/date.c
> index 950b88f..d75dff4 100644
> --- i/date.c
> +++ w/date.c
> @@ -89,6 +89,11 @@ const char *show_date(unsigned long time, int tz, enum date_mode mode)
>        struct tm *tm;
>        static char timebuf[200];
>
> +       if (mode == DATE_RAW) {
> +               snprintf(timebuf, sizeof(timebuf), "%lu %+05d", time, tz);
> +               return timebuf;
> +       }
> +
>        if (mode == DATE_RELATIVE) {
>                unsigned long diff;
>                struct timeval now;
> @@ -615,6 +620,8 @@ enum date_mode parse_date_format(const char *format)
>                return DATE_LOCAL;
>        else if (!strcmp(format, "default"))
>                return DATE_NORMAL;
> +       else if (!strcmp(format, "raw"))
> +               return DATE_RAW;
>        else
>                die("unknown date format %s", format);
>  }
>



-- 
Eugene

Re: [PATCH 2/2] --date=relative falls back to "short" format for commits older than a year

[Junio C Hamano]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> So here's a throw-away patch (meaning: I won't be re-sending it, because I 
> really don't think it's a big deal) to add "--date=raw". It just prints 
> out the internal raw git format - seconds since epoch plus timezone (put 
> another way: 'date +"%s %z"' format)

Heh, who can discard a patch *WITH DOCUMENTATION* from you ;-)

Thanks.

Re: [PATCH 2/2] --date=relative falls back to "short" format for commits older than a year

[Jeff King]

On Fri, Feb 20, 2009 at 01:23:54PM -0800, eletuchy@gmail.com wrote:

> From: Eugene Letuchy <eugene@facebook.com>
> 
> In the context of sizing the git blame time column, it doesn't make a
> lot of sense to see "12 months ago" next to an exact timestamp +
> timezone for something 13 months ago. This commit makes commits older
> than 12 months display the date only, not the time.

I think this is an improvement, though I was thinking of taking it a
step further:

diff --git a/date.c b/date.c
index d75dff4..6dbb8e8 100644
--- a/date.c
+++ b/date.c
@@ -128,12 +128,14 @@ const char *show_date(unsigned long time, int tz, enum date_mode mode)
 			snprintf(timebuf, sizeof(timebuf), "%lu weeks ago", (diff + 3) / 7);
 			return timebuf;
 		}
-		/* Say months for the past 12 months or so */
-		if (diff < 360) {
+		/* Say months for the past 24 months or so */
+		if (diff < 720) {
 			snprintf(timebuf, sizeof(timebuf), "%lu months ago", (diff + 15) / 30);
 			return timebuf;
 		}
-		/* Else fall back on absolute format.. */
+		/* Otherwise, years. Centuries is probably overkill. */
+		snprintf(timebuf, sizeof(timebuf), "%lu years ago", (diff + 183) / 365);
+		return timebuf;
 	}
 
 	if (mode == DATE_LOCAL)


but maybe other people actually like seeing the absolute time. I've
always found it jarring when reading relative times (but part of that
_was_ because it was so long and exact).

-Peff

Re: [PATCH 2/2] --date=relative falls back to "short" format for commits older than a year

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Fri, Feb 20, 2009 at 01:23:54PM -0800, eletuchy@gmail.com wrote:
>
>> From: Eugene Letuchy <eugene@facebook.com>
>> 
>> In the context of sizing the git blame time column, it doesn't make a
>> lot of sense to see "12 months ago" next to an exact timestamp +
>> timezone for something 13 months ago. This commit makes commits older
>> than 12 months display the date only, not the time.
>
> I think this is an improvement, though I was thinking of taking it a
> step further:
> ...
> +		/* Otherwise, years. Centuries is probably overkill. */
> +		snprintf(timebuf, sizeof(timebuf), "%lu years ago", (diff + 183) / 365);
> +		return timebuf;
>  	}
>  
>  	if (mode == DATE_LOCAL)
>
>
> but maybe other people actually like seeing the absolute time. I've
> always found it jarring when reading relative times (but part of that
> _was_ because it was so long and exact).

I agree this is an improvement.  It irritated me, too.  And I do not think
this change falls into the category of bad backward incompatibility.

I was hoping somebody would do a "N years M months", though.

Re: [PATCH 2/2] --date=relative falls back to "short" format for commits older than a year

[Jeff King]

On Sun, Feb 22, 2009 at 05:44:37PM -0800, Junio C Hamano wrote:

> > +		/* Otherwise, years. Centuries is probably overkill. */
> > +		snprintf(timebuf, sizeof(timebuf), "%lu years ago", (diff + 183) / 365);
> > +		return timebuf;
> 
> I agree this is an improvement.  It irritated me, too.  And I do not think
> this change falls into the category of bad backward incompatibility.
> 
> I was hoping somebody would do a "N years M months", though.

I thought about that, but I wanted to keep the maximum size down for
column output (like in git-blame). Which is why I bumped the "use
months" limit to 24 months instead of 12.

And that limit can also be tweaked.  Surely at some point there is a
range where you no longer care about the months and "N years" has high
enough resolution. But there is also a point where "N months" gets
cumbersome (75 months is a more annoying than "around 6 years"). The
question is whether we reach the "cumbersome" point before we reach the
"don't care about months" point.

Another option would to give higher resolution in number of years, like
"3.5 years" or even "3.1 years".

-Peff

Re: [PATCH 2/2] --date=relative falls back to "short" format for commits older than a year

[Marius Storm-Olsen]

[-- Attachment #1: Type: text/plain, Size: 1528 bytes --]

Jeff King said the following on 23.02.2009 04:16:
> On Sun, Feb 22, 2009 at 05:44:37PM -0800, Junio C Hamano wrote:
>>> +		/* Otherwise, years. Centuries is probably overkill. */
>>> +		snprintf(timebuf, sizeof(timebuf), "%lu years ago", (diff + 183) / 365);
>>> +		return timebuf;
>> I agree this is an improvement.  It irritated me, too.  And I do
>> not think this change falls into the category of bad backward
>> incompatibility.
>> 
>> I was hoping somebody would do a "N years M months", though.
> 
> I thought about that, but I wanted to keep the maximum size down
> for column output (like in git-blame). Which is why I bumped the
> "use months" limit to 24 months instead of 12.
> 
> And that limit can also be tweaked.  Surely at some point there is
> a range where you no longer care about the months and "N years" has
> high enough resolution. But there is also a point where "N months"
> gets cumbersome (75 months is a more annoying than "around 6
> years"). The question is whether we reach the "cumbersome" point
> before we reach the "don't care about months" point.
> 
> Another option would to give higher resolution in number of years,
> like "3.5 years" or even "3.1 years".

And using shorter names for the units would be a no-go?

   "3y 2m ago"    <--
   "3 years ago"
   "3 months ago"
   "3 weeks ago"
   "3 days ago"
   "3 hours ago"
   "3 mins ago"   <--
   "3 secs ago"   <--

-- 
.marius [@trolltech.com]
'if you know what you're doing, it's not research'


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 187 bytes --]

Re: [PATCH 2/2] --date=relative falls back to "short" format for commits older than a year

[Jeff King]

On Mon, Feb 23, 2009 at 09:09:02AM +0100, Marius Storm-Olsen wrote:

>> Another option would to give higher resolution in number of years,
>> like "3.5 years" or even "3.1 years".
>
> And using shorter names for the units would be a no-go?
>
>   "3y 2m ago"    <--

Personally I think that looks terrible. But I recognize that it is
very subjective. The only objective thing I can say is that "m" is not a
unique prefix of a time unit, due to "minutes". Yes, it is obvious if
you see the "y" first, but I actually parse the relative time backwards
in my head and think "2 minutes ago, oh wait, 3 years, that must be
months".

-Peff

Re: [PATCH 2/2] --date=relative falls back to "short" format for commits older than a year

[Marius Storm-Olsen]

[-- Attachment #1: Type: text/plain, Size: 883 bytes --]

Jeff King said the following on 24.02.2009 06:04:
> On Mon, Feb 23, 2009 at 09:09:02AM +0100, Marius Storm-Olsen wrote:
>>> Another option would to give higher resolution in number of
>>> years, like "3.5 years" or even "3.1 years".
>> And using shorter names for the units would be a no-go?
>> 
>> "3y 2m ago"    <--
> 
> Personally I think that looks terrible. But I recognize that it is 
> very subjective. The only objective thing I can say is that "m" is
> not a unique prefix of a time unit, due to "minutes". Yes, it is
> obvious if you see the "y" first, but I actually parse the relative
> time backwards in my head and think "2 minutes ago, oh wait, 3
> years, that must be months".

Ok, the standard abbreviation for month is "mo.", so

   "3y 2mo. ago"

then? ;-)

-- 
.marius [@trolltech.com]
'if you know what you're doing, it's not research'


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 187 bytes --]

Re: [PATCH 2/2] --date=relative falls back to "short" format for commits older than a year

[Jeff King]

On Tue, Feb 24, 2009 at 07:35:05AM +0100, Marius Storm-Olsen wrote:

> Ok, the standard abbreviation for month is "mo.", so
>
>   "3y 2mo. ago"
>
> then? ;-)

That is definitely better, but see the patch I just posted elsewhere in
the thread.

-Peff

Re: [PATCH 2/2] --date=relative falls back to "short" format for commits older than a year

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I thought about that, but I wanted to keep the maximum size down for
> column output (like in git-blame). Which is why I bumped the "use
> months" limit to 24 months instead of 12.
>
> And that limit can also be tweaked.  Surely at some point there is a
> range where you no longer care about the months and "N years" has high
> enough resolution. But there is also a point where "N months" gets
> cumbersome (75 months is a more annoying than "around 6 years"). The
> question is whether we reach the "cumbersome" point before we reach the
> "don't care about months" point.

Yes, "75 months" is unacceptable.  I suspect people's mind would not work
well with anything larger than 60 months.  I've actually thought about
"don't care about months" point, but 12 months is a long time.  You
certainly remember there still was a noticeable maturity difference
between classmates who were born in the earliest months of the school year
and in the last months before graduating grade school.  Perhaps after 20
years.

> Another option would to give higher resolution in number of years, like
> "3.5 years" or even "3.1 years".

But I do not think people think of years in terms of decimal fraction.

Re: [PATCH 2/2] --date=relative falls back to "short" format for commits older than a year

[Jeff King]

On Mon, Feb 23, 2009 at 08:33:37AM -0800, Junio C Hamano wrote:

> Yes, "75 months" is unacceptable.  I suspect people's mind would not work
> well with anything larger than 60 months.  I've actually thought about
> "don't care about months" point, but 12 months is a long time.  You
> certainly remember there still was a noticeable maturity difference
> between classmates who were born in the earliest months of the school year
> and in the last months before graduating grade school.  Perhaps after 20
> years.

I'm not sure human and code development necessarily follow the same
timelines. Git wouldn't even be in kindergarten yet. ;)

> > Another option would to give higher resolution in number of years, like
> > "3.5 years" or even "3.1 years".
> 
> But I do not think people think of years in terms of decimal fraction.

I think decimal fraction is overkill. Halves or quarters are more
reasonable.

But after sleeping on it, I think "Y years, M months" is not that bad.
So here is a patch (Eugene, note that this conflicts with your "fall
back to DATE_SHORT" patch).

-- >8 --
Subject: [PATCH] never fallback relative times to absolute

Previously, for dates older than 12 months we fell back to
just giving the absolute time. This can be a bit jarring
when reading a list of times. Instead, let's switch to "Y
years, M months" for five years, and then just "Y years"
after that.

No particular reason on the 5 year cutoff except that it
seemed reasonable to me.

Signed-off-by: Jeff King <peff@peff.net>
---
Please feel free to mark the 5 years up to 20, or whatever
you think is appropriate.

I think this should produce good output in all cases. There
are a surprising number of corner cases, and I spent an
embarrassing amount of time looking at the output of "git
log --pretty=tformat:'%ai / %ar'".

You could also argue for splitting this into "support N
years, M months" and then still fall back to absolute time
eventually (whether DATE_SHORT or not).

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

diff --git a/date.c b/date.c
index d75dff4..1165d30 100644
--- a/date.c
+++ b/date.c
@@ -133,7 +133,25 @@ const char *show_date(unsigned long time, int tz, enum date_mode mode)
 			snprintf(timebuf, sizeof(timebuf), "%lu months ago", (diff + 15) / 30);
 			return timebuf;
 		}
-		/* Else fall back on absolute format.. */
+		/* 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;
 	}
 
 	if (mode == DATE_LOCAL)
-- 
1.6.2.rc1.269.ga7d41

Re: [PATCH 2/2] --date=relative falls back to "short" format for commits older than a year

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Mon, Feb 23, 2009 at 08:33:37AM -0800, Junio C Hamano wrote:
>
>> Yes, "75 months" is unacceptable.  I suspect people's mind would not work
>> well with anything larger than 60 months.  I've actually thought about
>> "don't care about months" point, but 12 months is a long time.  You
>> certainly remember there still was a noticeable maturity difference
>> between classmates who were born in the earliest months of the school year
>> and in the last months before graduating grade school.  Perhaps after 20
>> years.
>
> I'm not sure human and code development necessarily follow the same
> timelines. Git wouldn't even be in kindergarten yet. ;)
>
>> > Another option would to give higher resolution in number of years, like
>> > "3.5 years" or even "3.1 years".
>> 
>> But I do not think people think of years in terms of decimal fraction.
>
> I think decimal fraction is overkill. Halves or quarters are more
> reasonable.
>
> But after sleeping on it, I think "Y years, M months" is not that bad.

That was what I thought.  There may be some very convincing reasoning I am
not seeing in the proposals to make it ultra-short like "Y yr M mo" or
"Y.x years" (i.e. "we _have_ to keep it under N characters"threshold), but
I doubt there is a particular place "Y years, M months" would make the
output too long to be acceptable.

Re: [PATCH 2/2] --date=relative falls back to "short" format for commits older than a year

[Jeff King]

On Mon, Feb 23, 2009 at 10:59:26PM -0800, Junio C Hamano wrote:

> That was what I thought.  There may be some very convincing reasoning I am
> not seeing in the proposals to make it ultra-short like "Y yr M mo" or
> "Y.x years" (i.e. "we _have_ to keep it under N characters"threshold), but
> I doubt there is a particular place "Y years, M months" would make the
> output too long to be acceptable.

Yeah, anything like blame that deals with arbitrary date formats has to
know how to handle at least

  strlen("Sun Feb 22 15:08:25 2009 -0500") = 30

anyway.  Even the default blame format is:

  strlen("2009-02-24 02:03:25 -0500") = 25

So "Y years, M months ago" is at least that short for the next ten
thousand years. I can live with setting the cutoff to just "years"
somewhere lower than 10,000. :)

-Peff