[PATCH] Add a --dateformat= option to git-for-each-ref

I wanted to get date information in RFC2822 format out of a tag using
git-for-each-ref; but there was no way to specify that.  This patch
addresses that omission by adding a --dateformat option.

For example (I'm in BST, +0100 at present):

 $ git-for-each-ref --dateformat=normal --format='%(taggerdate)' refs/tags/v1.5.2
 Sun May 20 00:30:42 2007 -0700
 $ git-for-each-ref --dateformat=relative --format='%(taggerdate)' refs/tags/v1.5.2
 4 months ago
 $ git-for-each-ref --dateformat=short --format='%(taggerdate)' refs/tags/v1.5.2
 2007-05-20
 $ git-for-each-ref --dateformat=local --format='%(taggerdate)' refs/tags/v1.5.2
 Sun May 20 08:30:42 2007
 $ git-for-each-ref --dateformat=iso8601 --format='%(taggerdate)' refs/tags/v1.5.2
 2007-05-20 00:30:42 -0700
 $ git-for-each-ref --dateformat=rfc2822 --format='%(taggerdate)' refs/tags/v1.5.2
 Sun, 20 May 2007 00:30:42 -0700

The default is to use 'normal', which leaves existing behaviour
unchanged.

Signed-off-by: Andy Parkins <redacted>
---
 Documentation/git-for-each-ref.txt |    6 ++++++
 builtin-for-each-ref.c             |   18 +++++++++++++++++-
 2 files changed, 23 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 6df8e85..1b8fdb8 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt

@@ -10,6 +10,7 @@ SYNOPSIS
 [verse]
 'git-for-each-ref' [--count=<count>]\*
                    [--shell|--perl|--python|--tcl]
+                   [--dateformat=normal|relative|short|local|iso8601|rfc2822]
                    [--sort=<key>]\* [--format=<format>] [<pattern>]
 
 DESCRIPTION

@@ -58,6 +59,11 @@ OPTIONS
 	the specified host language.  This is meant to produce
 	a scriptlet that can directly be `eval`ed.
 
+--dateformat::
+	If given, all timestamp fields will be output in the specified
+	format.  This is only really relevant for innvocations using the
+	--format option with a `%(date)`-type field.
+
 
 FIELD NAMES
 -----------

diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
index 0afa1c5..80e58fc 100644
--- a/builtin-for-each-ref.c
+++ b/builtin-for-each-ref.c

@@ -80,6 +80,7 @@ static struct {
 static const char **used_atom;
 static cmp_type *used_atom_type;
 static int used_atom_cnt, sort_atom_limit, need_tagged;
+static enum date_mode date_mode = DATE_NORMAL;
 
 /*
  * Used to parse format string and sort specifiers

@@ -362,7 +363,7 @@ static void grab_date(const char *buf, struct atom_value *v)
 	tz = strtol(zone, NULL, 10);
 	if ((tz == LONG_MIN || tz == LONG_MAX) && errno == ERANGE)
 		goto bad;
-	v->s = xstrdup(show_date(timestamp, tz, 0));
+	v->s = xstrdup(show_date(timestamp, tz, date_mode));
 	v->ul = timestamp;
 	return;
  bad:

@@ -870,6 +871,21 @@ int cmd_for_each_ref(int ac, const char **av, const char *prefix)
 			sort->atom = parse_atom(arg, arg+len);
 			continue;
 		}
+		if (!prefixcmp(arg, "--dateformat=")) {
+			arg += 13;
+			if (!prefixcmp(arg,"relative")) {
+				date_mode = DATE_RELATIVE;
+			} else if (!prefixcmp(arg,"short")) {
+				date_mode = DATE_SHORT;
+			} else if (!prefixcmp(arg,"local")) {
+				date_mode = DATE_LOCAL;
+			} else if (!prefixcmp(arg,"iso8601")) {
+				date_mode = DATE_ISO8601;
+			} else if (!prefixcmp(arg,"rfc2822")) {
+				date_mode = DATE_RFC2822;
+			}
+			continue;
+		}
 		break;
 	}
 	if (quote_style < 0)

-- 
1.5.3.1.5.g4e560-dirty

On Wed, Sep 26, 2007 at 10:09:18AM +0100, Andy Parkins wrote:

For example (I'm in BST, +0100 at present):

 $ git-for-each-ref --dateformat=normal --format='%(taggerdate)' refs/tags/v1.5.2
 Sun May 20 00:30:42 2007 -0700
 $ git-for-each-ref --dateformat=relative --format='%(taggerdate)' refs/tags/v1.5.2
 4 months ago

What if you want a format that contains two dates in different formats?
Something like:

 $ git-for-each-ref --format='%(committerdate:relative) %(authordate:normal)'

would be more flexible. Although perhaps that is a bit too unlikely to
be concerned with implementing, giving options to substitutions seems
like a sane way to implement these sorts of things (e.g.,
"%(objectsize:human)", "%(parent:1)", etc).

+		if (!prefixcmp(arg, "--dateformat=")) {
+			arg += 13;
+			if (!prefixcmp(arg,"relative")) {
+				date_mode = DATE_RELATIVE;
+			} else if (!prefixcmp(arg,"short")) {
+				date_mode = DATE_SHORT;
+			} else if (!prefixcmp(arg,"local")) {
+				date_mode = DATE_LOCAL;
+			} else if (!prefixcmp(arg,"iso8601")) {
+				date_mode = DATE_ISO8601;
+			} else if (!prefixcmp(arg,"rfc2822")) {
+				date_mode = DATE_RFC2822;
+			}
+			continue;
+		}

Surely this same code exists elsewhere, and could be easily factored out
into a parse_date_type function.

-Peff

On Wednesday 2007 September 26, Jeff King wrote:

would be more flexible. Although perhaps that is a bit too unlikely to
be concerned with implementing, giving options to substitutions seems
like a sane way to implement these sorts of things (e.g.,
"%(objectsize:human)", "%(parent:1)", etc).

I'd thought about doing it like that, but imagined that there would objections 
that it was overcomplicating git-for-each-ref.  As you think that's 
acceptable, I'll do it.

Surely this same code exists elsewhere, and could be easily factored out
into a parse_date_type function.

It was.  It was also in revisions.c.

A patch series that implements both your requested changes to follow.


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

In anticipation of supplying a per-field date format specifier, this
patch makes parse_atom() in builtin-for-each-ref.c allow atoms that have
a valid atom name (as determined by the valid_atom[] table) followed by
a colon, followed by an arbitrary string.

The arbitrary string is where the format for the atom will be specified.

Note, if different formats are specified for the same atom, multiple
entries will be made in the used_atoms table to allow them to be
distinguished by the grab_XXXX() functions.

Signed-off-by: Andy Parkins <redacted>
---
 builtin-for-each-ref.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
index 0afa1c5..3280516 100644
--- a/builtin-for-each-ref.c
+++ b/builtin-for-each-ref.c

@@ -106,7 +106,13 @@ static int parse_atom(const char *atom, const char *ep)
 	/* Is the atom a valid one? */
 	for (i = 0; i < ARRAY_SIZE(valid_atom); i++) {
 		int len = strlen(valid_atom[i].name);
-		if (len == ep - sp && !memcmp(valid_atom[i].name, sp, len))
+		/* If the atom name has a colon, strip it and everything after
+		 * it off - it specifies the format for this entry, and
+		 * shouldn't be used for checking against the valid_atom table */
+		const char *formatp = strrchr(sp, ':' );
+		if (formatp == NULL )
+			formatp = ep;
+		if (len == formatp - sp && !memcmp(valid_atom[i].name, sp, len))
 			break;
 	}
 

-- 
1.5.3.2.105.gf47f2-dirty

The --date parameter was previously handled in revisions.c with a list
of if(strcmp()) calls; now parse_date_format() is called instead.

Signed-off-by: Andy Parkins <redacted>
---
 revision.c |   17 +----------------
 1 files changed, 1 insertions(+), 16 deletions(-)

diff --git a/revision.c b/revision.c
index 33d092c..75cd0c6 100644
--- a/revision.c
+++ b/revision.c

@@ -1134,22 +1134,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 				continue;
 			}
 			if (!strncmp(arg, "--date=", 7)) {
-				if (!strcmp(arg + 7, "relative"))
-					revs->date_mode = DATE_RELATIVE;
-				else if (!strcmp(arg + 7, "iso8601") ||
-					 !strcmp(arg + 7, "iso"))
-					revs->date_mode = DATE_ISO8601;
-				else if (!strcmp(arg + 7, "rfc2822") ||
-					 !strcmp(arg + 7, "rfc"))
-					revs->date_mode = DATE_RFC2822;
-				else if (!strcmp(arg + 7, "short"))
-					revs->date_mode = DATE_SHORT;
-				else if (!strcmp(arg + 7, "local"))
-					revs->date_mode = DATE_LOCAL;
-				else if (!strcmp(arg + 7, "default"))
-					revs->date_mode = DATE_NORMAL;
-				else
-					die("unknown date format %s", arg);
+				revs->date_mode = parse_date_format(arg + 7);
 				continue;
 			}
 			if (!strcmp(arg, "--log-size")) {

-- 
1.5.3.2.105.gf47f2-dirty

parse_date_format() is passed a string that is compared against a
pre-defined list and converted to an enum date_format.  The table is as
follows:

 - "relative"         => DATE_RELATIVE
 - "iso8601" or "iso" => DATE_ISO8601
 - "rfc2822"          => DATE_RFC2822
 - "short"            => DATE_SHORT
 - "local"            => DATE_LOCAL
 - "default"          => DATE_NORMAL

In the event that none of these strings is found, the function die()s.

Signed-off-by: Andy Parkins <redacted>
---
 cache.h |    1 +
 date.c  |   20 ++++++++++++++++++++
 2 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/cache.h b/cache.h
index 8246500..5587f7e 100644
--- a/cache.h
+++ b/cache.h

@@ -432,6 +432,7 @@ const char *show_date(unsigned long time, int timezone, enum date_mode mode);
 int parse_date(const char *date, char *buf, int bufsize);
 void datestamp(char *buf, int bufsize);
 unsigned long approxidate(const char *);
+enum date_mode parse_date_format(const char *format);
 
 extern const char *git_author_info(int);
 extern const char *git_committer_info(int);

diff --git a/date.c b/date.c
index 93bef6e..8f70500 100644
--- a/date.c
+++ b/date.c

@@ -584,6 +584,26 @@ int parse_date(const char *date, char *result, int maxlen)
 	return date_string(then, offset, result, maxlen);
 }
 
+enum date_mode parse_date_format(const char *format)
+{
+	if (!strcmp(format, "relative"))
+		return DATE_RELATIVE;
+	else if (!strcmp(format, "iso8601") ||
+		 !strcmp(format, "iso"))
+		return DATE_ISO8601;
+	else if (!strcmp(format, "rfc2822") ||
+		 !strcmp(format, "rfc"))
+		return DATE_RFC2822;
+	else if (!strcmp(format, "short"))
+		return DATE_SHORT;
+	else if (!strcmp(format, "local"))
+		return DATE_LOCAL;
+	else if (!strcmp(format, "default"))
+		return DATE_NORMAL;
+	else
+		die("unknown date format %s", format);
+}
+
 void datestamp(char *buf, int bufsize)
 {
 	time_t now;

-- 
1.5.3.2.105.gf47f2-dirty

grab_date() gets an extra parameter - atomname; this extra parameter is
checked to see if it has a ":<format>" extra component in it, and if so
that "<format>" string is passed to parse_date_format() to produce an
enum date_mode value which is then further passed to show_date().

In short it allows the user of git-for-each-ref to do things like this:

 $ git-for-each-ref --format='%(taggerdate:default)' refs/tags/v1.5.2
 Sun May 20 00:30:42 2007 -0700
 $ git-for-each-ref --format='%(taggerdate:relative)' refs/tags/v1.5.2
 4 months ago
 $ git-for-each-ref --format='%(taggerdate:short)' refs/tags/v1.5.2
 2007-05-20
 $ git-for-each-ref --format='%(taggerdate:local)' refs/tags/v1.5.2
 Sun May 20 08:30:42 2007
 $ git-for-each-ref --format='%(taggerdate:iso8601)' refs/tags/v1.5.2
 2007-05-20 00:30:42 -0700
 $ git-for-each-ref --format='%(taggerdate:rfc2822)' refs/tags/v1.5.2
 Sun, 20 May 2007 00:30:42 -0700

The default, when no ":<format>" is specified is ":default", leaving the
existing behaviour unchanged.

Signed-off-by: Andy Parkins <redacted>
---
 Documentation/git-for-each-ref.txt |    5 +++++
 builtin-for-each-ref.c             |   26 +++++++++++++++++++-------
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 6df8e85..f1f90cc 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt

@@ -100,6 +100,11 @@ In any case, a field name that refers to a field inapplicable to
 the object referred by the ref does not cause an error.  It
 returns an empty string instead.
 
+As a special case for the date-type fields, you may specify a format for
+the date by adding one of `:default`, `:relative`, `:short`, `:local`,
+`:iso8601` or `:rfc2822` to the end of the fieldname; e.g.
+`%(taggerdate:relative)`.
+
 
 EXAMPLES
 --------

diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
index 3280516..2ca4fc6 100644
--- a/builtin-for-each-ref.c
+++ b/builtin-for-each-ref.c

@@ -353,12 +353,24 @@ static const char *copy_email(const char *buf)
 	return line;
 }
 
-static void grab_date(const char *buf, struct atom_value *v)
+static void grab_date(const char *buf, struct atom_value *v, const char *atomname)
 {
 	const char *eoemail = strstr(buf, "> ");
 	char *zone;
 	unsigned long timestamp;
 	long tz;
+	enum date_mode date_mode = DATE_NORMAL;
+	const char *formatp;
+
+	/* We got here because atomname ends in "date" or "date<something>",
+	 * it's not possible that <something> is not ":<format>" because
+	 * parse_atom() wouldn't have allowed it, so we can assume that no
+	 * ":" means no format is specified, use the default */
+	formatp = strrchr( atomname, ':' );
+	if (formatp != NULL) {
+		formatp++;
+		date_mode = parse_date_format(formatp);
+	}
 
 	if (!eoemail)
 		goto bad;

@@ -368,7 +380,7 @@ static void grab_date(const char *buf, struct atom_value *v)
 	tz = strtol(zone, NULL, 10);
 	if ((tz == LONG_MIN || tz == LONG_MAX) && errno == ERANGE)
 		goto bad;
-	v->s = xstrdup(show_date(timestamp, tz, 0));
+	v->s = xstrdup(show_date(timestamp, tz, date_mode));
 	v->ul = timestamp;
 	return;
  bad:

@@ -395,7 +407,7 @@ static void grab_person(const char *who, struct atom_value *val, int deref, stru
 		if (name[wholen] != 0 &&
 		    strcmp(name + wholen, "name") &&
 		    strcmp(name + wholen, "email") &&
-		    strcmp(name + wholen, "date"))
+		    prefixcmp(name + wholen, "date"))
 			continue;
 		if (!wholine)
 			wholine = find_wholine(who, wholen, buf, sz);

@@ -407,8 +419,8 @@ static void grab_person(const char *who, struct atom_value *val, int deref, stru
 			v->s = copy_name(wholine);
 		else if (!strcmp(name + wholen, "email"))
 			v->s = copy_email(wholine);
-		else if (!strcmp(name + wholen, "date"))
-			grab_date(wholine, v);
+		else if (!prefixcmp(name + wholen, "date"))
+			grab_date(wholine, v, name);
 	}
 
 	/* For a tag or a commit object, if "creator" or "creatordate" is

@@ -428,8 +440,8 @@ static void grab_person(const char *who, struct atom_value *val, int deref, stru
 		if (deref)
 			name++;
 
-		if (!strcmp(name, "creatordate"))
-			grab_date(wholine, v);
+		if (!prefixcmp(name, "creatordate"))
+			grab_date(wholine, v, name);
 		else if (!strcmp(name, "creator"))
 			v->s = copy_line(wholine);
 	}

-- 
1.5.3.2.105.gf47f2-dirty

Hi,

On Fri, 28 Sep 2007, Andy Parkins wrote:

The --date parameter was previously handled in revisions.c with a list
of if(strcmp()) calls; now parse_date_format() is called instead.

Since this is really more like a code move, 1/4 and 2/4 should be 
squashed.

Ciao,
Dscho

On Friday 2007, September 28, Johannes Schindelin wrote:

Since this is really more like a code move, 1/4 and 2/4 should be
squashed.

I have no problem with that.

Junio: would you like a resend?



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

On Fri, Sep 28, 2007 at 03:15:58PM +0100, Andy Parkins wrote:

quoted

like a sane way to implement these sorts of things (e.g.,
"%(objectsize:human)", "%(parent:1)", etc).

I'd thought about doing it like that, but imagined that there would
objections that it was overcomplicating git-for-each-ref.  As you
think that's acceptable, I'll do it.

Well, I'm not sure my opinion counts for much, but at least there are
now two of us. :)

A patch series that implements both your requested changes to follow.

Patches 1/2 look fine to me (but I agree with the squash suggestion).

3/4 are not exactly what I had in mind, but I think are reasonable in
this case. Rather than treating it was ":format", I had imagined more of
a ":attribute1:attribute2" style, where some attributes may be
understood by all substitutions (e.g., the moral equivalent of shell's
":-" and ":+"), and some only by some substitutions (such as date
formats). And on top of that, these sorts of substitutions should be
unified with the --pretty=format machinery.

Of course, that is a much larger task and you probably just want to do
your date formatting and get your other work done. So I think your
implementation is reasonable, in that it accomplishes what you want in a
reasonable amount of code, and its syntax doesn't prevent moving towards
what I described above (since %(foo:bar:baz) is currently nonsensical,
we would be free to adapt its meaning later).

So in a very verbose way,

Acked-by: Jeff King <redacted>

-Peff