Re: [PATCH] builtin-commit: add --date option

Jeff King [off-list ref] writes:

Do you really want to set the date to something arbitrary, or do you
just want to set it to "now"? If the latter case, do you really just
want the recently discussed --reset-author?

Also, is there a good reason why GIT_AUTHOR_DATE is not respected in
this case?  If not, should we simply be fixing that bug instead?

I expect I won't be on the list for the rest of the day (I've started the
preparation to tag -rc1), but I have two-and-half points before this
discussion goes too far:

 - The "--reset-author" patch will be in 1.6.6 (it is already in 'master'
   yesterday, I think);

 - We should honor GIT_AUTHOR_DATE if --reset-author is given.

 - I _think_ we should ignore GIT_AUTHOR_DATE if --reset-author is not
   given, as --amend/-c/-C is stronger for being command line options than
   an environment variable.

So I do not think --date is something we urgently need, even though it
might be nice to have it to be consistent with --author.

On Wed, Dec 02, 2009 at 11:38:05AM -0800, Junio C Hamano [off-list ref] wrote:

 - We should honor GIT_AUTHOR_DATE if --reset-author is given.

This is already a situation, as far as I see.

 - I _think_ we should ignore GIT_AUTHOR_DATE if --reset-author is not
   given, as --amend/-c/-C is stronger for being command line options than
   an environment variable.

We already ignore GIT_AUTHOR_DATE if --reset-author is not given, also
when I change it like this:

diff --git a/builtin-commit.c b/builtin-commit.c
index e93a647..7234c7d 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c

@@ -397,7 +397,8 @@ static void determine_author_info(void)

                name = xstrndup(a + 8, lb - (a + 8));
                email = xstrndup(lb + 2, rb - (lb + 2));
