git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
* [RFC PATCH] Add 'human' date format
@ 2018-07-07 19:38 Linus Torvalds
  2018-07-07 19:58 ` Linus Torvalds
  2018-07-07 22:02 ` [RFC PATCH v2] " Linus Torvalds
  0 siblings, 2 replies; 13+ messages in thread
From: Linus Torvalds @ 2018-07-07 19:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git


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

This adds --date=human, which skips the timezone if it matches the
current time-zone, and doesn't print the whole date if that matches (ie
skip printing year for dates that are "this year", but also skip the
whole date itself if it's in the last few days and we can just say what
weekday it was).

For really recent dates (within the last hour), use the relative date
stamp.

Also add 'auto' date mode, which defaults to human if we're using the
pager.  So you can do

	git config --add log.date auto

and your "git log" commands will show the human-legible format unless
you're scripting things.

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

Ok, I tried something like this long long ago, but that was a fairly nasty 
patch that just defaulted to "--date=relative" for recent dates, and then 
did the usual default for anything else.

But the issue kept bugging me, and this is a slightly more sane model (?). 
It keeps the notion of "let's use --date=relative for very recent dates", 
but for anything that is more than an hour old, it just uses a simplified 
date.

For example, for time stamps that are "today", just show the time. And if 
the year or the time zone matches the current year or timezone, skip them.  
And don't bother showing seconds, because humans won't care.

The end result is a slightly simplified view.

So for example, the date for this commit right now looks like

    Date:   Sat Jul 7 12:21:26 2018 -0700

to me, but with "--date=human", right now it just says

    Date:   12:21

(and maybe for a US locale, I should make it do the AM/PM thing).

This is marked RFC because

 (a) maybe I'm the only one who has ever wanted the simplified dates

 (b) the simplification rules themselves might be worthy of discussion.

For example, I also simplified the "a couple of days ago" case, so that it 
shows

    Date:   Fri 19:45

for one of my kernel commits that happened yesterday. The date didn't 
match _exactly_, but it's the same month, and just a few days ago, so it 
just shows the weekday. Is that easier to read? Maybe. I kind of like it.


 builtin/blame.c |   1 +
 cache.h         |   1 +
 date.c          | 139 +++++++++++++++++++++++++++++++++++++-----------
 3 files changed, 110 insertions(+), 31 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 5a0388aae..27c64b1c8 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -917,6 +917,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 		 */
 		blame_date_width = utf8_strwidth(_("4 years, 11 months ago")) + 1; /* add the null */
 		break;
+	case DATE_HUMAN:
 	case DATE_NORMAL:
 		blame_date_width = sizeof("Thu Oct 19 16:00:04 2006 -0700");
 		break;
