git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH/RFC 0/9] add long forms for format placeholders
@ 2011-03-28 23:17 Will Palmer
  2011-03-28 23:17 ` [PATCH/RFC 1/9] mention --date=raw in rev-list and blame help Will Palmer
                   ` (9 more replies)
  0 siblings, 10 replies; 14+ messages in thread
From: Will Palmer @ 2011-03-28 23:17 UTC (permalink / raw
  To: git; +Cc: Junio C Hamano, Jeff King, Will Palmer

I've been kicking around this series for a while now as part of a larger
effort of refactoring the pretty formats. A recent discussion on the
list has lead me to believe that this smaller subset may be of use
sooner, rather than later.

This series attempts to add "long forms" for common format placeholders
in the "git log" family of commands, making the way for yet more
placeholders to be added without needing to worry too much about the
increasingly limited set of available one-letter mnemonics. It also
moves towards the possibility of eventual unification with the format
options in for-each-ref.

For example: after this series, in place of %ad, %at, etc you would be
able to use %(authordate), %(authordate:unix), etc.

There are some changes in this series which may not seem justified at
this point, most noticeably the splitting of user-defined format
processing into separate "parse" and "format" stages. This is done to
allow for more-complicated formats to be defined, especially conditional
formats. Examples of these are not yet included in this series, because
this series only includes those parts related to allowing for the
"long form" placeholders. If this series needs to go through a few
cycles before it's ready for inclusion, examples more-complicated
formats may find their way into the series.

The primary purpose of the series is to allow new placeholders to be
defined without worrying about the increasingly diminishing set of
available one-or-two-letter mnemonics.

Cc: Jeff King <peff@peff.net>
Cc: Junio C Hamano <gitster@pobox.com>

Will Palmer (9):
  mention --date=raw in rev-list and blame help
  add support for --date=unix to complement %at
  interpret %C(invalid) as we would %%C(invalid)
  add sanity length check to format_person_part
  refactor pretty.c into "parse" and "format" steps
  add long-form %(wrap:...) for %w(...)
  add long form %(color:...) for %C(...)
  add long forms %(authordate) and %(committerdate)
  add long forms for author and committer identity

 .gitignore                         |    1 +
 Documentation/blame-options.txt    |    4 +-
 Documentation/git-rev-list.txt     |    2 +-
 Documentation/pretty-formats.txt   |   51 ++-
 Documentation/rev-list-options.txt |    4 +-
 Makefile                           |    1 +
 builtin/blame.c                    |    3 +
 cache.h                            |    5 +-
 color.c                            |   17 +-
 color.h                            |    1 +
 commit.h                           |   84 ++++
 date.c                             |   61 ++-
 pretty.c                           |  947 +++++++++++++++++++++++++++---------
 test-pretty.c                      |  250 ++++++++++
 14 files changed, 1164 insertions(+), 267 deletions(-)
 create mode 100644 test-pretty.c

-- 
1.7.4.2

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH/RFC 1/9] mention --date=raw in rev-list and blame help
  2011-03-28 23:17 [PATCH/RFC 0/9] add long forms for format placeholders Will Palmer
@ 2011-03-28 23:17 ` Will Palmer
  2011-03-28 23:17 ` [PATCH/RFC 2/9] add support for --date=unix to complement %at Will Palmer
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Will Palmer @ 2011-03-28 23:17 UTC (permalink / raw
  To: git; +Cc: Junio C Hamano, Jeff King, Will Palmer

Very trivial: the documentation for "blame" and "rev-list" failed to
mention the --date=raw option in the summary of the --date option.
Here we correct that.

Signed-off-by: Will Palmer <wmpalmer@gmail.com>
---
 Documentation/blame-options.txt |    4 ++--
 Documentation/git-rev-list.txt  |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index 16e3c68..e11a3da 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -72,8 +72,8 @@ of lines before or after the line given by <start>.
 
 --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
+	{relative,local,default,iso,rfc,short,raw}. 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].
diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt
index 8e1e329..ce3692c 100644
--- a/Documentation/git-rev-list.txt
+++ b/Documentation/git-rev-list.txt
@@ -37,7 +37,7 @@ SYNOPSIS
 	     [ \--regexp-ignore-case | -i ]
 	     [ \--extended-regexp | -E ]
 	     [ \--fixed-strings | -F ]
-	     [ \--date=(local|relative|default|iso|rfc|short) ]
+	     [ \--date=(local|relative|default|iso|rfc|short|raw) ]
 	     [ [\--objects | \--objects-edge] [ \--unpacked ] ]
 	     [ \--pretty | \--header ]
 	     [ \--bisect ]