-               date = xstrndup(rb + 2, eol - (rb + 2));
+               if (!date)
+                       date = xstrndup(rb + 2, eol - (rb + 2));
        }

        if (force_author) {

tests 27, 33, 38 and 39 fail in t7501-commit.sh.

---
 Documentation/date-formats.txt    |   23 +++++++++++++++++++++++
 Documentation/git-commit-tree.txt |    1 +
 Documentation/git-commit.txt      |    2 ++
 3 files changed, 26 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/date-formats.txt

diff --git a/Documentation/date-formats.txt b/Documentation/date-formats.txt
new file mode 100644
index 0000000..626c887
--- /dev/null
+++ b/Documentation/date-formats.txt

@@ -0,0 +1,23 @@
+DATE FORMATS
+------------
+
+The GIT_AUTHOR_DATE, GIT_COMMITTER_DATE environment variables
+ifdef::git-commit[]
+and the `--date` option
+endif::git-commit[]
+support the following date formats:
+
+Git native format::
+	It is `<unix timestamp> <timezone offset>`, where `<unix
+	timestamp>` is the number of seconds since the UNIX epoch.
+	`<timezone offset>` is a positive or negative offset from UTC.
+	For example CET (which is 2 hours ahead UTC) is `+0200`.
+
+RFC 2822::
+	The standard email format as described by RFC 2822, for example
+	`Thu, 07 Apr 2005 22:13:13 +0200`.
+
+ISO 8601::
+	Time and date specified by the ISO 8601 standard, for example
+	`2005-04-07T22:13:13`. The parser accepts a space instead of the
+	`T` character as well.

diff --git a/Documentation/git-commit-tree.txt b/Documentation/git-commit-tree.txt
index b8834ba..4fec5d5 100644
--- a/Documentation/git-commit-tree.txt
+++ b/Documentation/git-commit-tree.txt

@@ -73,6 +73,7 @@ A commit comment is read from stdin. If a changelog
 entry is not provided via "<" redirection, 'git-commit-tree' will just wait
 for one to be entered and terminated with ^D.
 
+include::date-formats.txt[]
 
 Diagnostics
 -----------

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index cbbbeeb..17783b4 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt

@@ -220,6 +220,8 @@ specified.
 	these files are also staged for the next commit on top
 	of what have been staged before.
 
+:git-commit: 1
+include::date-formats.txt[]
 
 EXAMPLES
 --------

-- 
1.6.5.2

This is like --author: allow a user to specify a given date without
using the GIT_AUTHOR_DATE environment variable.

Signed-off-by: Miklos Vajna <redacted>
---

On Wed, Dec 02, 2009 at 11:38:05AM -0800, Junio C Hamano [off-list ref] wrote:

So I do not think --date is something we urgently need, even though it
might be nice to have it to be consistent with --author.

Changes from the previous version:

* updated the commit message

* rebased on top of current master that has --reset-author

 Documentation/git-commit.txt |    5 ++++-
 builtin-commit.c             |    6 +++++-
 t/t7501-commit.sh            |   15 +++++++++++++++
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index d227cec..cbbbeeb 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt

@@ -11,7 +11,7 @@ SYNOPSIS
 'git commit' [-a | --interactive] [-s] [-v] [-u<mode>] [--amend] [--dry-run]
 	   [(-c | -C) <commit>] [-F <file> | -m <msg>] [--reset-author]
 	   [--allow-empty] [--no-verify] [-e] [--author=<author>]
-	   [--cleanup=<mode>] [--] [[-i | -o ]<file>...]
+	   [--date=<date>] [--cleanup=<mode>] [--] [[-i | -o ]<file>...]
 
 DESCRIPTION
 -----------

@@ -85,6 +85,9 @@ OPTIONS
 	an existing commit that matches the given string and its author
 	name is used.
 
+--date=<date>::
+	Override the date used in the commit.
+
 -m <msg>::
 --message=<msg>::
 	Use the given <msg> as the commit message.

diff --git a/builtin-commit.c b/builtin-commit.c
index e93a647..9a1264a 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c

@@ -52,7 +52,7 @@ static char *edit_message, *use_message;
 static char *author_name, *author_email, *author_date;
 static int all, edit_flag, also, interactive, only, amend, signoff;
 static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
-static char *untracked_files_arg;
+static char *untracked_files_arg, *force_date;
 /*
  * The default commit message cleanup mode will remove the lines
  * beginning with # (shell comments) and leading and trailing

@@ -90,6 +90,7 @@ static struct option builtin_commit_options[] = {
 
 	OPT_FILENAME('F', "file", &logfile, "read log from file"),
 	OPT_STRING(0, "author", &force_author, "AUTHOR", "override author for commit"),
+	OPT_STRING(0, "date", &force_date, "DATE", "override date for commit"),
 	OPT_CALLBACK('m', "message", &message, "MESSAGE", "specify commit message", opt_parse_m),
 	OPT_STRING('c', "reedit-message", &edit_message, "COMMIT", "reuse and edit message from specified commit"),
 	OPT_STRING('C', "reuse-message", &use_message, "COMMIT", "reuse message from specified commit"),

@@ -410,6 +411,9 @@ static void determine_author_info(void)
 		email = xstrndup(lb + 2, rb - (lb + 2));
 	}
 
+	if (force_date)
+		date = force_date;
+
 	author_name = name;
 	author_email = email;
 	author_date = date;

diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index a603f6d..a529701 100755
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh

@@ -211,6 +211,21 @@ test_expect_success 'amend commit to fix author' '
 
 '
 
+test_expect_success 'amend commit to fix date' '
+
+	test_tick &&
+	newtick=$GIT_AUTHOR_DATE &&
+	git reset --hard &&
+	git cat-file -p HEAD |
+	sed -e "s/author.*/author $author $newtick/" \
+		-e "s/^\(committer.*> \).*$/\1$GIT_COMMITTER_DATE/" > \
+		expected &&
+	git commit --amend --date="$newtick" &&
+	git cat-file -p HEAD > current &&
+	test_cmp expected current
+
+'
+
 test_expect_success 'sign off (1)' '
 
 	echo 1 >positive &&

-- 
1.6.5.2

Quoting Miklos Vajna [off-list ref]

---

I don't think any message needs to be there, but a Signed-Off-By: 
line should be.

+The GIT_AUTHOR_DATE, GIT_COMMITTER_DATE environment variables
+ifdef::git-commit[]
+and the `--date` option
+endif::git-commit[]
+support the following date formats:
+
+Git native format::
+	It is `<unix timestamp> <timezone offset>`, where `<unix
+	timestamp>` is the number of seconds since the UNIX epoch.
+	`<timezone offset>` is a positive or negative offset from UTC.
+	For example CET (which is 2 hours ahead UTC) is `+0200`.

It is better to call it 'internal' format, instead of 'native'. 
I think 'native' means the most natural way to Git, and users 
view what 'git show' outputs as 'native'.

  nana.git% git show -s v1.6.5^0 | grep Date
  Date:   Sat Oct 10 00:05:19 2009 -0700
  nana.git% ./test-date parse 'Sat Oct 10 00:05:19 2009 -0700'
  Sat Oct 10 00:05:19 2009 -0700 -> 2009-10-10 07:05:19 +0000

And 'Git native' format is also supported, I think.

+RFC 2822::
+	The standard email format as described by RFC 2822, for example
+	`Thu, 07 Apr 2005 22:13:13 +0200`.
+
+ISO 8601::
+	Time and date specified by the ISO 8601 standard, for example
+	`2005-04-07T22:13:13`. The parser accepts a space instead of the
+	`T` character as well.

I didn't know about "T", but it works (^_^).

  nana.git% ./test-date parse '2005-04-07T22:13:13'
  2005-04-07T22:13:13 -> 2005-04-07 13:13:13 +0000

Is there something wrong with 'show'?

  nana.git% ./test-date show '2005-04-07T22:13:13'
  2005-04-07T22:13:13 -> 40 years ago

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

On Thu, Dec 03, 2009 at 07:33:13AM +0900, Nanako Shiraishi wrote:

Is there something wrong with 'show'?

  nana.git% ./test-date show '2005-04-07T22:13:13'
  2005-04-07T22:13:13 -> 40 years ago

No. It doesn't call parse_date, but instead accepts a time_t-like
"seconds since epoch". I intentionally didn't use parse_date because I
didn't want bugs in parse_date to affect 'show' output, to keep testing
simple.

-Peff