diff --git a/cache.h b/cache.h
index d49092d94..8a6810ee6 100644
--- a/cache.h
+++ b/cache.h
@@ -1428,6 +1428,7 @@ extern struct object *peel_to_type(const char *name, int namelen,
 struct date_mode {
 	enum date_mode_type {
 		DATE_NORMAL = 0,
+		DATE_HUMAN,
 		DATE_RELATIVE,
 		DATE_SHORT,
 		DATE_ISO8601,
diff --git a/date.c b/date.c
index 49f943e25..9809ac334 100644
--- a/date.c
+++ b/date.c
@@ -77,22 +77,16 @@ static struct tm *time_to_tm_local(timestamp_t time)
 }
 
 /*
- * What value of "tz" was in effect back then at "time" in the
- * local timezone?
+ * Fill in the localtime 'struct tm' for the supplied time,
+ * and return the local tz.
  */
-static int local_tzoffset(timestamp_t time)
+static int local_time_tzoffset(time_t t, struct tm *tm)
 {
-	time_t t, t_local;
-	struct tm tm;
+	time_t t_local;
 	int offset, eastwest;
 
-	if (date_overflows(time))
-		die("Timestamp too large for this system: %"PRItime, time);
-
-	t = (time_t)time;
-	localtime_r(&t, &tm);
-	t_local = tm_to_time_t(&tm);
-
+	localtime_r(&t, tm);
+	t_local = tm_to_time_t(tm);
 	if (t_local == -1)
 		return 0; /* error; just use +0000 */
 	if (t_local < t) {
@@ -107,6 +101,20 @@ static int local_tzoffset(timestamp_t time)
 	return offset * eastwest;
 }
 
+/*
+ * What value of "tz" was in effect back then at "time" in the
+ * local timezone?
+ */
+static int local_tzoffset(timestamp_t time)
+{
+	struct tm tm;
+
+	if (date_overflows(time))
+		die("Timestamp too large for this system: %"PRItime, time);
+
+	return local_time_tzoffset((time_t)time, &tm);
+}
+
 void show_date_relative(timestamp_t time, int tz,
 			       const struct timeval *now,
 			       struct strbuf *timebuf)
@@ -191,27 +199,94 @@ struct date_mode *date_mode_from_type(enum date_mode_type type)
 	return &mode;
 }
 
+static void show_date_normal(struct strbuf *buf, struct tm *tm, int tz, struct tm *human_tm, int human_tz, int local)
+{
+	struct {
+		unsigned int	year:1,
+				date:1,
+				wday:1,
+				seconds:1,
+				tz:1;
+	} hide = { 0 };
+
+	hide.tz = local || tz == human_tz;
+	hide.year = tm->tm_year == human_tm->tm_year;
+	if (hide.year) {
+		if (tm->tm_mon == human_tm->tm_mon) {
+			if (tm->tm_mday > human_tm->tm_mday) {
+				/* Crazy future time */
+				hide.year = 0;
+			} else if (tm->tm_mday == human_tm->tm_mday) {
+				hide.date = hide.wday = 1;
+			} else if (tm->tm_mday + 5 > human_tm->tm_mday) {
+				/* Leave just weekday if it was a few days ago */
+				hide.date = 1;
+			}
+		}
+	}
+
+	/* Always hide seconds for human-readable */
+	hide.seconds = human_tm->tm_year > 0;
+
+	if (!hide.wday)
+		strbuf_addf(buf, "%.3s ", weekday_names[tm->tm_wday]);
+	if (!hide.date)
+		strbuf_addf(buf, "%.3s %d ", month_names[tm->tm_mon], tm->tm_mday);
+
+	/* Always show time. Do we want AM/PM depending on locale? */
+	strbuf_addf(buf, "%02d:%02d", tm->tm_hour, tm->tm_min);
+	if (!hide.seconds)
+		strbuf_addf(buf, ":%02d", tm->tm_sec);
+
+	if (!hide.year)
+		strbuf_addf(buf, " %d", tm->tm_year + 1900);
+
+	if (!hide.tz)
+		strbuf_addf(buf, " %+05d", tz);
+}
+
 const char *show_date(timestamp_t time, int tz, const struct date_mode *mode)
 {
+	int type = mode->type;
+	int local = mode->local;
 	struct tm *tm;
+	struct tm human_tm = { 0 };
+	int human_tz = -1;
 	static struct strbuf timebuf = STRBUF_INIT;
 
-	if (mode->type == DATE_UNIX) {
+	if (type == DATE_UNIX) {
 		strbuf_reset(&timebuf);
 		strbuf_addf(&timebuf, "%"PRItime, time);
 		return timebuf.buf;
 	}
 
-	if (mode->local)
+	if (type == DATE_HUMAN) {
+		struct timeval now;
+
+		gettimeofday(&now, NULL);
+
+		/* Fill in the data for "current time" in human_tz and human_tm */
+		human_tz = local_time_tzoffset(now.tv_sec, &human_tm);
+
+		/* Special case: if it's less than an hour ago, use relative time */
+		if (time - now.tv_sec < 60 * 60)
+			type = DATE_RELATIVE;
+
+		/* Don't print timezone if it matches */
+		if (tz == human_tz)
+			local = 1;
+	}
+
+	if (local)
 		tz = local_tzoffset(time);
 
-	if (mode->type == DATE_RAW) {
+	if (type == DATE_RAW) {
 		strbuf_reset(&timebuf);
 		strbuf_addf(&timebuf, "%"PRItime" %+05d", time, tz);
 		return timebuf.buf;
 	}
 
-	if (mode->type == DATE_RELATIVE) {
+	if (type == DATE_RELATIVE) {
 		struct timeval now;
 
 		strbuf_reset(&timebuf);
@@ -220,7 +295,7 @@ const char *show_date(timestamp_t time, int tz, const struct date_mode *mode)
 		return timebuf.buf;
 	}
 
-	if (mode->local)
+	if (local)
 		tm = time_to_tm_local(time);
 	else
 		tm = time_to_tm(time, tz);
@@ -230,17 +305,17 @@ const char *show_date(timestamp_t time, int tz, const struct date_mode *mode)
 	}
 
 	strbuf_reset(&timebuf);
-	if (mode->type == DATE_SHORT)
+	if (type == DATE_SHORT)
 		strbuf_addf(&timebuf, "%04d-%02d-%02d", tm->tm_year + 1900,
 				tm->tm_mon + 1, tm->tm_mday);
-	else if (mode->type == DATE_ISO8601)
+	else if (type == DATE_ISO8601)
 		strbuf_addf(&timebuf, "%04d-%02d-%02d %02d:%02d:%02d %+05d",
 				tm->tm_year + 1900,
 				tm->tm_mon + 1,
 				tm->tm_mday,
 				tm->tm_hour, tm->tm_min, tm->tm_sec,
 				tz);
-	else if (mode->type == DATE_ISO8601_STRICT) {
+	else if (type == DATE_ISO8601_STRICT) {
 		char sign = (tz >= 0) ? '+' : '-';
 		tz = abs(tz);
 		strbuf_addf(&timebuf, "%04d-%02d-%02dT%02d:%02d:%02d%c%02d:%02d",
@@ -249,23 +324,16 @@ const char *show_date(timestamp_t time, int tz, const struct date_mode *mode)
 				tm->tm_mday,
 				tm->tm_hour, tm->tm_min, tm->tm_sec,
 				sign, tz / 100, tz % 100);
-	} else if (mode->type == DATE_RFC2822)
+	} else if (type == DATE_RFC2822)
 		strbuf_addf(&timebuf, "%.3s, %d %.3s %d %02d:%02d:%02d %+05d",
 			weekday_names[tm->tm_wday], tm->tm_mday,
 			month_names[tm->tm_mon], tm->tm_year + 1900,
 			tm->tm_hour, tm->tm_min, tm->tm_sec, tz);
-	else if (mode->type == DATE_STRFTIME)
+	else if (type == DATE_STRFTIME)
 		strbuf_addftime(&timebuf, mode->strftime_fmt, tm, tz,
-				!mode->local);
+				!local);
 	else
-		strbuf_addf(&timebuf, "%.3s %.3s %d %02d:%02d:%02d %d%c%+05d",
-				weekday_names[tm->tm_wday],
-				month_names[tm->tm_mon],
-				tm->tm_mday,
-				tm->tm_hour, tm->tm_min, tm->tm_sec,
-				tm->tm_year + 1900,
-				mode->local ? 0 : ' ',
-				tz);
+		show_date_normal(&timebuf, tm, tz, &human_tm, human_tz, local);
 	return timebuf.buf;
 }
 
@@ -802,6 +870,11 @@ int parse_date(const char *date, struct strbuf *result)
 	return 0;
 }
 
+static int auto_date_style(void)
+{
+	return (isatty(1) || pager_in_use()) ? DATE_HUMAN : DATE_NORMAL;
+}
+
 static enum date_mode_type parse_date_type(const char *format, const char **end)
 {
 	if (skip_prefix(format, "relative", end))
@@ -819,6 +892,10 @@ static enum date_mode_type parse_date_type(const char *format, const char **end)
 		return DATE_SHORT;
 	if (skip_prefix(format, "default", end))
 		return DATE_NORMAL;
+	if (skip_prefix(format, "human", end))
+		return DATE_HUMAN;
+	if (skip_prefix(format, "auto", end))
+		return auto_date_style();
 	if (skip_prefix(format, "raw", end))
 		return DATE_RAW;
 	if (skip_prefix(format, "unix", end))
-- 
2.18.0.131.gc3213e20f.dirty


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

* Re: [RFC PATCH] Add 'human' date format
  2018-07-07 19:38 [RFC PATCH] Add 'human' date format Linus Torvalds
@ 2018-07-07 19:58 ` Linus Torvalds
  2018-07-07 20:12   ` Linus Torvalds
  2018-07-07 22:02 ` [RFC PATCH v2] " Linus Torvalds
  1 sibling, 1 reply; 13+ messages in thread
From: Linus Torvalds @ 2018-07-07 19:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List

On Sat, Jul 7, 2018 at 12:39 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> to me, but with "--date=human", right now it just says
>
>     Date:   12:21

Side note: this is probably my least favorite of the formats.

I'm playing with making all "today" dates just use the relative
format, and then the "a couple of days ago" dates would then have the

>     Date:   Fri 19:45

format.

But since it's _explicitly_ about a "human legible" format, I think
the format could be a bit fluid, and things like that might be tweaked
later. Anybody who would script this would be crazy.

I'm more looking for "no, that's just stupid" comments, or "no, you're
not the only one who has wanted this" kinds of replies.

                  Linus

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

* Re: [RFC PATCH] Add 'human' date format
  2018-07-07 19:58 ` Linus Torvalds
@ 2018-07-07 20:12   ` Linus Torvalds
  0 siblings, 0 replies; 13+ messages in thread
From: Linus Torvalds @ 2018-07-07 20:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List

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

On Sat, Jul 7, 2018 at 12:58 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> I'm playing with making all "today" dates just use the relative
> format.

Here's the incremental patch for that if people want to compare the output.

With this, you never get the "just time" case, because that will turn
into "2 hours ago" or similar. But you will get "Fri 19:45" for
something that happened yesterday.

So examples from my kernel logs look something like this:

  2 hours ago
  Fri 19:45
  Fri 10:44 +1000
  Fri Jun 22 15:46
  Tue Jun 19 15:41 -0600
  Thu Jun 15 12:57 2017 +0300

depending on how long ago they were and whether they were in the same
timezone etc.

              Linus

[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 1743 bytes --]

 date.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/date.c b/date.c
index 9809ac334..de0b03cf4 100644
--- a/date.c
+++ b/date.c
@@ -199,7 +199,7 @@ struct date_mode *date_mode_from_type(enum date_mode_type type)
 	return &mode;
 }
 
-static void show_date_normal(struct strbuf *buf, struct tm *tm, int tz, struct tm *human_tm, int human_tz, int local)
+static void show_date_normal(struct strbuf *buf, timestamp_t time, struct tm *tm, int tz, struct tm *human_tm, int human_tz, int local)
 {
 	struct {
 		unsigned int	year:1,
@@ -225,6 +225,14 @@ static void show_date_normal(struct strbuf *buf, struct tm *tm, int tz, struct t
 		}
 	}
 
+	/* Show "today" times as just relative times */
+	if (hide.wday) {
+		struct timeval now;
+		gettimeofday(&now, NULL);
+		show_date_relative(time, tz, &now, buf);
+		return;
+	}
+
 	/* Always hide seconds for human-readable */
 	hide.seconds = human_tm->tm_year > 0;
 
@@ -268,10 +276,6 @@ const char *show_date(timestamp_t time, int tz, const struct date_mode *mode)
 		/* Fill in the data for "current time" in human_tz and human_tm */
 		human_tz = local_time_tzoffset(now.tv_sec, &human_tm);
 
-		/* Special case: if it's less than an hour ago, use relative time */
-		if (time - now.tv_sec < 60 * 60)
-			type = DATE_RELATIVE;
-
 		/* Don't print timezone if it matches */
 		if (tz == human_tz)
 			local = 1;
@@ -333,7 +337,7 @@ const char *show_date(timestamp_t time, int tz, const struct date_mode *mode)
 		strbuf_addftime(&timebuf, mode->strftime_fmt, tm, tz,
 				!local);
 	else
-		show_date_normal(&timebuf, tm, tz, &human_tm, human_tz, local);
+		show_date_normal(&timebuf, time, tm, tz, &human_tm, human_tz, local);
 	return timebuf.buf;
 }
 

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

* [RFC PATCH v2] Add 'human' date format
  2018-07-07 19:38 [RFC PATCH] Add 'human' date format Linus Torvalds
  2018-07-07 19:58 ` Linus Torvalds
@ 2018-07-07 22:02 ` " Linus Torvalds
  2018-07-11 20:34   ` Andrei Rybak
                     ` (2 more replies)
  1 sibling, 3 replies; 13+ messages in thread
From: Linus Torvalds @ 2018-07-07 22:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git


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

This adds --date=human, which skips the timezone if it matches the
current time-zone, and doesn't print the whole date if that matches (ie
skip printing year for dates that are "this year", but also skip the
whole date itself if it's in the last few days and we can just say what
weekday it was).

For really recent dates (same day), use the relative date stamp, while
for old dates (year doesn't match), don't bother with time and timezone.

Also add 'auto' date mode, which defaults to human if we're using the
pager.  So you can do

	git config --add log.date auto

and your "git log" commands will show the human-legible format unless
you're scripting things.

Note that this time format still shows the timezone for recent enough
events (but not so recent that they show up as relative dates).  You can
combine it with the "-local" suffix to never show timezones for an even
more simplified view.

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

Slightly updated version after playing with this more. 

This tries to make the length somewhat more consistent (and shorter), 
which came about when looking at this in "git blame" output. 

Once you're talking "last year" patches, you don't tend to care about time 
of day or timezone. So the longest date is basically "Thu Oct 19 16:00", 
because if you show the year (four characters), you don't show the time 
(five characters). And the timezone (five characters) is only shown if not 
showing the date (5-6 characters).

Also, because the relative time is now handled entirely inside the 
show_date_normal() function, I could undo some of the changes to 
show_date() that were updating date->mode and date->local. So the patch 
has actually shrunk a bit, I think.

 builtin/blame.c |   4 ++
 cache.h         |   1 +
 date.c          | 130 ++++++++++++++++++++++++++++++++++++++++--------
 3 files changed, 115 insertions(+), 20 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 5a0388aae..7b6235321 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -917,6 +917,10 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 		 */
 		blame_date_width = utf8_strwidth(_("4 years, 11 months ago")) + 1; /* add the null */
 		break;
+	case DATE_HUMAN:
+		/* If the year is shown, no time is shown */
+		blame_date_width = sizeof("Thu Oct 19 16:00");
+		break;
 	case DATE_NORMAL:
 		blame_date_width = sizeof("Thu Oct 19 16:00:04 2006 -0700");
 		break;
diff --git a/cache.h b/cache.h
index d49092d94..8a6810ee6 100644
--- a/cache.h
+++ b/cache.h
@@ -1428,6 +1428,7 @@ extern struct object *peel_to_type(const char *name, int namelen,
 struct date_mode {
 	enum date_mode_type {
 		DATE_NORMAL = 0,
+		DATE_HUMAN,
 		DATE_RELATIVE,
 		DATE_SHORT,
 		DATE_ISO8601,
diff --git a/date.c b/date.c
index 49f943e25..4486c028a 100644
--- a/date.c
+++ b/date.c
@@ -77,22 +77,16 @@ static struct tm *time_to_tm_local(timestamp_t time)
 }
 
 /*
- * What value of "tz" was in effect back then at "time" in the
- * local timezone?
+ * Fill in the localtime 'struct tm' for the supplied time,
+ * and return the local tz.
  */
-static int local_tzoffset(timestamp_t time)
+static int local_time_tzoffset(time_t t, struct tm *tm)
 {
-	time_t t, t_local;
-	struct tm tm;
+	time_t t_local;
 	int offset, eastwest;
 
-	if (date_overflows(time))
-		die("Timestamp too large for this system: %"PRItime, time);
-
-	t = (time_t)time;
-	localtime_r(&t, &tm);
-	t_local = tm_to_time_t(&tm);
-
+	localtime_r(&t, tm);
+	t_local = tm_to_time_t(tm);
 	if (t_local == -1)
 		return 0; /* error; just use +0000 */
 	if (t_local < t) {
@@ -107,6 +101,20 @@ static int local_tzoffset(timestamp_t time)
 	return offset * eastwest;
 }
 
+/*
+ * What value of "tz" was in effect back then at "time" in the
+ * local timezone?
+ */
+static int local_tzoffset(timestamp_t time)
+{
+	struct tm tm;
+
+	if (date_overflows(time))
+		die("Timestamp too large for this system: %"PRItime, time);
+
+	return local_time_tzoffset((time_t)time, &tm);
+}
+
 void show_date_relative(timestamp_t time, int tz,
 			       const struct timeval *now,
 			       struct strbuf *timebuf)
@@ -191,9 +199,80 @@ struct date_mode *date_mode_from_type(enum date_mode_type type)
 	return &mode;
 }
 
+static void show_date_normal(struct strbuf *buf, timestamp_t time, struct tm *tm, int tz, struct tm *human_tm, int human_tz, int local)
+{
+	struct {
+		unsigned int	year:1,
+				date:1,
+				wday:1,
+				time:1,
+				seconds:1,
+				tz:1;
+	} hide = { 0 };
+
+	hide.tz = local || tz == human_tz;
+	hide.year = tm->tm_year == human_tm->tm_year;
+	if (hide.year) {
+		if (tm->tm_mon == human_tm->tm_mon) {
+			if (tm->tm_mday > human_tm->tm_mday) {
+				/* Future date: think timezones */
+			} else if (tm->tm_mday == human_tm->tm_mday) {
+				hide.date = hide.wday = 1;
+			} else if (tm->tm_mday + 5 > human_tm->tm_mday) {
+				/* Leave just weekday if it was a few days ago */
+				hide.date = 1;
+			}
+		}
+	}
+
+	/* Show "today" times as just relative times */
+	if (hide.wday) {
+		struct timeval now;
+		gettimeofday(&now, NULL);
+		show_date_relative(time, tz, &now, buf);
+		return;
+	}
+
+	/*
+	 * Always hide seconds for human-readable.
+	 * Hide timezone if showing date.
+	 * Hide weekday and time if showing year.
+	 *
+	 * The logic here is two-fold:
+	 *  (a) only show details when recent enough to matter
+	 *  (b) keep the maximum length "similar", and in check
+	 */
+	if (human_tm->tm_year) {
+		hide.seconds = 1;
+		hide.tz |= !hide.date;
+		hide.wday = hide.time = !hide.year;
+	}
+
+	if (!hide.wday)
+		strbuf_addf(buf, "%.3s ", weekday_names[tm->tm_wday]);
+	if (!hide.date)
+		strbuf_addf(buf, "%.3s %d ", month_names[tm->tm_mon], tm->tm_mday);
+
+	/* Do we want AM/PM depending on locale? */
+	if (!hide.time) {
+		strbuf_addf(buf, "%02d:%02d", tm->tm_hour, tm->tm_min);
+		if (!hide.seconds)
+			strbuf_addf(buf, ":%02d", tm->tm_sec);
+	} else
+		strbuf_rtrim(buf);
+
+	if (!hide.year)
+		strbuf_addf(buf, " %d", tm->tm_year + 1900);
+
+	if (!hide.tz)
+		strbuf_addf(buf, " %+05d", tz);
+}
+
 const char *show_date(timestamp_t time, int tz, const struct date_mode *mode)
 {
 	struct tm *tm;
+	struct tm human_tm = { 0 };
+	int human_tz = -1;
 	static struct strbuf timebuf = STRBUF_INIT;
 
 	if (mode->type == DATE_UNIX) {
@@ -202,6 +281,15 @@ const char *show_date(timestamp_t time, int tz, const struct date_mode *mode)
 		return timebuf.buf;
 	}
 
+	if (mode->type == DATE_HUMAN) {
+		struct timeval now;
+
+		gettimeofday(&now, NULL);
+
+		/* Fill in the data for "current time" in human_tz and human_tm */
+		human_tz = local_time_tzoffset(now.tv_sec, &human_tm);
+	}
+
 	if (mode->local)
 		tz = local_tzoffset(time);
 
@@ -258,14 +346,7 @@ const char *show_date(timestamp_t time, int tz, const struct date_mode *mode)
 		strbuf_addftime(&timebuf, mode->strftime_fmt, tm, tz,
 				!mode->local);
 	else
-		strbuf_addf(&timebuf, "%.3s %.3s %d %02d:%02d:%02d %d%c%+05d",
-				weekday_names[tm->tm_wday],
-				month_names[tm->tm_mon],
-				tm->tm_mday,
-				tm->tm_hour, tm->tm_min, tm->tm_sec,
-				tm->tm_year + 1900,
-				mode->local ? 0 : ' ',
-				tz);
+		show_date_normal(&timebuf, time, tm, tz, &human_tm, human_tz, mode->local);
 	return timebuf.buf;
 }
 
@@ -802,6 +883,11 @@ int parse_date(const char *date, struct strbuf *result)
 	return 0;
 }
 
+static int auto_date_style(void)
+{
+	return (isatty(1) || pager_in_use()) ? DATE_HUMAN : DATE_NORMAL;
+}
+
 static enum date_mode_type parse_date_type(const char *format, const char **end)
 {
 	if (skip_prefix(format, "relative", end))
@@ -819,6 +905,10 @@ static enum date_mode_type parse_date_type(const char *format, const char **end)
 		return DATE_SHORT;
 	if (skip_prefix(format, "default", end))
 		return DATE_NORMAL;
+	if (skip_prefix(format, "human", end))
+		return DATE_HUMAN;
+	if (skip_prefix(format, "auto", end))
+		return auto_date_style();
 	if (skip_prefix(format, "raw", end))
 		return DATE_RAW;
 	if (skip_prefix(format, "unix", end))
-- 
2.18.0.132.g95eda3d86


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

* Re: [RFC PATCH v2] Add 'human' date format
  2018-07-07 22:02 ` [RFC PATCH v2] " Linus Torvalds
@ 2018-07-11 20:34   ` Andrei Rybak
  2018-07-11 20:38     ` Andrei Rybak
  2018-07-11 20:49     ` Linus Torvalds
  2018-07-11 21:24   ` Ævar Arnfjörð Bjarmason
  2018-07-24 21:49   ` Junio C Hamano
  2 siblings, 2 replies; 13+ messages in thread
From: Andrei Rybak @ 2018-07-11 20:34 UTC (permalink / raw)
  To: torvalds, Junio C Hamano; +Cc: git

On 2018-07-08 00:02, Linus Torvalds wrote:
> diff --git a/date.c b/date.c
> index 49f943e25..4486c028a 100644
> --- a/date.c
> +++ b/date.c
> @@ -77,22 +77,16 @@ static struct tm *time_to_tm_local(timestamp_t time)
>  }
>
>  /*
> - * What value of "tz" was in effect back then at "time" in the
> - * local timezone?
> + * Fill in the localtime 'struct tm' for the supplied time,
> + * and return the local tz.
>   */
> -static int local_tzoffset(timestamp_t time)
> +static int local_time_tzoffset(time_t t, struct tm *tm)
>  {
> -     time_t t, t_local;
> -     struct tm tm;
> +     time_t t_local;
>       int offset, eastwest;
>
> -     if (date_overflows(time))
> -             die("Timestamp too large for this system: %"PRItime, time);
> -
> -     t = (time_t)time;
> -     localtime_r(&t, &tm);
> -     t_local = tm_to_time_t(&tm);
> -
> +     localtime_r(&t, tm);
> +     t_local = tm_to_time_t(tm);
>       if (t_local == -1)
>               return 0; /* error; just use +0000 */
>       if (t_local < t) {
> @@ -107,6 +101,20 @@ static int local_tzoffset(timestamp_t time)
>       return offset * eastwest;
>  }
>

[...]

> +
>  const char *show_date(timestamp_t time, int tz, const struct date_mode *mode)
>  {
>       struct tm *tm;
> +     struct tm human_tm = { 0 };
> +     int human_tz = -1;

Is -1 an OK initial value for timezone if local_time_tzoffset returns
negative values as well? It looks like it doesn't matter for from functional

>       static struct strbuf timebuf = STRBUF_INIT;
>
>       if (mode->type == DATE_UNIX) {
> @@ -202,6 +281,15 @@ const char *show_date(timestamp_t time, int tz, const struct date_mode *mode)
>               return timebuf.buf;
>       }
>
> +     if (mode->type == DATE_HUMAN) {
> +             struct timeval now;
> +
> +             gettimeofday(&now, NULL);
> +
> +             /* Fill in the data for "current time" in human_tz and human_tm */
> +             human_tz = local_time_tzoffset(now.tv_sec, &human_tm);
> +     }
> +
>       if (mode->local)
>               tz = local_tzoffset(time);
>

--
Best regards, Andrei Rybak

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

* Re: [RFC PATCH v2] Add 'human' date format
  2018-07-11 20:34   ` Andrei Rybak
@ 2018-07-11 20:38     ` Andrei Rybak
  2018-07-11 20:54       ` Junio C Hamano
  2018-07-11 20:49     ` Linus Torvalds
  1 sibling, 1 reply; 13+ messages in thread
From: Andrei Rybak @ 2018-07-11 20:38 UTC (permalink / raw)
  To: torvalds, Junio C Hamano; +Cc: git

On Wed, 11 Jul 2018 at 22:34, Andrei Rybak <rybak.a.v@gmail.com> wrote:
>
> Is -1 an OK initial value for timezone if local_time_tzoffset returns
> negative values as well? It looks like it doesn't matter for from functional
>

meant to say: "It looks like it doesn't matter from the functional
point of view".

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

* Re: [RFC PATCH v2] Add 'human' date format
  2018-07-11 20:34   ` Andrei Rybak
  2018-07-11 20:38     ` Andrei Rybak
@ 2018-07-11 20:49     ` Linus Torvalds
  2018-07-11 21:23       ` Linus Torvalds
  1 sibling, 1 reply; 13+ messages in thread
From: Linus Torvalds @ 2018-07-11 20:49 UTC (permalink / raw)
  To: rybak.a.v; +Cc: Junio C Hamano, Git Mailing List

On Wed, Jul 11, 2018 at 1:34 PM Andrei Rybak <rybak.a.v@gmail.com> wrote:
>
> > +     int human_tz = -1;
>
> Is -1 an OK initial value for timezone if local_time_tzoffset returns
> negative values as well? It looks like it doesn't matter for from functional

The value was intentionally picked to *not* be a valid timezone value,
so that the comparison of "human_tz == tz" would always fail if
DATE_HUMAN is not selected.

But it could be anything else invalid, of course. It could be MAX_INT
or something like that.

By picking something that isn't possibly a real timezone value, late
code can do things like

        hide.tz = local || tz == human_tz;

without worrying about whther it's really DATE_HUMAN or not.

The clearing of "human_tm" is done for a similar reason: the code does

        hide.year = tm->tm_year == human_tm->tm_year;

(and then later just checks "if (human_tm->tm_year)") knowing that a
non-zero tm_year will only ever happen for human_tz (and that 1900 is
not a valid git date, even though I guess in theory you could do it).

               Linus

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

* Re: [RFC PATCH v2] Add 'human' date format
  2018-07-11 20:38     ` Andrei Rybak
@ 2018-07-11 20:54       ` Junio C Hamano
  0 siblings, 0 replies; 13+ messages in thread
From: Junio C Hamano @ 2018-07-11 20:54 UTC (permalink / raw)
  To: Andrei Rybak; +Cc: torvalds, git

Andrei Rybak <rybak.a.v@gmail.com> writes:

> On Wed, 11 Jul 2018 at 22:34, Andrei Rybak <rybak.a.v@gmail.com> wrote:
>>
>> Is -1 an OK initial value for timezone if local_time_tzoffset returns
>> negative values as well? It looks like it doesn't matter for from functional
>>
>
> meant to say: "It looks like it doesn't matter from the functional
> point of view".

As long as we do not show data in a timezone that is exactly one
minute ahead (or is it behind???) of UTC, it does not cause an issue
in practice.

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

* Re: [RFC PATCH v2] Add 'human' date format
  2018-07-11 20:49     ` Linus Torvalds
@ 2018-07-11 21:23       ` Linus Torvalds
  0 siblings, 0 replies; 13+ messages in thread
From: Linus Torvalds @ 2018-07-11 21:23 UTC (permalink / raw)
  To: rybak.a.v; +Cc: Junio C Hamano, Git Mailing List

[ Trying to come up with crazy special cases ]

On Wed, Jul 11, 2018 at 1:49 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> But it could be anything else invalid, of course. It could be MAX_INT
> or something like that.

That might be better. A timezone of -1 isn't actually a valid
timezone, but I guess you could create a commit by hand that had
"-0001" as the timezone.

You can't do that with something like MAX_INT, without fsck
complaining - since it has to be exactly four digits.

> The clearing of "human_tm" is done for a similar reason: the code does
>
>         hide.year = tm->tm_year == human_tm->tm_year;
>
> (and then later just checks "if (human_tm->tm_year)") knowing that a
> non-zero tm_year will only ever happen for human_tz (and that 1900 is
> not a valid git date, even though I guess in theory you could do it).

Actually, the 1900 should be safe, because 'timestamp_t' is unsigned.
So a valid timestamp really can't be before 1970.

Of course, you can probably try to mess with it by giving values that
don't actually fit, because sometimes we do convert mindlessly from
'timestamp_t' to 'time_t'. In particular, if you use the
"default-local" time, it will use that

  static struct tm *time_to_tm_local(timestamp_t time)
  {
        time_t t = time;
        return localtime(&t);
  }

and not check the range of the timestamp.

But other proper time stamp functions will actually do range checking
with "date_overflow()", so in general that whole assumption of "a real
git date cannot be in the year 1900" is valid.

              Linus

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

* Re: [RFC PATCH v2] Add 'human' date format
  2018-07-07 22:02 ` [RFC PATCH v2] " Linus Torvalds
  2018-07-11 20:34   ` Andrei Rybak
@ 2018-07-11 21:24   ` Ævar Arnfjörð Bjarmason
  2018-07-11 21:49     ` Linus Torvalds
  2018-07-24 21:49   ` Junio C Hamano
  2 siblings, 1 reply; 13+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2018-07-11 21:24 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Junio C Hamano, git


On Sat, Jul 07 2018, Linus Torvalds wrote:

I really like where this is going in general. Having a "human" format
would be great.

> For really recent dates (same day), use the relative date stamp, while
> for old dates (year doesn't match), don't bother with time and timezone.
> [...]
> Once you're talking "last year" patches, you don't tend to care about time
> of day or timezone. So the longest date is basically "Thu Oct 19 16:00",
> because if you show the year (four characters), you don't show the time
> (five characters). And the timezone (five characters) is only shown if not
> showing the date (5-6 characters).

Just chiming in on this part, I think it's a worthwile trade-off to
always keep it relatively short, but I'd like to challenge the "you
don't tend to care about time [for really old commits]".

I think that's true for the likes of linux.git & git.git, but a lot of
users of git say work in some corporate setting entirely or mostly in
the same timezone.

In that case, knowing if some commit whose sole message was "fix"[1] was
made at 3am or in the afternoon, even if it's really old, is really
useful information, even years later.

Maybe something like v2 could be a human-lossy and v1 human-short (or
better names...). I.e. (AFAICT) v1 didn't lose any information, just
smartly abbreviated it, but v2 does.

1. Because let's face it, bothering to write good commit messages like
   git.git is the exception.

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

* Re: [RFC PATCH v2] Add 'human' date format
  2018-07-11 21:24   ` Ævar Arnfjörð Bjarmason
@ 2018-07-11 21:49     ` Linus Torvalds
  0 siblings, 0 replies; 13+ messages in thread
From: Linus Torvalds @ 2018-07-11 21:49 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Junio C Hamano, Git Mailing List

On Wed, Jul 11, 2018 at 2:24 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
> I think that's true for the likes of linux.git & git.git, but a lot of
> users of git say work in some corporate setting entirely or mostly in
> the same timezone.
>
> In that case, knowing if some commit whose sole message was "fix"[1] was
> made at 3am or in the afternoon, even if it's really old, is really
> useful information, even years later.

Heh.

Maybe. But if  you care about that kind of information, would you
actually want to use the "human" date? Wouldn't you want to use the
strftime thing instead, which gets you whatever field you care about,
and gets it consistently regardless of how old the data is?

That said, I do acknowledge that the "human" format may be a bit
inflexible and ad-hoc. Of course some more generic way that allowed
arbitrary rules might be better for some uses.

I'll just explain the cases that made me zero in on what that last patch did:

 (a) I do like the "relative" date for recent stuff.

Quite often, I look at how recent the commits are, for example, and
then I really like seeing "2 hours ago" rather than a time with a
timezone (which is a lot harder for me to mentally parse)

This was the primary impetus for my original "auto" patch many years
ago, that was (rightly) not merged. It really boiled down to just
"default or relative, depending on how recent it was".

 (b) I noticed that I was distracted by dates that were *too* terse.

My first patch had _just_ the time when it was today and the same
timezone (but older than two hours, so the original relative logic
didn't trigger).

That initially sounded great to me, which is why it was that first time.

But after _using_ it for a while, I actually found that it didn't have
enough context for me (visually) to really trigger my date parsing at
all.

So "five hours ago" actually parsed better than just "9:48" to me. I
didn't think it would do that, but it did. Which was why I changed the
"relative" time to trigger every time if it was the exact same date
(and in the past) - just to avoid the really terse model.

 (c) when I played around with other commands than just "git log", I
also noticed that a consistent length mattered.,

Again, my first version was more along the lines of "if it's long ago,
just use the full format, exactly like the default date". It wasn't
*quite* that, because it would always skip the seconds, but it was
close.

And with "git log", that worked fine, because dates were fairly
uniformly increasing, so the date format would slowly get longer, and
that was fine.

But then when I played with "git blame -C --date=human", I noticed
that not only did the human date actually make sense there too, it
actually made it easier for me to read - and that in particular, the
"extra" info was just annoying.

So now I find that shortened "only show the date" format to be really
good _particularly_ for "git blame". You can see very clearly whether
it's something recent or something old.

Maybe my use of "git blame" is unusual, but I don't think so. I tend
to do "git blame -C" when I'm looking for a bug, and then seeing
something like this:

        ...
          Apr 16 2005       437)
          Apr 16 2005       438)
          Jan 14 2016       439)
          Apr 16 2005       440)
          Apr 16 2005       441)
          Apr 16 2005       442)
          Thu Jun 14 15:26  443)
          Thu Jun 14 15:26  444)
          Thu Jun 14 15:26  445)
          Thu Jun 14 15:26  446)
          Thu Jun 14 15:26  447)
          Thu Jun 14 15:26  448)
          Thu Jun 14 15:26  449)
          Thu Jun 14 15:26  450)
          Apr 16 2005       451)
          Jul 30 2012       452)
          Jul 30 2012       453)
          Feb 13 2012       454)
          Apr 16 2005       455)
          Apr 16 2005       456)
        ....