-- 
1.7.4.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH/RFC 2/9] add support for --date=unix to complement %at
  2011-03-28 23:17 [PATCH/RFC 0/9] add long forms for format placeholders Will Palmer
  2011-03-28 23:17 ` [PATCH/RFC 1/9] mention --date=raw in rev-list and blame help Will Palmer
@ 2011-03-28 23:17 ` Will Palmer
  2011-03-28 23:17 ` [PATCH/RFC 3/9] interpret %C(invalid) as we would %%C(invalid) Will Palmer
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Will Palmer @ 2011-03-28 23:17 UTC (permalink / raw
  To: git; +Cc: Junio C Hamano, Jeff King, Will Palmer

this adds support for --date=unix to the "git log" family of commands,
which would cause %ad and %cd to act like %at and %ct respectively.

Signed-off-by: Will Palmer <wmpalmer@gmail.com>
---
 Documentation/blame-options.txt    |    2 +-
 Documentation/git-rev-list.txt     |    2 +-
 Documentation/rev-list-options.txt |    4 +++-
 builtin/blame.c                    |    3 +++
 cache.h                            |    3 ++-
 date.c                             |    7 +++++++
 6 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index e11a3da..2586606 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -72,7 +72,7 @@ of lines before or after the line given by <start>.
 
 --date <format>::
 	The value is one of the following alternatives:
-	{relative,local,default,iso,rfc,short,raw}. If --date is
+	{relative,local,default,iso,rfc,short,unix,raw}. 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
diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt
index ce3692c..7e1d4bf 100644
--- a/Documentation/git-rev-list.txt
+++ b/Documentation/git-rev-list.txt
@@ -37,7 +37,7 @@ SYNOPSIS
 	     [ \--regexp-ignore-case | -i ]
 	     [ \--extended-regexp | -E ]
 	     [ \--fixed-strings | -F ]
-	     [ \--date=(local|relative|default|iso|rfc|short|raw) ]
+	     [ \--date=(local|relative|default|iso|rfc|short|unix|raw) ]
 	     [ [\--objects | \--objects-edge] [ \--unpacked ] ]
 	     [ \--pretty | \--header ]
 	     [ \--bisect ]
diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index 9c47ad8..9ad68a7 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -13,7 +13,7 @@ include::pretty-options.txt[]
 
 	Synonym for `--date=relative`.
 
---date=(relative|local|default|iso|rfc|short|raw)::
+--date=(relative|local|default|iso|rfc|short|unix|raw)::
 
 	Only takes effect for dates shown in human-readable format, such
 	as when using "--pretty". `log.date` config variable sets a default
@@ -31,6 +31,8 @@ format, often found in E-mail messages.
 +
 `--date=short` shows only date but not time, in `YYYY-MM-DD` format.
 +
+`--date=unix` shows the date as a UNIX timestamp, ignoring timezone.
++
 `--date=raw` shows the date in the internal raw git format `%s %z` format.
 +
 `--date=default` shows timestamps in the original timezone
diff --git a/builtin/blame.c b/builtin/blame.c
index f6b03f7..c6e43a0 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2366,6 +2366,9 @@ parse_done:
 	case DATE_RAW:
 		blame_date_width = sizeof("1161298804 -0700");
 		break;
+	case DATE_UNIX:
+		blame_date_width = sizeof("1161298804");
+		break;
 	case DATE_SHORT:
 		blame_date_width = sizeof("2006-10-19");
 		break;
diff --git a/cache.h b/cache.h
index f2dabef..fa564fa 100644
--- a/cache.h
+++ b/cache.h
@@ -814,7 +814,8 @@ enum date_mode {
 	DATE_LOCAL,
 	DATE_ISO8601,
 	DATE_RFC2822,
-	DATE_RAW
+	DATE_RAW,
+	DATE_UNIX
 };
 
 const char *show_date(unsigned long time, int timezone, enum date_mode mode);
diff --git a/date.c b/date.c
index 00f9eb5..ce48220 100644
--- a/date.c
+++ b/date.c
@@ -157,6 +157,11 @@ const char *show_date(unsigned long time, int tz, enum date_mode mode)
 		return timebuf;
 	}
 
+	if (mode == DATE_UNIX) {
+		snprintf(timebuf, sizeof(timebuf), "%lu", time);
+		return timebuf;
+	}
+
 	if (mode == DATE_RELATIVE) {
 		struct timeval now;
 		gettimeofday(&now, NULL);
@@ -672,6 +677,8 @@ enum date_mode parse_date_format(const char *format)
 		return DATE_NORMAL;
 	else if (!strcmp(format, "raw"))
 		return DATE_RAW;
+	else if (!strcmp(format, "unix"))
+		return DATE_UNIX;
 	else
 		die("unknown date format %s", format);
 }
-- 
1.7.4.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH/RFC 3/9] interpret %C(invalid) as we would %%C(invalid)
  2011-03-28 23:17 [PATCH/RFC 0/9] add long forms for format placeholders Will Palmer
  2011-03-28 23:17 ` [PATCH/RFC 1/9] mention --date=raw in rev-list and blame help Will Palmer
  2011-03-28 23:17 ` [PATCH/RFC 2/9] add support for --date=unix to complement %at Will Palmer
@ 2011-03-28 23:17 ` Will Palmer
  2011-03-28 23:17 ` [PATCH/RFC 4/9] add sanity length check to format_person_part Will Palmer
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Will Palmer @ 2011-03-28 23:17 UTC (permalink / raw
  To: git; +Cc: Junio C Hamano, Jeff King, Will Palmer

%C(...) had the distinction of being the only format placeholder which
could trigger a die(), a side-effect of its ancestry in calling the
existing color_parse_mem(...). This was good, because it gave users an
explanation of what went wrong. It was also inconsistent, since every
other "unknown placeholder" was interpreted as a literal.

This removes the inconsistency by interpreting %C(invalid) as a literal.
Perhaps this is a step in the wrong direction.

Signed-off-by: Will Palmer <wmpalmer@gmail.com>
---
 color.c  |   17 +++++++++++------
 color.h  |    1 +
 pretty.c |    7 +++----
 3 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/color.c b/color.c
index 6a5a54e..9bc190b 100644
--- a/color.c
+++ b/color.c
@@ -45,6 +45,13 @@ void color_parse(const char *value, const char *var, char *dst)
 void color_parse_mem(const char *value, int value_len, const char *var,
 		char *dst)
 {
+	if (color_parse_len(value, value_len, dst))
+		return;
+	die("bad color value '%.*s' for variable '%s'", value_len, value, var);
+}
+
+int color_parse_len(const char *value, int value_len, char *dst)
+{
 	const char *ptr = value;
 	int len = value_len;
 	unsigned int attr = 0;
@@ -53,7 +60,7 @@ void color_parse_mem(const char *value, int value_len, const char *var,
 
 	if (!strncasecmp(value, "reset", len)) {
 		strcpy(dst, GIT_COLOR_RESET);
-		return;
+		return 1;
 	}
 
 	/* [fg [bg]] [attr]... */
@@ -82,13 +89,13 @@ void color_parse_mem(const char *value, int value_len, const char *var,
 				bg = val;
 				continue;
 			}
-			goto bad;
+			return 0;
 		}
 		val = parse_attr(word, wordlen);
 		if (0 <= val)
 			attr |= (1 << val);
 		else
-			goto bad;
+			return 0;
 	}
 
 	if (attr || fg >= 0 || bg >= 0) {
@@ -130,9 +137,7 @@ void color_parse_mem(const char *value, int value_len, const char *var,
 		*dst++ = 'm';
 	}
 	*dst = 0;
-	return;
-bad:
-	die("bad color value '%.*s' for variable '%s'", value_len, value, var);
+	return 1;
 }
 
 int git_config_colorbool(const char *var, const char *value, int stdout_is_tty)
diff --git a/color.h b/color.h
index 170ff40..084d85d 100644
--- a/color.h
+++ b/color.h
@@ -59,6 +59,7 @@ int git_color_default_config(const char *var, const char *value, void *cb);
 
 int git_config_colorbool(const char *var, const char *value, int stdout_is_tty);
 void color_parse(const char *value, const char *var, char *dst);
+int color_parse_len(const char *value, int len, char *dst);
 void color_parse_mem(const char *value, int len, const char *var, char *dst);
 __attribute__((format (printf, 3, 4)))
 int color_fprintf(FILE *fp, const char *color, const char *fmt, ...);
diff --git a/pretty.c b/pretty.c
index 8549934..d5a724f 100644
--- a/pretty.c
+++ b/pretty.c
@@ -752,11 +752,10 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,
 		if (placeholder[1] == '(') {
 			const char *end = strchr(placeholder + 2, ')');
 			char color[COLOR_MAXLEN];
-			if (!end)
+			if (!end || !color_parse_len(placeholder + 2,
+						     end - (placeholder + 2),
+						     color))
 				return 0;
-			color_parse_mem(placeholder + 2,
-					end - (placeholder + 2),
-					"--pretty format", color);
 			strbuf_addstr(sb, color);
 			return end - placeholder + 1;
 		}
-- 
1.7.4.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH/RFC 4/9] add sanity length check to format_person_part
  2011-03-28 23:17 [PATCH/RFC 0/9] add long forms for format placeholders Will Palmer
                   ` (2 preceding siblings ...)
  2011-03-28 23:17 ` [PATCH/RFC 3/9] interpret %C(invalid) as we would %%C(invalid) Will Palmer
@ 2011-03-28 23:17 ` Will Palmer
  2011-03-28 23:17 ` [PATCH/RFC 5/9] refactor pretty.c into "parse" and "format" steps Will Palmer
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Will Palmer @ 2011-03-28 23:17 UTC (permalink / raw
  To: git; +Cc: Junio C Hamano, Jeff King, Will Palmer

previously we were relying on the length check from ident.c, ie: relying
on having been given a good object to parse in the first place. If the
object should have triggered the "Impossibly long personal identifier"
check at commit-time, for example, it would have resulted in an overflow
here.

This is another one which, due to the input constraints, would not have
been a real-world problem for regular usage. I was able to cause an
overflow by viewing the log of a commit with an impossibly-long Author,
created via hash-object. This is admittedly a pretty far-fetched
scenario, though it could potentially be considered a security issue.

In any case, the lack of it rubbed me the wrong way when I was
refactoring this section, and it is trivial to add the sanity check.

Signed-off-by: Will Palmer <wmpalmer@gmail.com>
---
 pretty.c |   20 ++++++++++++++------
 1 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/pretty.c b/pretty.c
index d5a724f..8a288f0 100644
--- a/pretty.c
+++ b/pretty.c
@@ -449,6 +449,7 @@ static size_t format_person_part(struct strbuf *sb, char part,
 	unsigned long date = 0;
 	char *ep;
 	const char *name_start, *name_end, *mail_start, *mail_end, *msg_end = msg+len;
+	size_t name_len, mail_len;
 	char person_name[1024];
 	char person_mail[1024];
 
@@ -469,29 +470,36 @@ static size_t format_person_part(struct strbuf *sb, char part,
 	name_end = msg+end;
 	while (name_end > name_start && isspace(*(name_end-1)))
 		name_end--;
+	name_len = name_end-name_start;
+	if (name_len >= sizeof(person_name))
+		goto skip;
 	mail_start = msg+end+1;
 	mail_end = mail_start;
 	while (mail_end < msg_end && *mail_end != '>')
 		mail_end++;
+	mail_len = mail_end-mail_start;
+	if (mail_len >= sizeof(person_mail))
+		goto skip;
 	if (mail_end == msg_end)
 		goto skip;
 	end = mail_end-msg;
 
 	if (part == 'N' || part == 'E') { /* mailmap lookup */
-		strlcpy(person_name, name_start, name_end-name_start+1);
-		strlcpy(person_mail, mail_start, mail_end-mail_start+1);
+		/* copy up to, and including, the end delimiter */
+		strlcpy(person_name, name_start, name_len+1);
+		strlcpy(person_mail, mail_start, mail_len+1);
 		mailmap_name(person_mail, sizeof(person_mail), person_name, sizeof(person_name));
 		name_start = person_name;
-		name_end = name_start + strlen(person_name);
+		name_len = strlen(person_name);
 		mail_start = person_mail;
-		mail_end = mail_start +  strlen(person_mail);
+		mail_len = strlen(person_mail);
 	}
 	if (part == 'n' || part == 'N') {	/* name */
-		strbuf_add(sb, name_start, name_end-name_start);
+		strbuf_add(sb, name_start, name_len);
 		return placeholder_len;
 	}
 	if (part == 'e' || part == 'E') {	/* email */
-		strbuf_add(sb, mail_start, mail_end-mail_start);
+		strbuf_add(sb, mail_start, mail_len);
 		return placeholder_len;
 	}
 
-- 
1.7.4.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH/RFC 5/9] refactor pretty.c into "parse" and "format" steps
  2011-03-28 23:17 [PATCH/RFC 0/9] add long forms for format placeholders Will Palmer
                   ` (3 preceding siblings ...)
  2011-03-28 23:17 ` [PATCH/RFC 4/9] add sanity length check to format_person_part Will Palmer
@ 2011-03-28 23:17 ` Will Palmer
  2011-03-28 23:17 ` [PATCH/RFC 6/9] add long-form %(wrap:...) for %w(...) Will Palmer
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Will Palmer @ 2011-03-28 23:17 UTC (permalink / raw
  To: git; +Cc: Junio C Hamano, Jeff King, Will Palmer

Previously we parsed-out the user-defined "pretty" formats and outputted
the result in one pass, once for every commit. This was perfectly
reasonable, as the performance penalties for doing so are almost
nonexistant. However, it did make it cumbersome to define more-complex
placeholders.

Here we split the bulk of pretty.c into separate "parse" and "format"
steps, adding the bulk of the back-end for more-complicated formatting
rules in the process.

This also adds test-pretty, for debugging the parser. It prints out a
human-readable dump of the parsed format.

Signed-off-by: Will Palmer <wmpalmer@gmail.com>
---
 .gitignore    |    1 +
 Makefile      |    1 +
 commit.h      |   86 ++++++
 pretty.c      |  802 ++++++++++++++++++++++++++++++++++++++++-----------------
 test-pretty.c |  232 +++++++++++++++++
 5 files changed, 889 insertions(+), 233 deletions(-)
 create mode 100644 test-pretty.c

diff --git a/.gitignore b/.gitignore
index 3dd6ef7..c3b32e8 100644
--- a/.gitignore
+++ b/.gitignore
@@ -173,6 +173,7 @@
 /test-obj-pool
 /test-parse-options
 /test-path-utils
+/test-pretty
 /test-run-command
 /test-sha1
 /test-sigchain
diff --git a/Makefile b/Makefile
index 5c2b797..9cecf40 100644
--- a/Makefile
+++ b/Makefile
@@ -436,6 +436,7 @@ TEST_PROGRAMS_NEED_X += test-svn-fe
 TEST_PROGRAMS_NEED_X += test-treap
 TEST_PROGRAMS_NEED_X += test-index-version
 TEST_PROGRAMS_NEED_X += test-mktemp
+TEST_PROGRAMS_NEED_X += test-pretty
 
 TEST_PROGRAMS = $(patsubst %,%$X,$(TEST_PROGRAMS_NEED_X))
 
diff --git a/commit.h b/commit.h
index 4198513..fff1225 100644
--- a/commit.h
+++ b/commit.h
@@ -83,17 +83,103 @@ struct userformat_want {
 	unsigned notes:1;
 };
 
+enum format_part_type {
+	FORMAT_PART_UNKNOWN = 0,
+	FORMAT_PART_LITERAL,
+
+	FORMAT_PART_COMMIT_HASH,
+	FORMAT_PART_COMMIT_HASH_ABBREV,
+	FORMAT_PART_PARENT_HASHES,
+	FORMAT_PART_PARENT_HASHES_ABBREV,
+	FORMAT_PART_TREE_HASH,
+	FORMAT_PART_TREE_HASH_ABBREV,
+
+	FORMAT_PART_AUTHOR_NAME,
+	FORMAT_PART_AUTHOR_NAME_MAILMAP,
+	FORMAT_PART_AUTHOR_EMAIL,
+	FORMAT_PART_AUTHOR_EMAIL_MAILMAP,
+	FORMAT_PART_AUTHOR_DATE,
+	FORMAT_PART_COMMITTER_NAME,
+	FORMAT_PART_COMMITTER_NAME_MAILMAP,
+	FORMAT_PART_COMMITTER_EMAIL,
+	FORMAT_PART_COMMITTER_EMAIL_MAILMAP,
+	FORMAT_PART_COMMITTER_DATE,
+
+	FORMAT_PART_DECORATE,
+	FORMAT_PART_ENCODING,
+	FORMAT_PART_SUBJECT,
+	FORMAT_PART_SUBJECT_SANITIZED,
+	FORMAT_PART_BODY,
+	FORMAT_PART_RAW_BODY,
+	FORMAT_PART_NOTES,
+
+	FORMAT_PART_REFLOG_SELECTOR,
+	FORMAT_PART_REFLOG_SELECTOR_SHORT,
+	FORMAT_PART_REFLOG_SUBJECT,
+
+	FORMAT_PART_MARK,
+	FORMAT_PART_WRAP
+};
+
+enum format_part_magic {
+	NO_MAGIC,
+	ADD_LF_BEFORE_NON_EMPTY,
+	DEL_LF_BEFORE_EMPTY,
+	ADD_SP_BEFORE_NON_EMPTY
+};
+
+enum format_arg_type {
+	FORMAT_ARG_UINT,
+	FORMAT_ARG_DATE_MODE
+};
+
+struct format_arg {
+	enum format_arg_type type;
+	union {
+		unsigned long uint;
+		enum date_mode dmode;
+	};
+};
+
+struct format_part;
+struct format_parts {
+	size_t			format_len;
+	struct userformat_want	want;
+	size_t			len;
+	size_t			alloc;
+	struct format_part	*part;
+};
+
+struct format_part {
+	enum format_part_type	type;
+	enum format_part_magic	magic;
+
+	size_t			format_len;
+
+	size_t			literal_len;
+	char			*literal;
+
+	size_t			argc;
+	size_t			args_alloc;
+	struct format_arg	*args;
+};
+
 extern int has_non_ascii(const char *text);
 struct rev_info; /* in revision.h, it circularly uses enum cmit_fmt */
 extern char *logmsg_reencode(const struct commit *commit,
 			     const char *output_encoding);
 extern char *reencode_commit_message(const struct commit *commit,
 				     const char **encoding_p);
+extern struct format_parts *parse_format(const char *unparsed);
 extern void get_commit_format(const char *arg, struct rev_info *);
 extern void userformat_find_requirements(const char *fmt, struct userformat_want *w);
 extern void format_commit_message(const struct commit *commit,
 				  const char *format, struct strbuf *sb,
 				  const struct pretty_print_context *context);
+extern void format_commit_message_parsed(const struct commit *commit,
+					 const struct format_parts *parsed_format,
+					 struct strbuf *sb,
+					 const struct pretty_print_context *context);
 extern void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
 				struct strbuf *sb,
 				const struct pretty_print_context *context);
diff --git a/pretty.c b/pretty.c
index 8a288f0..cb02879 100644
--- a/pretty.c
+++ b/pretty.c
@@ -10,7 +10,7 @@
 #include "color.h"
 #include "reflog-walk.h"
 
-static char *user_format;
+static struct format_parts *user_format;
 static struct cmt_fmt_map {
 	const char *name;
 	enum cmit_fmt format;
@@ -23,10 +23,408 @@ static size_t commit_formats_len;
 static size_t commit_formats_alloc;
 static struct cmt_fmt_map *find_commit_format(const char *sought);
 
+#define WHITESPACE " \t\r\n"
+#define format_parts_alloc() \
+	((struct format_parts*)xcalloc(1, sizeof(struct format_parts)))
+#define format_part_alloc() \
+	((struct format_part*)xcalloc(1, sizeof(struct format_part)))
+static void format_part_free(struct format_part **part);
+static void format_parts_free(struct format_parts **parts)
+{
+	if((*parts)->part)
+		free((*parts)->part);
+	free(*parts);
+	*parts = NULL;
+}
+static void format_part_free(struct format_part **part)
+{
+	if ((*part)->literal)
+		free((*part)->literal);
+	if ((*part)->args)
+		free((*part)->args);
+	free(*part);
+	*part = NULL;
+}
+
+static struct format_part *parts_add(struct format_parts *parts,
+				     enum format_part_type type)
+{
+	ALLOC_GROW(parts->part, parts->len+1, parts->alloc);
+	memset(&parts->part[parts->len], 0,
+	       sizeof(parts->part[parts->len]));
+	parts->part[parts->len].type = type;
+	parts->len++;
+	return &parts->part[parts->len-1];
+}
+
+static struct format_part *parts_add_part(struct format_parts *parts,
+					   struct format_part *part)
+{
+	struct format_part *dst = parts_add(parts, FORMAT_PART_UNKNOWN);
+	memcpy(dst, part, sizeof(*dst));
+	if (part->type == FORMAT_PART_NOTES)
+		parts->want.notes = 1;
+	return dst;
+}
+
+static void parts_add_nliteral(struct format_parts *parts, const char *literal,
+			       size_t len)
+{
+	if (len == 0)
+		return;
+	parts_add(parts, FORMAT_PART_LITERAL);
+	parts->part[parts->len-1].literal = xmemdupz(literal, len);
+	parts->part[parts->len-1].literal_len = len;
+	parts->part[parts->len-1].format_len = len;
+	return;
+}
+
+static void part_add_arg_date_mode(struct format_part *part,
+				   enum date_mode dmode)
+{
+	part->args = xrealloc(part->args,
+			      sizeof(struct format_arg) * (part->argc+1));
+	part->args[part->argc].type = FORMAT_ARG_DATE_MODE;
+	part->args[part->argc].dmode = dmode;
+	part->argc++;
+	return;
+}
+
+/*
+* Parse a single argument of an extended format, up to the next delimiter
+* ie: up to ',' or ')'
+* The return value is the position of the found delimiter within *unparsed,
+* or NULL if the argument was invalid.
+*/
+const char *parse_arg(struct format_part *part, enum format_arg_type type,
+		      const char *unparsed)
+{
+	struct format_arg arg = {0};
+	const char *c = unparsed;
+	char *t;
+
+	if (type != FORMAT_ARG_UINT)
+		return NULL;
+	arg.type = type;
+
+	c += strspn(c, WHITESPACE);
+	if (!isdigit(*c))
+		return NULL;
+	arg.uint = strtoul(c, &t, 10);
+	if (t == c)
+		return NULL;
+	c = t + strspn(t, WHITESPACE);
+	if (*c == ',' || *c == ')'){
+		ALLOC_GROW(part->args, part->argc+1, part->args_alloc);
+		memcpy(&(part->args[part->argc]), &arg,
+		       sizeof(struct format_arg));
+		part->argc++;
+		return c;
+	}
+	return NULL;
+}
+
+static struct format_part *parse_special(const char *unparsed)
+{
+	struct format_part *part = NULL;
+	int h1,h2;
+	char c;
+	const char *s, *e;
+
+	/* these allocate their own part */
+	switch (unparsed[1]) {
+	case '-':
+	case '+':
+	case ' ':
+		if (*unparsed != '%')
+			goto fail;
+
+		part = parse_special(unparsed + 1);
+		if (part) {
+			part->format_len++;
+
+			switch (unparsed[1]) {
+			case '-':
+				part->magic = DEL_LF_BEFORE_EMPTY;
+				break;
+			case '+':
+				part->magic = ADD_LF_BEFORE_NON_EMPTY;
+				break;
+			case ' ':
+				part->magic = ADD_SP_BEFORE_NON_EMPTY;
+				break;
+			}
+		}
+		return part;
+	}
+
+	part = format_part_alloc();
+
+	/* most placeholders are 2 characters long */
+	part->format_len = 2;
+
+	switch (unparsed[1]) {
+	case 'h':
+		part->type = FORMAT_PART_COMMIT_HASH_ABBREV;
+		return part;
+	case 'H':
+		part->type = FORMAT_PART_COMMIT_HASH;
+		return part;
+	case 'p':
+		part->type = FORMAT_PART_PARENT_HASHES_ABBREV;
+		return part;
+	case 'P':
+		part->type = FORMAT_PART_PARENT_HASHES;
+		return part;
+	case 't':
+		part->type = FORMAT_PART_TREE_HASH_ABBREV;
+		return part;
+	case 'T':
+		part->type = FORMAT_PART_TREE_HASH;
+		return part;
+	case 'a':
+		part->format_len++;
+		switch (unparsed[2]) {
+		case 'n':
+			part->type = FORMAT_PART_AUTHOR_NAME;
+			return part;
+		case 'N':
+			part->type = FORMAT_PART_AUTHOR_NAME_MAILMAP;
+			return part;
+		case 'e':
+			part->type = FORMAT_PART_AUTHOR_EMAIL;
+			return part;
+		case 'E':
+			part->type = FORMAT_PART_AUTHOR_EMAIL_MAILMAP;
+			return part;
+		case 'd':
+			part->type = FORMAT_PART_AUTHOR_DATE;
+			return part;
+		case 'D':
+			part->type = FORMAT_PART_AUTHOR_DATE;
+			part_add_arg_date_mode(part, DATE_RFC2822);
+			return part;
+		case 'r':
+			part->type = FORMAT_PART_AUTHOR_DATE;
+			part_add_arg_date_mode(part, DATE_RELATIVE);
+			return part;
+		case 't':
+			part->type = FORMAT_PART_AUTHOR_DATE;
+			part_add_arg_date_mode(part, DATE_UNIX);
+			return part;
+		case 'i':
+			part->type = FORMAT_PART_AUTHOR_DATE;
+			part_add_arg_date_mode(part, DATE_ISO8601);
+			return part;
+		}
+		goto fail;
+	case 'c':
+		part->format_len++;
+		switch (unparsed[2]) {
+		case 'n':
+			part->type = FORMAT_PART_COMMITTER_NAME;
+			return part;
+		case 'N':
+			part->type = FORMAT_PART_COMMITTER_NAME_MAILMAP;
+			return part;
+		case 'e':
+			part->type = FORMAT_PART_COMMITTER_EMAIL;
+			return part;
+		case 'E':
+			part->type = FORMAT_PART_COMMITTER_EMAIL_MAILMAP;
+			return part;
+		case 'd':
+			part->type = FORMAT_PART_COMMITTER_DATE;
+			return part;
+		case 'D':
+			part->type = FORMAT_PART_COMMITTER_DATE;
+			part_add_arg_date_mode(part, DATE_RFC2822);
+			return part;
+		case 'r':
+			part->type = FORMAT_PART_COMMITTER_DATE;
+			part_add_arg_date_mode(part, DATE_RELATIVE);
+			return part;
+		case 't':
+			part->type = FORMAT_PART_COMMITTER_DATE;
+			part_add_arg_date_mode(part, DATE_UNIX);
+			return part;
+		case 'i':
+			part->type = FORMAT_PART_COMMITTER_DATE;
+			part_add_arg_date_mode(part, DATE_ISO8601);
+			return part;
+		}
+		goto fail;
+	case 'd':
+		part->type = FORMAT_PART_DECORATE;
+		return part;
+	case 'e':
+		part->type = FORMAT_PART_ENCODING;
+		return part;
+	case 's':
+		part->type = FORMAT_PART_SUBJECT;
+		return part;
+	case 'f':
+		part->type = FORMAT_PART_SUBJECT_SANITIZED;
+		return part;
+	case 'b':
+		part->type = FORMAT_PART_BODY;
+		return part;
+	case 'B':
+		part->type = FORMAT_PART_RAW_BODY;
+		return part;
+	case 'N':
+		part->type = FORMAT_PART_NOTES;
+		return part;
+	case 'g':
+		part->format_len++;
+		switch (unparsed[2]) {
+		case 'D':
+			part->type = FORMAT_PART_REFLOG_SELECTOR;
+			return part;
+		case 'd':
+			part->type = FORMAT_PART_REFLOG_SELECTOR_SHORT;
+			return part;
+		case 's':
+			part->type = FORMAT_PART_REFLOG_SUBJECT;
+			return part;
+		}
+		goto fail;
+	case 'C':
+		part->type = FORMAT_PART_LITERAL;
+		if (unparsed[2] == '(') {
+			e = strchr(unparsed + 3, ')');
+			part->literal = xcalloc(1, COLOR_MAXLEN);
+			if (!e || !color_parse_len(unparsed + 3,
+						  e - (unparsed + 3),
+						  part->literal))
+				goto fail;
+			part->literal_len = strlen(part->literal);
+			part->format_len = e - unparsed + 1;
+			return part;
+		}
+
+		if (!prefixcmp(&unparsed[2], "red")) {
+			part->literal = GIT_COLOR_RED;
+			part->literal_len = strlen(GIT_COLOR_RED);
+			part->format_len = 5;
+		} else if (!prefixcmp(&unparsed[2], "green")) {
+			part->literal = GIT_COLOR_GREEN;
+			part->literal_len = strlen(GIT_COLOR_GREEN);
+			part->format_len = 7;
+		} else if (!prefixcmp(&unparsed[2], "blue")) {
+			part->literal = GIT_COLOR_BLUE;
+			part->literal_len = strlen(GIT_COLOR_BLUE);
+			part->format_len = 6;
+		} else if (!prefixcmp(&unparsed[2], "reset")) {
+			part->literal = GIT_COLOR_RESET;
+			part->literal_len = strlen(GIT_COLOR_RESET);
+			part->format_len = 7;
+		}
+
+		if (part->literal)
+			return part;
+		goto fail;
+	case 'm':
+		part->type = FORMAT_PART_MARK;
+		return part;
+	case 'w':
+		if (unparsed[2] != '(')
+			goto fail;
+
+		part->type = FORMAT_PART_WRAP;
+
+		s = unparsed + 3;
+		while (part->argc <= 3) {
+			s += strspn(s, WHITESPACE);
+			if (*s == ')'){
+				part->format_len = s - unparsed + 1;
+				return part;
+			}
+			if (part->argc) {
+				if (*s != ',')
+					goto fail;
+				s++;
+			}
+			if (part->argc == 3)
+				goto fail;
+
+			s = parse_arg(part, FORMAT_ARG_UINT, s);
+			if (!s)
+				goto fail;
+		}
+		goto fail;
+	case 'x':
+		/* %x00 == NUL, %x0a == LF, etc. */
+		if (0 <= (h1 = hexval_table[0xff & unparsed[2]]) &&
+		    h1 <= 16 &&
+		    0 <= (h2 = hexval_table[0xff & unparsed[3]]) &&
+		    h2 <= 16) {
+			part->type = FORMAT_PART_LITERAL;
+			part->format_len = 4;
+			c = (h1<<4)|h2;
+			part->literal = xmemdupz(&c, 1);
+			part->literal_len = 1;
+			return part;
+		}
+		goto fail;
+	case 'n':
+		part->type = FORMAT_PART_LITERAL;
+		part->literal = "\n";
+		part->literal_len = 1;
+		return part;
+	case '%':
+		part->type = FORMAT_PART_LITERAL;
+		part->literal = xstrndup(&unparsed[1], 1);
+		part->literal_len = 1;
+		return part;
+	}
+
+fail:
+	if (part)
+		format_part_free(&part);
+	return NULL;
+}
+
+struct format_parts *parse_format(const char *unparsed)
+{
+	struct format_parts *parts = format_parts_alloc();
+	struct format_part *part;
+	const char *c = unparsed;
+	const char *last = NULL;
+
+	while (*c) {
+		if (!last)
+			last = c;
+
+		c += strcspn(c, "%");
+		if (!*c)
+			break;
+
+		part = parse_special(c);
+		if (part) {
+			parts_add_nliteral(parts, last, c - last);
+			last = NULL;
+
+			parts_add_part(parts, part);
+			c += part->format_len;
+			free(part);
+			continue;
+		}
+		c++;
+	}
+
+	if (last)
+		parts_add_nliteral(parts, last, c - last);
+
+	parts->format_len = c - unparsed + 1;
+	return parts;
+}
+
 static void save_user_format(struct rev_info *rev, const char *cp, int is_tformat)
 {
-	free(user_format);
-	user_format = xstrdup(cp);
+	if (user_format)
+		format_parts_free(&user_format);
+	user_format = parse_format(cp);
 	if (is_tformat)
 		rev->use_terminator = 1;
 	rev->commit_format = CMIT_FMT_USERFORMAT;
@@ -440,11 +838,9 @@ static int mailmap_name(char *email, int email_len, char *name, int name_len)
 	return mail_map->nr && map_user(mail_map, email, email_len, name, name_len);
 }
 
-static size_t format_person_part(struct strbuf *sb, char part,
-				 const char *msg, int len, enum date_mode dmode)
+static void format_person_part(struct strbuf *sb, struct format_part *part,
+			       const char *msg, int len, enum date_mode dmode)
 {
-	/* currently all placeholders have same length */
-	const int placeholder_len = 2;
 	int start, end, tz = 0;
 	unsigned long date = 0;
 	char *ep;
@@ -463,7 +859,7 @@ static size_t format_person_part(struct strbuf *sb, char part,
 	 * below len - 1.
 	 */
 	if (end >= len - 2)
-		goto skip;
+		return;
 
 	/* Seek for both name and email part */
 	name_start = msg;
@@ -472,19 +868,22 @@ static size_t format_person_part(struct strbuf *sb, char part,
 		name_end--;
 	name_len = name_end-name_start;
 	if (name_len >= sizeof(person_name))
-		goto skip;
+		return;
 	mail_start = msg+end+1;
 	mail_end = mail_start;
 	while (mail_end < msg_end && *mail_end != '>')
 		mail_end++;
 	mail_len = mail_end-mail_start;
 	if (mail_len >= sizeof(person_mail))
-		goto skip;
+		return;
 	if (mail_end == msg_end)
-		goto skip;
+		return;
 	end = mail_end-msg;
 
-	if (part == 'N' || part == 'E') { /* mailmap lookup */
+	if (part->type == FORMAT_PART_AUTHOR_NAME_MAILMAP ||
+	    part->type == FORMAT_PART_AUTHOR_EMAIL_MAILMAP ||
+	    part->type == FORMAT_PART_COMMITTER_NAME_MAILMAP ||
+	    part->type == FORMAT_PART_COMMITTER_EMAIL_MAILMAP) {
 		/* copy up to, and including, the end delimiter */
 		strlcpy(person_name, name_start, name_len+1);
 		strlcpy(person_mail, mail_start, mail_len+1);
@@ -494,27 +893,37 @@ static size_t format_person_part(struct strbuf *sb, char part,
 		mail_start = person_mail;
 		mail_len = strlen(person_mail);
 	}
-	if (part == 'n' || part == 'N') {	/* name */
+	if (part->type == FORMAT_PART_AUTHOR_NAME ||
+	    part->type == FORMAT_PART_AUTHOR_NAME_MAILMAP ||
+	    part->type == FORMAT_PART_COMMITTER_NAME ||
+	    part->type == FORMAT_PART_COMMITTER_NAME_MAILMAP) {
 		strbuf_add(sb, name_start, name_len);
-		return placeholder_len;
+		return;
 	}
-	if (part == 'e' || part == 'E') {	/* email */
+	if (part->type == FORMAT_PART_AUTHOR_EMAIL ||
+	    part->type == FORMAT_PART_AUTHOR_EMAIL_MAILMAP ||
+	    part->type == FORMAT_PART_COMMITTER_EMAIL ||
+	    part->type == FORMAT_PART_COMMITTER_EMAIL_MAILMAP) {
 		strbuf_add(sb, mail_start, mail_len);
-		return placeholder_len;
+		return;
 	}
 
 	/* advance 'start' to point to date start delimiter */
 	for (start = end + 1; start < len && isspace(msg[start]); start++)
 		; /* do nothing */
 	if (start >= len)
-		goto skip;
+		return;
 	date = strtoul(msg + start, &ep, 10);
 	if (msg + start == ep)
-		goto skip;
+		return;
 
-	if (part == 't') {	/* date, UNIX timestamp */
+	if (part->type != FORMAT_PART_AUTHOR_DATE &&
+	    part->type != FORMAT_PART_COMMITTER_DATE)
+		return;
+
+	if (part->argc && part->args[0].dmode == DATE_UNIX) {
 		strbuf_add(sb, msg + start, ep - (msg + start));
-		return placeholder_len;
+		return;
 	}
 
 	/* parse tz */
@@ -526,31 +935,11 @@ static size_t format_person_part(struct strbuf *sb, char part,
 			tz = -tz;
 	}
 
-	switch (part) {
-	case 'd':	/* date */
+	if (part->argc)
+		strbuf_addstr(sb, show_date(date, tz, part->args[0].dmode));
+	else
 		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));
-		return placeholder_len;
-	case 'r':	/* date, relative */
-		strbuf_addstr(sb, show_date(date, tz, DATE_RELATIVE));
-		return placeholder_len;
-	case 'i':	/* date, ISO 8601 */
-		strbuf_addstr(sb, show_date(date, tz, DATE_ISO8601));
-		return placeholder_len;
-	}
-
-skip:
-	/*
-	 * bogus commit, 'sb' cannot be updated, but we still need to
-	 * compute a valid return value.
-	 */
-	if (part == 'n' || part == 'e' || part == 't' || part == 'd'
-	    || part == 'D' || part == 'r' || part == 'i')
-		return placeholder_len;
-
-	return 0; /* unknown placeholder */
+	return;
 }
 
 struct chunk {
@@ -745,116 +1134,65 @@ static void rewrap_message_tail(struct strbuf *sb,
 	c->indent2 = new_indent2;
 }
 
-static size_t format_commit_one(struct strbuf *sb, const char *placeholder,
-				void *context)
+void format_commit_message_part(struct format_part *part,
+				struct strbuf *sb, void *context)
 {
 	struct format_commit_context *c = context;
 	const struct commit *commit = c->commit;
-	const char *msg = c->message;
+	const char *msg = commit->buffer;
 	struct commit_list *p;
-	int h1, h2;
+	unsigned long width = 0, indent1 = 0, indent2 = 0;
 
 	/* these are independent of the commit */
-	switch (placeholder[0]) {
-	case 'C':
-		if (placeholder[1] == '(') {
-			const char *end = strchr(placeholder + 2, ')');
-			char color[COLOR_MAXLEN];
-			if (!end || !color_parse_len(placeholder + 2,
-						     end - (placeholder + 2),
-						     color))
-				return 0;
-			strbuf_addstr(sb, color);
-			return end - placeholder + 1;
-		}
-		if (!prefixcmp(placeholder + 1, "red")) {
-			strbuf_addstr(sb, GIT_COLOR_RED);
-			return 4;
-		} else if (!prefixcmp(placeholder + 1, "green")) {
-			strbuf_addstr(sb, GIT_COLOR_GREEN);
-			return 6;
-		} else if (!prefixcmp(placeholder + 1, "blue")) {
-			strbuf_addstr(sb, GIT_COLOR_BLUE);
-			return 5;
-		} else if (!prefixcmp(placeholder + 1, "reset")) {
-			strbuf_addstr(sb, GIT_COLOR_RESET);
-			return 6;
-		} else
-			return 0;
-	case 'n':		/* newline */
-		strbuf_addch(sb, '\n');
-		return 1;
-	case 'x':
-		/* %x00 == NUL, %x0a == LF, etc. */
-		if (0 <= (h1 = hexval_table[0xff & placeholder[1]]) &&
-		    h1 <= 16 &&
-		    0 <= (h2 = hexval_table[0xff & placeholder[2]]) &&
-		    h2 <= 16) {
-			strbuf_addch(sb, (h1<<4)|h2);
-			return 3;
-		} else
-			return 0;
-	case 'w':
-		if (placeholder[1] == '(') {
-			unsigned long width = 0, indent1 = 0, indent2 = 0;
-			char *next;
-			const char *start = placeholder + 2;
-			const char *end = strchr(start, ')');
-			if (!end)
-				return 0;
-			if (end > start) {
-				width = strtoul(start, &next, 10);
-				if (*next == ',') {
-					indent1 = strtoul(next + 1, &next, 10);
-					if (*next == ',') {
-						indent2 = strtoul(next + 1,
-								 &next, 10);
-					}
-				}
-				if (*next != ')')
-					return 0;
-			}
-			rewrap_message_tail(sb, c, width, indent1, indent2);
-			return end - placeholder + 1;
-		} else
-			return 0;
+	switch (part->type) {
+	case FORMAT_PART_LITERAL:
+		strbuf_add(sb, part->literal, part->literal_len);
+		return;
+	case FORMAT_PART_WRAP:
+		width = (part->argc > 0) ? part->args[0].uint : 0;
+		indent1 = (part->argc > 1) ? part->args[1].uint : 0;
+		indent2 = (part->argc > 2) ? part->args[2].uint : 0;
+		rewrap_message_tail(sb, c, width, indent1, indent2);
+		return;
+	default:
+		break;
 	}
 
 	/* these depend on the commit */
 	if (!commit->object.parsed)
 		parse_object(commit->object.sha1);
 
-	switch (placeholder[0]) {
-	case 'H':		/* commit hash */
+	switch (part->type) {
+	case FORMAT_PART_COMMIT_HASH:
 		strbuf_addstr(sb, sha1_to_hex(commit->object.sha1));
-		return 1;
-	case 'h':		/* abbreviated commit hash */
+		return;
+	case FORMAT_PART_COMMIT_HASH_ABBREV:
 		if (add_again(sb, &c->abbrev_commit_hash))
-			return 1;
+			return;
 		strbuf_addstr(sb, find_unique_abbrev(commit->object.sha1,
-						     c->pretty_ctx->abbrev));
+					     c->pretty_ctx->abbrev));
 		c->abbrev_commit_hash.len = sb->len - c->abbrev_commit_hash.off;
-		return 1;
-	case 'T':		/* tree hash */
+		return;
+	case FORMAT_PART_TREE_HASH:
 		strbuf_addstr(sb, sha1_to_hex(commit->tree->object.sha1));
-		return 1;
-	case 't':		/* abbreviated tree hash */
+		return;
+	case FORMAT_PART_TREE_HASH_ABBREV:
 		if (add_again(sb, &c->abbrev_tree_hash))
-			return 1;
+			return;
 		strbuf_addstr(sb, find_unique_abbrev(commit->tree->object.sha1,
 						     c->pretty_ctx->abbrev));
 		c->abbrev_tree_hash.len = sb->len - c->abbrev_tree_hash.off;
-		return 1;
-	case 'P':		/* parent hashes */
+		return;
+	case FORMAT_PART_PARENT_HASHES:
 		for (p = commit->parents; p; p = p->next) {
 			if (p != commit->parents)
 				strbuf_addch(sb, ' ');
 			strbuf_addstr(sb, sha1_to_hex(p->item->object.sha1));
 		}
-		return 1;
-	case 'p':		/* abbreviated parent hashes */
+		return;
+	case FORMAT_PART_PARENT_HASHES_ABBREV:
 		if (add_again(sb, &c->abbrev_parent_hashes))
-			return 1;
+			return;
 		for (p = commit->parents; p; p = p->next) {
 			if (p != commit->parents)
 				strbuf_addch(sb, ' ');
@@ -864,159 +1202,138 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,
 		}
 		c->abbrev_parent_hashes.len = sb->len -
 		                              c->abbrev_parent_hashes.off;
-		return 1;
-	case 'm':		/* left/right/bottom */
+		return;
+	case FORMAT_PART_MARK:
 		strbuf_addch(sb, (commit->object.flags & BOUNDARY)
 		                 ? '-'
 		                 : (commit->object.flags & SYMMETRIC_LEFT)
 		                 ? '<'
 		                 : '>');
-		return 1;
-	case 'd':
+		return;
+	case FORMAT_PART_DECORATE:
 		format_decoration(sb, commit);
-		return 1;
-	case 'g':		/* reflog info */
-		switch(placeholder[1]) {
-		case 'd':	/* reflog selector */
-		case 'D':
-			if (c->pretty_ctx->reflog_info)
-				get_reflog_selector(sb,
-						    c->pretty_ctx->reflog_info,
-						    c->pretty_ctx->date_mode,
-						    (placeholder[1] == 'd'));
-			return 2;
-		case 's':	/* reflog message */
-			if (c->pretty_ctx->reflog_info)
-				get_reflog_message(sb, c->pretty_ctx->reflog_info);
-			return 2;
+		return;
+	case FORMAT_PART_REFLOG_SELECTOR:
+	case FORMAT_PART_REFLOG_SELECTOR_SHORT:
+		if (c->pretty_ctx->reflog_info) {
+			get_reflog_selector(sb,
+					    c->pretty_ctx->reflog_info,
+					    c->pretty_ctx->date_mode,
+					    (part->type == FORMAT_PART_REFLOG_SELECTOR_SHORT));
 		}
-		return 0;	/* unknown %g placeholder */
-	case 'N':
+		return;
+	case FORMAT_PART_REFLOG_SUBJECT:
+		if (c->pretty_ctx->reflog_info)
+			get_reflog_message(sb, c->pretty_ctx->reflog_info);
+		return;
+	case FORMAT_PART_NOTES:
 		if (c->pretty_ctx->show_notes) {
 			format_display_notes(commit->object.sha1, sb,
 				    get_log_output_encoding(), 0);
-			return 1;
 		}
-		return 0;
+		return;
+	default:
+		break;
 	}
 
 	/* For the rest we have to parse the commit header. */
 	if (!c->commit_header_parsed)
 		parse_commit_header(c);
 
-	switch (placeholder[0]) {
-	case 'a':	/* author ... */
-		return format_person_part(sb, placeholder[1],
-				   msg + c->author.off, c->author.len,
-				   c->pretty_ctx->date_mode);
-	case 'c':	/* committer ... */
-		return format_person_part(sb, placeholder[1],
-				   msg + c->committer.off, c->committer.len,
-				   c->pretty_ctx->date_mode);
-	case 'e':	/* encoding */
+	switch (part->type) {
+	case FORMAT_PART_AUTHOR_NAME:
+	case FORMAT_PART_AUTHOR_NAME_MAILMAP:
+	case FORMAT_PART_AUTHOR_EMAIL:
+	case FORMAT_PART_AUTHOR_EMAIL_MAILMAP:
+	case FORMAT_PART_AUTHOR_DATE:
+		format_person_part(sb, part, commit->buffer + c->author.off,
+				   c->author.len, c->pretty_ctx->date_mode);
+		return;
+	case FORMAT_PART_COMMITTER_NAME:
+	case FORMAT_PART_COMMITTER_NAME_MAILMAP:
+	case FORMAT_PART_COMMITTER_EMAIL:
+	case FORMAT_PART_COMMITTER_EMAIL_MAILMAP:
+	case FORMAT_PART_COMMITTER_DATE:
+		format_person_part(sb, part, commit->buffer + c->committer.off,
+				   c->committer.len, c->pretty_ctx->date_mode);
+		return;
+	case FORMAT_PART_ENCODING:
 		strbuf_add(sb, msg + c->encoding.off, c->encoding.len);
-		return 1;
-	case 'B':	/* raw body */
+		return;
+	case FORMAT_PART_RAW_BODY:
 		/* message_off is always left at the initial newline */
 		strbuf_addstr(sb, msg + c->message_off + 1);
-		return 1;
+		return;
+	default:
+		break;
 	}
 
 	/* Now we need to parse the commit message. */
 	if (!c->commit_message_parsed)
 		parse_commit_message(c);
 
-	switch (placeholder[0]) {
-	case 's':	/* subject */
+	switch (part->type) {
+	case FORMAT_PART_SUBJECT:
 		format_subject(sb, msg + c->subject_off, " ");
-		return 1;
-	case 'f':	/* sanitized subject */
+		return;
+	case FORMAT_PART_SUBJECT_SANITIZED:
 		format_sanitized_subject(sb, msg + c->subject_off);
-		return 1;
-	case 'b':	/* body */
+		return;
+	case FORMAT_PART_BODY:
 		strbuf_addstr(sb, msg + c->body_off);
-		return 1;
-	}
-	return 0;	/* unknown placeholder */
-}
-
-static size_t format_commit_item(struct strbuf *sb, const char *placeholder,
-				 void *context)
-{
-	int consumed;
-	size_t orig_len;
-	enum {
-		NO_MAGIC,
-		ADD_LF_BEFORE_NON_EMPTY,
-		DEL_LF_BEFORE_EMPTY,
-		ADD_SP_BEFORE_NON_EMPTY
-	} magic = NO_MAGIC;
-
-	switch (placeholder[0]) {
-	case '-':
-		magic = DEL_LF_BEFORE_EMPTY;
-		break;
-	case '+':
-		magic = ADD_LF_BEFORE_NON_EMPTY;
-		break;
-	case ' ':
-		magic = ADD_SP_BEFORE_NON_EMPTY;
-		break;
+		return;
 	default:
 		break;
 	}
-	if (magic != NO_MAGIC)
-		placeholder++;
-
-	orig_len = sb->len;
-	consumed = format_commit_one(sb, placeholder, context);
-	if (magic == NO_MAGIC)
-		return consumed;
-
-	if ((orig_len == sb->len) && magic == DEL_LF_BEFORE_EMPTY) {
-		while (sb->len && sb->buf[sb->len - 1] == '\n')
-			strbuf_setlen(sb, sb->len - 1);
-	} else if (orig_len != sb->len) {
-		if (magic == ADD_LF_BEFORE_NON_EMPTY)
-			strbuf_insert(sb, orig_len, "\n", 1);
-		else if (magic == ADD_SP_BEFORE_NON_EMPTY)
-			strbuf_insert(sb, orig_len, " ", 1);
-	}
-	return consumed + 1;
+	return;
 }
 
-static size_t userformat_want_item(struct strbuf *sb, const char *placeholder,
-				   void *context)
+void format_commit_message_parts(const struct format_parts *parsed,
+				 struct strbuf *sb, void *context)
 {
-	struct userformat_want *w = context;
+	size_t i, orig_len;
+	enum format_part_magic magic;
 
-	if (*placeholder == '+' || *placeholder == '-' || *placeholder == ' ')
-		placeholder++;
+	for (i = 0; i < parsed->len; i++) {
+		orig_len = sb->len;
+		magic = parsed->part[i].magic;
+		format_commit_message_part(&parsed->part[i], sb, context);
 
-	switch (*placeholder) {
-	case 'N':
-		w->notes = 1;
-		break;
+		if (magic == NO_MAGIC)
+			continue;
+
+		if ((orig_len == sb->len) && magic == DEL_LF_BEFORE_EMPTY) {
+			while (sb->len && sb->buf[sb->len - 1] == '\n')
+				strbuf_setlen(sb, sb->len - 1);
+		} else if (orig_len != sb->len) {
+			if (magic == ADD_LF_BEFORE_NON_EMPTY)
+				strbuf_insert(sb, orig_len, "\n", 1);
+			else if (magic == ADD_SP_BEFORE_NON_EMPTY)
+				strbuf_insert(sb, orig_len, " ", 1);
+		}
 	}
-	return 0;
 }
 
 void userformat_find_requirements(const char *fmt, struct userformat_want *w)
 {
-	struct strbuf dummy = STRBUF_INIT;
+	struct format_parts *dummy;
 
 	if (!fmt) {
 		if (!user_format)
 			return;
-		fmt = user_format;
+		memcpy(w, &user_format->want, sizeof(*w));
+		return;
 	}
-	strbuf_expand(&dummy, user_format, userformat_want_item, w);
-	strbuf_release(&dummy);
+
+	dummy = parse_format(fmt);
+	memcpy(w, &dummy->want, sizeof(*w));
+	format_parts_free(&dummy);
 }
 
-void format_commit_message(const struct commit *commit,
-			   const char *format, struct strbuf *sb,
-			   const struct pretty_print_context *pretty_ctx)
+void format_commit_message_parsed(const struct commit *commit,
+				  const struct format_parts *parsed_format,
+				  struct strbuf *sb,
+				  const struct pretty_print_context *pretty_ctx)
 {
 	struct format_commit_context context;
 	static const char utf8[] = "UTF-8";
@@ -1035,13 +1352,32 @@ void format_commit_message(const struct commit *commit,
 			context.message = logmsg_reencode(commit, output_enc);
 	}
 
-	strbuf_expand(sb, format, format_commit_item, &context);
+	format_commit_message_parts(parsed_format, sb, &context);
 	rewrap_message_tail(sb, &context, 0, 0, 0);
 
 	if (context.message != commit->buffer)
 		free(context.message);
 }
 
+void format_commit_message(const struct commit *commit,
+			   const char *format, struct strbuf *sb,
+			   const struct pretty_print_context *pretty_ctx)
+{
+	static char *last = NULL;
+	static struct format_parts *parsed = NULL;
+
+	if( !parsed || strcmp(last, format) ){
+		if (parsed){
+			format_parts_free(&parsed);
+			free(last);
+		}
+		parsed = parse_format(format);
+		last = xstrdup(format);
+	}
+
+	format_commit_message_parsed(commit, parsed, sb, pretty_ctx);
+}
+
 static void pp_header(enum cmit_fmt fmt,
 		      int abbrev,
 		      enum date_mode dmode,
@@ -1198,7 +1534,7 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
 	int need_8bit_cte = context->need_8bit_cte;
 
 	if (fmt == CMIT_FMT_USERFORMAT) {
-		format_commit_message(commit, user_format, sb, context);
+		format_commit_message_parsed(commit, user_format, sb, context);
 		return;
 	}
 
diff --git a/test-pretty.c b/test-pretty.c
new file mode 100644
index 0000000..57c1c65
--- /dev/null
+++ b/test-pretty.c
@@ -0,0 +1,232 @@
+#include <ctype.h>
+#include "cache.h"
+#include "utf8.h"
+#include "commit.h"
+
+static const char *usage_msg = "\n"
+"  test-pretty <format>\n"
+"  test-pretty -a\n"
+"  test-pretty -- <format>\n";
+
+static const char *all = "a"
+"%-h%+h% h"
+"%h%H%p%P%t%T"
+"%an%aN%ae%aE%ad%aD%ar%at%ai"
+"%cn%cN%ce%cE%cd%cD%cr%ct%ci"
+"%d%e%s%f%b%B%N"
+"%gD%gd%gs"
+"%Cred%Cgreen%Cblue%Creset%C(reset)"
+"%m%w()%w(1)%w(1,2)%w(1,2,3)"
+"%x0a%n%%%@";
+
+static struct strbuf *parts_debug(struct format_parts *parts,
+				  const char *unparsed)
+{
+	struct format_part *part;
+	struct strbuf *buf = xcalloc(1, sizeof(*buf));
+	size_t indent = 0;
+	struct {enum format_part_type type; char *label;} labels[] = {
+		{FORMAT_PART_LITERAL, "LITERAL"},
+		{FORMAT_PART_COMMIT_HASH, "COMMIT_HASH"},
+		{FORMAT_PART_COMMIT_HASH_ABBREV, "COMMIT_HASH_ABBREV"},
+		{FORMAT_PART_PARENT_HASHES, "PARENT_HASHES"},
+		{FORMAT_PART_PARENT_HASHES_ABBREV, "PARENT_HASHES_ABBREV"},
+		{FORMAT_PART_TREE_HASH, "TREE_HASH"},
+		{FORMAT_PART_TREE_HASH_ABBREV, "TREE_HASH_ABBREV"},
+		{FORMAT_PART_AUTHOR_NAME, "AUTHOR_NAME"},
+		{FORMAT_PART_AUTHOR_NAME_MAILMAP, "AUTHOR_NAME_MAILMAP"},
+		{FORMAT_PART_AUTHOR_EMAIL, "AUTHOR_EMAIL"},
+		{FORMAT_PART_AUTHOR_EMAIL_MAILMAP, "AUTHOR_EMAIL_MAILMAP"},
+		{FORMAT_PART_AUTHOR_DATE, "AUTHOR_DATE"},
+		{FORMAT_PART_COMMITTER_NAME, "COMMITTER_NAME"},
+		{FORMAT_PART_COMMITTER_NAME_MAILMAP, "COMMITTER_NAME_MAILMAP"},
+		{FORMAT_PART_COMMITTER_EMAIL, "COMMITTER_EMAIL"},
+		{FORMAT_PART_COMMITTER_EMAIL_MAILMAP, "COMMITTER_EMAIL_MAILMAP"},
+		{FORMAT_PART_COMMITTER_DATE, "COMMITTER_DATE"},
+
+		{FORMAT_PART_DECORATE, "DECORATE"},
+		{FORMAT_PART_ENCODING, "ENCODING"},
+		{FORMAT_PART_SUBJECT, "SUBJECT"},
+		{FORMAT_PART_SUBJECT_SANITIZED, "SUBJECT_SANITIZED"},
+		{FORMAT_PART_BODY, "BODY"},
+		{FORMAT_PART_RAW_BODY, "RAW_BODY"},
+		{FORMAT_PART_NOTES, "NOTES"},
+
+		{FORMAT_PART_REFLOG_SELECTOR, "REFLOG_SELECTOR"},
+		{FORMAT_PART_REFLOG_SELECTOR_SHORT, "REFLOG_SELECTOR_SHORT"},
+		{FORMAT_PART_REFLOG_SUBJECT, "REFLOG_SUBJECT"},
+
+		{FORMAT_PART_MARK, "MARK"},
+		{FORMAT_PART_WRAP, "WRAP"}
+	};
+	char *label;
+	size_t i,j,t = 0;
+	strbuf_init(buf, 0);
+
+	strbuf_add_wrapped_text(buf, "{[PARTS:", indent++, 0, 0);
+	strbuf_addf(buf, "%li]\n", parts->len);
+	strbuf_add_wrapped_text(buf, "[FORMAT:", indent, 0, 0);
+	strbuf_addf(buf, "%s]\n", unparsed);
+	strbuf_add_wrapped_text(buf, "(REMADE:", indent, 0, 0);
+	for (i = 0; i < parts->len; i++) {
+		strbuf_add(buf, unparsed + t, parts->part[i].format_len);
+		t += parts->part[i].format_len;
+	}
+	strbuf_addstr(buf, ")\n");
+
+	for (i = 0; i < parts->len; i++) {
+		part = &parts->part[i];
+		label = "UNKNOWN";
+		for (j = 0; j < ARRAY_SIZE(labels); j++) {
+			if (labels[j].type == part->type) {
+				label = labels[j].label;
+			}
+		}
+
+		strbuf_add_wrapped_text(buf, "{[", indent, 0, 0);
+		strbuf_add(buf, unparsed, part->format_len);
+		unparsed += part->format_len;
+		strbuf_add(buf, "] ", 2);
+		strbuf_addstr(buf, label);
+
+		switch(part->magic){
+		case NO_MAGIC:
+			break;
+		case ADD_LF_BEFORE_NON_EMPTY:
+			strbuf_addstr(buf, " (ADD_LF_BEFORE_NON_EMPTY)");
+			break;
+		case DEL_LF_BEFORE_EMPTY:
+			strbuf_addstr(buf, " (DEL_LF_BEFORE_EMPTY)");
+			break;
+		case ADD_SP_BEFORE_NON_EMPTY:
+			strbuf_addstr(buf, " (ADD_SP_BEFORE_NON_EMPTY)");
+			break;
+		}
+
+		if (part->literal) {
+			strbuf_addstr(buf, " \"");
+			t = 0;
+			while (t < part->literal_len) {
+				switch (part->literal[t]) {
+				case '\n':
+					strbuf_addstr(buf, "\\n");
+					break;
+				case '\r':
+					strbuf_addstr(buf, "\\r");
+					break;
+				case '\t':
+					strbuf_addstr(buf, "\\t");
+					break;
+				case '\\':
+					strbuf_addstr(buf, "\\\\");
+					break;
+				default:
+					if (isprint(part->literal[t]))
+						strbuf_add(buf, &part->literal[t],
+							   1);
+					else
+						strbuf_addf(buf, "\\x%02x",
+							    part->literal[t]);
+					break;
+				}
+				t++;
+			}
+			strbuf_addstr(buf, "\"");
+		}
+
+		if (part->argc) {
+			strbuf_addstr(buf, "\n");
+			strbuf_add_wrapped_text(buf, "ARGS: [", indent+1, 0, 0);
+			for (j = 0; j < part->argc; j++) {
+				switch(part->args[j].type){
+				case FORMAT_ARG_UINT:
+					strbuf_addstr(buf, "UINT:");
+					strbuf_addf(buf, "%lu", part->args[j].uint);
+					break;
+				case FORMAT_ARG_DATE_MODE:
+					strbuf_addstr(buf, "DATE_MODE:");
+					switch(part->args[j].dmode){
+					case DATE_NORMAL:
+						strbuf_addstr(buf, "DATE_NORMAL");
+						break;
+					case DATE_RELATIVE:
+						strbuf_addstr(buf, "DATE_RELATIVE");
+						break;
+					case DATE_SHORT:
+						strbuf_addstr(buf, "DATE_SHORT");
+						break;
+					case DATE_LOCAL:
+						strbuf_addstr(buf, "DATE_LOCAL");
+						break;
+					case DATE_ISO8601:
+						strbuf_addstr(buf, "DATE_ISO8601");
+						break;
+					case DATE_RFC2822:
+						strbuf_addstr(buf, "DATE_RFC2822");
+						break;
+					case DATE_RAW:
+						strbuf_addstr(buf, "DATE_RAW");
+						break;
+					case DATE_UNIX:
+						strbuf_addstr(buf, "DATE_UNIX");
+						break;
+					default:
+						strbuf_addf(buf, "(UNKNOWN:%u)",
+							    part->args[j].dmode);
+						break;
+					}
+					break;
+				default:
+					strbuf_addstr(buf, "(UNKNOWN)");
+					break;
+				}
+
+				if (j < part->argc - 1)
+					strbuf_addstr(buf, ", ");
+			}
+			strbuf_addstr(buf, "]\n");
+		}
+
+		if (part->argc)
+			strbuf_add_wrapped_text(buf, "}\n", indent, 0, 0);
+		else
+			strbuf_addstr(buf, "}\n");
+	}
+	strbuf_add_wrapped_text(buf, "}\n", --indent, 0, 0);
+
+	if (!indent) {
+		printf("%s", buf->buf);
+		strbuf_release(buf);
+		free(buf);
+		return NULL;
+	}
+	return buf;
+}
+
+int main(int argc, char **argv)
+{
+	const char *unparsed = NULL;
+	struct format_parts *parsed;
+
+	if (argc < 2) {
+		usage(usage_msg);
+		return 1;
+	}
+
+	if (*argv[1] == '-') {
+		if (argv[1][1] == 'a' && argc == 2)
+			unparsed = all;
+		if (argv[1][1] == '-' && !argv[1][2] && argc == 3)
+			unparsed = argv[2];
+	} else
+		unparsed = argv[1];
+
+	if (!unparsed) {
+		usage(usage_msg);
+		return 1;
+	}
+
+	parsed = parse_format(unparsed);
+	parts_debug(parsed, unparsed);
+	return 0;
+}
-- 
1.7.4.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH/RFC 6/9] add long-form %(wrap:...) for %w(...)
  2011-03-28 23:17 [PATCH/RFC 0/9] add long forms for format placeholders Will Palmer
                   ` (4 preceding siblings ...)
  2011-03-28 23:17 ` [PATCH/RFC 5/9] refactor pretty.c into "parse" and "format" steps Will Palmer
@ 2011-03-28 23:17 ` Will Palmer
  2011-03-28 23:17 ` [PATCH/RFC 7/9] add long form %(color:...) for %C(...) Will Palmer
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Will Palmer @ 2011-03-28 23:17 UTC (permalink / raw
  To: git; +Cc: Junio C Hamano, Jeff King, Will Palmer

the list of user-defined format placeholders has grown steadily longer
since they were first introduced. We currently have about forty
placeholders, and the room for new mnemonics is growing scarce. To make
more room, we introduce "long forms" for placeholders, which take the
form:
	'%(' <label> [ ':' <arg> [ ',' <arg> ]* ] ')'
eg:
	%(wrap: 80, 0, 4)

We start by adding a long-form to the %w(...) placeholder, mostly
because as it already takes multiple arguments, it is a good example.

Signed-off-by: Will Palmer <wmpalmer@gmail.com>
---
 Documentation/pretty-formats.txt |    1 +
 pretty.c                         |   47 +++++++++++++++++++++++++++++++++-----
 test-pretty.c                    |    1 +
 3 files changed, 43 insertions(+), 6 deletions(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 561cc9f..c9f3fb6 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -144,6 +144,7 @@ The placeholders are:
 - '%x00': print a byte from a hex code
 - '%w([<w>[,<i1>[,<i2>]]])': switch line wrapping, like the -w option of
   linkgit:git-shortlog[1].
+- '%(wrap:[<w>[,<i1>[,<i2>]]])': alternative form of %w(...)
 
 NOTE: Some placeholders may depend on other options given to the
 revision traversal engine. For example, the `%g*` reflog options will
diff --git a/pretty.c b/pretty.c
index cb02879..8301008 100644
--- a/pretty.c
+++ b/pretty.c
@@ -108,12 +108,10 @@ const char *parse_arg(struct format_part *part, enum format_arg_type type,
 	arg.type = type;
 
 	c += strspn(c, WHITESPACE);
-	if (!isdigit(*c))
-		return NULL;
-	arg.uint = strtoul(c, &t, 10);
-	if (t == c)
-		return NULL;
-	c = t + strspn(t, WHITESPACE);
+	if (isdigit(*c)) {
+		arg.uint = strtoul(c, &t, 10);
+		c = t + strspn(t, WHITESPACE);
+	}
 	if (*c == ',' || *c == ')'){
 		ALLOC_GROW(part->args, part->argc+1, part->args_alloc);
 		memcpy(&(part->args[part->argc]), &arg,
@@ -124,6 +122,41 @@ const char *parse_arg(struct format_part *part, enum format_arg_type type,
 	return NULL;
 }
 
+static struct format_part *parse_extended(const char *unparsed)
+{
+	struct format_part *part = format_part_alloc();
+	const char *c = unparsed + 2; /* "%(..." + strlen("%(") */
+
+	c += strspn(c, WHITESPACE);
+
+	if (!prefixcmp(c, "wrap")) {
+		part->type = FORMAT_PART_WRAP;
+		c += 4;
+		while(part->argc <= 3){
+			c += strspn(c, WHITESPACE);
+			if (*c == ')')
+				goto success;
+			if (*c != (part->argc ? ',' : ':'))
+				goto fail;
+			if (part->argc == 3)
+				goto fail;
+
+			c = parse_arg(part, FORMAT_ARG_UINT, c+1);
+			if (!c)
+				goto fail;
+		}
+		goto fail;
+	}
+
+fail:
+	format_part_free(&part);
+	return NULL;
+
+success:
+	part->format_len = c - unparsed + 1;
+	return part;
+}
+
 static struct format_part *parse_special(const char *unparsed)
 {
 	struct format_part *part = NULL;
@@ -156,6 +189,8 @@ static struct format_part *parse_special(const char *unparsed)
 			}
 		}
 		return part;
+	case '(':
+		return parse_extended(unparsed);
 	}
 
 	part = format_part_alloc();
diff --git a/test-pretty.c b/test-pretty.c
index 57c1c65..64a8218 100644
--- a/test-pretty.c
+++ b/test-pretty.c
@@ -17,6 +17,7 @@ static const char *all = "a"
 "%gD%gd%gs"
 "%Cred%Cgreen%Cblue%Creset%C(reset)"
 "%m%w()%w(1)%w(1,2)%w(1,2,3)"
+"%(wrap)%(wrap:1)%(wrap:1,2)%(wrap:1,2,3)"
 "%x0a%n%%%@";
 
 static struct strbuf *parts_debug(struct format_parts *parts,
-- 
1.7.4.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH/RFC 7/9] add long form %(color:...) for %C(...)
  2011-03-28 23:17 [PATCH/RFC 0/9] add long forms for format placeholders Will Palmer
                   ` (5 preceding siblings ...)
  2011-03-28 23:17 ` [PATCH/RFC 6/9] add long-form %(wrap:...) for %w(...) Will Palmer
@ 2011-03-28 23:17 ` Will Palmer
  2011-03-28 23:17 ` [PATCH/RFC 8/9] add long forms %(authordate) and %(committerdate) Will Palmer
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Will Palmer @ 2011-03-28 23:17 UTC (permalink / raw
  To: git; +Cc: Junio C Hamano, Jeff King, Will Palmer

this adds %(color:...) as an alternative to %C(...) in the "git log"
family of commands. We now have a "long form" for all of the existing
complex placeholders.

Signed-off-by: Will Palmer <wmpalmer@gmail.com>
---
 Documentation/pretty-formats.txt |    1 +
 pretty.c                         |   22 ++++++++++++++++++++++
 test-pretty.c                    |    1 +
 3 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index c9f3fb6..d987102 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -138,6 +138,7 @@ The placeholders are:
 - '%Cblue': switch color to blue
 - '%Creset': reset color
 - '%C(...)': color specification, as described in color.branch.* config option
+- '%(color:...)': alternative form of %C(...)
 - '%m': left, right or boundary mark
 - '%n': newline
 - '%%': a raw '%'
diff --git a/pretty.c b/pretty.c
index 8301008..616b857 100644
--- a/pretty.c
+++ b/pretty.c
@@ -126,9 +126,31 @@ static struct format_part *parse_extended(const char *unparsed)
 {
 	struct format_part *part = format_part_alloc();
 	const char *c = unparsed + 2; /* "%(..." + strlen("%(") */
+	const char *e;
 
 	c += strspn(c, WHITESPACE);
 
+	if (!prefixcmp(c, "color")) {
+		part->type = FORMAT_PART_LITERAL;
+		c += 5 + strspn(c + 5, WHITESPACE);
+		if (*c == ')') {
+			part->literal = xstrdup(GIT_COLOR_RESET);
+			part->literal_len = strlen(part->literal);
+			goto success;
+		}
+		if (*c != ':')
+			goto fail;
+		c++;
+		e = strchr(c, ')');
+		part->literal = xcalloc(1, COLOR_MAXLEN);
+		if (!e || !color_parse_len(c, e - c,
+					   part->literal))
+			goto fail;
+		part->literal_len = strlen(part->literal);
+		c = e;
+		goto success;
+	}
+
 	if (!prefixcmp(c, "wrap")) {
 		part->type = FORMAT_PART_WRAP;
 		c += 4;
diff --git a/test-pretty.c b/test-pretty.c
index 64a8218..eb88e3a 100644
--- a/test-pretty.c
+++ b/test-pretty.c
@@ -18,6 +18,7 @@ static const char *all = "a"
 "%Cred%Cgreen%Cblue%Creset%C(reset)"
 "%m%w()%w(1)%w(1,2)%w(1,2,3)"
 "%(wrap)%(wrap:1)%(wrap:1,2)%(wrap:1,2,3)"
+"%(color)%(color:red)%(color:red bold)%(color:red green bold)"
 "%x0a%n%%%@";
 
 static struct strbuf *parts_debug(struct format_parts *parts,
-- 
1.7.4.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH/RFC 8/9] add long forms %(authordate) and %(committerdate)
  2011-03-28 23:17 [PATCH/RFC 0/9] add long forms for format placeholders Will Palmer
                   ` (6 preceding siblings ...)
  2011-03-28 23:17 ` [PATCH/RFC 7/9] add long form %(color:...) for %C(...) Will Palmer
@ 2011-03-28 23:17 ` Will Palmer
  2011-03-28 23:17 ` [PATCH/RFC 9/9] add long forms for author and committer identity Will Palmer
  2011-03-29  0:28 ` [PATCH/RFC 0/9] add long forms for format placeholders Junio C Hamano
  9 siblings, 0 replies; 14+ messages in thread
