threads / patch / 6993

patchshow_date(): rename the "relative" parameter to "mode"

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

## tl;dr

18 messages between Feb 27, 2007 and Feb 28, 2007. Diffs are folded; open one to read it.

replies: 17people: 4as markdown or json

Johannes Schindelin· Feb 27, 2007, 15:21 UTC · lore

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(-)
Show changes to 2 files +14 −9

cache.h, date.c

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
Linus Torvalds· Feb 27, 2007, 21:41 UTC · re: Johannes Schindelin · lore

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

On Tue, 27 Feb 2007, Johannes Schindelin wrote:
Show 7 quoted lines
> 
> 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
Junio C Hamano· Feb 27, 2007, 21:51 UTC · re: Linus Torvalds · lore

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

Linus Torvalds <torvalds@linux-foundation.org> writes:
Show 11 quoted lines
> 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.
Linus Torvalds· Feb 27, 2007, 21:59 UTC · re: Linus Torvalds · lore

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

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
Johannes Schindelin· Feb 27, 2007, 22:24 UTC · re: Linus Torvalds · lore

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

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...

Linus Torvalds· Feb 27, 2007, 23:04 UTC · re: Johannes Schindelin · lore

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

On Tue, 27 Feb 2007, Johannes Schindelin wrote:
Show 10 quoted lines
> 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
Johannes Schindelin· Feb 27, 2007, 23:13 UTC · re: Linus Torvalds · lore

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

Hi,
On Tue, 27 Feb 2007, Linus Torvalds wrote:
Show 17 quoted lines
> 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

Junio C Hamano· Feb 27, 2007, 23:09 UTC · re: Johannes Schindelin · lore

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

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 6 quoted lines
>> 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.

Johannes Schindelin· Feb 27, 2007, 23:20 UTC · re: Junio C Hamano · lore

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

Hi,
On Tue, 27 Feb 2007, Junio C Hamano wrote:
Show 13 quoted lines
> 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

Junio C Hamano· Feb 27, 2007, 23:34 UTC · re: Johannes Schindelin · lore

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

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.

Johannes Schindelin· Feb 27, 2007, 23:52 UTC · re: Junio C Hamano · lore

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

Hi,
On Tue, 27 Feb 2007, Junio C Hamano wrote:
Show 8 quoted lines
> 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

Linus Torvalds· Feb 27, 2007, 23:59 UTC · re: Junio C Hamano · lore

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

On Tue, 27 Feb 2007, Junio C Hamano wrote:
Show 5 quoted lines
> 
> 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
Johannes Schindelin· Feb 27, 2007, 22:13 UTC · re: Linus Torvalds · lore

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

Hi,
On Tue, 27 Feb 2007, Linus Torvalds wrote:
Show 11 quoted lines
> 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

Linus Torvalds· Feb 27, 2007, 23:02 UTC · re: Johannes Schindelin · lore

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

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
Johannes Schindelin· Feb 27, 2007, 23:10 UTC · re: Linus Torvalds · lore

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

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

Linus Torvalds· Feb 27, 2007, 23:53 UTC · re: Johannes Schindelin · lore

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

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
Johannes Schindelin· Feb 27, 2007, 23:18 UTC · re: Linus Torvalds · lore

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

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.
Show 5 quoted lines
> 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

Andy Parkins· Feb 28, 2007, 09:48 UTC · re: Linus Torvalds · lore

OT: Favourite log message layout

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

← back to recent threads