in that date field (yeah. that happens to be "kernel/fork.c" in the
current kernel - I just edited out all the other stuff than time and
line number) is actually very visually easy to see what parts are old,
and which ones are recent, because it changes the format pretty
clearly and unambiguously, without changing the size of that field
_dramatically_.

(Sure, the size changes, but it's not a radical difference, it's a
fairly small variation, and the variation only highlights the
different time range, without making it compltely unbalanced).

Anyway, enough excuses. I'l just trying to explain some of the things
that I noticed simply _while_ making some of the decisions I made.

Are they the "right" decisions? I don't know. But I've been running with that

        git config --add log.date auto

in my kernel repo since I posted the patches, and so far I'm still liking it.

                 Linus

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

* Re: [RFC PATCH v2] Add 'human' date format
  2018-07-07 22:02 ` [RFC PATCH v2] " Linus Torvalds
  2018-07-11 20:34   ` Andrei Rybak
  2018-07-11 21:24   ` Ævar Arnfjörð Bjarmason
@ 2018-07-24 21:49   ` Junio C Hamano
  2018-07-24 22:58     ` Linus Torvalds
  2 siblings, 1 reply; 13+ messages in thread
From: Junio C Hamano @ 2018-07-24 21:49 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git

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

> From: Linus Torvalds <torvalds@linux-foundation.org>
>
> This adds --date=human, which skips the timezone if it matches the
> current time-zone, and doesn't print the whole date if that matches (ie
> skip printing year for dates that are "this year", but also skip the
> whole date itself if it's in the last few days and we can just say what
> weekday it was).

The behavior of the code does not quite match my intuition, though.

    $ date ;# to show that I am in -0700 zone
    Tue Jul 24 14:42:09 PDT 2018
    $ git show -s pk/rebase-in-c | head -n 3
    commit d18b5221ba98fe8254c3f9922ba31b21d7c954af
    Author: Pratik Karki <predatoramigo@gmail.com>
    Date:   Sun Jul 8 23:46:04 2018 +0545
    $ git show --date=human -s pk/rebase-in-c | head -n 3
    commit d18b5221ba98fe8254c3f9922ba31b21d7c954af
    Author: Pratik Karki <predatoramigo@gmail.com>
    Date:   Sun Jul 8 23:46

It is sensible to omit the seconds; I do not really care about that
level of precision for an event that happened two weeks ago in a
different continent.

But lack of TZ does not give me enough hint about which content it
happened.  The fact that this was done late at night on weekend is
indeed interesting, and I may not care what time it locally was for
me, so perhaps this is an intended behaviour.


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

* Re: [RFC PATCH v2] Add 'human' date format
  2018-07-24 21:49   ` Junio C Hamano