From: Will Palmer @ 2011-03-28 23:17 UTC (permalink / raw
  To: git; +Cc: Junio C Hamano, Jeff King, Will Palmer

this adds the long forms %(authordate) and %(committerdate) to
complement the existing placeholders %ad, %aD, %ar, %at, %ai, %cd, %cC,
%cr, %ct, and %ci. The specific format is specified as with --date, eg:
	%(committerdate:rfc2822)

This change exists mostly to give a place for new date formatting
options to go, though it also has the benefit of matching the format of
the date placeholders in git for-each-ref.

Signed-off-by: Will Palmer <wmpalmer@gmail.com>
---
 Documentation/pretty-formats.txt |    4 ++
 cache.h                          |    2 +
 date.c                           |   58 +++++++++++++++++++++++++------------
 pretty.c                         |   51 +++++++++++++++++++++++++++++----
 test-pretty.c                    |   10 ++++++
 5 files changed, 100 insertions(+), 25 deletions(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index d987102..dfb81a7 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -114,6 +114,8 @@ The placeholders are:
 - '%ar': author date, relative
 - '%at': author date, UNIX timestamp
 - '%ai': author date, ISO 8601 format
+- '%(authordate[:<format>])': author date. Without a <format>, the --date= option is respected.
+  Otherwise, a format of the type which can be specified via the --date= option is taken.
 - '%cn': committer name
 - '%cN': committer name (respecting .mailmap, see linkgit:git-shortlog[1] or linkgit:git-blame[1])
 - '%ce': committer email
@@ -123,6 +125,8 @@ The placeholders are:
 - '%cr': committer date, relative
 - '%ct': committer date, UNIX timestamp
 - '%ci': committer date, ISO 8601 format
+- '%(committerdate[:<format>])': committer date. Without a <format>, the --date= option is respected.
+  Otherwise, a format of the type which can be specified via the --date= option is taken.
 - '%d': ref names, like the --decorate option of linkgit:git-log[1]
 - '%e': encoding
 - '%s': subject
diff --git a/cache.h b/cache.h
index fa564fa..b769cf8 100644
--- a/cache.h
+++ b/cache.h
@@ -829,6 +829,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);
+#define DATE_FORMAT_MAX 9 /* strlen("relative") + 1 */
+size_t parse_date_format_len(const char *format, enum date_mode *dmode);
 enum date_mode parse_date_format(const char *format);
 
 #define IDENT_WARN_ON_NO_NAME  1
diff --git a/date.c b/date.c
index ce48220..1834ab6 100644
--- a/date.c
+++ b/date.c
@@ -659,28 +659,48 @@ int parse_date(const char *date, char *result, int maxlen)
 	return date_string(timestamp, offset, result, maxlen);
 }
 
+size_t parse_date_format_len(const char *format, enum date_mode *dmode)
+{
+	if (!strcmp(format, "relative")) {
+		*dmode = DATE_RELATIVE;
+		return 8;
+	} else if (!strcmp(format, "iso8601")) {
+		*dmode = DATE_ISO8601;
+		return 7;
+	} else if (!strcmp(format, "iso")) {
+		*dmode = DATE_ISO8601;
+		return 3;
+	} else if (!strcmp(format, "rfc2822")) {
+		*dmode = DATE_RFC2822;
+		return 7;
+	} else if (!strcmp(format, "rfc")) {
+		*dmode = DATE_RFC2822;
+		return 3;
+	} else if (!strcmp(format, "short")) {
+		*dmode = DATE_SHORT;
+		return 5;
+	} else if (!strcmp(format, "local")) {
+		*dmode = DATE_LOCAL;
+		return 5;
+	} else if (!strcmp(format, "default")) {
+		*dmode = DATE_NORMAL;
+		return 7;
+	} else if (!strcmp(format, "raw")) {
+		*dmode = DATE_RAW;
+		return 3;
+	} else if (!strcmp(format, "unix")) {
+		*dmode = DATE_UNIX;
+		return 4;
+	} else
+		return 0;
+}
+
 enum date_mode parse_date_format(const char *format)
 {
-	if (!strcmp(format, "relative"))
-		return DATE_RELATIVE;
-	else if (!strcmp(format, "iso8601") ||
-		 !strcmp(format, "iso"))
-		return DATE_ISO8601;
-	else if (!strcmp(format, "rfc2822") ||
-		 !strcmp(format, "rfc"))
-		return DATE_RFC2822;
-	else if (!strcmp(format, "short"))
-		return DATE_SHORT;
-	else if (!strcmp(format, "local"))
-		return DATE_LOCAL;
-	else if (!strcmp(format, "default"))
-		return DATE_NORMAL;
-	else if (!strcmp(format, "raw"))
-		return DATE_RAW;
-	else if (!strcmp(format, "unix"))
-		return DATE_UNIX;
-	else
+	enum date_mode dmode;
+	if (!parse_date_format_len(format, &dmode))
 		die("unknown date format %s", format);
+	return dmode;
 }
 
 void datestamp(char *buf, int bufsize)
diff --git a/pretty.c b/pretty.c
index 616b857..006bbe3 100644
--- a/pretty.c
+++ b/pretty.c
@@ -102,19 +102,38 @@ const char *parse_arg(struct format_part *part, enum format_arg_type type,
 	struct format_arg arg = {0};
 	const char *c = unparsed;
 	char *t;
+	size_t len;
+	char date_format[DATE_FORMAT_MAX];
 
-	if (type != FORMAT_ARG_UINT)
-		return NULL;
 	arg.type = type;
 
 	c += strspn(c, WHITESPACE);
-	if (isdigit(*c)) {
-		arg.uint = strtoul(c, &t, 10);
-		c = t + strspn(t, WHITESPACE);
+
+	switch (type){
+	case FORMAT_ARG_UINT:
+		if (isdigit(*c)) {
+			arg.uint = strtoul(c, &t, 10);
+			c = t;
+		}
+		break;
+	case FORMAT_ARG_DATE_MODE:
+		len = strcspn(c, WHITESPACE ",)");
+		if (len >= DATE_FORMAT_MAX)
+			return NULL;
+		strncpy(date_format, c, len);
+		len = parse_date_format_len(date_format, &arg.dmode);
+		if (!len)
+			return NULL;
+		c += len;
+		break;
+	default:
+		return NULL;
 	}
+
+	c += strspn(c, WHITESPACE);
 	if (*c == ',' || *c == ')'){
 		ALLOC_GROW(part->args, part->argc+1, part->args_alloc);
-		memcpy(&(part->args[part->argc]), &arg,
+		memcpy(&part->args[part->argc], &arg,
 		       sizeof(struct format_arg));
 		part->argc++;
 		return c;
@@ -130,6 +149,26 @@ static struct format_part *parse_extended(const char *unparsed)
 
 	c += strspn(c, WHITESPACE);
 
+	if (!prefixcmp(c, "author") || !prefixcmp(c, "committer")) {
+		e = c;
+		c += (*e == 'a') ? 6 : 9;
+		if (!prefixcmp(c, "date")) {
+			part->type = (*e == 'a') ? FORMAT_PART_AUTHOR_DATE :
+						   FORMAT_PART_COMMITTER_DATE;
+			c += 4 + strspn(c + 4, WHITESPACE);
+			if (*c == ')')
+				goto success;
+			if (*c != ':')
+				goto fail;
+			c = parse_arg(part, FORMAT_ARG_DATE_MODE, c+1);
+			if (!c)
+				goto fail;
+			goto success;
+		}
+
+		c = e;
+	}
+
 	if (!prefixcmp(c, "color")) {
 		part->type = FORMAT_PART_LITERAL;
 		c += 5 + strspn(c + 5, WHITESPACE);
diff --git a/test-pretty.c b/test-pretty.c
index eb88e3a..6a92c65 100644
--- a/test-pretty.c
+++ b/test-pretty.c
@@ -19,6 +19,16 @@ static const char *all = "a"
 "%m%w()%w(1)%w(1,2)%w(1,2,3)"
 "%(wrap)%(wrap:1)%(wrap:1,2)%(wrap:1,2,3)"
 "%(color)%(color:red)%(color:red bold)%(color:red green bold)"
+"%(authordate)%(authordate:default)%(authordate:relative)"
+"%(authordate:short)%(authordate:local)"
+"%(authordate:iso8601)%(authordate:iso)"
+"%(authordate:rfc2822)%(authordate:rfc)"
+"%(authordate:unix)%(authordate:raw)"
+"%(committerdate)%(committerdate:default)%(committerdate:relative)"
+"%(committerdate:short)%(committerdate:local)"
+"%(committerdate:iso8601)%(committerdate:iso)"
+"%(committerdate:rfc2822)%(committerdate:rfc)"
+"%(committerdate:unix)%(committerdate:raw)"
 "%x0a%n%%%@";
 
 static struct strbuf *parts_debug(struct format_parts *parts,
-- 
1.7.4.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH/RFC 9/9] add long forms for author and committer identity
  2011-03-28 23:17 [PATCH/RFC 0/9] add long forms for format placeholders Will Palmer
                   ` (7 preceding siblings ...)
  2011-03-28 23:17 ` [PATCH/RFC 8/9] add long forms %(authordate) and %(committerdate) Will Palmer
@ 2011-03-28 23:17 ` Will Palmer
  2011-03-29  0:28 ` [PATCH/RFC 0/9] add long forms for format placeholders Junio C Hamano
  9 siblings, 0 replies; 14+ messages in thread
From: Will Palmer @ 2011-03-28 23:17 UTC (permalink / raw
  To: git; +Cc: Junio C Hamano, Jeff King, Will Palmer

this adds the long forms %(authorname), %(committername),
%(authoremail), and %(committeremail) to complement the existing
placeholders %an, %ae, %cn, and %ce; and, :mailmap forms of each, eg:
    %(authoremail:mailmap)

The main purpose of this change is to match the format of the
placeholders supported in git for-each-ref, though the optional :mailmap
parameter seemed like a sensible extension.

At this point we have enough "long form" placeholders to justify giving
them their own section in the documentation, so we do.

Signed-off-by: Will Palmer <wmpalmer@gmail.com>
---
 Documentation/pretty-formats.txt |   57 ++++++++++++++++++++++++++++----
 commit.h                         |    6 +--
 pretty.c                         |   68 ++++++++++++++++++++++++++++----------
 test-pretty.c                    |   14 +++++--
 4 files changed, 112 insertions(+), 33 deletions(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index dfb81a7..e9d6634 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -97,7 +97,13 @@ The title was >>t4119: test autocomputing -p<n> for traditional diff input.<<
 
 --------
 +
-The placeholders are:
+The placeholders are divided into two categories, "short forms",
+for quickly typing the more-common placeholders, and "long forms",
+which may be more readable, and may support various additional
+options.
++
+--
+Short forms:
 
 - '%H': commit hash
 - '%h': abbreviated commit hash
@@ -114,8 +120,6 @@ The placeholders are:
 - '%ar': author date, relative
 - '%at': author date, UNIX timestamp
 - '%ai': author date, ISO 8601 format
-- '%(authordate[:<format>])': author date. Without a <format>, the --date= option is respected.
-  Otherwise, a format of the type which can be specified via the --date= option is taken.
 - '%cn': committer name
 - '%cN': committer name (respecting .mailmap, see linkgit:git-shortlog[1] or linkgit:git-blame[1])
 - '%ce': committer email
@@ -125,8 +129,6 @@ The placeholders are:
 - '%cr': committer date, relative
 - '%ct': committer date, UNIX timestamp
 - '%ci': committer date, ISO 8601 format
-- '%(committerdate[:<format>])': committer date. Without a <format>, the --date= option is respected.
-  Otherwise, a format of the type which can be specified via the --date= option is taken.
 - '%d': ref names, like the --decorate option of linkgit:git-log[1]
 - '%e': encoding
 - '%s': subject
@@ -142,20 +144,61 @@ The placeholders are:
 - '%Cblue': switch color to blue
 - '%Creset': reset color
 - '%C(...)': color specification, as described in color.branch.* config option
-- '%(color:...)': alternative form of %C(...)
 - '%m': left, right or boundary mark
 - '%n': newline
 - '%%': a raw '%'
 - '%x00': print a byte from a hex code
 - '%w([<w>[,<i1>[,<i2>]]])': switch line wrapping, like the -w option of
   linkgit:git-shortlog[1].
-- '%(wrap:[<w>[,<i1>[,<i2>]]])': alternative form of %w(...)
+
+Long forms:
+
+- '%(authorname[:mailmap])':
++
+author name, optionally respecting .mailmap (see linkgit:git-shortlog[1]
+or linkgit:git-blame[1]).
+
+- '%(authoremail[:mailmap])':
++
+author email, optionally respecting .mailmap (see linkgit:git-shortlog[1]
+or linkgit:git-blame[1]).
+
+- '%(authordate[:<format>])':
++
+author date. Without a <format>, the --date= option is respected.
+Otherwise, a format of the type which can be specified via the --date=
+option is taken.
+
+- '%(committername[:mailmap])':
++
+committer name, optionally respecting .mailmap (see
+linkgit:git-shortlog[1] or linkgit:git-blame[1]).
+
+- '%(committeremail[:mailmap])':
++
+committer email, optionally respecting .mailmap (see
+linkgit:git-shortlog[1] or linkgit:git-blame[1]).
+
+- '%(committerdate[:<format>])':
++
+committer date. Without a <format>, the --date= option is respected.
+Otherwise, a format of the type which can be specified via the --date=
+option is taken.
+
+- '%(color:[<spec>])':
++
+color specification, as described in color.branch.* config option.
+
+- '%(wrap:[<w>[,<i1>[,<i2>]]])':
++
+switch line wrapping, like the -w option of linkgit:git-shortlog[1].
 
 NOTE: Some placeholders may depend on other options given to the
 revision traversal engine. For example, the `%g*` reflog options will
 insert an empty string unless we are traversing reflog entries (e.g., by
 `git log -g`). The `%d` placeholder will use the "short" decoration
 format if `--decorate` was not already provided on the command line.
+--
 
 If you add a `{plus}` (plus sign) after '%' of a placeholder, a line-feed
 is inserted immediately before the expansion if and only if the
diff --git a/commit.h b/commit.h
index fff1225..a175444 100644
--- a/commit.h
+++ b/commit.h
@@ -95,14 +95,10 @@ enum format_part_type {
 	FORMAT_PART_TREE_HASH_ABBREV,
 
 	FORMAT_PART_AUTHOR_NAME,
-	FORMAT_PART_AUTHOR_NAME_MAILMAP,
 	FORMAT_PART_AUTHOR_EMAIL,
-	FORMAT_PART_AUTHOR_EMAIL_MAILMAP,
 	FORMAT_PART_AUTHOR_DATE,
 	FORMAT_PART_COMMITTER_NAME,
-	FORMAT_PART_COMMITTER_NAME_MAILMAP,
 	FORMAT_PART_COMMITTER_EMAIL,
-	FORMAT_PART_COMMITTER_EMAIL_MAILMAP,
 	FORMAT_PART_COMMITTER_DATE,
 
 	FORMAT_PART_DECORATE,
@@ -130,6 +126,7 @@ enum format_part_magic {
 
 enum format_arg_type {
 	FORMAT_ARG_UINT,
+	FORMAT_ARG_BOOLEAN,
 	FORMAT_ARG_DATE_MODE
 };
 
@@ -137,6 +134,7 @@ struct format_arg {
 	enum format_arg_type type;
 	union {
 		unsigned long uint;
+		int boolean : 1;
 		enum date_mode dmode;
 	};
 };
diff --git a/pretty.c b/pretty.c
index 006bbe3..ef6c3c1 100644
--- a/pretty.c
+++ b/pretty.c
@@ -90,6 +90,16 @@ static void part_add_arg_date_mode(struct format_part *part,
 	return;
 }
 
+static void part_add_arg_boolean(struct format_part *part, int value)
+{
+	part->args = xrealloc(part->args,
+			      sizeof(struct format_arg) * (part->argc+1));
+	part->args[part->argc].type = FORMAT_ARG_BOOLEAN;
+	part->args[part->argc].boolean = value ? 1 : 0;
+	part->argc++;
+	return;
+}
+
 /*
 * Parse a single argument of an extended format, up to the next delimiter
 * ie: up to ',' or ')'
@@ -165,6 +175,31 @@ static struct format_part *parse_extended(const char *unparsed)
 				goto fail;
 			goto success;
 		}
+		if (!prefixcmp(c, "name") || !prefixcmp(c, "email")) {
+			if (*c == 'n') { /* name */
+				part->type = (*e == 'a') ? FORMAT_PART_AUTHOR_NAME :
+							   FORMAT_PART_COMMITTER_NAME;
+				c += 4;
+			} else { /* email */
+				part->type = (*e == 'a') ? FORMAT_PART_AUTHOR_EMAIL :
+							   FORMAT_PART_COMMITTER_EMAIL;
+				c += 5;
+			}
+
+			strspn(c, WHITESPACE);
+			if (*c == ')')
+				goto success;
+			if (*c != ':')
+				goto fail;
+			c += 1 + strspn(c + 1, WHITESPACE);
+			if (!prefixcmp(c, "mailmap")) {
+				part_add_arg_boolean(part, 1);
+				c += 7 + strspn(c + 7, WHITESPACE);
+				if (*c == ')')
+					goto success;
+			}
+			goto fail;
+		}
 
 		c = e;
 	}
@@ -285,13 +320,15 @@ static struct format_part *parse_special(const char *unparsed)
 			part->type = FORMAT_PART_AUTHOR_NAME;
 			return part;
 		case 'N':
-			part->type = FORMAT_PART_AUTHOR_NAME_MAILMAP;
+			part->type = FORMAT_PART_AUTHOR_NAME;
+			part_add_arg_boolean(part, 1);
 			return part;
 		case 'e':
 			part->type = FORMAT_PART_AUTHOR_EMAIL;
 			return part;
 		case 'E':
-			part->type = FORMAT_PART_AUTHOR_EMAIL_MAILMAP;
+			part->type = FORMAT_PART_AUTHOR_EMAIL;
+			part_add_arg_boolean(part, 1);
 			return part;
 		case 'd':
 			part->type = FORMAT_PART_AUTHOR_DATE;
@@ -321,13 +358,15 @@ static struct format_part *parse_special(const char *unparsed)
 			part->type = FORMAT_PART_COMMITTER_NAME;
 			return part;
 		case 'N':
-			part->type = FORMAT_PART_COMMITTER_NAME_MAILMAP;
+			part->type = FORMAT_PART_COMMITTER_NAME;
+			part_add_arg_boolean(part, 1);
 			return part;
 		case 'e':
 			part->type = FORMAT_PART_COMMITTER_EMAIL;
 			return part;
 		case 'E':
-			part->type = FORMAT_PART_COMMITTER_EMAIL_MAILMAP;
+			part->type = FORMAT_PART_COMMITTER_EMAIL;
+			part_add_arg_boolean(part, 1);
 			return part;
 		case 'd':
 			part->type = FORMAT_PART_COMMITTER_DATE;
@@ -976,10 +1015,11 @@ static void format_person_part(struct strbuf *sb, struct format_part *part,
 		return;
 	end = mail_end-msg;
 
-	if (part->type == FORMAT_PART_AUTHOR_NAME_MAILMAP ||
-	    part->type == FORMAT_PART_AUTHOR_EMAIL_MAILMAP ||
-	    part->type == FORMAT_PART_COMMITTER_NAME_MAILMAP ||
-	    part->type == FORMAT_PART_COMMITTER_EMAIL_MAILMAP) {
+	if ((part->type == FORMAT_PART_AUTHOR_NAME ||
+	     part->type == FORMAT_PART_AUTHOR_EMAIL ||
+	     part->type == FORMAT_PART_COMMITTER_NAME ||
+	     part->type == FORMAT_PART_COMMITTER_EMAIL) &&
+	    part->argc && part->args[0].boolean) { /* mailmap */
 		/* copy up to, and including, the end delimiter */
 		strlcpy(person_name, name_start, name_len+1);
 		strlcpy(person_mail, mail_start, mail_len+1);
@@ -990,16 +1030,12 @@ static void format_person_part(struct strbuf *sb, struct format_part *part,
 		mail_len = strlen(person_mail);
 	}
 	if (part->type == FORMAT_PART_AUTHOR_NAME ||
-	    part->type == FORMAT_PART_AUTHOR_NAME_MAILMAP ||
-	    part->type == FORMAT_PART_COMMITTER_NAME ||
-	    part->type == FORMAT_PART_COMMITTER_NAME_MAILMAP) {
+	    part->type == FORMAT_PART_COMMITTER_NAME) {
 		strbuf_add(sb, name_start, name_len);
 		return;
 	}
 	if (part->type == FORMAT_PART_AUTHOR_EMAIL ||
-	    part->type == FORMAT_PART_AUTHOR_EMAIL_MAILMAP ||
-	    part->type == FORMAT_PART_COMMITTER_EMAIL ||
-	    part->type == FORMAT_PART_COMMITTER_EMAIL_MAILMAP) {
+	    part->type == FORMAT_PART_COMMITTER_EMAIL) {
 		strbuf_add(sb, mail_start, mail_len);
 		return;
 	}
@@ -1338,17 +1374,13 @@ void format_commit_message_part(struct format_part *part,
 
 	switch (part->type) {
 	case FORMAT_PART_AUTHOR_NAME:
-	case FORMAT_PART_AUTHOR_NAME_MAILMAP:
 	case FORMAT_PART_AUTHOR_EMAIL:
-	case FORMAT_PART_AUTHOR_EMAIL_MAILMAP:
 	case FORMAT_PART_AUTHOR_DATE:
 		format_person_part(sb, part, commit->buffer + c->author.off,
 				   c->author.len, c->pretty_ctx->date_mode);
 		return;
 	case FORMAT_PART_COMMITTER_NAME:
-	case FORMAT_PART_COMMITTER_NAME_MAILMAP:
 	case FORMAT_PART_COMMITTER_EMAIL:
-	case FORMAT_PART_COMMITTER_EMAIL_MAILMAP:
 	case FORMAT_PART_COMMITTER_DATE:
 		format_person_part(sb, part, commit->buffer + c->committer.off,
 				   c->committer.len, c->pretty_ctx->date_mode);
diff --git a/test-pretty.c b/test-pretty.c
index 6a92c65..f9d44fa 100644
--- a/test-pretty.c
+++ b/test-pretty.c
@@ -19,11 +19,15 @@ static const char *all = "a"
 "%m%w()%w(1)%w(1,2)%w(1,2,3)"
 "%(wrap)%(wrap:1)%(wrap:1,2)%(wrap:1,2,3)"
 "%(color)%(color:red)%(color:red bold)%(color:red green bold)"
+"%(authorname)%(authorname:mailmap)"
+"%(authoremail)%(authoremail:mailmap)"
 "%(authordate)%(authordate:default)%(authordate:relative)"
 "%(authordate:short)%(authordate:local)"
 "%(authordate:iso8601)%(authordate:iso)"
 "%(authordate:rfc2822)%(authordate:rfc)"
 "%(authordate:unix)%(authordate:raw)"
+"%(committername)%(committername:mailmap)"
+"%(committeremail)%(committeremail:mailmap)"
 "%(committerdate)%(committerdate:default)%(committerdate:relative)"
 "%(committerdate:short)%(committerdate:local)"
 "%(committerdate:iso8601)%(committerdate:iso)"
@@ -46,14 +50,10 @@ static struct strbuf *parts_debug(struct format_parts *parts,
 		{FORMAT_PART_TREE_HASH, "TREE_HASH"},
 		{FORMAT_PART_TREE_HASH_ABBREV, "TREE_HASH_ABBREV"},
 		{FORMAT_PART_AUTHOR_NAME, "AUTHOR_NAME"},
-		{FORMAT_PART_AUTHOR_NAME_MAILMAP, "AUTHOR_NAME_MAILMAP"},
 		{FORMAT_PART_AUTHOR_EMAIL, "AUTHOR_EMAIL"},
-		{FORMAT_PART_AUTHOR_EMAIL_MAILMAP, "AUTHOR_EMAIL_MAILMAP"},
 		{FORMAT_PART_AUTHOR_DATE, "AUTHOR_DATE"},
 		{FORMAT_PART_COMMITTER_NAME, "COMMITTER_NAME"},
-		{FORMAT_PART_COMMITTER_NAME_MAILMAP, "COMMITTER_NAME_MAILMAP"},
 		{FORMAT_PART_COMMITTER_EMAIL, "COMMITTER_EMAIL"},
-		{FORMAT_PART_COMMITTER_EMAIL_MAILMAP, "COMMITTER_EMAIL_MAILMAP"},
 		{FORMAT_PART_COMMITTER_DATE, "COMMITTER_DATE"},
 
 		{FORMAT_PART_DECORATE, "DECORATE"},
@@ -155,6 +155,12 @@ static struct strbuf *parts_debug(struct format_parts *parts,
 					strbuf_addstr(buf, "UINT:");
 					strbuf_addf(buf, "%lu", part->args[j].uint);
 					break;
+				case FORMAT_ARG_BOOLEAN:
+					strbuf_addstr(buf, "BOOLEAN:");
+					strbuf_addstr(buf,
+						      part->args[j].boolean ?
+						       "TRUE" : "FALSE");
+					break;
 				case FORMAT_ARG_DATE_MODE:
 					strbuf_addstr(buf, "DATE_MODE:");
 					switch(part->args[j].dmode){
-- 
1.7.4.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH/RFC 0/9] add long forms for format placeholders
  2011-03-28 23:17 [PATCH/RFC 0/9] add long forms for format placeholders Will Palmer
                   ` (8 preceding siblings ...)
  2011-03-28 23:17 ` [PATCH/RFC 9/9] add long forms for author and committer identity Will Palmer
@ 2011-03-29  0:28 ` Junio C Hamano
  2011-03-29  6:44   ` Will Palmer
  2011-03-29  6:46   ` Michael J Gruber
  9 siblings, 2 replies; 14+ messages in thread
From: Junio C Hamano @ 2011-03-29  0:28 UTC (permalink / raw
  To: Will Palmer; +Cc: git, Jeff King

Will Palmer <wmpalmer@gmail.com> writes:

> I've been kicking around this series for a while now as part of a larger
> effort of refactoring the pretty formats. A recent discussion on the
> list has lead me to believe that this smaller subset may be of use
> sooner, rather than later.
>
> This series attempts to add "long forms" for common format placeholders
> in the "git log" family of commands, making the way for yet more
> placeholders to be added without needing to worry too much about the
> increasingly limited set of available one-letter mnemonics. It also
> moves towards the possibility of eventual unification with the format
> options in for-each-ref.

I don't claim that I read 1300+ long [PATCH 5/9] carefully, but I like the
direction in which this topic is going very much.

Except that [PATCH 2/9] looked quite out of place---more like "I wanted to
sneak this feature in" than "this was needed to keep the resulting code
backward compatible" or anything like that.

Off the top of my head, I don't think of a reason to say that [PATCH 3/9]
is going in a wrong direction---is there a reason to make you worried in
the particular change?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH/RFC 0/9] add long forms for format placeholders
  2011-03-29  0:28 ` [PATCH/RFC 0/9] add long forms for format placeholders Junio C Hamano
@ 2011-03-29  6:44   ` Will Palmer
  2011-03-29  6:46   ` Michael J Gruber
  1 sibling, 0 replies; 14+ messages in thread
From: Will Palmer @ 2011-03-29  6:44 UTC (permalink / raw
  To: Junio C Hamano; +Cc: git, Jeff King

On Mon, 2011-03-28 at 17:28 -0700, Junio C Hamano wrote:
> Will Palmer <wmpalmer@gmail.com> writes:
> 
> > I've been kicking around this series for a while now as part of a larger
> > effort of refactoring the pretty formats. A recent discussion on the
> > list has lead me to believe that this smaller subset may be of use
> > sooner, rather than later.
> >
> > This series attempts to add "long forms" for common format placeholders
> > in the "git log" family of commands, making the way for yet more
> > placeholders to be added without needing to worry too much about the
> > increasingly limited set of available one-letter mnemonics. It also
> > moves towards the possibility of eventual unification with the format
> > options in for-each-ref.
> 
> I don't claim that I read 1300+ long [PATCH 5/9] carefully, but I like the
> direction in which this topic is going very much.
> 
> Except that [PATCH 2/9] looked quite out of place---more like "I wanted to
> sneak this feature in" than "this was needed to keep the resulting code
> backward compatible" or anything like that.

just for context, we're talking about:
 [PATCH/RFC 2/9] add support for --date=unix to complement %at

I was warned that I should tweak that message!

This one is actually in there to make the later [PATCH 5/9] more
consistent in how it handled dates, as: {AUTHOR,COMMITTER}_DATE: <TYPE>,
rather than having a special case just for _UNIX.

When I added DATE_UNIX to the enum, gcc started complaining about
unhandled enum values in switch()es. To get around those (and noticing
that %at was the only format that wasn't available as a --date= switch)
--date=unix was added. It seemed like a good idea to move that change to
earlier in the series, rather than "sneaking it in" as part of [PATCH
5/9]

Of course, [PATCH 1/9] is only in there to make the documentation tweaks
in [PATCH 2/9] more readable.

> 
> Off the top of my head, I don't think of a reason to say that [PATCH 3/9]
> is going in a wrong direction---is there a reason to make you worried in
> the particular change?

 [PATCH/RFC 3/9] interpret %C(invalid) as we would %%C(invalid)

This one I was iffy on. On the one hand, it's inconsistent to treat
%C(invalid) any differently from %Z(doesn't even exist), but on the
other hand we lose feedback of telling the user why it's actually not
working as intended.

The real purpose of it was to prevent strange messages later on in the
larger series, which adds support for %(alias:<aliasname>). Seeing the
message "bad color value 'bkue' for variable '--pretty format'" when
what you actually typed was:
    commit %h%+(alias:mergeline)%+(alias:message)
could be confusing.

> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
-- Will

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH/RFC 0/9] add long forms for format placeholders
  2011-03-29  0:28 ` [PATCH/RFC 0/9] add long forms for format placeholders Junio C Hamano
  2011-03-29  6:44   ` Will Palmer
@ 2011-03-29  6:46   ` Michael J Gruber
  2011-03-29  7:27     ` Will Palmer
  1 sibling, 1 reply; 14+ messages in thread
From: Michael J Gruber @ 2011-03-29  6:46 UTC (permalink / raw
  To: Junio C Hamano; +Cc: Will Palmer, git, Jeff King

Junio C Hamano venit, vidit, dixit 29.03.2011 02:28:
> Will Palmer <wmpalmer@gmail.com> writes:
> 
>> I've been kicking around this series for a while now as part of a larger
>> effort of refactoring the pretty formats. A recent discussion on the
>> list has lead me to believe that this smaller subset may be of use
>> sooner, rather than later.
>>
>> This series attempts to add "long forms" for common format placeholders
>> in the "git log" family of commands, making the way for yet more
>> placeholders to be added without needing to worry too much about the
>> increasingly limited set of available one-letter mnemonics. It also
>> moves towards the possibility of eventual unification with the format
>> options in for-each-ref.
> 
> I don't claim that I read 1300+ long [PATCH 5/9] carefully, but I like the
> direction in which this topic is going very much.
> 
> Except that [PATCH 2/9] looked quite out of place---more like "I wanted to
> sneak this feature in" than "this was needed to keep the resulting code
> backward compatible" or anything like that.
> 
> Off the top of my head, I don't think of a reason to say that [PATCH 3/9]
> is going in a wrong direction---is there a reason to make you worried in
> the particular change?

I'm wondering how much of this could and should be shared with
for-each-ref. Notable differences that I'm aware of:

- for-each-ref is about (named) refs which can point to any type of
object; rev-list/log is about commit objects

- for-each-ref deals with "few" objects typically, rev-list/log with many

So, other than %(refname), %(upstream) and %(tagger...), all
for-each-ref placeholders make sense for rev-list/log.

Sharing the parser would serve several purposes:

- reduced code
- increased test coverage (for-each-ref tests would test the parser)
- speed up for for-each-ref (due to your nice separation)
- short placeholders for for-each-ref
- automatic consistency between the two

Michael

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH/RFC 0/9] add long forms for format placeholders
  2011-03-29  6:46   ` Michael J Gruber
@ 2011-03-29  7:27     ` Will Palmer
  0 siblings, 0 replies; 14+ messages in thread
From: Will Palmer @ 2011-03-29  7:27 UTC (permalink / raw
  To: Michael J Gruber; +Cc: Junio C Hamano, git, Jeff King

On Tue, 2011-03-29 at 08:46 +0200, Michael J Gruber wrote:
> Junio C Hamano venit, vidit, dixit 29.03.2011 02:28:
> > Will Palmer <wmpalmer@gmail.com> writes:
> > 
> >> I've been kicking around this series for a while now as part of a larger
> >> effort of refactoring the pretty formats. A recent discussion on the
> >> list has lead me to believe that this smaller subset may be of use
> >> sooner, rather than later.
> >>
> >> This series attempts to add "long forms" for common format placeholders
> >> in the "git log" family of commands, making the way for yet more
> >> placeholders to be added without needing to worry too much about the
> >> increasingly limited set of available one-letter mnemonics. It also
> >> moves towards the possibility of eventual unification with the format
> >> options in for-each-ref.
> > 
> > I don't claim that I read 1300+ long [PATCH 5/9] carefully, but I like the
> > direction in which this topic is going very much.
> > 
> > Except that [PATCH 2/9] looked quite out of place---more like "I wanted to
> > sneak this feature in" than "this was needed to keep the resulting code
> > backward compatible" or anything like that.
> > 
> > Off the top of my head, I don't think of a reason to say that [PATCH 3/9]
> > is going in a wrong direction---is there a reason to make you worried in
> > the particular change?
> 
> I'm wondering how much of this could and should be shared with
> for-each-ref. ......................................

I agree with this.
Not only that, but I think the "list" modes of branch and tag should
also call for-each-ref internally, and I hope that some of the
conditional formats that this series is moving slowly towards will help
with that.

> ............. Notable differences that I'm aware of:
> 
> - for-each-ref is about (named) refs which can point to any type of
> object; rev-list/log is about commit objects
> 
> - for-each-ref deals with "few" objects typically, rev-list/log with many
> 
> So, other than %(refname), %(upstream) and %(tagger...), all
> for-each-ref placeholders make sense for rev-list/log.

I think the "right thing to do" here is to allow the parser to accept
any of the for-each-ref specifications, but for the formatter to return
an empty string for anything that doesn't make sense in context. This is
what for-each-ref currently does. for-each-ref also gives an empty
string for some invalid specifications, such as %(tree:short), but I
assume this is a bug.

I'm not sure what the implications are in terms of what additional
structures we'll need to pass in to the formatter, as I haven't looked
much at the for-each-ref code. It may also be that there are some
commit-related things which for-each-ref doesn't currently bother to
grab, since its placeholder list is comparatively smaller than the
rev-list one.

> 
> Sharing the parser would serve several purposes:
> 
> - reduced code
> - increased test coverage (for-each-ref tests would test the parser)
> - speed up for for-each-ref (due to your nice separation)
> - short placeholders for for-each-ref
> - automatic consistency between the two
> 

This is already a part of my longer-term plans, though those were mostly
as a "I bet it would be fairly simple to do this once the rest is done".
What I'm actually working towards is strictly related to the --pretty=
formats, so I expect it will be a while before I get to anything like
for-each-ref unification. It may also be worth noting that the last part
of this work I submitted, "pretty aliases", was sent nearly a year ago.
I am not going to be working on any of this full-time.
The point here is: I would not be offended if someone were to snatch
for-each-ref unification up from me, since I really don't know when I
would get to it myself.


> Michael

-- 
-- Will

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2011-03-29  7:27 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-28 23:17 [PATCH/RFC 0/9] add long forms for format placeholders Will Palmer
2011-03-28 23:17 ` [PATCH/RFC 1/9] mention --date=raw in rev-list and blame help Will Palmer
2011-03-28 23:17 ` [PATCH/RFC 2/9] add support for --date=unix to complement %at Will Palmer
2011-03-28 23:17 ` [PATCH/RFC 3/9] interpret %C(invalid) as we would %%C(invalid) Will Palmer
2011-03-28 23:17 ` [PATCH/RFC 4/9] add sanity length check to format_person_part Will Palmer
2011-03-28 23:17 ` [PATCH/RFC 5/9] refactor pretty.c into "parse" and "format" steps Will Palmer
2011-03-28 23:17 ` [PATCH/RFC 6/9] add long-form %(wrap:...) for %w(...) Will Palmer
2011-03-28 23:17 ` [PATCH/RFC 7/9] add long form %(color:...) for %C(...) Will Palmer
2011-03-28 23:17 ` [PATCH/RFC 8/9] add long forms %(authordate) and %(committerdate) Will Palmer
2011-03-28 23:17 ` [PATCH/RFC 9/9] add long forms for author and committer identity Will Palmer
2011-03-29  0:28 ` [PATCH/RFC 0/9] add long forms for format placeholders Junio C Hamano
2011-03-29  6:44   ` Will Palmer
2011-03-29  6:46   ` Michael J Gruber
2011-03-29  7:27     ` Will Palmer

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).