[PATCH] show_date(): rename the "relative" parameter to "mode"

[PATCH] show_date(): rename the "relative" parameter to "mode"

[Johannes Schindelin]

Now, show_date() can print three different kinds of dates: normal,
relative and short (%Y-%m-%s) dates.

To achieve this, the "int relative" was changed to "enum date_mode
mode", which has three states: DATE_NORMAL, DATE_RELATIVE and
DATE_SHORT.

Since existing users of show_date() only call it with relative_date
being either 0 or 1, and DATE_NORMAL and DATE_RELATIVE having these
values, no behaviour is changed.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 cache.h |    3 ++-
 date.c  |   20 ++++++++++++--------
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/cache.h b/cache.h
index 8bbc142..8f89a8b 100644
--- a/cache.h
+++ b/cache.h
@@ -326,7 +326,8 @@ extern void *read_object_with_reference(const unsigned char *sha1,
 					unsigned long *size,
 					unsigned char *sha1_ret);
 
-const char *show_date(unsigned long time, int timezone, int relative);
+enum date_mode { DATE_NORMAL = 0, DATE_RELATIVE, DATE_SHORT };
+const char *show_date(unsigned long time, int timezone, enum date_mode mode);
 const char *show_rfc2822_date(unsigned long time, int timezone);
 int parse_date(const char *date, char *buf, int bufsize);
 void datestamp(char *buf, int bufsize);
diff --git a/date.c b/date.c
index 542c004..0ceccbe 100644
--- a/date.c
+++ b/date.c
@@ -55,12 +55,12 @@ static struct tm *time_to_tm(unsigned long time, int tz)
 	return gmtime(&t);
 }
 