@ 2018-07-24 22:58     ` Linus Torvalds
  0 siblings, 0 replies; 13+ messages in thread
From: Linus Torvalds @ 2018-07-24 22:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List

On Tue, Jul 24, 2018 at 2:49 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> But lack of TZ does not give me enough hint about which content it
> happened.  The fact that this was done late at night on weekend is
> indeed interesting, and I may not care what time it locally was for
> me, so perhaps this is an intended behaviour.

I'm not sure you could call it "intended". The TZ hiding did change as
I was playing with this.

The first version of the patch only hid the timezone if it matched the
current one.

Because, as you say, the time zone can be interesting not so much
because you care about *when* the commit happened, but you care about
*where* the commit happened. And that can be true even if the commit
is very old.

So the timezone data in some sense isn't necessarily about the date at all.

When I used it a bit more (I still have the "--date=auto" as my
default for the kernel), I decided I really don't much care about the
timezone. In any _individual_ case, the timezone looks fine, but when
you look at many different dates, it looks really odd how it sometimes
shows, and sometimes does not, particularly for old dates when it
really doesn't matter for the *time* reason.

So I decided that it's better to not show the timezone at all when you
show a real date.

But honestly, I don't claim to have a really strong argument. It's
just a choice. Nothing says it's the absolute right choice.

I pointed out that you can use "--date=human-local" to get an even
denser representation that gives you the human date without ever
having a TZ. But we don't have the reverse of "-local", which would
explicitly show the timezones.

Again, I think this is really because the timezone is about something
other than just the time. I think the "do we care *where* it was done
or not?" question in many ways is entirely independent of the time
question.

So right now the patch says

                hide.tz |= !hide.date;

which ends up being good for the "times are roughly the same size"
(which I decided was a good thing - again, I don't really have a
hugely strong argument for it, it was a matter of me playing with
options).

But it would make equally much sense to say

                hide.tz |= hide.time;

and just say that the timezone is hidden if it matches the current
one, or if the commit is just so old that we don't show the time at
all.

OR you could just say "timezone is always interesting, because you
want to know _where_ it was done regardless of _when_ it was done",
and just not hide the timezone at all.

I think all are "technically valid" choices to make. The one I made
was just a random personal preference, not necessarily the right one.

Could we extend on the "decorations" (like the "-local" thing)?
Absolutely.  I'm not sure it's worth doing, but it would certainly
solve the "different people have different preferences" issue.

I think that *if* we want to extend on the decorations, that would
probably still be a separate patch from the basic patch.

               Linus

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

end of thread, back to index

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-07 19:38 [RFC PATCH] Add 'human' date format Linus Torvalds
2018-07-07 19:58 ` Linus Torvalds
2018-07-07 20:12   ` Linus Torvalds
2018-07-07 22:02 ` [RFC PATCH v2] " Linus Torvalds
2018-07-11 20:34   ` Andrei Rybak
2018-07-11 20:38     ` Andrei Rybak
2018-07-11 20:54       ` Junio C Hamano
2018-07-11 20:49     ` Linus Torvalds
2018-07-11 21:23       ` Linus Torvalds
2018-07-11 21:24   ` Ævar Arnfjörð Bjarmason
2018-07-11 21:49     ` Linus Torvalds
2018-07-24 21:49   ` Junio C Hamano
2018-07-24 22:58     ` Linus Torvalds

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox