[RFC 1/5] Light refactoring of date infrastructure

[RFC 1/5] Light refactoring of date infrastructure

[Michael Witten]

Date: Fri, 11 Feb 2011 18:34:29 +0000
date_mode_explicit -> date_mode_from_command_line
    It's more understandable by itself now.

DATE_NORMAL -> DATE_DEFAULT
    The user input to select this value is `default'.

Signed-off-by: Michael Witten <mfwitten@gmail.com>
---
 archive.c              |    2 +-
 builtin/blame.c        |    3 +--
 builtin/commit.c       |    2 +-
 builtin/for-each-ref.c |    2 +-
 builtin/shortlog.c     |    2 +-
 cache.h                |    2 +-
 date.c                 |    2 +-
 log-tree.c             |    6 +++---
 revision.c             |    4 ++--
 revision.h             |    2 +-
 submodule.c            |    2 +-
 11 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/archive.c b/archive.c
index 42f2d2f..dd871ca 100644
--- a/archive.c
+++ b/archive.c
@@ -32,7 +32,7 @@ static void format_subst(const struct commit *commit,
 	char *to_free = NULL;
 	struct strbuf fmt = STRBUF_INIT;
 	struct pretty_print_context ctx = {0};
-	ctx.date_mode = DATE_NORMAL;
+	ctx.date_mode = DATE_DEFAULT;
 	ctx.abbrev = DEFAULT_ABBREV;
 
 	if (src == buf->buf)
diff --git a/builtin/blame.c b/builtin/blame.c
index 4639788..dd597f4 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2377,9 +2377,8 @@ parse_done:
 		blame_date_width = sizeof("2006-10-19");
 		break;
 	case DATE_RELATIVE:
-		/* "normal" is used as the fallback for "relative" */
 	case DATE_LOCAL:
-	case DATE_NORMAL:
+	case DATE_DEFAULT:
 		blame_date_width = sizeof("Thu Oct 19 16:00:04 2006 -0700");
 		break;
 	}
diff --git a/builtin/commit.c b/builtin/commit.c
index 67757e9..194db99 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -933,7 +933,7 @@ static const char *find_author_by_nickname(const char *name)
 	commit = get_revision(&revs);
 	if (commit) {
 		struct pretty_print_context ctx = {0};
-		ctx.date_mode = DATE_NORMAL;
+		ctx.date_mode = DATE_DEFAULT;
 		strbuf_release(&buf);
 		format_commit_message(commit, "%an <%ae>", &buf, &ctx);
 		return strbuf_detach(&buf, NULL);
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 89e75c6..60d6b32 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -367,7 +367,7 @@ static void grab_date(const char *buf, struct atom_value *v, const char *atomnam
 	char *zone;
 	unsigned long timestamp;
 	long tz;
-	enum date_mode date_mode = DATE_NORMAL;
+	enum date_mode date_mode = DATE_DEFAULT;
 	const char *formatp;
 
 	/*
diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index f5efc67..5815f55 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -165,7 +165,7 @@ void shortlog_add_commit(struct shortlog *log, struct commit *commit)
 		ctx.abbrev = log->abbrev;
 		ctx.subject = "";
 		ctx.after_subject = "";
-		ctx.date_mode = DATE_NORMAL;
+		ctx.date_mode = DATE_DEFAULT;
 		pretty_print_commit(CMIT_FMT_USERFORMAT, commit, &ufbuf, &ctx);
 		buffer = ufbuf.buf;
 	} else if (*buffer) {
diff --git a/cache.h b/cache.h
index 5b896d9..bf639f7 100644
--- a/cache.h
+++ b/cache.h
@@ -829,7 +829,7 @@ extern struct object *peel_to_type(const char *name, int namelen,
 				   struct object *o, enum object_type);
 
 enum date_mode {
-	DATE_NORMAL = 0,
+	DATE_DEFAULT = 0,
 	DATE_RELATIVE,
 	DATE_SHORT,
 	DATE_LOCAL,
diff --git a/date.c b/date.c
index 00f9eb5..096468f 100644
--- a/date.c
+++ b/date.c
@@ -669,7 +669,7 @@ enum date_mode parse_date_format(const char *format)
 	else if (!strcmp(format, "local"))
 		return DATE_LOCAL;
 	else if (!strcmp(format, "default"))
-		return DATE_NORMAL;
+		return DATE_DEFAULT;
 	else if (!strcmp(format, "raw"))
 		return DATE_RAW;
 	else
diff --git a/log-tree.c b/log-tree.c
index 2a1e3a9..b059446 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -272,7 +272,7 @@ void get_patch_filename(struct commit *commit, int nr, const char *suffix,
 	if (commit) {
 		int max_len = start_len + FORMAT_PATCH_NAME_MAX - suffix_len;
 		struct pretty_print_context ctx = {0};
-		ctx.date_mode = DATE_NORMAL;
+		ctx.date_mode = DATE_DEFAULT;
 
 		format_commit_message(commit, "%f", buf, &ctx);
 		if (max_len < buf->len)
@@ -465,9 +465,9 @@ void show_log(struct rev_info *opt)
 			 */
 			show_reflog_message(opt->reflog_info,
 				    opt->commit_format == CMIT_FMT_ONELINE,
-				    opt->date_mode_explicit ?
+				    opt->date_mode_from_command_line ?
 					opt->date_mode :
-					DATE_NORMAL);
+					DATE_DEFAULT);
 			if (opt->commit_format == CMIT_FMT_ONELINE)
 				return;
 		}
diff --git a/revision.c b/revision.c
index 541f09e..462c311 100644
--- a/revision.c
+++ b/revision.c
@@ -1434,10 +1434,10 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		revs->simplify_history = 0;
 	} else if (!strcmp(arg, "--relative-date")) {
 		revs->date_mode = DATE_RELATIVE;
-		revs->date_mode_explicit = 1;
+		revs->date_mode_from_command_line = 1;
 	} else if ((argcount = parse_long_opt("date", argv, &optarg))) {
 		revs->date_mode = parse_date_format(optarg);
-		revs->date_mode_explicit = 1;
+		revs->date_mode_from_command_line = 1;
 		return argcount;
 	} else if (!strcmp(arg, "--log-size")) {
 		revs->show_log_size = 1;
diff --git a/revision.h b/revision.h
index 9fd8f30..e5ca939 100644
--- a/revision.h
+++ b/revision.h
@@ -92,7 +92,7 @@ struct rev_info {
 			abbrev_commit:1,
 			use_terminator:1,
 			missing_newline:1,
-			date_mode_explicit:1;
+			date_mode_from_command_line:1;
 	unsigned int	disable_stdin:1;
 
 	enum date_mode date_mode;
diff --git a/submodule.c b/submodule.c
index 5294cef..0a45da9 100644
--- a/submodule.c
+++ b/submodule.c
@@ -605,7 +605,7 @@ static void print_commit(struct commit *commit)
 {
 	struct strbuf sb = STRBUF_INIT;
 	struct pretty_print_context ctx = {0};
-	ctx.date_mode = DATE_NORMAL;
+	ctx.date_mode = DATE_DEFAULT;
 	format_commit_message(commit, " %h: %m %s", &sb, &ctx);
 	fprintf(stderr, "%s\n", sb.buf);
 	strbuf_release(&sb);
-- 
1.7.4.18.g68fe8

[RFC 2/5] Pretty Print: show tz when using DATE_LOCAL

[Michael Witten]

Date: Fri, 11 Feb 2011 16:06:36 +0000
Currently, when the date mode is DATE_LOCAL, the
time zone is never pretty printed; this seems
to be an unnecessary pecularliarity, especially
when the time zone data could be useful to
the user.

This commit removes that special handling of
time zones.

Now, for instance, fast-import.c's write_crash_report()
will produce reports that provide more meaningful
dates.

Signed-off-by: Michael Witten <mfwitten@gmail.com>
---
 date.c                  |    3 +--
 t/t6300-for-each-ref.sh |    4 ++--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/date.c b/date.c
index 096468f..caa14fe 100644
--- a/date.c
+++ b/date.c
@@ -186,13 +186,12 @@ const char *show_date(unsigned long time, int tz, enum date_mode mode)
 			month_names[tm->tm_mon], tm->tm_year + 1900,
 			tm->tm_hour, tm->tm_min, tm->tm_sec, tz);
 	else
-		sprintf(timebuf, "%.3s %.3s %d %02d:%02d:%02d %d%c%+05d",
+		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,
-				(mode == DATE_LOCAL) ? 0 : ' ',
 				tz);
 	return timebuf;
 }
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 7dc8a51..050ed7d 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -175,8 +175,8 @@ test_expect_success 'Check format "short" date fields output' '
 '
 
 cat >expected <<\EOF
-'refs/heads/master' 'Mon Jul 3 15:18:43 2006' 'Mon Jul 3 15:18:44 2006'
-'refs/tags/testtag' 'Mon Jul 3 15:18:45 2006'
+'refs/heads/master' 'Mon Jul 3 15:18:43 2006 +0000' 'Mon Jul 3 15:18:44 2006 +0000'
+'refs/tags/testtag' 'Mon Jul 3 15:18:45 2006 +0000'
 EOF
 
 test_expect_success 'Check format "local" date fields output' '
-- 
1.7.4.18.g68fe8

[RFC 3/5] Date Mode: Implementation

[Michael Witten]

Date: Sat, 12 Feb 2011 03:56:17 +0000
This introduces and employs infrastructure for manipulating
and bundling time zone and date format specifications in
code, on the command line, and in git configuration files.

In particular, this commit includes the following:

  * `struct date_mode' along with functions and macros for
     working with it.

  *  The new `--time-zone' option (which takes as a value
     either `default' or `local').

  *  The new date-type mode specifiers `@default' and `@local'
     for use with the formats of "git for-each-ref".

  *  The configuration variables `log.timezone' and
    `blame.timezone' and the functions for handling their
     parsing.

Moreover, the value `local' for a date mode format has
been deprecated in favor of the value `local' for a
date mode time zone; a use of the deprecated `local'
format causes an informational warning to be emitted
on stderr (though this message might be missed quite
easily due to further normal output on stdout).

Essentially, what was:

  git log --date=local

should now be:

  git log --time-zone=local

or, perhaps more accurately (depending on what the
actual unspecified date mode format is):

  git log --date=default --time-zone=local

The other valid time zone value is `default', which
specifies that the time zone stored by git be used.

Signed-off-by: Michael Witten <mfwitten@gmail.com>
---
 archive.c              |    2 +-
 builtin/blame.c        |   23 +++++++-------
 builtin/cat-file.c     |    3 +-
 builtin/commit.c       |    2 +-
 builtin/for-each-ref.c |   65 +++++++++++++++++++++++++++++----------
 builtin/log.c          |   17 ++++++----
 builtin/shortlog.c     |    2 +-
 builtin/show-branch.c  |    3 +-
 cache.h                |   39 +++++++++++++++++++++--
 commit.h               |    4 +-
 date.c                 |   79 +++++++++++++++++++++++++++++++++++++++++++-----
 fast-import.c          |    4 ++-
 http-backend.c         |    3 +-
 log-tree.c             |   10 ++++--
 pretty.c               |   18 +++++++----
 reflog-walk.c          |    6 ++--
 reflog-walk.h          |    4 +-
 refs.c                 |    6 ++-
 revision.c             |   11 ++++--
 revision.h             |    4 +-
 sha1_name.c            |    5 ++-
 submodule.c            |    2 +-
 test-date.c            |   11 ++++--
 23 files changed, 236 insertions(+), 87 deletions(-)