-const char *show_date(unsigned long time, int tz, int relative)
+const char *show_date(unsigned long time, int tz, enum date_mode mode)
 {
 	struct tm *tm;
 	static char timebuf[200];
 
-	if (relative) {
+	if (mode == DATE_RELATIVE) {
 		unsigned long diff;
 		struct timeval now;
 		gettimeofday(&now, NULL);
@@ -105,12 +105,16 @@ const char *show_date(unsigned long time, int tz, int relative)
 	tm = time_to_tm(time, tz);
 	if (!tm)
 		return NULL;
-	sprintf(timebuf, "%.3s %.3s %d %02d:%02d:%02d %d %+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, tz);
+	if (mode == DATE_SHORT)
+		sprintf(timebuf, "%04d-%02d-%02d", tm->tm_year + 1900,
+				tm->tm_mon + 1, tm->tm_mday);
+	else
+		sprintf(timebuf, "%.3s %.3s %d %02d:%02d:%02d %d %+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, tz);
 	return timebuf;
 }
 
-- 
1.5.0.2.2410.g737b

Re: [PATCH] show_date(): rename the "relative" parameter to "mode"

[Linus Torvalds]

On Tue, 27 Feb 2007, Johannes Schindelin wrote:
> 
> Now, show_date() can print three different kinds of dates: normal,
> relative and short (%Y-%m-%s) dates.
> 
> To achieve this, the "int relative" was changed to "enum date_mode
> mode", which has three states: DATE_NORMAL, DATE_RELATIVE and
> DATE_SHORT.

Ack. I think this kind of thing is worth it regardless of any of the other 
issues (ie the whole "changelog" thing). 

		Linus

Re: [PATCH] show_date(): rename the "relative" parameter to "mode"

[Junio C Hamano]

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

> On Tue, 27 Feb 2007, Johannes Schindelin wrote:
>> 
>> Now, show_date() can print three different kinds of dates: normal,
>> relative and short (%Y-%m-%s) dates.
>> 
>> To achieve this, the "int relative" was changed to "enum date_mode
>> mode", which has three states: DATE_NORMAL, DATE_RELATIVE and
>> DATE_SHORT.
>
> Ack. I think this kind of thing is worth it regardless of any of the other 
> issues (ie the whole "changelog" thing). 

Ack-on-Ack.

Re: [PATCH] show_date(): rename the "relative" parameter to "mode"

[Linus Torvalds]

On Tue, 27 Feb 2007, Linus Torvalds wrote:
> 
> Ack. I think this kind of thing is worth it regardless of any of the other 
> issues (ie the whole "changelog" thing). 

Side note: one of the reasons I like having more fine-grained date flags 
is that I've occasionally wanted a "show in UTC time" or "show everything 
in *my* local time" option. Sometimes it's a bit hard to compare dates 
just because we show them as they were for the people who did them, not 
in any unified format ;)

I'm not sure how useful that really would end up being, but the whole 
approach of just having a "date mode" flag should at least make those 
kinds of things trivial if we ever really want it.

		Linus

Re: [PATCH] show_date(): rename the "relative" parameter to "mode"

[Johannes Schindelin]

Hi,

On Tue, 27 Feb 2007, Linus Torvalds wrote:

> On Tue, 27 Feb 2007, Johannes Schindelin wrote:
> > 
> > Now, show_date() can print three different kinds of dates: normal, 
> > relative and short (%Y-%m-%s) dates.
> > 
> > To achieve this, the "int relative" was changed to "enum date_mode 
> > mode", which has three states: DATE_NORMAL, DATE_RELATIVE and 
> > DATE_SHORT.
> 
> Ack. I think this kind of thing is worth it regardless of any of the 
> other issues (ie the whole "changelog" thing).

Gee, thanks!

And you are no supporter of a GNU ChangeLog? Imagine being able to produce 
a GNU style ChangeLog of the Linux kernel in no time at all...

Ciao,
Dscho

Re: [PATCH] show_date(): rename the "relative" parameter to "mode"

[Johannes Schindelin]

Hi,

On Tue, 27 Feb 2007, Linus Torvalds wrote:

> Side note: one of the reasons I like having more fine-grained date flags 
> is that I've occasionally wanted a "show in UTC time" or "show 
> everything in *my* local time" option.

I found the --relative-date option very useful to that end, if a bit long 
to type.

Ciao,
Dscho

P.S.: It is refreshing to see somebody using the same convention of 
one-space-after-full-stop as me. Europeans...

Re: [PATCH] show_date(): rename the "relative" parameter to "mode"

[Linus Torvalds]

On Tue, 27 Feb 2007, Johannes Schindelin wrote:
> 
> And you are no supporter of a GNU ChangeLog? Imagine being able to produce 
> a GNU style ChangeLog of the Linux kernel in no time at all...

I don't think the GNU style ChangeLog is particularly good. In fact, I 
think it has an unfortunate tendency (thanks to being per-file-log) to 
encourage people to commit unrelated changes and then explain them per 
file.

Of course, you can't do that under git anyway, but I sure hope people 
don't start thinking that they should explain their changes in the git 
commit messages that way - my point being that certain log formats tend to 
encourage certain behaviour, and the GNU log format I think tends to do 
that exactly the wrong way around.

That said, listing the functions that got changed (which I don't know if 
you did, but some GNU changelogs do) may be a nice thing.

And hey, if some project wants GNU changelogs, I'm not against them. I 
just don't think they are in any way "superior", and the per-file comments 
really turn me off.

But the "short date + author name" on one line part I certainly don't 
disagree with. I often use "git log --stat", and the three-line 
default header that git uses is a bit verbose (but at least a shortened 
commit name would be good - one of the things I then do is that I may want 
to look in more detail at a commit).

			Linus

Re: [PATCH] show_date(): rename the "relative" parameter to "mode"

[Linus Torvalds]

On Tue, 27 Feb 2007, Johannes Schindelin wrote:

> Hi,
> 
> On Tue, 27 Feb 2007, Linus Torvalds wrote:
> 
> > Side note: one of the reasons I like having more fine-grained date flags 
> > is that I've occasionally wanted a "show in UTC time" or "show 
> > everything in *my* local time" option.
> 
> I found the --relative-date option very useful to that end, if a bit long 
> to type.

That really doesn't work for stuff that is more than a day old. The 
difference between "3 weeks ago" and "3 weeks ago" is not usually very 
obvious.

(Hint: often they are days apart, but the relative date-format will just 
hide any differences due to excessive granularity).

			Linus

Re: [PATCH] show_date(): rename the "relative" parameter to "mode"

[Junio C Hamano]

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

>> Side note: one of the reasons I like having more fine-grained date flags 
>> is that I've occasionally wanted a "show in UTC time" or "show 
>> everything in *my* local time" option.
>
> I found the --relative-date option very useful to that end, if a bit long 
> to type.

How about --ago as a synonym, and --my-date for obeying TZ?
"show in UTC time" can be had by something like:

	$ TZ=UTC git log --my-date

I personally am not very happy that "reflog show @{now}" gives
relative date and not the full datestamp.

Re: [PATCH] show_date(): rename the "relative" parameter to "mode"

[Johannes Schindelin]

Hi,

On Tue, 27 Feb 2007, Linus Torvalds wrote:

> But the "short date + author name" on one line part I certainly don't 
> disagree with.

If you use "next" Git:

	$ git log --pretty=format:"%h %ad %an"

Ciao,
Dscho

Re: [PATCH] show_date(): rename the "relative" parameter to "mode"

[Johannes Schindelin]

Hi,

On Tue, 27 Feb 2007, Linus Torvalds wrote:

> On Tue, 27 Feb 2007, Johannes Schindelin wrote:
> 
> > On Tue, 27 Feb 2007, Linus Torvalds wrote:
> > 
> > > Side note: one of the reasons I like having more fine-grained date 
> > > flags is that I've occasionally wanted a "show in UTC time" or "show 
> > > everything in *my* local time" option.
> > 
> > I found the --relative-date option very useful to that end, if a bit 
> > long to type.
> 
> That really doesn't work for stuff that is more than a day old. The 
> difference between "3 weeks ago" and "3 weeks ago" is not usually very 
> obvious.
> 
> (Hint: often they are days apart, but the relative date-format will just 
> hide any differences due to excessive granularity).

Maybe we need --relative-but-verbose-date?

Ciao,
Dscho

GNU ChangeLogs, was Re: [PATCH] show_date(): rename the "relative" parameter to "mode"

[Johannes Schindelin]

Hi,

On Tue, 27 Feb 2007, Linus Torvalds wrote:

> I don't think the GNU style ChangeLog is particularly good. In fact, I 
> think it has an unfortunate tendency (thanks to being per-file-log) to 
> encourage people to commit unrelated changes and then explain them per 
> file.

Yes, that is some bad thing.

> Of course, you can't do that under git anyway, but I sure hope people 
> don't start thinking that they should explain their changes in the git 
> commit messages that way - my point being that certain log formats tend 
> to encourage certain behaviour, and the GNU log format I think tends to 
> do that exactly the wrong way around.

Well, if you have to change your committing behaviour to get sensible 
automatic GNU ChangeLog from history, that would be a start, wouldn't it?

> That said, listing the functions that got changed (which I don't know if 
> you did, but some GNU changelogs do) may be a nice thing.

No I did not.

It is not _that_ easy with existing interfaces. Like I illustrated in an 
example I sent in another reply, you miss newly introduced functions.

Besides, without my Java-methods patch I sent yesterday, our function name 
extraction is utterly unusable on Java projects (and C++ projects with 
inlined methods).

> And hey, if some project wants GNU changelogs, I'm not against them. I 
> just don't think they are in any way "superior", and the per-file 
> comments really turn me off.

I think that the standard behaviour of "git log" rocks. But it would not 
hurt to be able to point to "git log --pretty=gnucl", to prove that Git is 
the ultimate SCM.

Ciao,
Dscho

Re: [PATCH] show_date(): rename the "relative" parameter to "mode"

[Johannes Schindelin]

Hi,

On Tue, 27 Feb 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> Side note: one of the reasons I like having more fine-grained date 
> >> flags is that I've occasionally wanted a "show in UTC time" or "show 
> >> everything in *my* local time" option.
> >
> > I found the --relative-date option very useful to that end, if a bit 
> > long to type.
> 
> How about --ago as a synonym, and --my-date for obeying TZ? "show in UTC 
> time" can be had by something like:
> 
> 	$ TZ=UTC git log --my-date

I like both.

> I personally am not very happy that "reflog show @{now}" gives relative 
> date and not the full datestamp.

$ git log -g @{now} 

Ciao,
Dscho

Re: [PATCH] show_date(): rename the "relative" parameter to "mode"

[Junio C Hamano]

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

>> I personally am not very happy that "reflog show @{now}" gives relative 
>> date and not the full datestamp.
>
> $ git log -g @{now} 

I said "reflog show" because I wanted to have oneline and
abbrev.

Re: [PATCH] show_date(): rename the "relative" parameter to "mode"

[Johannes Schindelin]

Hi,

On Tue, 27 Feb 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> I personally am not very happy that "reflog show @{now}" gives relative 
> >> date and not the full datestamp.
> >
> > $ git log -g @{now} 
> 
> I said "reflog show" because I wanted to have oneline and abbrev.

Ah. Yes.

However, when I look at reflogs, usually I know an approximate relative 
date of the commit I am looking for. I _do_ arithmetic in my head, only 
that base-60 is a little slow...

Ciao,
Dscho

Re: [PATCH] show_date(): rename the "relative" parameter to "mode"

[Linus Torvalds]

On Wed, 28 Feb 2007, Johannes Schindelin wrote:
> 
> If you use "next" Git:
> 
> 	$ git log --pretty=format:"%h %ad %an"

Well, that has the long-format date, which really isn't appropriate for a 
one-liner format. 

It's one of the reasons I agreed with your show_date() changes: I think 
our date output formats are fairly limited. I love the long date format, 
but if I want to show a commit name and author name and email on the same 
line, no way can we use 30+ characters for just the date.

So the part about the GNU ChangeLog format I like is the single-line 
header thing.. Right now, the choice between "--pretty=xxx" choices is 
"oneline", "short", "medium", and there's something lacking there in 
between ("terse").

The "format:" thing certainly gives us more options to play with, though.

			Linus

Re: [PATCH] show_date(): rename the "relative" parameter to "mode"

[Linus Torvalds]

On Tue, 27 Feb 2007, Junio C Hamano wrote:
> 
> How about --ago as a synonym, and --my-date for obeying TZ?
> "show in UTC time" can be had by something like:
> 
> 	$ TZ=UTC git log --my-date

Yes, although I would suggest that at that point we also use a shorter 
date format, since the timezone (by definition) is no longer valid or 
interesting.

But in general, there are other date issues - if we start showing the date 
in other non-rfc2822 formats (like skipping the timezone), we migth as 
well also support denser times..

For example, I occasionally love seeing the seconds, especially when I 
apply a series of patches from Andrew, and I see myself committing 5-6 
patches per second - but it's just noise if you want a single-line thing. 
So showing dates as "yyyy-mm-dd hh:mm" makes sense (and sorts properly).

So instead of "--my-date", I'd suggest "--date[-format]=xyz" as an option, 
possibly even with a way to set some defaults for "git log" using the 
config file. 

		Linus

OT: Favourite log message layout

[Andy Parkins]

On Tuesday 2007 February 27 23:02, Linus Torvalds wrote:

> I don't think the GNU style ChangeLog is particularly good. In fact, I
> think it has an unfortunate tendency (thanks to being per-file-log) to
> encourage people to commit unrelated changes and then explain them per
> file.

Me either; however, I do like per-file information.  My own favoured format 
for log messages is (with all but the shortlog optional)

---- 8< ----
Short log message summarises changes in one line

Longer message summarises changes, perhaps with reasons for change and why 
this change fixes something.

file1.c
 * detailed description of change to this file
 * more changes

file2.c
 * detailed description of change to this file
---- >8 ----

That's not to say that this is an excuse to lump unrelated changes together, 
but it does allow one to say

 file1.c
  * Added function xyz()

 file2.c
  * Call new function xyz() to achieve clever feature

when it's appropriate.  I've found this particularly useful in conjunction 
with git-blame.  You find the revision that made a change in a particular 
file then the log message is easily searched to find the exact reasons for 
that change on that line in that file.

Perhaps I'm overly verbose though.  I do like the sound of my own voice - even 
on log messages :-)


Andy
-- 
Dr Andy Parkins, M Eng (hons), MIET
andyparkins@gmail.com