diff --git a/archive.c b/archive.c
index dd871ca..cb3e731 100644
--- a/archive.c
+++ b/archive.c
@@ -32,7 +32,7 @@ static void format_subst(const struct commit *commit,
 	char *to_free = NULL;
 	struct strbuf fmt = STRBUF_INIT;
 	struct pretty_print_context ctx = {0};
-	ctx.date_mode = DATE_DEFAULT;
+	SET_DATE_MODE_DEFAULT(ctx.date_mode);
 	ctx.abbrev = DEFAULT_ABBREV;
 
 	if (src == buf->buf)
diff --git a/builtin/blame.c b/builtin/blame.c
index dd597f4..c28d870 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -43,8 +43,8 @@ static int incremental;
 static int xdl_opts;
 static int abbrev = -1;
 
-static enum date_mode blame_date_mode = DATE_ISO8601;
 static size_t blame_date_width;
+static struct date_mode blame_date_mode = INIT_DATE_MODE_FORMAT(DATE_ISO8601);
 
 static struct string_list mailmap;
 
@@ -2023,6 +2023,8 @@ static void prepare_blame_range(struct scoreboard *sb,
 
 static int git_blame_config(const char *var, const char *value, void *cb)
 {
+	int ret;
+
 	if (!strcmp(var, "blame.showroot")) {
 		show_root = git_config_bool(var, value);
 		return 0;
@@ -2031,12 +2033,9 @@ static int git_blame_config(const char *var, const char *value, void *cb)
 		blank_boundary = git_config_bool(var, value);
 		return 0;
 	}
-	if (!strcmp(var, "blame.date")) {
-		if (!value)
-			return config_error_nonbool(var);
-		blame_date_mode = parse_date_format(value);
-		return 0;
-	}
+
+	if (parse_date_mode_config("blame", var, value, &blame_date_mode, &ret))
+		return ret;
 
 	switch (userdiff_config(var, value)) {
 	case 0:
@@ -2347,6 +2346,8 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 parse_done:
 	argc = parse_options_end(&ctx);
 
+	sanitize_date_mode("blame", &revs.date_mode);
+
 	if (abbrev == -1)
 		abbrev = default_abbrev;
 	/* one more abbrev length is needed for the boundary commit */
@@ -2357,13 +2358,13 @@ parse_done:
 
 	if (cmd_is_annotate) {
 		output_option |= OUTPUT_ANNOTATE_COMPAT;
-		blame_date_mode = DATE_ISO8601;
+		blame_date_mode.format = DATE_ISO8601;
 	} else {
 		blame_date_mode = revs.date_mode;
 	}
 
 	/* The maximum width used to show the dates */
-	switch (blame_date_mode) {
+	switch (blame_date_mode.format) {
 	case DATE_RFC2822:
 		blame_date_width = sizeof("Thu, 19 Oct 2006 16:00:04 -0700");
 		break;
@@ -2376,9 +2377,7 @@ parse_done:
 	case DATE_SHORT:
 		blame_date_width = sizeof("2006-10-19");
 		break;
-	case DATE_RELATIVE:
-	case DATE_LOCAL:
-	case DATE_DEFAULT:
+	default:
 		blame_date_width = sizeof("Thu Oct 19 16:00:04 2006 -0700");
 		break;
 	}
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 94632db..2252c83 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -46,6 +46,7 @@ static void pprint_tag(const unsigned char *sha1, const char *buf, unsigned long
 					char *ep;
 					unsigned long date;
 					long tz;
+					struct date_mode d = INIT_DATE_MODE;
 					while (sp < cp && *sp != '>')
 						sp++;
 					if (sp == cp) {
@@ -60,7 +61,7 @@ static void pprint_tag(const unsigned char *sha1, const char *buf, unsigned long
 					write_or_die(1, tagger, sp - tagger);
 					date = strtoul(sp, &ep, 10);
 					tz = strtol(ep, NULL, 10);
-					sp = show_date(date, tz, 0);
+					sp = show_date(date, tz, d);
 					write_or_die(1, sp, strlen(sp));
 					xwrite(1, "\n", 1);
 					break;
diff --git a/builtin/commit.c b/builtin/commit.c
index 194db99..79343c4 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -933,7 +933,7 @@ static const char *find_author_by_nickname(const char *name)
 	commit = get_revision(&revs);
 	if (commit) {
 		struct pretty_print_context ctx = {0};
-		ctx.date_mode = DATE_DEFAULT;
+		SET_DATE_MODE_DEFAULT(ctx.date_mode);
 		strbuf_release(&buf);
 		format_commit_message(commit, "%an <%ae>", &buf, &ctx);
 		return strbuf_detach(&buf, NULL);
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 60d6b32..add9299 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -118,9 +118,8 @@ static int parse_atom(const char *atom, const char *ep)
 		 * shouldn't be used for checking against the valid_atom
 		 * table.
 		 */
-		const char *formatp = strchr(sp, ':');
-		if (!formatp || ep < formatp)
-			formatp = ep;
+		const char *formatp = sp;
+		for (; formatp < ep && *formatp != ':' && *formatp != '@'; ++formatp);
 		if (len == formatp - sp && !memcmp(valid_atom[i].name, sp, len))
 			break;
 	}
@@ -361,26 +360,58 @@ static const char *copy_email(const char *buf)
 	return xmemdupz(email, eoemail + 1 - email);
 }
 
+#define PARSE_DATE_MODE(marker_0, parser_0, date_mode_member_0, \
+                        marker_1, parser_1, date_mode_member_1) \
+	if (*atomname == marker_0) { \
+		const char *spec_0 = ++atomname; \
+		do { \
+			if (*atomname == marker_1) { \
+				date_mode_member_1 = parser_1(atomname+1); \
+				char *spec_0_dup  = xmemdupz(spec_0, atomname - spec_0); \
+				date_mode_member_0 = parser_0(spec_0_dup); \
+				free(spec_0_dup); \
+				goto finish; \
+			} \
+		} while (*++atomname); \
+		date_mode_member_0 = parser_0(spec_0); \
+		goto finish; \
+	}
+
+static void parse_date_mode(const char *atomname, struct date_mode *date_mode)
+{
+	static int need_to_warn = 1;
+
+	do {
+		/* :date_format@time_zone */
+		PARSE_DATE_MODE(':', parse_date_format, date_mode->format,
+		                '@', parse_time_zone,   date_mode->zone);
+
+		/* @time_zone:date_format */
+		PARSE_DATE_MODE('@', parse_time_zone,   date_mode->zone,
+		                ':', parse_date_format, date_mode->format);
+
+	} while (*++atomname);
+
+finish:
+
+	if (sanitize_date_mode(NULL, date_mode) && need_to_warn) {
+		need_to_warn = 0;
+		warning("The date format `:local' is deprecated; "
+		        "use the time zone specifier `@local' instead.");
+	}
+}
+
+#undef PARSE_DATE_MODE
+
 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_DEFAULT;
-	const char *formatp;
+	struct date_mode date_mode = INIT_DATE_MODE;
 
-	/*
-	 * 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, and use the default.
-	 */
-	formatp = strchr(atomname, ':');
-	if (formatp != NULL) {
-		formatp++;
-		date_mode = parse_date_format(formatp);
-	}
+	parse_date_mode(atomname, &date_mode);
 
 	if (!eoemail)
 		goto bad;
diff --git a/builtin/log.c b/builtin/log.c
index d43ad3a..fd6b212 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -20,8 +20,7 @@
 #include "string-list.h"
 #include "parse-options.h"
 
-/* Set a default date-time format for git log ("log.date" config variable) */
-static const char *default_date_mode = NULL;
+static struct date_mode default_date_mode = INIT_DATE_MODE;
 
 static int default_show_root = 1;
 static int decoration_style;
@@ -61,8 +60,7 @@ static void cmd_log_init_defaults(struct rev_info *rev)
 	rev->subject_prefix = fmt_patch_subject_prefix;
 	DIFF_OPT_SET(&rev->diffopt, ALLOW_TEXTCONV);
 
-	if (default_date_mode)
-		rev->date_mode = parse_date_format(default_date_mode);
+	rev->date_mode = default_date_mode;
 }
 
 static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
@@ -79,6 +77,8 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 		usage(builtin_log_usage);
 	argc = setup_revisions(argc, argv, rev, opt);
 
+	sanitize_date_mode("log", &rev->date_mode);
+
 	memset(&w, 0, sizeof(w));
 	userformat_find_requirements(NULL, &w);
 
@@ -305,12 +305,14 @@ static int cmd_log_walk(struct rev_info *rev)
 
 static int git_log_config(const char *var, const char *value, void *cb)
 {
+	int ret;
+
 	if (!strcmp(var, "format.pretty"))
 		return git_config_string(&fmt_pretty, var, value);
 	if (!strcmp(var, "format.subjectprefix"))
 		return git_config_string(&fmt_patch_subject_prefix, var, value);
-	if (!strcmp(var, "log.date"))
-		return git_config_string(&default_date_mode, var, value);
+	if (parse_date_mode_config("log", var, value, &default_date_mode, &ret))
+		return ret;
 	if (!strcmp(var, "log.decorate")) {
 		decoration_style = parse_decoration_style(var, value);
 		if (decoration_style < 0)
@@ -748,6 +750,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 	struct diff_options opts;
 	int need_8bit_cte = 0;
 	struct commit *commit = NULL;
+	struct date_mode date_mode = INIT_DATE_MODE_FORMAT(DATE_RFC2822);
 
 	if (rev->commit_format != CMIT_FMT_EMAIL)
 		die(_("Cover letter needs email format"));
@@ -787,7 +790,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 			need_8bit_cte = 1;
 
 	msg = body;
-	pp_user_info(NULL, CMIT_FMT_EMAIL, &sb, committer, DATE_RFC2822,
+	pp_user_info(NULL, CMIT_FMT_EMAIL, &sb, committer, date_mode,
 		     encoding);
 	pp_title_line(CMIT_FMT_EMAIL, &msg, &sb, subject_start, extra_headers,
 		      encoding, need_8bit_cte);
diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index 5815f55..8560847 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -165,7 +165,7 @@ void shortlog_add_commit(struct shortlog *log, struct commit *commit)
 		ctx.abbrev = log->abbrev;
 		ctx.subject = "";
 		ctx.after_subject = "";
-		ctx.date_mode = DATE_DEFAULT;
+		SET_DATE_MODE_DEFAULT(ctx.date_mode);
 		pretty_print_commit(CMIT_FMT_USERFORMAT, commit, &ufbuf, &ctx);
 		buffer = ufbuf.buf;
 	} else if (*buffer) {
diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index 1abcd9e..4981db2 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -765,6 +765,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
 			const char *msg;
 			unsigned long timestamp;
 			int tz;
+			struct date_mode d = INIT_DATE_MODE_FORMAT(DATE_RELATIVE);
 
 			if (read_ref_at(ref, 0, base+i, sha1, &logmsg,
 					&timestamp, &tz, NULL)) {
@@ -778,7 +779,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
 				msg++;
 			m = xmalloc(strlen(msg) + 200);
 			sprintf(m, "(%s) %s",
-				show_date(timestamp, tz, 1),
+				show_date(timestamp, tz, d),
 				msg);
 			reflog_msg[i] = m;
 			free(logmsg);
diff --git a/cache.h b/cache.h
index bf639f7..910eccf 100644
--- a/cache.h
+++ b/cache.h
@@ -828,17 +828,47 @@ extern void *read_object_with_reference(const unsigned char *sha1,
 extern struct object *peel_to_type(const char *name, int namelen,
 				   struct object *o, enum object_type);
 
-enum date_mode {
+enum date_format {
 	DATE_DEFAULT = 0,
 	DATE_RELATIVE,
 	DATE_SHORT,
-	DATE_LOCAL,
+	DATE_LOCAL,           /* Deprecated; use TIME_ZONE_LOCAL */
 	DATE_ISO8601,
 	DATE_RFC2822,
 	DATE_RAW
 };
 
-const char *show_date(unsigned long time, int timezone, enum date_mode mode);
+enum time_zone {
+	TIME_ZONE_DEFAULT = 0,
+	TIME_ZONE_LOCAL
+};
+
+struct date_mode {
+	enum date_format format;
+	enum time_zone zone;
+};
+
+#define INIT_DATE_MODE_FORMAT_AND_ZONE(m,z) { .format = m, .zone = z }
+#define INIT_DATE_MODE_FORMAT(m) INIT_DATE_MODE_FORMAT_AND_ZONE(m, TIME_ZONE_DEFAULT)
+#define INIT_DATE_MODE_ZONE(z) INIT_DATE_MODE_FORMAT_AND_ZONE(DATE_DEFAULT, z)
+#define INIT_DATE_MODE INIT_DATE_MODE_FORMAT_AND_ZONE(DATE_DEFAULT, TIME_ZONE_DEFAULT)
+#define SET_DATE_MODE_DEFAULT(date_mode)   \
+	date_mode.format = DATE_DEFAULT; \
+	date_mode.zone = TIME_ZONE_DEFAULT;
+
+int parse_date_mode_config_internal(const char *var_date,
+                                     const char *var_time_zone,
+                                     const char *var,
+                                     const char *value,
+                                     struct date_mode *d,
+                                     int *ret);
+
+#define parse_date_mode_config(command_name, var, value, date_mode, ret) \
+	parse_date_mode_config_internal(command_name ".date", command_name ".timezone", var, value, date_mode, ret)
+
+int sanitize_date_mode(const char* command_name, struct date_mode *d);
+
+const char *show_date(unsigned long time, int timezone, struct date_mode date_mode);
 const char *show_date_relative(unsigned long time, int tz,
 			       const struct timeval *now,
 			       char *timebuf,
@@ -849,7 +879,8 @@ void datestamp(char *buf, int bufsize);
 #define approxidate(s) approxidate_careful((s), NULL)
 unsigned long approxidate_careful(const char *, int *);
 unsigned long approxidate_relative(const char *date, const struct timeval *now);
-enum date_mode parse_date_format(const char *format);
+enum date_format parse_date_format(const char *format);
+enum time_zone parse_time_zone(const char *time_zone);
 
 #define IDENT_WARN_ON_NO_NAME  1
 #define IDENT_ERROR_ON_NO_NAME 2
diff --git a/commit.h b/commit.h
index 4198513..04b03d1 100644
--- a/commit.h
+++ b/commit.h
@@ -72,7 +72,7 @@ struct pretty_print_context {
 	int abbrev;
 	const char *subject;
 	const char *after_subject;
-	enum date_mode date_mode;
+	struct date_mode date_mode;
 	int need_8bit_cte;
 	int show_notes;
 	struct reflog_walk_info *reflog_info;
@@ -98,7 +98,7 @@ extern void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
 				struct strbuf *sb,
 				const struct pretty_print_context *context);
 void pp_user_info(const char *what, enum cmit_fmt fmt, struct strbuf *sb,
-		   const char *line, enum date_mode dmode,
+		   const char *line, struct date_mode dmode,
 		   const char *encoding);
 void pp_title_line(enum cmit_fmt fmt,
 		   const char **msg_p,
diff --git a/date.c b/date.c
index caa14fe..879b0e1 100644
--- a/date.c
+++ b/date.c
@@ -147,40 +147,42 @@ const char *show_date_relative(unsigned long time, int tz,
 	return timebuf;
 }
 
-const char *show_date(unsigned long time, int tz, enum date_mode mode)
+const char *show_date(unsigned long time, int tz, struct date_mode date_mode)
 {
 	struct tm *tm;
 	static char timebuf[200];
 
-	if (mode == DATE_RAW) {
+	enum date_format format = date_mode.format;
+
+	if (format == DATE_RAW) {
 		snprintf(timebuf, sizeof(timebuf), "%lu %+05d", time, tz);
 		return timebuf;
 	}
 
-	if (mode == DATE_RELATIVE) {
+	if (format == DATE_RELATIVE) {
 		struct timeval now;
 		gettimeofday(&now, NULL);
 		return show_date_relative(time, tz, &now,
 					  timebuf, sizeof(timebuf));
 	}
 
-	if (mode == DATE_LOCAL)
+	if (date_mode.zone == TIME_ZONE_LOCAL)
 		tz = local_tzoffset(time);
 
 	tm = time_to_tm(time, tz);
 	if (!tm)
 		return NULL;
-	if (mode == DATE_SHORT)
+	if (format == DATE_SHORT)
 		sprintf(timebuf, "%04d-%02d-%02d", tm->tm_year + 1900,
 				tm->tm_mon + 1, tm->tm_mday);
-	else if (mode == DATE_ISO8601)
+	else if (format == DATE_ISO8601)
 		sprintf(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 == DATE_RFC2822)
+	else if (format == DATE_RFC2822)
 		sprintf(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,
@@ -653,7 +655,7 @@ int parse_date(const char *date, char *result, int maxlen)
 	return date_string(timestamp, offset, result, maxlen);
 }
 
-enum date_mode parse_date_format(const char *format)
+enum date_format parse_date_format(const char *format)
 {
 	if (!strcmp(format, "relative"))
 		return DATE_RELATIVE;
@@ -675,6 +677,67 @@ enum date_mode parse_date_format(const char *format)
 		die("unknown date format %s", format);
 }
 
+enum time_zone parse_time_zone(const char *time_zone)
+{
+	if (!strcmp(time_zone, "local"))
+		return TIME_ZONE_LOCAL;
+	else if (!strcmp(time_zone, "default"))
+		return TIME_ZONE_DEFAULT;
+	else
+		die("unknown time zone %s", time_zone);
+}
+
+int parse_date_mode_config_internal(const char *var_date,
+                                     const char *var_timezone,
+                                     const char *var,
+                                     const char *value,
+                                     struct date_mode *d,
+                                     int *ret)
+{
+	if (!strcmp(var, var_date)) {
+		if (!value)
+			*ret = config_error_nonbool(var);
+
+		d->format = parse_date_format(value);
+
+		return 1;
+	}
+
+	if (!strcmp(var, var_timezone)) {
+		if (!value)
+			*ret = config_error_nonbool(var);
+
+		d->zone = parse_time_zone(value);
+
+		return 1;
+	}
+
+	return 0;
+}
+
+int sanitize_date_mode(const char* command_name, struct date_mode *d)
+{
+	if (d->format == DATE_LOCAL) {
+		if (command_name)
+			warning("The following are now deprecated:\n\n"
+			        "    %s.date = local\n"
+			        "    --date=local\n\n"
+			        "Use these instead:\n\n"
+			        "    %s.timezone = local\n"
+			        "    --time-zone=local\n\n"
+			        "Now assuming you meant:\n\n"
+			        "    --date=default --time-zone=local",
+			        command_name, command_name);
+
+		d->format = DATE_DEFAULT;
+		d->zone = TIME_ZONE_LOCAL;
+
+		return 1;
+	}
+
+	return 0;
+}
+
 void datestamp(char *buf, int bufsize)
 {
 	time_t now;
diff --git a/fast-import.c b/fast-import.c
index 3e4e655..bd5c113 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -401,6 +401,8 @@ static void write_crash_report(const char *err)
 	unsigned long lu;
 	struct recent_command *rc;
 
+	struct date_mode date_mode = INIT_DATE_MODE_ZONE(TIME_ZONE_LOCAL);
+
 	if (!rpt) {
 		error("can't write crash report %s: %s", loc, strerror(errno));
 		return;
@@ -411,7 +413,7 @@ static void write_crash_report(const char *err)
 	fprintf(rpt, "fast-import crash report:\n");
 	fprintf(rpt, "    fast-import process: %"PRIuMAX"\n", (uintmax_t) getpid());
 	fprintf(rpt, "    parent process     : %"PRIuMAX"\n", (uintmax_t) getppid());
-	fprintf(rpt, "    at %s\n", show_date(time(NULL), 0, DATE_LOCAL));
+	fprintf(rpt, "    at %s\n", show_date(time(NULL), 0, date_mode));
 	fputc('\n', rpt);
 
 	fputs("fatal: ", rpt);
diff --git a/http-backend.c b/http-backend.c
index 8501504..15794c9 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -89,7 +89,8 @@ static void hdr_int(const char *name, uintmax_t value)
 
 static void hdr_date(const char *name, unsigned long when)
 {
-	const char *value = show_date(when, 0, DATE_RFC2822);
+	struct date_mode d = INIT_DATE_MODE_FORMAT(DATE_RFC2822);
+	const char *value = show_date(when, 0, d);
 	hdr_str(name, value);
 }
 
diff --git a/log-tree.c b/log-tree.c
index b059446..3a6e110 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -272,7 +272,7 @@ void get_patch_filename(struct commit *commit, int nr, const char *suffix,
 	if (commit) {
 		int max_len = start_len + FORMAT_PATCH_NAME_MAX - suffix_len;
 		struct pretty_print_context ctx = {0};
-		ctx.date_mode = DATE_DEFAULT;
+		SET_DATE_MODE_DEFAULT(ctx.date_mode);
 
 		format_commit_message(commit, "%f", buf, &ctx);
 		if (max_len < buf->len)
@@ -463,11 +463,13 @@ void show_log(struct rev_info *opt)
 			 * so we don't need to worry about printing the
 			 * graph info here.
 			 */
+
+			if (!opt->date_format_from_command_line)
+				opt->date_mode.format = DATE_DEFAULT;
+
 			show_reflog_message(opt->reflog_info,
 				    opt->commit_format == CMIT_FMT_ONELINE,
-				    opt->date_mode_from_command_line ?
-					opt->date_mode :
-					DATE_DEFAULT);
+				    opt->date_mode);
 			if (opt->commit_format == CMIT_FMT_ONELINE)
 				return;
 		}
diff --git a/pretty.c b/pretty.c
index e1d8a8f..d702575 100644
--- a/pretty.c
+++ b/pretty.c
@@ -267,7 +267,7 @@ needquote:
 }
 
 void pp_user_info(const char *what, enum cmit_fmt fmt, struct strbuf *sb,
-		  const char *line, enum date_mode dmode,
+		  const char *line, struct date_mode dmode,
 		  const char *encoding)
 {
 	char *date;
@@ -306,7 +306,8 @@ void pp_user_info(const char *what, enum cmit_fmt fmt, struct strbuf *sb,
 		strbuf_addf(sb, "Date:   %s\n", show_date(time, tz, dmode));
 		break;
 	case CMIT_FMT_EMAIL:
-		strbuf_addf(sb, "Date: %s\n", show_date(time, tz, DATE_RFC2822));
+		dmode.format = DATE_RFC2822;
+		strbuf_addf(sb, "Date: %s\n", show_date(time, tz, dmode));
 		break;
 	case CMIT_FMT_FULLER:
 		strbuf_addf(sb, "%sDate: %s\n", what, show_date(time, tz, dmode));
@@ -458,7 +459,7 @@ static int mailmap_name(char *email, int email_len, char *name, int name_len)
 }
 
 static size_t format_person_part(struct strbuf *sb, char part,
-				 const char *msg, int len, enum date_mode dmode)
+				 const char *msg, int len, struct date_mode dmode)
 {
 	/* currently all placeholders have same length */
 	const int placeholder_len = 2;
@@ -540,13 +541,16 @@ static size_t format_person_part(struct strbuf *sb, char part,
 		strbuf_addstr(sb, show_date(date, tz, dmode));
 		return placeholder_len;
 	case 'D':	/* date, RFC2822 style */
-		strbuf_addstr(sb, show_date(date, tz, DATE_RFC2822));
+		dmode.format = DATE_RFC2822;
+		strbuf_addstr(sb, show_date(date, tz, dmode));
 		return placeholder_len;
 	case 'r':	/* date, relative */
-		strbuf_addstr(sb, show_date(date, tz, DATE_RELATIVE));
+		dmode.format = DATE_RELATIVE;
+		strbuf_addstr(sb, show_date(date, tz, dmode));
 		return placeholder_len;
 	case 'i':	/* date, ISO 8601 */
-		strbuf_addstr(sb, show_date(date, tz, DATE_ISO8601));
+		dmode.format = DATE_ISO8601;
+		strbuf_addstr(sb, show_date(date, tz, dmode));
 		return placeholder_len;
 	}
 
@@ -1050,7 +1054,7 @@ void format_commit_message(const struct commit *commit,
 
 static void pp_header(enum cmit_fmt fmt,
 		      int abbrev,
-		      enum date_mode dmode,
+		      struct date_mode dmode,
 		      const char *encoding,
 		      const struct commit *commit,
 		      const char **msg_p,
diff --git a/reflog-walk.c b/reflog-walk.c
index 5d81d39..e1bc64a 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -243,7 +243,7 @@ void fake_reflog_parent(struct reflog_walk_info *info, struct commit *commit)
 
 void get_reflog_selector(struct strbuf *sb,
 			 struct reflog_walk_info *reflog_info,
-			 enum date_mode dmode,
+			 struct date_mode dmode,
 			 int shorten)
 {
 	struct commit_reflog *commit_reflog = reflog_info->last_commit_reflog;
@@ -263,7 +263,7 @@ void get_reflog_selector(struct strbuf *sb,
 	}
 
 	strbuf_addf(sb, "%s@{", printed_ref);
-	if (commit_reflog->flag || dmode) {
+	if (commit_reflog->flag || dmode.format) {
 		info = &commit_reflog->reflogs->items[commit_reflog->recno+1];
 		strbuf_addstr(sb, show_date(info->timestamp, info->tz, dmode));
 	} else {
@@ -292,7 +292,7 @@ void get_reflog_message(struct strbuf *sb,
 }
 
 void show_reflog_message(struct reflog_walk_info *reflog_info, int oneline,
-	enum date_mode dmode)
+	struct date_mode dmode)
 {
 	if (reflog_info && reflog_info->last_commit_reflog) {
 		struct commit_reflog *commit_reflog = reflog_info->last_commit_reflog;
diff --git a/reflog-walk.h b/reflog-walk.h
index 7bd2cd4..30805cc 100644
--- a/reflog-walk.h
+++ b/reflog-walk.h
@@ -11,12 +11,12 @@ extern int add_reflog_for_walk(struct reflog_walk_info *info,
 extern void fake_reflog_parent(struct reflog_walk_info *info,
 		struct commit *commit);
 extern void show_reflog_message(struct reflog_walk_info *info, int,
-		enum date_mode);
+		struct date_mode);
 extern void get_reflog_message(struct strbuf *sb,
 		struct reflog_walk_info *reflog_info);
 extern void get_reflog_selector(struct strbuf *sb,
 		struct reflog_walk_info *reflog_info,
-		enum date_mode dmode,
+		struct date_mode dmode,
 		int shorten);
 
 #endif
diff --git a/refs.c b/refs.c
index e3c0511..75ad7e6 100644
--- a/refs.c
+++ b/refs.c
@@ -1633,8 +1633,9 @@ int read_ref_at(const char *ref, unsigned long at_time, int cnt, unsigned char *
 				if (get_sha1_hex(rec + 41, sha1))
 					die("Log %s is corrupt.", logfile);
 				if (hashcmp(logged_sha1, sha1)) {
+					struct date_mode d = INIT_DATE_MODE_FORMAT(DATE_RFC2822);
 					warning("Log %s has gap after %s.",
-						logfile, show_date(date, tz, DATE_RFC2822));
+						logfile, show_date(date, tz, d));
 				}
 			}
 			else if (date == at_time) {
@@ -1645,8 +1646,9 @@ int read_ref_at(const char *ref, unsigned long at_time, int cnt, unsigned char *
 				if (get_sha1_hex(rec + 41, logged_sha1))
 					die("Log %s is corrupt.", logfile);
 				if (hashcmp(logged_sha1, sha1)) {
+					struct date_mode d = INIT_DATE_MODE_FORMAT(DATE_RFC2822);
 					warning("Log %s unexpectedly ended on %s.",
-						logfile, show_date(date, tz, DATE_RFC2822));
+						logfile, show_date(date, tz, d));
 				}
 			}
 			munmap(log_mapped, mapsz);
diff --git a/revision.c b/revision.c
index 462c311..fd87c67 100644
--- a/revision.c
+++ b/revision.c
@@ -1433,11 +1433,14 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 	} else if (!strcmp(arg, "--full-history")) {
 		revs->simplify_history = 0;
 	} else if (!strcmp(arg, "--relative-date")) {
-		revs->date_mode = DATE_RELATIVE;
-		revs->date_mode_from_command_line = 1;
+		revs->date_mode.format = DATE_RELATIVE;
+		revs->date_format_from_command_line = 1;
 	} else if ((argcount = parse_long_opt("date", argv, &optarg))) {
-		revs->date_mode = parse_date_format(optarg);
-		revs->date_mode_from_command_line = 1;
+		revs->date_mode.format = parse_date_format(optarg);
+		revs->date_format_from_command_line = 1;
+		return argcount;
+	} else if ((argcount = parse_long_opt("time-zone", argv, &optarg))) {
+		revs->date_mode.zone = parse_time_zone(optarg);
 		return argcount;
 	} else if (!strcmp(arg, "--log-size")) {
 		revs->show_log_size = 1;
diff --git a/revision.h b/revision.h
index e5ca939..0ced0fb 100644
--- a/revision.h
+++ b/revision.h
@@ -92,10 +92,10 @@ struct rev_info {
 			abbrev_commit:1,
 			use_terminator:1,
 			missing_newline:1,
-			date_mode_from_command_line:1;
+			date_format_from_command_line:1;
 	unsigned int	disable_stdin:1;
 
-	enum date_mode date_mode;
+	struct date_mode date_mode;
 
 	unsigned int	abbrev;
 	enum cmit_fmt	commit_format;
diff --git a/sha1_name.c b/sha1_name.c
index 69cd6c8..718093c 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -428,9 +428,12 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
 		if (read_ref_at(real_ref, at_time, nth, sha1, NULL,
 				&co_time, &co_tz, &co_cnt)) {
 			if (at_time)
+			{
+				struct date_mode d = INIT_DATE_MODE_FORMAT(DATE_RFC2822);
 				warning("Log for '%.*s' only goes "
 					"back to %s.", len, str,
-					show_date(co_time, co_tz, DATE_RFC2822));
+					show_date(co_time, co_tz, d));
+			}
 			else {
 				free(real_ref);
 				die("Log for '%.*s' only has %d entries.",
diff --git a/submodule.c b/submodule.c
index 0a45da9..f61a0a2 100644
--- a/submodule.c
+++ b/submodule.c
@@ -605,7 +605,7 @@ static void print_commit(struct commit *commit)
 {
 	struct strbuf sb = STRBUF_INIT;
 	struct pretty_print_context ctx = {0};
-	ctx.date_mode = DATE_DEFAULT;
+	SET_DATE_MODE_DEFAULT(ctx.date_mode);
 	format_commit_message(commit, " %h: %m %s", &sb, &ctx);
 	fprintf(stderr, "%s\n", sb.buf);
 	strbuf_release(&sb);
diff --git a/test-date.c b/test-date.c
index 6bcd5b0..6d49c2d 100644
--- a/test-date.c
+++ b/test-date.c
@@ -26,8 +26,11 @@ static void parse_dates(char **argv, struct timeval *now)
 		result[0] = 0;
 		parse_date(*argv, result, sizeof(result));
 		if (sscanf(result, "%lu %d", &t, &tz) == 2)
+		{
+			struct date_mode d = INIT_DATE_MODE_FORMAT(DATE_ISO8601);
 			printf("%s -> %s\n",
-			       *argv, show_date(t, tz, DATE_ISO8601));
+			       *argv, show_date(t, tz, d));
+		}
 		else
 			printf("%s -> bad\n", *argv);
 	}
@@ -36,9 +39,9 @@ static void parse_dates(char **argv, struct timeval *now)
 static void parse_approxidate(char **argv, struct timeval *now)
 {
 	for (; *argv; argv++) {
-		time_t t;
-		t = approxidate_relative(*argv, now);
-		printf("%s -> %s\n", *argv, show_date(t, 0, DATE_ISO8601));
+		time_t t = approxidate_relative(*argv, now);
+		struct date_mode d = INIT_DATE_MODE_FORMAT(DATE_ISO8601);
+		printf("%s -> %s\n", *argv, show_date(t, 0, d));
 	}
 }
 
-- 
1.7.4.18.g68fe8

[RFC 4/5] Date Mode: Documentation

[Michael Witten]

Date: Tue, 19 Apr 2011 19:06:02 +0000
This commit updates the git documentation to reflect the new
date mode time zone features. As a result, the common date
mode documentation has been factored out into one file:

  Documentation/date-mode-docs.txt

which is included where necessary (sometimes with attribute
reference substitutions); this file is somewhat messy at
first glance (AsciiDoc is difficult to tame), but now all
of the essential information is centralized and quite
modular.

Signed-off-by: Michael Witten <mfwitten@gmail.com>
---
 Documentation/blame-options.txt    |    9 +---
 Documentation/config.txt           |   32 +++++++------
 Documentation/date-mode-docs.txt   |   87 ++++++++++++++++++++++++++++++++++++
 Documentation/git-for-each-ref.txt |   19 +++++++-
 Documentation/git-log.txt          |   43 ------------------
 Documentation/git-rev-list.txt     |    4 +-
 Documentation/git-svn.txt          |    2 +-
 Documentation/rev-list-options.txt |   28 +-----------
 8 files changed, 128 insertions(+), 96 deletions(-)
 create mode 100644 Documentation/date-mode-docs.txt

diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index 16e3c68..deb824e 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -70,13 +70,8 @@ of lines before or after the line given by <start>.
 	tree copy has the contents of the named file (specify
 	`-` to make the command read from the standard input).
 
---date <format>::
-	The value is one of the following alternatives:
-	{relative,local,default,iso,rfc,short}. If --date is not
-	provided, the value of the blame.date config variable is
-	used. If the blame.date config variable is also not set, the
-	iso format is used. For more information, See the discussion
-	of the --date option at linkgit:git-log[1].
+:date mode options:
+include::date-mode-docs.txt[]
 
 -M|<num>|::
 	Detect moved or copied lines within a file. When a commit
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 480dd0a..5bf9366 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -607,6 +607,9 @@ apply.whitespace::
 	Tells 'git apply' how to handle whitespaces, in the same way
 	as the '--whitespace' option. See linkgit:git-apply[1].
 
+:date mode config: blame
+include::date-mode-docs.txt[]
+
 branch.autosetupmerge::
 	Tells 'git branch' and 'git checkout' to set up new branches
 	so that linkgit:git-pull[1] will appropriately merge from the
@@ -1262,7 +1265,8 @@ i18n.commitEncoding::
 
 i18n.logOutputEncoding::
 	Character encoding the commit messages are converted to when
-	running 'git log' and friends.
+	running 'git log' and friends.  Defaults to the value of
+	`i18n.commitEncoding` if set, UTF-8 otherwise.
 
 imap::
 	The configuration variables in the 'imap' section are described
@@ -1300,12 +1304,8 @@ interactive.singlekey::
 	linkgit:git-add[1].  Note that this setting is silently
 	ignored if portable keystroke input is not available.
 
-log.date::
-	Set the default date-time mode for the 'log' command.
-	Setting a value for log.date is similar to using 'git log''s
-	`\--date` option.  Possible values are `relative`, `local`,
-	`default`, `iso`, `rfc`, and `short`; see linkgit:git-log[1]
-	for details.
+:date mode config: log
+include::date-mode-docs.txt[]
 
 log.decorate::
 	Print out the ref names of any commits that are shown by the log
@@ -1315,10 +1315,11 @@ log.decorate::
 	This is the same as the log commands '--decorate' option.
 
 log.showroot::
-	If true, the initial commit will be shown as a big creation event.
-	This is equivalent to a diff against an empty tree.
-	Tools like linkgit:git-log[1] or linkgit:git-whatchanged[1], which
-	normally hide the root commit will now show it. True by default.
+	If `true`, the initial commit will be shown as a big creation event;
+	this is equivalent to a diff against an empty tree. If `false`,
+	'git log' and related commands will not treat the initial commit as
+	a big creation event.  Any root commits in `git log -p` output would
+	be shown without a diff attached.  The default is `true`.
 
 mailmap.file::
 	The location of an augmenting mailmap file. The default
@@ -1391,13 +1392,14 @@ notes.displayRef::
 	exist, but a glob that does not match any refs is silently
 	ignored.
 +
-This setting can be overridden with the `GIT_NOTES_DISPLAY_REF`
-environment variable, which must be a colon separated list of refs or
-globs.
-+
 The effective value of "core.notesRef" (possibly overridden by
 GIT_NOTES_REF) is also implicitly added to the list of refs to be
 displayed.
++
+This setting can be overridden with the `GIT_NOTES_DISPLAY_REF`
+environment variable, which must be a colon separated list of refs or
+globs. It can also be overridden with 'git log''s `--notes` option.
+Also, this setting can be disabled by using 'git log''s `--no-notes` option.
 
 notes.rewrite.<command>::
 	When rewriting commits with <command> (currently `amend` or
diff --git a/Documentation/date-mode-docs.txt b/Documentation/date-mode-docs.txt
new file mode 100644
index 0000000..8f07d3f
--- /dev/null
+++ b/Documentation/date-mode-docs.txt
@@ -0,0 +1,87 @@
+ifdef::datemodeoptions[]
+
+:datemodeoptions!:
+
+--time-zone=<zone>::
+
+	Only takes effect for timestamps shown in a human-readable format (such
+	as when using "--pretty") and when the timestamp is interpreted in terms
+	of a time zone (for instance, this option has no effect when
+	`--date=raw` is specified).
++
+The <zone> may be one of the following:
++
+:date mode time zone values:
+include::date-mode-docs.txt[]
+
+--date=<format>::
+
+	Only takes effect for timestamps shown in a human-readable format (such
+	as when using "--pretty").
++
+The <format> may be one of the following:
++
+:date mode format values:
+include::date-mode-docs.txt[]
+
+--relative-date::
+
+	Synonym for `--date=relative`.
+
+endif::datemodeoptions[]
+
+ifdef::datemodetimezonevalues[]
+* *default* causes timestamps to be interpreted in terms of the time zone
+            recorded in the history.
+* *local*   causes timestamps to be interpreted in terms of the user's
+            time zone; on POSIX systems, the time zone may be selected
+            by setting the `TZ` environment variable appropriately
+            (for example, on GNU systems, the timestamps could be
+            interpreted in the `+0000` (UTC) time zone by something
+            like the following: `TZ=:UTC git log --time-zone=local`).
+
+:datemodetimezonevalues!:
+endif::datemodetimezonevalues[]
+
+ifdef::datemodeformatvalues[]
+* *default*  shows timestamps in git's original human-friendly format,
+             e.g. "Sat Jul 20 00:00:00 1985 -0400".
+* *relative* shows timestamps relative to the current time,
+             e.g. "2 hours ago".
+* *iso8601* (or *iso*) shows timestamps in ISO 8601 format,
+             kke.g. "1985-07-20 00:00:00 -0400".
+* *rfc2822* (or *rfc*) shows timestamps in RFC 2822 format
+            (which is used in email messages),
+             e.g. "Sat, 20 Jul 1985 00:00:00 -0400".
+* *short*    shows timestamps as only the date (no time) in `YYYY-MM-DD` format,
+             e.g. "1985-07-20".
+* *raw*      shows timestamps in the internal git format (as in GNU `date`'s
+            `"+%s %z"` format; that is,
+            "<seconds since The Epoch> <time zone in +HHMM format>"),
+             e.g. "490680000 -0400".
+
+:datemodeformatvalues!:
+endif::datemodeformatvalues[]
+
+ifdef::datemodeconfig[]
+:command: {datemodeconfig}
+:datemodeconfig!:
+
+{command}.timezone::
+	Set the default date mode time zone for the '{command}' command.
+	Setting a value for `{command}.timezone` is similar to using
+	'git {command}''s `\--time-zone` option.  Possible values are:
++
+:date mode time zone values:
+include::date-mode-docs.txt[]
+
+{command}.date::
+	Set the default date mode format for the '{command}' command.
+	Setting a value for `{command}.date` is similar to using
+	'git {command}''s `\--date` option.  Possible values are:
++
+:date mode format values:
+include::date-mode-docs.txt[]
+
+:command!:
+endif::datemodeconfig[]
diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 152e695..a91d2bf 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -114,9 +114,22 @@ 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)`.
+the date by adding one of the following to the end of the fieldname
+(e.g. `%(taggerdate:rfc)`):
+
+:date mode format values:
+include::date-mode-docs.txt[]
+
+Similarly, you may specify the time zone in which to interpret the
+date by adding one of the following to the end of the fieldname
+(e.g. `%(taggerdate&#64;local)`):
+
+:date mode time zone values:
+include::date-mode-docs.txt[]
+
+Of course, both the format and time zone specifications may be added
+at the same time (e.g. `%(taggerdate:rfc@local)` or, equivalently,
+`%(taggerdate&#64;local:rfc)`).
 
 
 EXAMPLES
diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index de5c0d3..03e0ae3 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -139,49 +139,6 @@ Discussion
 
 include::i18n.txt[]
 
-Configuration
--------------
-
-See linkgit:git-config[1] for core variables and linkgit:git-diff[1]
-for settings related to diff generation.
-
-format.pretty::
-	Default for the `--format` option.  (See "PRETTY FORMATS" above.)
-	Defaults to "medium".
-
-i18n.logOutputEncoding::
-	Encoding to use when displaying logs.  (See "Discussion", above.)
-	Defaults to the value of `i18n.commitEncoding` if set, UTF-8
-	otherwise.
-
-log.date::
-	Default format for human-readable dates.  (Compare the
-	`--date` option.)  Defaults to "default", which means to write
-	dates like `Sat May 8 19:35:34 2010 -0500`.
-
-log.showroot::
-	If `false`, 'git log' and related commands will not treat the
-	initial commit as a big creation event.  Any root commits in
-	`git log -p` output would be shown without a diff attached.
-	The default is `true`.
-
-mailmap.file::
-	See linkgit:git-shortlog[1].
-
-notes.displayRef::
-	Which refs, in addition to the default set by `core.notesRef`
-	or 'GIT_NOTES_REF', to read notes from when showing commit
-	messages with the 'log' family of commands.  See
-	linkgit:git-notes[1].
-+
-May be an unabbreviated ref name or a glob and may be specified
-multiple times.  A warning will be issued for refs that do not exist,
-but a glob that does not match any refs is silently ignored.
-+
-This setting can be disabled by the `--no-notes` option,
-overridden by the 'GIT_NOTES_DISPLAY_REF' environment variable,
-and overridden by the `--notes=<ref>` option.
-
 GIT
 ---
 Part of the linkgit:git[1] suite
diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt
index 415f4f0..5909913 100644
--- a/Documentation/git-rev-list.txt
+++ b/Documentation/git-rev-list.txt
@@ -44,7 +44,9 @@ SYNOPSIS
 	     [ \--regexp-ignore-case | -i ]
 	     [ \--extended-regexp | -E ]
 	     [ \--fixed-strings | -F ]
-	     [ \--date=(local|relative|default|iso|rfc|short) ]
+	     [ \--time-zone=<zone> ]
+	     [ \--date=<format> ]
+	     [ \--time-zone=<zone> ]
 	     [ [\--objects | \--objects-edge] [ \--unpacked ] ]
 	     [ \--pretty | \--header ]
 	     [ \--bisect ]
diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 71fc0ae..3e3ef7f 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -104,7 +104,7 @@ COMMANDS
 
 --localtime;;
 	Store Git commit times in the local timezone instead of UTC.  This
-	makes 'git log' (even without --date=local) show the same times
+	makes 'git log' (even without --time-zone=local) show the same times
 	that `svn log` would in the local timezone.
 +
 This doesn't interfere with interoperating with the Subversion
diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index 73111bb..1494247 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -634,32 +634,8 @@ endif::git-rev-list[]
 
 include::pretty-options.txt[]
 
---relative-date::
-
-	Synonym for `--date=relative`.
-
---date=(relative|local|default|iso|rfc|short|raw)::
-
-	Only takes effect for dates shown in human-readable format, such
-	as when using "--pretty". `log.date` config variable sets a default
-	value for log command's --date option.
-+
-`--date=relative` shows dates relative to the current time,
-e.g. "2 hours ago".
-+
-`--date=local` shows timestamps in user's local timezone.
-+
-`--date=iso` (or `--date=iso8601`) shows timestamps in ISO 8601 format.
-+
-`--date=rfc` (or `--date=rfc2822`) shows timestamps in RFC 2822
-format, often found in E-mail messages.
-+
-`--date=short` shows only date but not time, in `YYYY-MM-DD` format.
-+
-`--date=raw` shows the date in the internal raw git format `%s %z` format.
-+
-`--date=default` shows timestamps in the original timezone
-(either committer's or author's).
+:date mode options:
+include::date-mode-docs.txt[]
 
 ifdef::git-rev-list[]
 --header::
-- 
1.7.4.18.g68fe8

[RFC 5/5] Date Mode: Tests

[Michael Witten]

Date: Mon, 14 Mar 2011 20:39:44 +0000
This commit introduces infrastructure for testing nearly all
combinations of date mode input (only the format `relative'
is currently excluded); test scripts source a date mode shell
library to set up the environment and define the general date
mode testing driver that is then started with a test-specific
callback and optional parameters.

Care was taken to reduce the amount of runtime overhead,
particularly by only updating the values of git configuration
variables when necessary.

Signed-off-by: Michael Witten <mfwitten@gmail.com>
---
 t/date-modes-lib.sh          |  168 ++++++++++++++++++++++++++++++++++++++++++
 t/date-modes-permutator.perl |   94 +++++++++++++++++++++++
 t/t4202-log.sh               |    3 +
 t/t6300-for-each-ref.sh      |   11 +++
 t/t8002-blame.sh             |    9 ++
 5 files changed, 285 insertions(+), 0 deletions(-)
 create mode 100755 t/date-modes-lib.sh
 create mode 100755 t/date-modes-permutator.perl

diff --git a/t/date-modes-lib.sh b/t/date-modes-lib.sh
new file mode 100755
index 0000000..5ff6192
--- /dev/null
+++ b/t/date-modes-lib.sh
@@ -0,0 +1,168 @@
+#!/bin/sh
+#
+# Copyright (c) 2011 Michael F Witten
+
+if ! test_have_prereq PERL; then
+	skip_all='skipping all date mode tests; perl not available'
+	test_done
+fi
+
+date_mode_file=d.$$
+
+test_expect_success 'date mode: prepare reference commit' "
+	time='2131675200 -0400'
+
+	export GIT_COMMITTER_DATE=\"\$time\"  \
+	       GIT_COMMITTER_NAME=C           \
+	       GIT_COMMITTER_EMAIL=C@test.git \
+	       GIT_AUTHOR_DATE=\"\$time\"     \
+	       GIT_AUTHOR_NAME=A              \
+	       GIT_AUTHOR_EMAIL=A@test.git
+
+	d=$date_mode_file
+
+	echo > \$d && git add \$d && git commit -am d
+"
+
+date_mode_perm=$TEST_DIRECTORY/date-modes-permutator.perl
+
+# $1 : Configuration section (the command name being configured, such as
+#      the `log' in `log.date', etc.).
+#
+date_mode_unset_configs()
+{
+	var_t=$1.timezone
+	var_d=$1.date
+
+	test_expect_success "date mode: unset configs: '$var_t' and '$var_d'" "
+		git config --unset-all $var_t
+		git config --unset-all $var_d
+		! git config $var_t &&
+		! git config $var_d
+	"
+
+	return 0
+}
+
+# $1    : `section.variable'
+#
+# stdin : One line that is a value that is suitable for setting "$1";
+#         an empty line commands that "$1" be unset.
+#
+date_mode_set_config()
+{
+	read -r v || return 1
+
+	if [ -z "$v" ]; then
+		test_expect_success \
+			"date mode: unset config '$1'" \
+			"git config --unset-all '$1';! git config '$1'"
+	else
+		test_expect_success \
+			"date mode: set config: '$1 = $v'" \
+			"git config '$1' '$v'"
+	fi
+
+	return 0
+}
+
+date_mode_set_configs()
+{
+	date_mode_set_config "$1.timezone" &&
+	read -r; # skip `=' line
+	date_mode_set_config "$1.date"
+}
+
+# $1    : A value that is suitable as a date mode time zone.
+#
+# stdin : 2 lines:
+#
+#           A value that is suitable as a date mode format.
+#
+#           Expected output from running "$date_mode_test_code" (see below)
+#
+#         If "$1" or the first line is empty, then the corresponding option
+#         is not used.
+#
+# Global variable `date_mode_test_code':
+#
+#         Shell code that can accept the arguments "$1" and the value of the
+#         first line of stdin, and then output a date that is equal to the
+#         second line of stdin.
+#
+date_mode_run_test()
+{
+	read -r format   &&
+	read -r expected || return 1
+
+	test_expect_success "date mode:${1:+ '$1'}${format:+ '$format'}" "
+		$date_mode_test_code '$1' '$format' > actual   &&
+		echo '$expected'                    > expected &&
+		test_cmp expected actual
+	"
+
+	return 0
+}
+
+date_mode_error()
+{
+	error "date mode: The test is in a confused state; aborting..."
+}
+
+# $1    : Shell code that can accept the following input (empty strings
+#         represent no specified value):
+#
+#           $1 : A value suitable as a date mode time zone.
+#           $2 : A value suitable as a date mode format.
+#
+#         and then output a date.
+#
+# $2    : Configuration section (the command name being configured, such as
+#         the `log' in `log.date', etc.); this may be the empty string, in
+#         which case git configuration variables are not tested.
+#
+# $3    : The date format that is the default; the default is `default'.
+# $4    : The date timzezone that is the default; the default is `default'.
+#
+# stdin : See the "$date_mode_perm" script.
+#
+date_mode_run_tests()
+{
+	date_mode_test_code=$1
+
+	if [ -z "$2" ]; then
+
+		"$PERL_PATH" "$date_mode_perm" '' "$3" "$4" |
+		while read -r line; do
+			date_mode_run_test "$line" || date_mode_error
+		done
+
+	else
+		date_mode_unset_configs "$2"
+
+		"$PERL_PATH" "$date_mode_perm" config "$3" "$4" |
+		while read -r line; do
+
+			if   [ "$line" = = ]; then
+				date_mode_set_config  "$2.date" && read -r line
+			elif [ "$line" = - ]; then
+				date_mode_set_configs "$2"      && read -r line
+			fi &&
+
+			date_mode_run_test "$line" || date_mode_error
+
+		done
+	fi
+
+	return 0
+}
+
+date_mode_std_args()
+{
+	echo "_date_mode_std_args '$(echo "$1" | sed "s/'/'\\\\''/g")'";
+}
+
+_date_mode_std_args()
+{
+	eval "$1${2:+ --time-zone='$2'}${3:+ --date='$3'}"
+}
diff --git a/t/date-modes-permutator.perl b/t/date-modes-permutator.perl
new file mode 100755
index 0000000..729e3e7
--- /dev/null
+++ b/t/date-modes-permutator.perl
@@ -0,0 +1,94 @@
+#!/usr/bin/perl
+#
+# Copyright (c) 2011 Michael F Witten
+
+use strict;
+use warnings;
+
+# The values expected to be generated by the test
+# when the given {zone}{format} is in use;
+my %expected = ();
+
+#########   ZONES    FORMATS            EXPECTED
+$expected {default} {default} = 'Mon Jul 20 00:00:00 2037 -0400';
+$expected {default} {  ''   } = $expected{default}{default};
+$expected {default} {rfc2822} = 'Mon, 20 Jul 2037 00:00:00 -0400';
+$expected {default} {  rfc  } = $expected{default}{rfc2822};
+$expected {default} {iso8601} = '2037-07-20 00:00:00 -0400';
+$expected {default} {  iso  } = $expected{default}{iso8601};
+$expected {default} { short } = '2037-07-20';
+$expected {default} {  raw  } = '2131675200 -0400';
+$expected {default} { local } = 'Mon Jul 20 04:00:00 2037 +0000';
+$expected {  ''   }           = $expected{default};
+$expected { local } {default} = 'Mon Jul 20 04:00:00 2037 +0000';
+$expected { local } {  ''   } = $expected{local}{default};
+$expected { local } {rfc2822} = 'Mon, 20 Jul 2037 04:00:00 +0000';
+$expected { local } {  rfc  } = $expected{local}{rfc2822};
+$expected { local } {iso8601} = '2037-07-20 04:00:00 +0000';
+$expected { local } {  iso  } = $expected{local}{iso8601};
+$expected { local } { short } = '2037-07-20';
+$expected { local } {  raw  } = '2131675200 -0400';
+$expected { local } { local } = 'Mon Jul 20 04:00:00 2037 +0000';
+
+my @zones   = keys %expected;
+my @formats = keys %{$expected{$zones[0]}}; # Assuming unvarying formats
+
+my ($config_zone, $config_format);
+
+sub print_option_sets_and_expected_outputs
+{
+	foreach my $z (@zones)
+	{
+		foreach my $f (@formats)
+		{
+			print $z, "\n", $f, "\n";
+			print $expected { $z ||  $config_zone  }
+					{ $f || $config_format },
+					"\n";
+		}
+	}
+}
+
+sub print_config_sets_and_option_sets_and_expected_outputs
+{
+	foreach (@zones)
+	{
+		$config_zone = $_;
+		print "-\n", $_, "\n";
+
+		foreach (@formats)
+		{
+			$config_format = $_;
+			print "=\n", $_, "\n";
+			print_option_sets_and_expected_outputs();
+		}
+	}
+}
+
+#########################
+#### Start of Output ####
+#########################
+
+if (defined $ARGV[1])
+{
+	foreach (@zones)
+	{
+		my $e = $expected{$_};
+		$e->{''} = $e->{$ARGV[1]};
+	}
+}
+
+if (defined $ARGV[2])
+{
+	foreach (@formats) {
+		$expected{''} = $expected{$ARGV[2]};
+	}
+}
+
+
+if (defined $ARGV[0] && $ARGV[0] eq 'config') {
+	print_config_sets_and_option_sets_and_expected_outputs();
+} else {
+	$config_zone = $config_format = '';
+	print_option_sets_and_expected_outputs();
+}
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 2fcc31a..cdfea8b 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -463,4 +463,7 @@ test_expect_success 'show added path under "--follow -M"' '
 	)
 '
 
+. "$TEST_DIRECTORY"/date-modes-lib.sh
+date_mode_run_tests "$(date_mode_std_args 'git log -1 --format=%ad')" log
+
 test_done
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 050ed7d..d6ac4dd 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -359,4 +359,15 @@ test_expect_success 'an unusual tag with an incomplete line' '
 
 '
 
+. "$TEST_DIRECTORY"/date-modes-lib.sh
+date_mode_run_tests '
+	b()
+	{
+		git for-each-ref \
+			--count=1 \
+			--format="%(authordate${2:+:$2}${1:+@$1})" \
+			"$(git symbolic-ref HEAD)"
+	}
+	b'
+
 test_done
diff --git a/t/t8002-blame.sh b/t/t8002-blame.sh
index d3a51e1..f049439 100755
--- a/t/t8002-blame.sh
+++ b/t/t8002-blame.sh
@@ -11,4 +11,13 @@ test_expect_success 'Blame --show-email works' '
     check_count "<A@test.git>" 1 "<B@test.git>" 1 "<B1@test.git>" 1 "<B2@test.git>" 1 "<author@example.com>" 1 "<C@test.git>" 1 "<D@test.git>" 1
 '
 
+. "$TEST_DIRECTORY"/date-modes-lib.sh
+date_mode_run_tests "$(date_mode_std_args "
+	b()
+	{
+		git blame -c -L1,1 \"\$@\" '$date_mode_file' |
+			awk -F'\t' '{print \$3}'
+	}
+	b")" blame iso8601
+
 test_done
-- 
1.7.4.18.g68fe8

[RFC 0/5] Date Mode: Add --time-zone; deprecate --date=local

[Michael Witten]

One of the possible values for a date format is `local', which
specifies that a date should be output as though the date format
were instead `default' but in terms of the user's time zone
instead of the time zone stored by git; clearly, then, `local'
does not really provide just another format, but rather the
combination of 2 specifications:

  * A format for the date (`default')
  * A time zone in which to interpret the date (`local', if you will)

Interestingly, the existing documentation suggests that the original
purpose of the `--date' option was to specify the time zone rather
than a format:

  $ git grep -hC1 '`--date=local`\|`--date=default`' \
  >   6ceb270ce6f65cf4bc2a22830f96e0cc838b3fec       \
  >   -- Documentation/rev-list-options.txt
  +
  `--date=local` shows timestamps in user's local timezone.
  +
  --
  +
  `--date=default` shows timestamps in the original timezone
  (either committer's or author's).

This patch series reimplements the original purpose of `--date' by allowing
the time zone mode to be specified independently of the date format
(see the commit message for [2] and the documentation provided by [3]):

  [0] Light refactoring of date infrastructure
  [1] Pretty Print: show tz when using DATE_LOCAL
  [2] Date Mode: Implementation
  [3] Date Mode: Documentation
  [4] Date Mode: Tests

The date format `local' has thus been deprecated, though it is still
supported (with a warning on stderr).

These patches apply cleanly to the current tip of Junio's `next' branch:

  commit 63e4ee5f87eede11d1377370c385c26c5b90c6e7
  Merge: 359ea0b 6ceb270
  Author: Junio C Hamano <gitster@pobox.com>
  Date:   Tue Apr 19 11:51:25 2011 -0700
  
      Merge branch 'master' into next
      
      * master:
        Git 1.7.5-rc3
        Git 1.7.4.5
        git-svn.txt: Document --mergeinfo
  
and it would appear that `make test' runs successfully (including the
extensive testing that I have implemented for date mode features, both
old and new).

There are some whitespace warnings (a la `git diff --check'), but I
have reviewed them and personally approve of them; if you think that
appraisal is incorrect, then you don't know what you're talking about :-P

Sincerely,
Michael Witten

  Documentation/blame-options.txt    |    9 +--
  Documentation/config.txt           |   32 ++++---
  Documentation/date-mode-docs.txt   |   87 +++++++++++++++++++
  Documentation/git-for-each-ref.txt |   19 ++++-
  Documentation/git-log.txt          |   43 ---------
  Documentation/git-rev-list.txt     |    4 +-
  Documentation/git-svn.txt          |    2 +-
  Documentation/rev-list-options.txt |   28 +------
  archive.c                          |    2 +-
  builtin/blame.c                    |   24 +++---
  builtin/cat-file.c                 |    3 +-
  builtin/commit.c                   |    2 +-
  builtin/for-each-ref.c             |   65 ++++++++++----
  builtin/log.c                      |   17 ++--
  builtin/shortlog.c                 |    2 +-
  builtin/show-branch.c              |    3 +-
  cache.h                            |   41 ++++++++-
  commit.h                           |    4 +-
  date.c                             |   84 ++++++++++++++++---
  fast-import.c                      |    4 +-
  http-backend.c                     |    3 +-
  log-tree.c                         |   10 ++-
  pretty.c                           |   18 +++--
  reflog-walk.c                      |    6 +-
  reflog-walk.h                      |    4 +-
  refs.c                             |    6 +-
  revision.c                         |   11 ++-
  revision.h                         |    4 +-
  sha1_name.c                        |    5 +-
  submodule.c                        |    2 +-
  t/date-modes-lib.sh                |  168 ++++++++++++++++++++++++++++++++++++
  t/date-modes-permutator.perl       |   94 ++++++++++++++++++++
  t/t4202-log.sh                     |    3 +
  t/t6300-for-each-ref.sh            |   15 +++-
  t/t8002-blame.sh                   |    9 ++
  test-date.c                        |   11 ++-
  36 files changed, 654 insertions(+), 190 deletions(-)
  create mode 100644 Documentation/date-mode-docs.txt
  create mode 100755 t/date-modes-lib.sh
  create mode 100755 t/date-modes-permutator.perl

-- 
1.7.4.18.g68fe8

Re: [RFC 0/5] Date Mode: Add --time-zone; deprecate --date=local

[Jeff King]

On Wed, Apr 20, 2011 at 02:53:36AM +0000, Michael Witten wrote:

> One of the possible values for a date format is `local', which
> specifies that a date should be output as though the date format
> were instead `default' but in terms of the user's time zone
> instead of the time zone stored by git; clearly, then, `local'
> does not really provide just another format, but rather the
> combination of 2 specifications:
> 
>   * A format for the date (`default')
>   * A time zone in which to interpret the date (`local', if you will)
>
> [...]
>
> This patch series reimplements the original purpose of `--date' by allowing
> the time zone mode to be specified independently of the date format
> (see the commit message for [2] and the documentation provided by [3]):

I think the intent of this series is good. See also this thread from
quite a while back:

  http://article.gmane.org/gmane.comp.version-control.git/112026

> The date format `local' has thus been deprecated, though it is still
> supported (with a warning on stderr).

I don't know if we need to go that far. We can leave it forever as a
historical compatibility feature. Its existence is not really hurting
anyone as long as the documentation marks it as deprecated (or doesn't
even mention it).

> These patches apply cleanly to the current tip of Junio's `next' branch:

Why not "master"? Usually we try to develop features independently on
top of master, and then merge them. That way topics can graduate
independently to master, and it is easier to see which topics are
responsible for breakages. If you really need something in another topic
(because you are directly building on it, or the merge would just be too
painful), then build straight on that topic's commits, not on top of
next (and of course tell everyone which topic it's built on).

> There are some whitespace warnings (a la `git diff --check'), but I
> have reviewed them and personally approve of them; if you think that
> appraisal is incorrect, then you don't know what you're talking about :-P

All of the warnings I saw are related to indentation with spaces, and it
looks like your intent in each case is to line up the opening paren of a
function call with a line of arguments below, like:

   foo(bar, baz
       bleep, moof);

That's fine, style-wise, but the run of spaces should be collapsed into
tabs followed by spaces, with each tab representing 8 spaces (some would
argue that it should be "one tab for each level of structural
indentation, plus spaces to line up the arguments", but I don't find
that we tend to follow such a rule in git).

-Peff

Re: [RFC 0/5] Date Mode: Add --time-zone; deprecate --date=local

[Michael Witten]

On Wed, 20 Apr 2011 02:43:18 -0400, Jeff King wrote:

> On Wed, Apr 20, 2011 at 02:53:36AM +0000, Michael Witten wrote:
>
>> One of the possible values for a date format is `local', which
>> specifies that a date should be output as though the date format
>> were instead `default' but in terms of the user's time zone
>> instead of the time zone stored by git; clearly, then, `local'
>> does not really provide just another format, but rather the
>> combination of 2 specifications:
>> 
>>   * A format for the date (`default')
>>   * A time zone in which to interpret the date (`local', if you will)
>>
>> [...]
>>
>> This patch series reimplements the original purpose of `--date' by allowing
>> the time zone mode to be specified independently of the date format
>> (see the commit message for [2] and the documentation provided by [3]):
>
> I think the intent of this series is good. See also this thread from
> quite a while back:
>
>  http://article.gmane.org/gmane.comp.version-control.git/112026

I took a cursory look, but I've spent so much time on this series already
that I don't really care what it says; if there are particular concerns
that you think I might not have met, then let me know in this discussion :-)

In any case, my approach is not only infintely cleaner than some of the
stuff I saw in that thread, but it's also complete and working (and it is
extensive without being invasive).

Re: [RFC 0/5] Date Mode: Add --time-zone; deprecate --date=local

[Michael Witten]

On Wed, 20 Apr 2011 02:43:18 -0400, Jeff King wrote:

> On Wed, Apr 20, 2011 at 02:53:36AM +0000, Michael Witten wrote:
>
>> The date format `local' has thus been deprecated, though it is still
>> supported (with a warning on stderr).
>
> I don't know if we need to go that far. We can leave it forever as a
> historical compatibility feature. Its existence is not really hurting
> anyone as long as the documentation marks it as deprecated (or doesn't
> even mention it).

The references to a `local' *format* have indeed been removed from
the documentation, and the warning is just an indication that the
code that supports it will also be removed at some point too.

However, the warning mechanism (in general) could be improved by
producing the message only when exiting the program (after the
pager has been destroyed); currently, the message can be quite
easy to miss otherwise (depending on the way the terminal handles
the buffer history).

I don't see the point of leaving crufty code in place forever; it's
ridiculous that my modern CPU starts up like a 35 year old chip
when the very few people who need that functionality (if anybody)
can emulate it if necessary.

As an aside, I've always found it amusing that most programmers
are adamant about justifying an existing use case for a new
feature before even considering an implementation for it, but yet
they will gladly keep around dead code until the next Big Bang.

Re: [RFC 0/5] Date Mode: Add --time-zone; deprecate --date=local

[Michael Witten]

On Wed, 20 Apr 2011 02:43:18 -0400, Jeff King wrote:

> On Wed, Apr 20, 2011 at 02:53:36AM +0000, Michael Witten wrote:
>
>> These patches apply cleanly to the current tip of Junio's `next' branch:
>
> Why not "master"? Usually we try to develop features independently on
> top of master, and then merge them. That way topics can graduate
> independently to master, and it is easier to see which topics are
> responsible for breakages. If you really need something in another topic
> (because you are directly building on it, or the merge would just be too
> painful), then build straight on that topic's commits, not on top of
> next (and of course tell everyone which topic it's built on).

Interesting points.

I was building on `master', but I figured I would expedite the testing
of my patch by making sure that it applies to the recent tip of `next'
without trouble.

In fact, when I did rebase onto `next', there were a couple of minor
conflicts that I had to resolve, so I can safely say that the current
patch series depends upon the following 2 topics:

  commit 84393bfd731c435120dc1dda63432a70124821cb
  Author: Namhyung Kim <namhyung@gmail.com>
  Date:   Wed Apr 6 11:20:50 2011 +0900

      blame: add --abbrev command line option and make it honor core.abbrev
      
      If user sets config.abbrev option, use it as if --abbrev was given.  This
      is the default value and user can override different abbrev length by
      specifying the --abbrev=N command line option.
      
      Signed-off-by: Namhyung Kim <namhyung@gmail.com>
      Signed-off-by: Junio C Hamano <gitster@pobox.com>

  commit ef803fd4b09bca707c7c27669a2789bb050b488c
  Author: Michael J Gruber <git@drmicha.warpmail.net>
  Date:   Fri Apr 1 11:20:31 2011 +0200

      builtin/log.c: separate default and setup of cmd_log_init()
      
      cmd_log_init() sets up some default rev options and then calls
      setup_revisions(), so that a caller cannot set up own defaults: Either
      they get overriden by cmd_log_init() (if set before) or they override
      the command line (if set after). We even complain about this in a
      comment to cmd_log_reflog().
      
      Therefore, separate the two steps so that one can still call
      cmd_log_init() or, alternatively, cmd_log_init_defaults() followed by
      cmd_log_init_finish() (and set defaults in between).
      
      No functional change so far.
      
      Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
      Signed-off-by: Junio C Hamano <gitster@pobox.com>

Those look likely to go into master, so it actually seems like a good idea to
have gone ahead and taken care of the conflict. However, in the future, I will
send patches to `master' only.

Re: [RFC 0/5] Date Mode: Add --time-zone; deprecate --date=local

[Michael Witten]

On Wed, 20 Apr 2011 02:43:18 -0400, Jeff King wrote:

>> There are some whitespace warnings (a la `git diff --check'), but I
>> have reviewed them and personally approve of them; if you think that
>> appraisal is incorrect, then you don't know what you're talking about :-P
>
> All of the warnings I saw are related to indentation with spaces, and it
> looks like your intent in each case is to line up the opening paren of a
> function call with a line of arguments below, like:
>
>   foo(bar, baz
>       bleep, moof);
>
> That's fine, style-wise, but the run of spaces should be collapsed into
> tabs followed by spaces, with each tab representing 8 spaces.

I understand this, as I've read:

  Documentation/CodingGuidelines

and I'm also responsible for this massive git flamewar of yore on the same
subject:

  http://article.gmane.org/gmane.comp.version-control.git/61095
  Message-ID: 634393B0-734A-4884-93E3-42F7D3CB157F@mit.edu

However - and this is the key point - if you are going to be mixing
tabs and spaces ANYWAY, then you might as well do it in a way that
maintains alignment within a tab level regardless of the current
setting for the tabwidth:

> (some would argue that it should be "one tab for each level of
> structural indentation, plus spaces to line up the arguments",
> but I don't find that we tend to follow such a rule in git).

That approach is the most logical and the most robust, and if
somebody messes it up, then the whitespace simply reduces
(or degenerates) back to just the very approach you espouse
anyway.

Tabs and spaces (Re: [RFC 0/5] Date Mode: Add --time-zone; deprecate --date=local)

[Jonathan Nieder]

Hi Michael,

Michael Witten wrote:

> and I'm also responsible for this massive git flamewar of yore on the same
> subject:
> 
>   http://article.gmane.org/gmane.comp.version-control.git/61095
>   Message-ID: 634393B0-734A-4884-93E3-42F7D3CB157F@mit.edu

Heh. :)

> However - and this is the key point - if you are going to be mixing
> tabs and spaces ANYWAY, then you might as well do it in a way that
> maintains alignment within a tab level regardless of the current
> setting for the tabwidth:

In principle, I generally agree.  But as mentioned in the thread you
reference, most text editors don't make that very easy.

I personally use a tabwidth of 6 when I really want to concentrate on
reading.  When coding in a rush for other people, that leads to using
tabstop of 8 and only aligning text that is much shorter than one tab:

	if (foo && bar && baz &&
	    qux && quux) {
		...
	} else if (quuux(quuuux, quuuuux,
				long_expresion_comes_here(quuuuuux))) {
		...
	}

As you can see, this is following the "put continuation lines near the
right margin" convention advocated in linux-2.6's
Documentation/CodingStyle.

Two advantages:

 - looks sensible with any tabstop
 - no need to cascade changes on following lines when the width of a
   function name changes

One major disadvantage:

 - annoys people who like everything nicely lined up.

For what it's worth.  (Not much, of course --- the best rule is as
always to make sure your code fits well with the code around it.)

Hope that helps,
Jonathan

Re: [RFC 0/5] Date Mode: Add --time-zone; deprecate --date=local

[Junio C Hamano]

Michael Witten <mfwitten@gmail.com> writes:

>> I think the intent of this series is good. See also this thread from
>> quite a while back:
>>
>>  http://article.gmane.org/gmane.comp.version-control.git/112026
>
> I took a cursory look, but I've spent so much time on this series already
> that I don't really care what it says...

This is not a very good way to motivate somebody who is already tired at
the end of the day to reivew the RFC series, I would have to say.

Re: Tabs and spaces

[Michael Witten]

On Wed, 20 Apr 2011 19:07:01 -0500, Jonathan Nieder wrote:

>> However - and this is the key point - if you are going to be mixing
>> tabs and spaces ANYWAY, then you might as well do it in a way that
>> maintains alignment within a tab level regardless of the current
>> setting for the tabwidth:
>
> In principle, I generally agree.  But as mentioned in the thread you
> reference, most text editors don't make that very easy.

You just need an editor that can insert into a newly created line the
same leading whitespace characters that were on the previous line (of
course, it's nice if lines with only whitespace are automatically
converted to empty lines); if your text editor can't do that, then any
style of indentation/alignment is going to be painful.

> I personally use a tabwidth of 6 when I really want to concentrate on
> reading.  When coding in a rush for other people, that leads to using
> tabstop of 8 and only aligning text that is much shorter than one tab:
>
> 	if (foo && bar && baz &&
> 	    qux && quux) {
> 		...
> 	} else if (quuux(quuuux, quuuuux,
> 				long_expresion_comes_here(quuuuuux))) {
> 		...
> 	}
>
> As you can see, this is following the "put continuation lines near the
> right margin" convention advocated in linux-2.6's
> Documentation/CodingStyle.

It's not hard to see why "near the right margin" is an important remark;
with a tabstop of 8, your code is rendered like this:

        if (foo && bar && baz &&
            qux && quux) {
                ...
        } else if (quuux(quuuux, quuuuux,
                                long_expresion_comes_here(quuuuuux))) {
                ...
        }

However, with a tabstop of 2, it looks like this:

  if (foo && bar && baz &&
      qux && quux) {
    ...
  } else if (quuux(quuuux, quuuuux,
        long_expresion_comes_here(quuuuuux))) {
    ...
  }

Notice that your `qux && quux' line (which follows the method we both like)
remained nicely aligned, but your `long_expression_comes_here' line has
moved back so much that it doesn't even satisfy the `right margin' rule
anymore.

> Two advantages:
>
>  - looks sensible with any tabstop

As we have seen, the sensibility of using tabs for *alignment* is precarious;
on the contrary, using spaces for alignment [across lines of the same level of
tabular indentation] remains *exactly* as sensible as intended by the original
author, regardless of the tabstop setting.

The problem is that there are really two issues that are being conflated"

	* Indentation : This can be implemented with tabs
	* Alignment   : This should be implemented with only spaces

> - no need to cascade changes on following lines when the width of a
>   function name changes

That is indeed the best reason to avoid alignment (after all, by its
very nature, alignment implies a coupling between lines), and I find
myself in constant intellectual conflict over that very issue. However,
if the number of lines to be aligned is rather small or alignment
obviously helps readability, then it's a rather minor risk.

Now, your example of a changing function name is a good one; for instance,
consider one of the functions that my patch series introduces:

	int parse_date_mode_config_internal(const char *var_date,
	                                     const char *var_time_zone,
	                                     const char *var,
	                                     const char *value,
	                                     struct date_mode *d,
	                                     int *ret);

I just noticed that the parameters are misaligned by one space; it should
be the following:

	int parse_date_mode_config_internal(const char *var_date,
	                                    const char *var_time_zone,
	                                    const char *var,
	                                    const char *value,
	                                    struct date_mode *d,
	                                    int *ret);

I probably changed the function name by 1 character at some point and then
forgot to update the alignment of the parameters. :-(

Now, I actually hate that style of continuation, and I only used it because
that's basically what everybody else on the planet (and in the git project)
tries to do (albeit with some apalling combination of tabs and spaces).

In such a case, I would much rather treat parameters as `subelements' of
the function construct (prototype, call, etc.) by explicitly giving them
their own indentation level; this transforms the problem from one of
indentation and alignment to a problem of just indentation, thereby allowing
for nothing but tabs:

	int parse_date_mode_config_internal
	(
		const char *var_date,
		const char *var_time_zone,
		const char *var,
		const char *value,
		struct date_mode *d,
		int *ret
	);

A function call might look like this:

	parse_date_mode_config_internal
	(
		"Some value of some sort",
		"Another value of some sort",
		"dingleberry",
		d,
		ret
	);

Of course, this opens up the debate about where the opening parenthesis should
go; if the `{' of a block is routinely put on the same line as, say, an `if',
then perhaps this would look more natural:

	parse_date_mode_config_internal (
		"Some value of some sort",
		"Another value of some sort",
		"dingleberry",
		date_mode,
		return_value
	);

An `if' statement is kind of gross to mold in this way due to the fact that
there are two parts, each with its own subelements; at some point, you might
as well just make some temporary variables with reasonably short names in
order to hold the condition value, thereby obviating this nightmare in the
first place.

However, if you must write every expression explicitly in place, then your
example could be written thusly:

	if (
 		foo && bar &&
 		baz && qux &&
		quux
	){
 		...
	} else if (
		quuux (
			quuuux,
			quuuuux,
			long_expresion_comes_here (
				quuuuuux
			)
		)
	){
		...
	}

or more drawn out:

	if
	(
 		foo &&
 		bar &&
 		baz &&
		qux &&
		quux
	)
	{
 		...
	}
	else if
	(
		quuux
		(
			quuuux,
			quuuuux,
			long_expresion_comes_here
			(
				quuuuuux
			)
		)
	)
	{
		...
	}

Again, it should be recognized that there are 2 issues:

	* Indentation : This can be implemented with tabs
	* Alignment   : This should be implemented with only spaces

and it should also be recognized that treating tabs as a primitive
means of space-saving compression by having it always represent
some constant number of spaces (8) is just as flaky as expecting
people to properly use spaces for alignment; thus you might as
well go for the latter in order to get the best of both worlds,
because somebody is going to mess it up either way.

Sincerely,
	Michael
		Witten

Re: [RFC 0/5] Date Mode: Add --time-zone; deprecate --date=local

[Michael Witten]

On Thu, Apr 21, 2011 at 01:50, Junio C Hamano <gitster@pobox.com> wrote:
> Michael Witten <mfwitten@gmail.com> writes:
>
>>> I think the intent of this series is good. See also this thread from
>>> quite a while back:
>>>
>>>  http://article.gmane.org/gmane.comp.version-control.git/112026
>>
>> I took a cursory look, but I've spent so much time on this series already
>> that I don't really care what it says...
>
> This is not a very good way to motivate somebody who is already tired at
> the end of the day to reivew the RFC series, I would have to say.

Then you can save us both time and just apply my series immediately :-P

You're taking it wholly out of context; the full sentence is indeed:
"I took a cursory look, but I've spent so much time on this series
already that I don't really care what it says; if there are particular
concerns that you think I might not have met, then let me know in this
discussion :-)"

Two points, though:

  * Mentioning another thread is not in any
    way a review of this current patch series.

  * I'm passed the phase of worrying about which
    cases I might have missed; that's why I
    have submitted my work to the list in order
    to give less-weary eyes a chance to think
    about it.

Re: Tabs and spaces

[Jonathan Nieder]

Michael Witten wrote:

> You just need an editor that [...]

which many contributors don't have (or haven't configured that way).
Really.

[...]
> However, with a tabstop of 2, it looks like this:
>
>   if (foo && bar && baz &&
>       qux && quux) {
>     ...
>   } else if (quuux(quuuux, quuuuux,
>         long_expresion_comes_here(quuuuuux))) {
>     ...
>   }
>
> Notice that your `qux && quux' line (which follows the method we both like)
> remained nicely aligned, but your `long_expression_comes_here' line has
> moved back so much that it doesn't even satisfy the `right margin' rule
> anymore.

Oh, I still think a tabstop of 2 is insane (for various reasons, some
explained in the thread you mentioned).

No matter --- Junio will use --whitespace=fixup to convert your spaces
to tabs anyway.  Just thought I could give some survival tips.

Regards,
Jonathan

Re: Tabs and spaces

[Michael Witten]

On Thu, Apr 21, 2011 at 02:18, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Michael Witten wrote:
>
>> You just need an editor that [...]
>
> which many contributors don't have (or haven't configured that way).
> Really.

to which I already said: "if your text editor can't do that, then any
style of indentation/alignment is going to be painful."

>
> [...]
>> However, with a tabstop of 2, it looks like this:
>>
>>   if (foo && bar && baz &&
>>       qux && quux) {
>>     ...
>>   } else if (quuux(quuuux, quuuuux,
>>         long_expresion_comes_here(quuuuuux))) {
>>     ...
>>   }
>>
>> Notice that your `qux && quux' line (which follows the method we both like)
>> remained nicely aligned, but your `long_expression_comes_here' line has
>> moved back so much that it doesn't even satisfy the `right margin' rule
>> anymore.
>
> Oh, I still think a tabstop of 2 is insane (for various reasons, some
> explained in the thread you mentioned).

That's not really the point, though, is it.

> No matter --- Junio will use --whitespace=fixup to convert your spaces
> to tabs anway.

Good!

That means I can be satisfied with having written sane patches, and
Junio can be satisfied with having forced the insane use of tabs. It's
a win-win :-D

Re: Tabs and spaces

[Thiago Farina]

On Thu, Apr 21, 2011 at 12:15 AM, Michael Witten <mfwitten@gmail.com> wrote:
>> Oh, I still think a tabstop of 2 is insane (for various reasons, some
>> explained in the thread you mentioned).
>
tabstop of 2 is what we use on Chromium code (with spaces, no tabs).
And works very well. Just my 0.02 cents. ;)

Re: [RFC 0/5] Date Mode: Add --time-zone; deprecate --date=local

[Junio C Hamano]

Michael Witten <mfwitten@gmail.com> writes:

>>> I took a cursory look, but I've spent so much time on this series already
>>> that I don't really care what it says...
>>
>> This is not a very good way to motivate somebody who is already tired at
>> the end of the day to reivew the RFC series, I would have to say.
>
> Then you can save us both time and just apply my series immediately :-P

That's not how things work around here.

The default is not to apply, unless the goal of the change is worthy and
the patch implements that change well.  It is submitter's job to convince
people that the change is worthy, justify that it is worth reviewers' time
to review the patch, and respond to questions and suggestions for
improvements.

By doing these responsibly, you win support for the particular patchset,
and win trust from others in you, which would affect further patches from
you.

It is not exactly a good way to win support to dump a patchset, to declare
that you are beyond the point of rethinking, to say a take-it-or-leave-it,
nor to defend your private style that goes against the project's coding
style (style is subjective and there is no right or wrong).

I know you know all of the above from your ":-P" (and your past patch
submissions), but I am writing these down so that people new to the list
do not get a wrong impression from this exchange.

Re: [RFC 0/5] Date Mode: Add --time-zone; deprecate --date=local

[Michael Witten]

On Thu, Apr 21, 2011 at 03:57, Junio C Hamano <gitster@pobox.com> wrote:

> It is not exactly a good way to win support to dump a patchset, to declare
> that you are beyond the point of rethinking, to say a take-it-or-leave-it,
> nor to defend your private style that goes against the project's coding
> style (style is subjective and there is no right or wrong).

Well, I wanted to respond here with:

    "But I did nothing of the sort!"

However, you stole my thunder:

> I know you know all of the above from your ":-P" (and your past patch
> submissions), but I am writing these down so that people new to the list
> do not get a wrong impression from this exchange.

Firstly, I'm not sure that this exchange has been that bad; perhaps
one of us perceives too little or too much when there is just email
text.

Secondly, don't you think that particular worry is a bit paranoid?

Re: Tabs and spaces

[Alex Riesen]

On Thu, Apr 21, 2011 at 05:25, Thiago Farina <tfransosi@gmail.com> wrote:
> On Thu, Apr 21, 2011 at 12:15 AM, Michael Witten <mfwitten@gmail.com> wrote:
>>> Oh, I still think a tabstop of 2 is insane (for various reasons, some
>>> explained in the thread you mentioned).
>>
> tabstop of 2 is what we use on Chromium code (with spaces, no tabs).

It's indentation. You indent your code by 2 space.
It has nothing to do with tabstop.

Re: Tabs and spaces

[Michael Witten]

On Thu, Apr 21, 2011 at 10:46, Alex Riesen <raa.lkml@gmail.com> wrote:

> On Thu, Apr 21, 2011 at 05:25, Thiago Farina <tfransosi@gmail.com> wrote:
>> On Thu, Apr 21, 2011 at 12:15 AM, Michael Witten <mfwitten@gmail.com> wrote:
>>>> Oh, I still think a tabstop of 2 is insane (for various reasons, some
>>>> explained in the thread you mentioned).
>>>
>> tabstop of 2 is what we use on Chromium code (with spaces, no tabs).
>
> It's indentation. You indent your code by 2 space.
> It has nothing to do with tabstop.

In this discussion, it has everything to do with the tabstops
(1235e29d-6cbb-445b-9b6f-4e174c03ba8f-mfwitten@gmail.com):

> Again, it should be recognized that there are 2 issues:
>
>        * Indentation : This *can* be implemented with tabs
>        * Alignment   : This *should* be implemented with only spaces
>
> and it should also be recognized that treating tabs as a primitive
> means of space-saving compression by having it always represent
> some constant number of spaces (8) is just as flaky as expecting
> people to properly use spaces for alignment; thus you might as
> well go for the latter in order to get the best of both worlds,
> because somebody is going to mess it up either way.

Re: [RFC 2/5] Pretty Print: show tz when using DATE_LOCAL

[Junio C Hamano]

Michael Witten <mfwitten@gmail.com> writes:

> Date: Fri, 11 Feb 2011 16:06:36 +0000
> Currently, when the date mode is DATE_LOCAL, the
> time zone is never pretty printed;...

I am not opposed to have a new mode that shows timestamps in local
timezone with zone information, but this change is a regression to people
who have known and relied upon that asking for timestamps in their local
timezone will give everything in the local timezone and result in a short
output by the virtue of not needing to repeat the zone string, as they are
the same (this is not strictly true near your dst boundary, and that is
why I am open to the idea as a separate option).

This is a tangent but it is funny to see the unnecessary Date: in-body
header for a series about date display.  Please drop it.

Backdating the author timestamp will make it harder to find the related
discussion from the list archive; the only plausible benefit I can see is
that you may get "I thought of this much earlier than when I posted it to
the public for the first time" pee-in-the-snow value out of doing so, but
that is done at the cost to all others who need to inspect the history
later.  Please don't.

As a future reference, when you have a valid reason to override the
header information your MUA would give your message with an in-body
header, please leave a blank line after the in-body header to make the
result easier to read, like this:

        Date: Fri, 11 Feb 2011 16:06:36 +0000

        Currently, when the date mode is DATE_LOCAL, the
        time zone is never pretty printed;...

Also paragraphs that wrap lines at too narrow a margin is just as hard to
read as paragraphs wrapped at a margin that is too wide.  You seem to have
wrapped at around 50 columns?

Re: [RFC 3/5] Date Mode: Implementation

[Junio C Hamano]

Michael Witten <mfwitten@gmail.com> writes:

> Date: Sat, 12 Feb 2011 03:56:17 +0000
> This introduces and employs infrastructure for manipulating
> and bundling time zone and date format specifications in
> code, on the command line, and in git configuration files.
>
> In particular, this commit includes the following:

Even though I think it is going in the right direction to separate "in
what timezone do we show timestamps (original vs user-specified)" and "in
what format do we show them (short? with tz? like in e-mail? iso? etc.),
this particular commit seems to do too many things at once.

Can you think of a way split this into smaller steps?  I am not asking
"Could you pretty please split this?" nor "Are you willing to do so?"
I am asking if the task is possible to begin with.

A valid answer could be "everything is so intertwined that this patch is
the smallest logical unit of change", and I would be somewhat sympathetic
to that.  Once you change the type of revs->date_mode from the current
single scalar to a struct, all the callers need to change the way they
initialize and update the field, and I understand that such a change would
have to touch a major part of the system.

But if that step can be done without changing the semantics nor behaviour
at all, it would be easier to review and verify. The the next patch can
enhance the struct in revs->date_mode to express timezone information as a
new field, opening the door to the new command line option.

> Essentially, what was:
>
>   git log --date=local
>
> should now be:
>
>   git log --time-zone=local

The above _should_ read something like:

	"git log --date=local"

        is now a short-hand for

        "git log --date-format=local --time-zone=local"

        It specifies that the timestamps should be stringified using the
        local timezone, and the value should be shown like "Tue Apr 19
        12:34:56 2011" without the timezone.

        "git log --time-zone=local" would give the same output but you
        will also see the timezone, e.g. "Tue Apr 19 12:34:56 2011 -0800",
        because --date-format defaults to "default".

In other words, this new feature should allow people who do not care about
it to keep saying "--date=local" without being told that they now _should_
say things differently.

If it is absolutely impossible to implement the separation between the
timezone and the format without breaking existing option handling, we
would have to consider if the new feature (i.e. separation of zone and
format) is worth breaking scripts and existing users, and we may end up
rejecting the implementation of new feature.  I however do think this
particular feature is possible to implement without such a regression.

This patch makes the date-related infrastructure rich enough to deserve
its own "date.h" that is included by "cache.h" near the beginning of it.
The documentation part in [4/5] has such a separation, which I think is a
sensible thing to do, except that date-mode-docs.txt is probably a wrong
name for the file.  If you followed the precedence set by most other
includable pieces about the set of options, you would probably call it
date-opts.txt or something.

> diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
> index 60d6b32..add9299 100644
> --- a/builtin/for-each-ref.c
> +++ b/builtin/for-each-ref.c
> @@ -361,26 +360,58 @@ static const char *copy_email(const char *buf)
>  	return xmemdupz(email, eoemail + 1 - email);
>  }
>  
> +#define PARSE_DATE_MODE(marker_0, parser_0, date_mode_member_0, \
> +                        marker_1, parser_1, date_mode_member_1) \
> +	if (*atomname == marker_0) { \
> +		const char *spec_0 = ++atomname; \
> +		do { \
> +			if (*atomname == marker_1) { \
> +				date_mode_member_1 = parser_1(atomname+1); \
> +				char *spec_0_dup  = xmemdupz(spec_0, atomname - spec_0); \
> +				date_mode_member_0 = parser_0(spec_0_dup); \
> +				free(spec_0_dup); \
> +				goto finish; \
> +			} \

This introduces:

builtin/for-each-ref.c:386: error: ISO C90 forbids mixed declarations and code
builtin/for-each-ref.c:390: error: ISO C90 forbids mixed declarations and code

> diff --git a/date.c b/date.c
> index caa14fe..879b0e1 100644
> --- a/date.c
> +++ b/date.c
> @@ -147,40 +147,42 @@ const char *show_date_relative(unsigned long time, int tz,
>  	return timebuf;
>  }
>  
> -const char *show_date(unsigned long time, int tz, enum date_mode mode)
> +const char *show_date(unsigned long time, int tz, struct date_mode date_mode)
>  {
> ...

I may be being old-fashioned, but I'd really prefer to see structures
passed by pointer, not by value, even if the structure starts out as a
small container for just two enum fields.  There are a few other places
you pass this structure by value.

> @@ -653,7 +655,7 @@ int parse_date(const char *date, char *result, int maxlen)
>  	return date_string(timestamp, offset, result, maxlen);
>  }
>  
> -enum date_mode parse_date_format(const char *format)
> +enum date_format parse_date_format(const char *format)
>  {
>  	if (!strcmp(format, "relative"))
>  		return DATE_RELATIVE;
> @@ -675,6 +677,67 @@ enum date_mode parse_date_format(const char *format)
>  		die("unknown date format %s", format);
>  }
>  
> +enum time_zone parse_time_zone(const char *time_zone)
> +{
> +	if (!strcmp(time_zone, "local"))
> +		return TIME_ZONE_LOCAL;
> +	else if (!strcmp(time_zone, "default"))
> +		return TIME_ZONE_DEFAULT;
> +	else
> +		die("unknown time zone %s", time_zone);
> +}

Hmm, I was hoping that we can enhance this to allow feeding a string
(e.g. "GMT", "+0900"), but "enum time_zone" makes it almost impossible.

> +int parse_date_mode_config_internal(const char *var_date,
> +                                     const char *var_timezone,
> +                                     const char *var,
> +                                     const char *value,
> +                                     struct date_mode *d,
> +                                     int *ret)
> +{

I'd rather see you drop the preprocessor trick to synthesize "blame.date"
and "blame.timezone" out of "blame", drop _internal from this function
(which is not static to the file scope to begin with), and take a single
"const char *var" as the first parameter.  Otherwise, future users of the
API cannot pass a variable to (your version of) parse_date_mode_config().
It is not implausible that someday somebody might want to parse per-branch
timezone specification out of "branch.master.timezone", and most likely
"branch.master" part would be created runtime in a strbuf, not limited to
hardcoded program names such as "blame".

No other config callback function uses "a pointer to int as an argument to
store the return value"; if you absolutely need a new calling convention,
it needs to be documented better.  What do *ret and the return value from
this function mean from the caller's point of view?

Thanks.

Re: [RFC 5/5] Date Mode: Tests

[Junio C Hamano]

I'd like to have part of the tests in this patch at the beginning of the
series to document and protect the behaviour of the current --date related
options, then your enhancement that adds --zone that lets the users
specify the timezone and the format independently, and finally the
remainder of this patch as an addition to the test script to document and
protect the interaction between the two options (e.g. what happens when
none or only one is specified? what happens when conflicting options such
as "--date=local --zone=gmt" is given?).

Thanks.

Dates in Commits and other issues of style (Re: [RFC 2/5] Pretty Print: show tz when using DATE_LOCAL)

[Michael Witten]

On Thu, 21 Apr 2011 15:34:49 -0700, Junio C Hamano wrote:

> Michael Witten <mfwitten@gmail.com> writes:
>
>> Date: Fri, 11 Feb 2011 16:06:36 +0000
>> Currently, when the date mode is DATE_LOCAL, the
>> time zone is never pretty printed;...
>
> This is a tangent but it is funny to see the unnecessary Date: in-body
> header for a series about date display.  Please drop it.

No.

> Backdating the author timestamp will make it harder to find the related
> discussion from the list archive; the only plausible benefit I can see is
> that you may get "I thought of this much earlier than when I posted it to
> the public for the first time" pee-in-the-snow value out of doing so, but
> that is done at the cost to all others who need to inspect the history
> later.  Please don't.

What if I had submitted a pull request instead of inlined patches? Would
you be asking me to wipe the dates in my repository? Would you rewrite
the commits on your end?

Let's suppose that I like backdating specifically for the "pee-in-the-snow"
value; well, that's one of the prime motivators for doing unpaid, volunteer
work, and that's one of the reasons that distributed SCM tools like git are
so great: Unlike with, say, CVS, the actual author gets his or her information
officially recorded.

Perhaps you think we should dispense with identity information as well, given
that it's just a pee-in-the-snow value...

BUT WAIT!

Names and email addresses are important because of copyright issues; we need
to know whence came a contribution, after all.

Well... don't you think the particular date at which something was written
might be similarly valuable in a dispute over copyright?

Junio, you'll take my pee-in-the-snow and you'll like it.

> As a future reference, when you have a valid reason to override the
> header information your MUA would give your message with an in-body
> header, please leave a blank line after the in-body header to make the
> result easier to read, like this:
>
>         Date: Fri, 11 Feb 2011 16:06:36 +0000
>
>         Currently, when the date mode is DATE_LOCAL, the
>         time zone is never pretty printed;...

Fair enough.

> Also paragraphs that wrap lines at too narrow a margin is just as hard to
> read as paragraphs wrapped at a margin that is too wide.

I disagree that
it's too narrow,
and I feel like
you are now
nitpicking.

Sincerely,
Michael Witten

Re: [RFC 2/5] Pretty Print: show tz when using DATE_LOCAL

[Michael Witten]

On Thu, 21 Apr 2011 15:34:49 -0700, Junio C Hamano wrote:

> Michael Witten <mfwitten@gmail.com> writes:
>
>> Date: Fri, 11 Feb 2011 16:06:36 +0000
>> Currently, when the date mode is DATE_LOCAL, the
>> time zone is never pretty printed;...
>
> I am not opposed to have a new mode that shows timestamps in local
> timezone with zone information, but this change is a regression to people
> who have known and relied upon that asking for timestamps in their local
> timezone will give everything in the local timezone and result in a short
> output by the virtue of not needing to repeat the zone string, as they are
> the same (this is not strictly true near your dst boundary, and that is
> why I am open to the idea as a separate option).

Allow me to restate what I think you are saying:

How about having [the still deprecated] --date=local (that is, DATE_LOCAL)
option produce the old behavior, but have every combination of date mode
options that does not involve DATE_LOCAL always emit the time zone.

In other words, all of these do not emit a time zone:

  --date=local
  --date=local --time-zone=local
  --date=local --time-zone=default   # --time-zone is ignored for simplicity.

and all of these always emit a time zone:

  --date=default
  --date=rfc
  --date=rfc2822
  --date=iso
  --date=iso8601
  --date=short
  --date=raw
  --date=default --time-zone=default
  --date=rfc     --time-zone=default
  --date=rfc2822 --time-zone=default
  --date=iso     --time-zone=default
  --date=iso8601 --time-zone=default
  --date=short   --time-zone=default
  --date=raw     --time-zone=default
  --date=default --time-zone=local
  --date=rfc2822 --time-zone=local
  --date=rfc     --time-zone=local
  --date=iso     --time-zone=local
  --date=iso8601 --time-zone=local
  --date=short   --time-zone=local
  --date=raw     --time-zone=local
                 --time-zone=local
                 --time-zone=default

Re: [RFC 2/5] Pretty Print: show tz when using DATE_LOCAL

[Junio C Hamano]

Michael Witten <mfwitten@gmail.com> writes:

> Allow me to restate what I think you are saying:
>
> How about having [the still deprecated] --date=local (that is, DATE_LOCAL)
> option produce the old behavior, but have every combination of date mode
> options that does not involve DATE_LOCAL always emit the time zone.
>
> In other words, all of these do not emit a time zone:
>
>   --date=local
>   --date=local --time-zone=local
>   --date=local --time-zone=default   # --time-zone is ignored for simplicity.

Yeah.  It is even plausible to extend --date further so that we can
express things like

    --date='%a %b %d %H:%M:%S %Y'
    --date='%a %b %d %H:%M:%S %Y %z'

The former would be equivalent to --date[-format]=local, the latter would
be equivalent to --date[-format]=default.

Or something like that [warning: without morning caffeine yet] :-) 

Re: [RFC 5/5] Date Mode: Tests

[Michael Witten]

On Thu, 21 Apr 2011 15:44:41 -0700, Junio C Hamano wrote:

> I'd like to have part of the tests in this patch at the beginning of the
> series to document and protect the behaviour of the current --date related
> options, then your enhancement that adds --zone that lets the users
> specify the timezone and the format independently, and finally the
> remainder of this patch as an addition to the test script to document and
> protect the interaction between the two options (e.g. what happens when
> none or only one is specified? what happens when conflicting options such
> as "--date=local --zone=gmt" is given?).

I would split it up as requested, but I'm not sure that it's worthwhile; while
there is a certain appeal to such a natural progression, my feeling is that
the separate patches won't turn out to be usefully smaller (and thus more
easily verified, as I'm sure you desire), because every data structure and
code path deals with both simultaneously.

Moreover, the beauty of your proposed progression is lost on the fact that
these patches will be applied so close in time to each other; why bother
dealing with solely date mode formats when such a version will almost
certainly never be used?

Thus, it seems cleaner just to introduce the whole set of tests in one go.

Time zone option name (Re: [RFC 5/5] Date Mode: Tests)

[Michael Witten]

On Thu, 21 Apr 2011 15:44:41 -0700, Junio C Hamano wrote:

> I'd like to have part of the tests in this patch at the beginning of the
> series to document and protect the behaviour of the current --date related
> options, then your enhancement that adds --zone that lets the users
> specify the timezone and the format independently, and finally the
> remainder of this patch as an addition to the test script to document and
> protect the interaction between the two options (e.g. what happens when
> none or only one is specified? what happens when conflicting options such
> as "--date=local --zone=gmt" is given?).

I'd like to clarify what you intend the new time zone option to be named; my
patch series currently uses `--time-zone' (which is admittedly long), but it
would appear here that you find `--zone' attractive. My only qualm there is
that `zone' is such a rich word; not only is it somewhat incomprehensible out
of context, but it also might be useful for something else in the future.

Re: [RFC 5/5] Date Mode: Tests

[Michael Witten]

On Wed, 20 Apr 2011 02:45:23 +0000, Michael Witten wrote:

> +	foreach (@formats) {
> +		$expected{''} = $expected{$ARGV[2]};
> +	}

Woops!

Well, I guess nobody read through this patch (including myself) :-P

Re: [RFC 5/5] Date Mode: Tests

[Michael Witten]

On Sat, 23 Apr 2011 03:42:24 +0000

> On Thu, 21 Apr 2011 15:44:41 -0700, Junio C Hamano wrote:
>
>> I'd like to have part of the tests in this patch at the beginning of the
>> series to document and protect the behaviour of the current --date related
>> options, then your enhancement that adds --zone that lets the users
>> specify the timezone and the format independently, and finally the
>> remainder of this patch as an addition to the test script to document and
>> protect the interaction between the two options (e.g. what happens when
>> none or only one is specified? what happens when conflicting options such
>> as "--date=local --zone=gmt" is given?).
>
> I would split it up as requested, but I'm not sure that it's worthwhile;
> while there is a certain appeal to such a natural progression, my feeling
> is that the separate patches won't turn out to be usefully smaller (and
> thus more easily verified, as I'm sure you desire), because every data
> structure and code path deals with both simultaneously.

Maybe I spoke to soon; let me play around with it.

Still, though:

> Moreover, the beauty of your proposed progression is lost on the fact that
> these patches will be applied so close in time to each other; why bother
> dealing with solely date mode formats when such a version will almost
> certainly never be used?
>
> Thus, it seems cleaner just to introduce the whole set of tests in one go.

Re: Time zone option name (Re: [RFC 5/5] Date Mode: Tests)

[Junio C Hamano]

Michael Witten <mfwitten@gmail.com> writes:

> I'd like to clarify what you intend the new time zone option to be named; my
> patch series currently uses `--time-zone' (which is admittedly long), but it
> would appear here that you find `--zone' attractive.

Sorry, that was just me being sloppy, writing barely enough for you and
readers to be able to tell which one of the two options you used in your
patch is what I was referring to.

The word "zone" may mean something other than timezone in future versions
of git, and I do not want to see us regret this feature squatting on a
short and sweet "--zone" option if/when it happens.  Your "--time-zone"
(or a single-word "--timezone") would be far better.

The "--date={local,default,short,...}" option might want to have a longer
synonym "--date-format={...ditto...}", as some commands take the value of
the timestamp as a parameter to their "--date=" option, but that is minor
and definitely should not be part of this series.

Thanks.

Re: Dates in Commits and other issues of style (Re: [RFC 2/5] Pretty Print: show tz when using DATE_LOCAL)

[Miles Bader]

Michael Witten <mfwitten@gmail.com> writes:
> and I feel like
> you are now
> nitpicking.

Wait, isn't nitpicking his job?!

-miles

-- 
Apologize, v. To lay the foundation for a future offense.

Re: Dates in Commits and other issues of style (Re: [RFC 2/5] Pretty Print: show tz when using DATE_LOCAL)

[Junio C Hamano]

Miles Bader <miles@gnu.org> writes:

> Michael Witten <mfwitten@gmail.com> writes:
>> and I feel like
>> you are now
>> nitpicking.
>
> Wait, isn't nitpicking his job?!

Enforcing consistency is one of the important tasks the maintainers do in
their projects.  Besides ensuring that the intent of the change each patch
brings to the codebase is good, that the log entry describes the change in
a useful way for future readers, and that the patch correctly implements
the described change, we also need to make sure that the resulting code
matches the style of the surrounding code, and the style, structure and
tone the log messages are delivered in a consistent voice.  Otherwise it
would quickly get very tiring when you have to dig into the history of the
codebase.  The code and the history are read a lot more often than are
written.

When a casual and occasional contributor sends in a good change whose only
sin is style violation, I do not mind touching up its proposed commit log
message or the patch text, and I often do.

But I expect contributors who return here often to be more considerate
than one-time contributors, and that is why I give style reminders.

The purpose of the style reminder is so that they can help me and other
reviewers concentrate on the substance (Is what it does good? Is it
explained well? Is it implemented well?) without having to spend
unnecessary time fixing the form (Does the new code fit well with the
surrounding code? Does the message flow well in "git log" and easy to
understand?) when they submit their changes the next time.

Please do not mistake a style reminder as an opportunity to promote your
own style that would not match what we have established here.  I could
make a black-list of people I should avoid giving style reminders and
instead fix all of their submissions silently, because giving style
remainders to them will waste people's time by creating a discussion
thread like this one.

I really wish I do not have to do so.

Such an arrangement would not scale.  Given two sets of patches with equal
goodness in substance, if one also matches our style and the other
deliberately asks me to spend extra time to whip it into our style, the
latter naturally has to be assigned a lower priority.  After all, there is
only 24 hours in a day, and my time is better spent on substance not form,
and definitely not on responding to quibblings about styles.

The only two "workable" solutions are either (1) everybody tries to be
consistent with the project's style, or (2) allow everybody to stick to
their own style, making the resulting code and history unpleasant to read.

I'd be failing the community if I took the latter approach.

Re: Dates in Commits and other issues of style (Re: [RFC 2/5] Pretty Print: show tz when using DATE_LOCAL)

[Jakub Narebski]

Junio C Hamano <gitster@pobox.com> writes:

> Enforcing consistency is one of the important tasks the maintainers do in
> their projects.  Besides ensuring that the intent of the change each patch
> brings to the codebase is good, that the log entry describes the change in
> a useful way for future readers, and that the patch correctly implements
> the described change, we also need to make sure that the resulting code
> matches the style of the surrounding code, and the style, structure and
> tone the log messages are delivered in a consistent voice.  Otherwise it
> would quickly get very tiring when you have to dig into the history of the
> codebase.  The code and the history are read a lot more often than are
> written. [...]

This information should be put e.g. in SubmittingPatches, or
CodingGuidelines, or MaintNotes, isn't it?

-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: Dates in Commits and other issues of style (Re: [RFC 2/5] Pretty Print: show tz when using DATE_LOCAL)

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Enforcing consistency is one of the important tasks the maintainers do...
>
> This information should be put e.g. in SubmittingPatches, or
> CodingGuidelines, or MaintNotes, isn't it?

The above "we value consistency" should be already inferrable by reading
SubmittingPatches and CodingStyle in reverse?  If it were up to the
contributors to ignore them, there is no point to have these guides to
begin with.

If we want to keep these words somewhere, I think the place to put it
would probably be the Policy section of Doc/howto/maintain-git.txt.