git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/15] Towards a more awesome git branch
@ 2013-06-09 17:54 Ramkumar Ramachandra
  2013-06-09 17:54 ` [PATCH v2 01/15] for-each-ref, quote: convert *_quote_print -> *_quote_buf Ramkumar Ramachandra
                   ` (15 more replies)
  0 siblings, 16 replies; 19+ messages in thread
From: Ramkumar Ramachandra @ 2013-06-09 17:54 UTC (permalink / raw)
  To: Git List; +Cc: Nguyễn Thái Ngọc Duy, Junio C Hamano

Hi,

This iteration contains some minor fixups (courtesy reviews by Eric
Sunshine and Junio), and some tests from Duy squashed in.  Also,
missing signoffs from Duy filled in.

Let's get this merged and work on stuff to do on top.

Thanks.

Nguyễn Thái Ngọc Duy (8):
  for-each-ref, quote: convert *_quote_print -> *_quote_buf
  for-each-ref: don't print out elements directly
  pretty: extend pretty_print_context with callback
  pretty: allow passing NULL commit to format_commit_message()
  for-each-ref: get --pretty using format_commit_message()
  for-each-ref: teach verify_format() about pretty's syntax
  for-each-ref: introduce format specifier %>(*) and %<(*)
  for-each-ref: improve responsiveness of %(upstream:track)

Ramkumar Ramachandra (7):
  tar-tree: remove dependency on sq_quote_print()
  quote: remove sq_quote_print()
  pretty: limit recursion in format_commit_one()
  for-each-ref: introduce %(HEAD) marker
  for-each-ref: introduce %(upstream:track[short])
  pretty: introduce get_pretty_userformat
  for-each-ref: use get_pretty_userformat in --pretty

 Documentation/git-for-each-ref.txt |  43 +++++-
 builtin/for-each-ref.c             | 279 ++++++++++++++++++++++++++++++-------
 builtin/tar-tree.c                 |  11 +-
 commit.h                           |   9 ++
 pretty.c                           |  77 +++++++++-
 quote.c                            |  61 +++-----
 quote.h                            |   8 +-
 t/t6300-for-each-ref.sh            | 143 +++++++++++++++++++
 8 files changed, 521 insertions(+), 110 deletions(-)

-- 
1.8.3.247.g485169c

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

* [PATCH v2 01/15] for-each-ref, quote: convert *_quote_print -> *_quote_buf
  2013-06-09 17:54 [PATCH v2 00/15] Towards a more awesome git branch Ramkumar Ramachandra
@ 2013-06-09 17:54 ` Ramkumar Ramachandra
  2013-06-09 17:54 ` [PATCH v2 02/15] for-each-ref: don't print out elements directly Ramkumar Ramachandra
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Ramkumar Ramachandra @ 2013-06-09 17:54 UTC (permalink / raw)
  To: Git List; +Cc: Nguyễn Thái Ngọc Duy, Junio C Hamano

From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

for-each-ref.c:print_value() currently prints values to stdout
immediately using {sq|perl|python|tcl}_quote_print, giving us no
opportunity to do any further processing.  In preparation for getting
print_value() to accept an additional strbuf argument to write to,
convert the *_quote_print functions and callers to *_quote_buf.

[rr: commit message, minor modifications]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 builtin/for-each-ref.c | 13 +++++++++----
 quote.c                | 44 ++++++++++++++++++++++----------------------
 quote.h                |  6 +++---
 3 files changed, 34 insertions(+), 29 deletions(-)

diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 7f059c3..1d4083c 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -867,24 +867,29 @@ static void sort_refs(struct ref_sort *sort, struct refinfo **refs, int num_refs
 static void print_value(struct refinfo *ref, int atom, int quote_style)
 {
 	struct atom_value *v;
+	struct strbuf sb = STRBUF_INIT;
 	get_value(ref, atom, &v);
 	switch (quote_style) {
 	case QUOTE_NONE:
 		fputs(v->s, stdout);
 		break;
 	case QUOTE_SHELL:
-		sq_quote_print(stdout, v->s);
+		sq_quote_buf(&sb, v->s);
 		break;
 	case QUOTE_PERL:
-		perl_quote_print(stdout, v->s);
+		perl_quote_buf(&sb, v->s);
 		break;
 	case QUOTE_PYTHON:
-		python_quote_print(stdout, v->s);
+		python_quote_buf(&sb, v->s);
 		break;
 	case QUOTE_TCL:
-		tcl_quote_print(stdout, v->s);
+		tcl_quote_buf(&sb, v->s);
 		break;
 	}
+	if (quote_style != QUOTE_NONE) {
+		fputs(sb.buf, stdout);
+		strbuf_release(&sb);
+	}
 }
 
 static int hex1(char ch)
diff --git a/quote.c b/quote.c
index 911229f..8c294df 100644
--- a/quote.c
+++ b/quote.c
@@ -463,72 +463,72 @@ int unquote_c_style(struct strbuf *sb, const char *quoted, const char **endp)
 
 /* quoting as a string literal for other languages */
 
-void perl_quote_print(FILE *stream, const char *src)
+void perl_quote_buf(struct strbuf *sb, const char *src)
 {
 	const char sq = '\'';
 	const char bq = '\\';
 	char c;
 
-	fputc(sq, stream);
+	strbuf_addch(sb, sq);
 	while ((c = *src++)) {
 		if (c == sq || c == bq)
-			fputc(bq, stream);
-		fputc(c, stream);
+			strbuf_addch(sb, bq);
+		strbuf_addch(sb, c);
 	}
-	fputc(sq, stream);
+	strbuf_addch(sb, sq);
 }
 
-void python_quote_print(FILE *stream, const char *src)
+void python_quote_buf(struct strbuf *sb, const char *src)
 {
 	const char sq = '\'';
 	const char bq = '\\';
 	const char nl = '\n';
 	char c;
 
-	fputc(sq, stream);
+	strbuf_addch(sb, sq);
 	while ((c = *src++)) {
 		if (c == nl) {
-			fputc(bq, stream);
-			fputc('n', stream);
+			strbuf_addch(sb, bq);
+			strbuf_addch(sb, 'n');
 			continue;
 		}
 		if (c == sq || c == bq)
-			fputc(bq, stream);
-		fputc(c, stream);
+			strbuf_addch(sb, bq);
+		strbuf_addch(sb, c);
 	}
-	fputc(sq, stream);
+	strbuf_addch(sb, sq);
 }
 
-void tcl_quote_print(FILE *stream, const char *src)
+void tcl_quote_buf(struct strbuf *sb, const char *src)
 {
 	char c;
 
-	fputc('"', stream);
+	strbuf_addch(sb, '"');
 	while ((c = *src++)) {
 		switch (c) {
 		case '[': case ']':
 		case '{': case '}':
 		case '$': case '\\': case '"':
-			fputc('\\', stream);
+			strbuf_addch(sb, '\\');
 		default:
-			fputc(c, stream);
+			strbuf_addch(sb, c);
 			break;
 		case '\f':
-			fputs("\\f", stream);
+			strbuf_addstr(sb, "\\f");
 			break;
 		case '\r':
-			fputs("\\r", stream);
+			strbuf_addstr(sb, "\\r");
 			break;
 		case '\n':
-			fputs("\\n", stream);
+			strbuf_addstr(sb, "\\n");
 			break;
 		case '\t':
-			fputs("\\t", stream);
+			strbuf_addstr(sb, "\\t");
 			break;
 		case '\v':
-			fputs("\\v", stream);
+			strbuf_addstr(sb, "\\v");
 			break;
 		}
 	}
-	fputc('"', stream);
+	strbuf_addch(sb, '"');
 }
diff --git a/quote.h b/quote.h
index 133155a..ed06df5 100644
--- a/quote.h
+++ b/quote.h
@@ -69,8 +69,8 @@ extern char *quote_path_relative(const char *in, int len,
 			  struct strbuf *out, const char *prefix);
 
 /* quoting as a string literal for other languages */
-extern void perl_quote_print(FILE *stream, const char *src);
-extern void python_quote_print(FILE *stream, const char *src);
-extern void tcl_quote_print(FILE *stream, const char *src);
+extern void perl_quote_buf(struct strbuf *sb, const char *src);
+extern void python_quote_buf(struct strbuf *sb, const char *src);
+extern void tcl_quote_buf(struct strbuf *sb, const char *src);
 
 #endif
-- 
1.8.3.247.g485169c

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

* [PATCH v2 02/15] for-each-ref: don't print out elements directly
  2013-06-09 17:54 [PATCH v2 00/15] Towards a more awesome git branch Ramkumar Ramachandra
  2013-06-09 17:54 ` [PATCH v2 01/15] for-each-ref, quote: convert *_quote_print -> *_quote_buf Ramkumar Ramachandra
@ 2013-06-09 17:54 ` Ramkumar Ramachandra
  2013-06-09 17:54 ` [PATCH v2 03/15] tar-tree: remove dependency on sq_quote_print() Ramkumar Ramachandra
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Ramkumar Ramachandra @ 2013-06-09 17:54 UTC (permalink / raw)
  To: Git List; +Cc: Nguyễn Thái Ngọc Duy, Junio C Hamano

From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

Currently, the entire callchain starting from show_ref() parses and
prints immediately.  This inflexibility limits our ability to extend the
parser.  So, convert the entire callchain to accept a strbuf argument to
write to.  Also introduce a show_refs() helper that calls show_ref() in
a loop to avoid cluttering up cmd_for_each_ref() with the task of
initializing/freeing the strbuf.

[rr: commit message]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 builtin/for-each-ref.c | 55 ++++++++++++++++++++++++++++++++------------------
 1 file changed, 35 insertions(+), 20 deletions(-)

diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 1d4083c..e2d6c5a 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -864,31 +864,31 @@ static void sort_refs(struct ref_sort *sort, struct refinfo **refs, int num_refs
 	qsort(refs, num_refs, sizeof(struct refinfo *), compare_refs);
 }
 
-static void print_value(struct refinfo *ref, int atom, int quote_style)
+static void print_value(struct strbuf *sb, struct refinfo *ref,
+			int atom, int quote_style)
 {
 	struct atom_value *v;
-	struct strbuf sb = STRBUF_INIT;
 	get_value(ref, atom, &v);
 	switch (quote_style) {
 	case QUOTE_NONE:
-		fputs(v->s, stdout);
+		strbuf_addstr(sb, v->s);
 		break;
 	case QUOTE_SHELL:
-		sq_quote_buf(&sb, v->s);
+		sq_quote_buf(sb, v->s);
 		break;
 	case QUOTE_PERL:
-		perl_quote_buf(&sb, v->s);
+		perl_quote_buf(sb, v->s);
 		break;
 	case QUOTE_PYTHON:
-		python_quote_buf(&sb, v->s);
+		python_quote_buf(sb, v->s);
 		break;
 	case QUOTE_TCL:
-		tcl_quote_buf(&sb, v->s);
+		tcl_quote_buf(sb, v->s);
 		break;
 	}
 	if (quote_style != QUOTE_NONE) {
-		fputs(sb.buf, stdout);
-		strbuf_release(&sb);
+		fputs(sb->buf, stdout);
+		strbuf_release(sb);
 	}
 }
 
@@ -910,7 +910,7 @@ static int hex2(const char *cp)
 		return -1;
 }
 
-static void emit(const char *cp, const char *ep)
+static void emit(struct strbuf *sb, const char *cp, const char *ep)
 {
 	while (*cp && (!ep || cp < ep)) {
 		if (*cp == '%') {
@@ -919,32 +919,47 @@ static void emit(const char *cp, const char *ep)
 			else {
 				int ch = hex2(cp + 1);
 				if (0 <= ch) {
-					putchar(ch);
+					strbuf_addch(sb, ch);
 					cp += 3;
 					continue;
 				}
 			}
 		}
-		putchar(*cp);
+		strbuf_addch(sb, *cp);
 		cp++;
 	}
 }
 
-static void show_ref(struct refinfo *info, const char *format, int quote_style)
+static void show_ref(struct strbuf *sb, struct refinfo *info,
+		     const char *format, int quote_style)
 {
 	const char *cp, *sp, *ep;
 
 	for (cp = format; *cp && (sp = find_next(cp)); cp = ep + 1) {
 		ep = strchr(sp, ')');
 		if (cp < sp)
-			emit(cp, sp);
-		print_value(info, parse_atom(sp + 2, ep), quote_style);
+			emit(sb, cp, sp);
+		print_value(sb, info, parse_atom(sp + 2, ep), quote_style);
 	}
 	if (*cp) {
 		sp = cp + strlen(cp);
-		emit(cp, sp);
+		emit(sb, cp, sp);
 	}
-	putchar('\n');
+	strbuf_addch(sb, '\n');
+}
+
+static void show_refs(struct refinfo **refs, int maxcount,
+		      const char *format, int quote_style)
+{
+	struct strbuf sb = STRBUF_INIT;
+	int i;
+
+	for (i = 0; i < maxcount; i++) {
+		strbuf_reset(&sb);
+		show_ref(&sb, refs[i], format, quote_style);
+		fputs(sb.buf, stdout);
+	}
+	strbuf_release(&sb);
 }
 
 static struct ref_sort *default_sort(void)
@@ -987,7 +1002,7 @@ static char const * const for_each_ref_usage[] = {
 
 int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 {
-	int i, num_refs;
+	int num_refs;
 	const char *format = "%(objectname) %(objecttype)\t%(refname)";
 	struct ref_sort *sort = NULL, **sort_tail = &sort;
 	int maxcount = 0, quote_style = 0;
@@ -1041,7 +1056,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 
 	if (!maxcount || num_refs < maxcount)
 		maxcount = num_refs;
-	for (i = 0; i < maxcount; i++)
-		show_ref(refs[i], format, quote_style);
+
+	show_refs(refs, maxcount, format, quote_style);
 	return 0;
 }
-- 
1.8.3.247.g485169c

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

* [PATCH v2 03/15] tar-tree: remove dependency on sq_quote_print()
  2013-06-09 17:54 [PATCH v2 00/15] Towards a more awesome git branch Ramkumar Ramachandra
  2013-06-09 17:54 ` [PATCH v2 01/15] for-each-ref, quote: convert *_quote_print -> *_quote_buf Ramkumar Ramachandra
  2013-06-09 17:54 ` [PATCH v2 02/15] for-each-ref: don't print out elements directly Ramkumar Ramachandra
@ 2013-06-09 17:54 ` Ramkumar Ramachandra
  2013-06-09 17:54 ` [PATCH v2 04/15] quote: remove sq_quote_print() Ramkumar Ramachandra
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Ramkumar Ramachandra @ 2013-06-09 17:54 UTC (permalink / raw)
  To: Git List; +Cc: Nguyễn Thái Ngọc Duy, Junio C Hamano

Currently, there is exactly one caller of sq_quote_print(), namely
cmd_tar_tree().  In the interest of removing sq_quote_print() and
simplification, replace it with an equivalent call to sq_quote_argv().
No functional changes intended.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/tar-tree.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/builtin/tar-tree.c b/builtin/tar-tree.c
index 3f1e701..ba3ffe6 100644
--- a/builtin/tar-tree.c
+++ b/builtin/tar-tree.c
@@ -26,8 +26,8 @@ int cmd_tar_tree(int argc, const char **argv, const char *prefix)
 	 * $0 tree-ish basedir ==>
 	 * 	git archive --format-tar --prefix=basedir tree-ish
 	 */
-	int i;
 	const char **nargv = xcalloc(sizeof(*nargv), argc + 3);
+	struct strbuf sb = STRBUF_INIT;
 	char *basedir_arg;
 	int nargc = 0;
 
@@ -65,11 +65,10 @@ int cmd_tar_tree(int argc, const char **argv, const char *prefix)
 	fprintf(stderr,
 		"*** \"git tar-tree\" is now deprecated.\n"
 		"*** Running \"git archive\" instead.\n***");
-	for (i = 0; i < nargc; i++) {
-		fputc(' ', stderr);
-		sq_quote_print(stderr, nargv[i]);
-	}
-	fputc('\n', stderr);
+	sq_quote_argv(&sb, nargv, 0);
+	strbuf_addch(&sb, '\n');
+	fputs(sb.buf, stderr);
+	strbuf_release(&sb);
 	return cmd_archive(nargc, nargv, prefix);
 }
 
-- 
1.8.3.247.g485169c

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

* [PATCH v2 04/15] quote: remove sq_quote_print()
  2013-06-09 17:54 [PATCH v2 00/15] Towards a more awesome git branch Ramkumar Ramachandra
                   ` (2 preceding siblings ...)
  2013-06-09 17:54 ` [PATCH v2 03/15] tar-tree: remove dependency on sq_quote_print() Ramkumar Ramachandra
@ 2013-06-09 17:54 ` Ramkumar Ramachandra
  2013-06-09 17:54 ` [PATCH v2 05/15] pretty: extend pretty_print_context with callback Ramkumar Ramachandra
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Ramkumar Ramachandra @ 2013-06-09 17:54 UTC (permalink / raw)
  To: Git List; +Cc: Nguyễn Thái Ngọc Duy, Junio C Hamano

Remove sq_quote_print() since it has no callers.  A nicer alternative
sq_quote_buf() exists: its callers aren't forced to print immediately.

For historical context, sq_quote_print() was first introduced in
575ba9d6 (GIT_TRACE: show which built-in/external commands are executed,
2006-06-25) for the purpose of printing argv for $GIT_TRACE.  Today, we
achieve this using trace_argv_printf() -> sq_quote_argv() ->
sq_quote_buf(), which ultimately fills in a strbuf.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 quote.c | 17 -----------------
 quote.h |  2 --
 2 files changed, 19 deletions(-)

diff --git a/quote.c b/quote.c
index 8c294df..778b39a 100644
--- a/quote.c
+++ b/quote.c
@@ -42,23 +42,6 @@ void sq_quote_buf(struct strbuf *dst, const char *src)
 	free(to_free);
 }
 
-void sq_quote_print(FILE *stream, const char *src)
-{
-	char c;
-
-	fputc('\'', stream);
-	while ((c = *src++)) {
-		if (need_bs_quote(c)) {
-			fputs("'\\", stream);
-			fputc(c, stream);
-			fputc('\'', stream);
-		} else {
-			fputc(c, stream);
-		}
-	}
-	fputc('\'', stream);
-}
-
 void sq_quote_argv(struct strbuf *dst, const char** argv, size_t maxlen)
 {
 	int i;
diff --git a/quote.h b/quote.h
index ed06df5..251f3cc 100644
--- a/quote.h
+++ b/quote.h
@@ -27,8 +27,6 @@ struct strbuf;
  * excluding the final null regardless of the buffer size.
  */
 
-extern void sq_quote_print(FILE *stream, const char *src);
-
 extern void sq_quote_buf(struct strbuf *, const char *src);
 extern void sq_quote_argv(struct strbuf *, const char **argv, size_t maxlen);
 
-- 
1.8.3.247.g485169c

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

* [PATCH v2 05/15] pretty: extend pretty_print_context with callback
  2013-06-09 17:54 [PATCH v2 00/15] Towards a more awesome git branch Ramkumar Ramachandra
                   ` (3 preceding siblings ...)
  2013-06-09 17:54 ` [PATCH v2 04/15] quote: remove sq_quote_print() Ramkumar Ramachandra
@ 2013-06-09 17:54 ` Ramkumar Ramachandra
  2013-06-09 17:54 ` [PATCH v2 06/15] pretty: limit recursion in format_commit_one() Ramkumar Ramachandra
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Ramkumar Ramachandra @ 2013-06-09 17:54 UTC (permalink / raw)
  To: Git List; +Cc: Nguyễn Thái Ngọc Duy, Junio C Hamano

From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

The struct pretty_print_context contains the context in which the
placeholders in format_commit_one() should be parsed.  Although
format_commit_one() primarily acts as a parser, there is no way for a
caller to plug in custom callbacks.  Now, callers can:

1. Parse a custom placeholder that is not supported by
   format_commit_one(), and act on it independently of the pretty
   machinery.

2. Parse a custom placeholder to substitute the custom placeholder with
   a placeholder that format_commit_one() understands.  This is
   especially useful for supporting %>(*), where * is substituted with a
   length computed by the caller.

To support these two usecases, the interface for the function looks
like:

typedef size_t (*format_message_fn)(struct strbuf *sb,
				const char *placeholder,
				void *format_context,
				void *user_data,
				struct strbuf *placeholder_subst)

It is exactly like format_commit_one(), except that there are two
additional fields: user_data (to pass custom data to the callback), and
placeholder_subst (to set the substitution).  The callback should return
the length of the original string parsed, and optionally set
placeholder_subst.

[rr: commit message, minor modifications]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 commit.h |  8 ++++++++
 pretty.c | 25 +++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/commit.h b/commit.h
index 6e9c7cd..38dc0c0 100644
--- a/commit.h
+++ b/commit.h
@@ -78,6 +78,12 @@ enum cmit_fmt {
 	CMIT_FMT_UNSPECIFIED
 };
 
+typedef size_t (*format_message_fn)(struct strbuf *sb,
+				const char *placeholder,
+				void *format_context,
+				void *user_data,
+				struct strbuf *placeholder_subst);
+
 struct pretty_print_context {
 	enum cmit_fmt fmt;
 	int abbrev;
@@ -92,6 +98,8 @@ struct pretty_print_context {
 	const char *output_encoding;
 	struct string_list *mailmap;
 	int color;
+	format_message_fn format;
+	void *user_data;
 };
 
 struct userformat_want {
diff --git a/pretty.c b/pretty.c
index 9e43154..095e5ba 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1069,6 +1069,31 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
 	struct commit_list *p;
 	int h1, h2;
 
+	if (c->pretty_ctx->format) {
+		struct strbuf subst = STRBUF_INIT;
+		int ret = c->pretty_ctx->format(sb, placeholder, context,
+						c->pretty_ctx->user_data,
+						&subst);
+		if (ret && subst.len) {
+			/*
+			 * Something was parsed by format(), and a
+			 * placeholder-substitution was set.
+			 * Recursion is required to override the
+			 * return value of format_commit_one() with
+			 * ret: the length of the original string
+			 * before substitution.
+			 */
+			ret = format_commit_one(sb, subst.buf, context) ? ret : 0;
+			strbuf_release(&subst);
+			return ret;
+		} else if (ret)
+			/*
+			 * Something was parsed by format(), but
+			 * no placeholder-substitution was set.
+			 */
+			return ret;
+	}
+
 	/* these are independent of the commit */
 	switch (placeholder[0]) {
 	case 'C':
-- 
1.8.3.247.g485169c

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

* [PATCH v2 06/15] pretty: limit recursion in format_commit_one()
  2013-06-09 17:54 [PATCH v2 00/15] Towards a more awesome git branch Ramkumar Ramachandra
                   ` (4 preceding siblings ...)
  2013-06-09 17:54 ` [PATCH v2 05/15] pretty: extend pretty_print_context with callback Ramkumar Ramachandra
@ 2013-06-09 17:54 ` Ramkumar Ramachandra
  2013-06-09 17:54 ` [PATCH v2 07/15] pretty: allow passing NULL commit to format_commit_message() Ramkumar Ramachandra
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Ramkumar Ramachandra @ 2013-06-09 17:54 UTC (permalink / raw)
  To: Git List; +Cc: Nguyễn Thái Ngọc Duy, Junio C Hamano

To make sure that a pretty_ctx->format substitution doesn't result in an
infinite recursion, change the prototype of format_commit_one() to
accept one last argument: no_recurse.  So, a single substitution by
format() must yield a result that can be parsed by format_commit_one()
without the help of format().

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 pretty.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/pretty.c b/pretty.c
index 095e5ba..0063f2d 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1061,7 +1061,8 @@ static size_t parse_padding_placeholder(struct strbuf *sb,
 
 static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
 				const char *placeholder,
-				void *context)
+				void *context,
+				int no_recurse)
 {
 	struct format_commit_context *c = context;
 	const struct commit *commit = c->commit;
@@ -1069,7 +1070,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
 	struct commit_list *p;
 	int h1, h2;
 
-	if (c->pretty_ctx->format) {
+	if (!no_recurse && c->pretty_ctx->format) {
 		struct strbuf subst = STRBUF_INIT;
 		int ret = c->pretty_ctx->format(sb, placeholder, context,
 						c->pretty_ctx->user_data,
@@ -1083,7 +1084,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
 			 * ret: the length of the original string
 			 * before substitution.
 			 */
-			ret = format_commit_one(sb, subst.buf, context) ? ret : 0;
+			ret = format_commit_one(sb, subst.buf, context, 1) ? ret : 0;
 			strbuf_release(&subst);
 			return ret;
 		} else if (ret)
@@ -1332,7 +1333,7 @@ static size_t format_and_pad_commit(struct strbuf *sb, /* in UTF-8 */
 	}
 	while (1) {
 		int modifier = *placeholder == 'C';
-		int consumed = format_commit_one(&local_sb, placeholder, c);
+		int consumed = format_commit_one(&local_sb, placeholder, c, 0);
 		total_consumed += consumed;
 
 		if (!modifier)
@@ -1452,7 +1453,7 @@ static size_t format_commit_item(struct strbuf *sb, /* in UTF-8 */
 	if (((struct format_commit_context *)context)->flush_type != no_flush)
 		consumed = format_and_pad_commit(sb, placeholder, context);
 	else
-		consumed = format_commit_one(sb, placeholder, context);
+		consumed = format_commit_one(sb, placeholder, context, 0);
 	if (magic == NO_MAGIC)
 		return consumed;
 
-- 
1.8.3.247.g485169c

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

* [PATCH v2 07/15] pretty: allow passing NULL commit to format_commit_message()
  2013-06-09 17:54 [PATCH v2 00/15] Towards a more awesome git branch Ramkumar Ramachandra
                   ` (5 preceding siblings ...)
  2013-06-09 17:54 ` [PATCH v2 06/15] pretty: limit recursion in format_commit_one() Ramkumar Ramachandra
@ 2013-06-09 17:54 ` Ramkumar Ramachandra
  2013-06-09 17:54 ` [PATCH v2 08/15] for-each-ref: get --pretty using format_commit_message() Ramkumar Ramachandra
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Ramkumar Ramachandra @ 2013-06-09 17:54 UTC (permalink / raw)
  To: Git List; +Cc: Nguyễn Thái Ngọc Duy, Junio C Hamano

From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

The new formatter, for-each-ref, may use non-commit placeholders only.
While it could audit the format line and warn/exclude commit
placeholders, that's a lot more work than simply ignore them.
Unrecognized placeholders are displayed as-is, pretty obvious that they
are not handled.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 pretty.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/pretty.c b/pretty.c
index 0063f2d..816aa32 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1156,6 +1156,9 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
 	}
 
 	/* these depend on the commit */
+	if (!commit)
+		return 0;
+
 	if (!commit->object.parsed)
 		parse_object(commit->object.sha1);
 
@@ -1276,6 +1279,9 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
 	}
 
 
+	if (!c->message)
+		return 0;
+
 	/* For the rest we have to parse the commit header. */
 	if (!c->commit_header_parsed)
 		parse_commit_header(c);
@@ -1510,9 +1516,10 @@ void format_commit_message(const struct commit *commit,
 	context.commit = commit;
 	context.pretty_ctx = pretty_ctx;
 	context.wrap_start = sb->len;
-	context.message = logmsg_reencode(commit,
-					  &context.commit_encoding,
-					  output_enc);
+	if (commit)
+		context.message = logmsg_reencode(commit,
+						  &context.commit_encoding,
+						  output_enc);
 
 	strbuf_expand(sb, format, format_commit_item, &context);
 	rewrap_message_tail(sb, &context, 0, 0, 0);
@@ -1535,7 +1542,8 @@ void format_commit_message(const struct commit *commit,
 	}
 
 	free(context.commit_encoding);
-	logmsg_free(context.message, commit);
+	if (commit)
+		logmsg_free(context.message, commit);
 	free(context.signature_check.gpg_output);
 	free(context.signature_check.signer);
 }
-- 
1.8.3.247.g485169c

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

* [PATCH v2 08/15] for-each-ref: get --pretty using format_commit_message()
  2013-06-09 17:54 [PATCH v2 00/15] Towards a more awesome git branch Ramkumar Ramachandra
                   ` (6 preceding siblings ...)
  2013-06-09 17:54 ` [PATCH v2 07/15] pretty: allow passing NULL commit to format_commit_message() Ramkumar Ramachandra
@ 2013-06-09 17:54 ` Ramkumar Ramachandra
  2013-06-09 17:54 ` [PATCH v2 09/15] for-each-ref: teach verify_format() about pretty's syntax Ramkumar Ramachandra
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Ramkumar Ramachandra @ 2013-06-09 17:54 UTC (permalink / raw)
  To: Git List; +Cc: Nguyễn Thái Ngọc Duy, Junio C Hamano

From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

--format is very limited in its capabilities.  Introduce --pretty, which
extends the existing --format with pretty-formats.  In --pretty:

 - Existing --format %(atom) is available. They also accept some pretty
   magic.  For example, you can use "% (atom)" to only display a leading
   space if the atom produces something.

 - %ab to display a hex character 0xab is not available as it may
   conflict with other pretty's placeholders.  Use %xab instead.

 - Many pretty placeholders are designed to work on commits.  While some
   of them should work on tags too, they don't (yet).

 - Unsupported atoms cause for-each-ref to exit early and report.
   Unsupported pretty placeholders are displayed as-is.

 - Pretty placeholders can not be used as a sorting criteria.

--format is considered deprecated. If the user hits a bug specific in
--format code, they are advised to migrate to --pretty.

[rr: documentation]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 Documentation/git-for-each-ref.txt |  23 ++++++-
 builtin/for-each-ref.c             |  72 +++++++++++++++++++++-
 t/t6300-for-each-ref.sh            | 123 +++++++++++++++++++++++++++++++++++++
 3 files changed, 214 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index f2e08d1..d8ad758 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -9,7 +9,8 @@ SYNOPSIS
 --------
 [verse]
 'git for-each-ref' [--count=<count>] [--shell|--perl|--python|--tcl]
-		   [(--sort=<key>)...] [--format=<format>] [<pattern>...]
+		   [(--sort=<key>)...] [--format=<format>|--pretty=<pretty>]
+		   [<pattern>...]
 
 DESCRIPTION
 -----------
@@ -47,6 +48,26 @@ OPTIONS
 	`xx`; for example `%00` interpolates to `\0` (NUL),
 	`%09` to `\t` (TAB) and `%0a` to `\n` (LF).
 
+<pretty>::
+	A format string with supporting placeholders described in the
+	"PRETTY FORMATS" section in linkgit:git-log[1].  Additionally
+	supports placeholders from `<format>`
+	(i.e. `%[*](fieldname)`).
++
+Caveats:
+
+1. Many of the placeholders in "PRETTY FORMATS" are designed to work
+   specifically on commit objects: when non-commit objects are
+   supplied, those placeholders won't work (i.e. they will be emitted
+   literally).
+
+2. Does not interpolate `%ab` (where `ab` are hex digits) with the
+   corresponding hex code.  To print a byte from a hex code, use
+   `%xab` (from pretty-formats) instead.
+
+3. Only the placeholders inherited from `<format>` will respect
+   quoting settings.
+
 <pattern>...::
 	If one or more patterns are given, only refs are shown that
 	match against at least one pattern, either using fnmatch(3) or
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index e2d6c5a..8611777 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -962,6 +962,63 @@ static void show_refs(struct refinfo **refs, int maxcount,
 	strbuf_release(&sb);
 }
 
+struct format_one_atom_context {
+	struct refinfo *info;
+	int quote_style;
+};
+
+static size_t format_one_atom(struct strbuf *sb, const char *placeholder,
+			      void *format_context, void *user_data,
+			      struct strbuf *subst)
+{
+	struct format_one_atom_context *ctx = user_data;
+	const char *ep;
+
+	if (*placeholder == '%') {
+		strbuf_addch(sb, '%');
+		return 1;
+	}
+
+	if (*placeholder != '(')
+		return 0;
+
+	ep = strchr(placeholder + 1, ')');
+	if (!ep)
+		return 0;
+	print_value(sb, ctx->info, parse_atom(placeholder + 1, ep),
+		    ctx->quote_style);
+	return ep + 1 - placeholder;
+}
+
+static void show_pretty_refs(struct refinfo **refs, int maxcount,
+			     const char *format, int quote_style)
+{
+	struct pretty_print_context ctx = {0};
+	struct format_one_atom_context fctx;
+	struct strbuf sb = STRBUF_INIT;
+	int i;
+
+	/*
+	 * FIXME: add --date= for %ad, --decorate for %d and --color
+	 * for %C
+	 */
+	ctx.abbrev = DEFAULT_ABBREV;
+	ctx.format = format_one_atom;
+	ctx.user_data = &fctx;
+	fctx.quote_style = quote_style;
+	for (i = 0; i < maxcount; i++) {
+		struct commit *commit = NULL;
+		fctx.info = refs[i];
+		if (sha1_object_info(refs[i]->objectname, NULL) == OBJ_COMMIT)
+			commit = lookup_commit(refs[i]->objectname);
+		strbuf_reset(&sb);
+		format_commit_message(commit, format, &sb, &ctx);
+		strbuf_addch(&sb, '\n');
+		fputs(sb.buf, stdout);
+	}
+	strbuf_release(&sb);
+}
+
 static struct ref_sort *default_sort(void)
 {
 	static const char cstr_name[] = "refname";
@@ -1003,7 +1060,9 @@ static char const * const for_each_ref_usage[] = {
 int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 {
 	int num_refs;
-	const char *format = "%(objectname) %(objecttype)\t%(refname)";
+	const char *default_format = "%(objectname) %(objecttype)\t%(refname)";
+	const char *format = default_format;
+	const char *pretty = NULL;
 	struct ref_sort *sort = NULL, **sort_tail = &sort;
 	int maxcount = 0, quote_style = 0;
 	struct refinfo **refs;
@@ -1022,6 +1081,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 		OPT_GROUP(""),
 		OPT_INTEGER( 0 , "count", &maxcount, N_("show only <n> matched refs")),
 		OPT_STRING(  0 , "format", &format, N_("format"), N_("format to use for the output")),
+		OPT_STRING(  0 , "pretty", &pretty, N_("format"), N_("alternative format to use for the output")),
 		OPT_CALLBACK(0 , "sort", sort_tail, N_("key"),
 			    N_("field name to sort on"), &opt_parse_sort),
 		OPT_END(),
@@ -1036,7 +1096,10 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 		error("more than one quoting style?");
 		usage_with_options(for_each_ref_usage, opts);
 	}
-	if (verify_format(format))
+	if (format != default_format && pretty)
+		die("--format and --pretty cannot be used together");
+	if ((pretty && verify_format(pretty)) ||
+	    (!pretty && verify_format(format)))
 		usage_with_options(for_each_ref_usage, opts);
 
 	if (!sort)
@@ -1057,6 +1120,9 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 	if (!maxcount || num_refs < maxcount)
 		maxcount = num_refs;
 
-	show_refs(refs, maxcount, format, quote_style);
+	if (pretty)
+		show_pretty_refs(refs, maxcount, pretty, quote_style);
+	else
+		show_refs(refs, maxcount, format, quote_style);
 	return 0;
 }
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 752f5cb..d39e0b4 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -48,6 +48,20 @@ test_atom() {
 	"
 }
 
+test_pretty() {
+	case "$1" in
+		head) ref=refs/heads/master ;;
+		 tag) ref=refs/tags/testtag ;;
+		   *) ref=$1 ;;
+	esac
+	printf '%s\n' "$3" >expected
+	test_expect_${4:-success} $PREREQ "basic pretty: $1 $2" "
+		git for-each-ref --pretty='$2' $ref >actual &&
+		sanitize_pgp <actual >actual.clean &&
+		test_cmp expected actual.clean
+	"
+}
+
 test_atom head refname refs/heads/master
 test_atom head upstream refs/remotes/origin/master
 test_atom head objecttype commit
@@ -114,6 +128,115 @@ test_atom tag contents:signature ''
 test_atom tag contents 'Tagging at 1151939927
 '
 
+echo "Mailmap'd <map@example.com> <author@example.com>" > $HOME/.mailmap
+
+test_pretty head '%(refname)' refs/heads/master
+test_pretty head '%(upstream)' refs/remotes/origin/master
+test_pretty head '%(objecttype)' commit
+test_pretty head '%(objectsize)' 171
+test_pretty head '%(objectname)' 67a36f10722846e891fbada1ba48ed035de75581
+test_pretty head '%H' 67a36f10722846e891fbada1ba48ed035de75581
+test_pretty head '%h' 67a36f1
+test_pretty head '%(tree)' 0e51c00fcb93dffc755546f27593d511e1bdb46f
+test_pretty head '%T' 0e51c00fcb93dffc755546f27593d511e1bdb46f
+test_pretty head '%t' 0e51c00
+test_pretty head '%(parent)' ''
+test_pretty head '%P' ''
+test_pretty head '%(numparent)' 0
+test_pretty head '%(object)' ''
+test_pretty head '%(type)' ''
+test_pretty head '%(author)' 'A U Thor <author@example.com> 1151939924 +0200'
+test_pretty head '%(authorname)' 'A U Thor'
+test_pretty head '%an' 'A U Thor'
+test_pretty head '%aN' "Mailmap'd"
+test_pretty head '%(authoremail)' '<author@example.com>'
+test_pretty head '%ae' 'author@example.com'
+test_pretty head '%aE' 'map@example.com'
+test_pretty head '%(authordate)' 'Mon Jul 3 17:18:44 2006 +0200'
+test_pretty head '%aD' 'Mon, 3 Jul 2006 17:18:44 +0200'
+test_pretty head '%(committer)' 'C O Mitter <committer@example.com> 1151939923 +0200'
+test_pretty head '%(committername)' 'C O Mitter'
+test_pretty head '%cn' 'C O Mitter'
+test_pretty head '%(committeremail)' '<committer@example.com>'
+test_pretty head '%ce' 'committer@example.com'
+test_pretty head '%(committerdate)' 'Mon Jul 3 17:18:43 2006 +0200'
+test_pretty head '%cD' 'Mon, 3 Jul 2006 17:18:43 +0200'
+test_pretty head '%(tag)' ''
+test_pretty head '%(tagger)' ''
+test_pretty head '%(taggername)' ''
+test_pretty head '%(taggeremail)' ''
+test_pretty head '%(taggerdate)' ''
+test_pretty head '%(creator)' 'C O Mitter <committer@example.com> 1151939923 +0200'
+test_pretty head '%(creatordate)' 'Mon Jul 3 17:18:43 2006 +0200'
+test_pretty head '%(subject)' 'Initial'
+test_pretty head '%(contents:subject)' 'Initial'
+test_pretty head '%(body)' ''
+test_pretty head '%(contents:body)' ''
+test_pretty head '%(contents:signature)' ''
+test_pretty head '%(contents)' 'Initial
+'
+
+test_pretty head '%d' ' (HEAD, tag: testtag, origin/master, master)'
+test_pretty head '%x20' ' '
+test_pretty head '%g' '%g'
+test_pretty head '%unknown' '%unknown'
+test_pretty head '% (parent)' ''
+test_pretty head '% P' ''
+test_pretty head '% (tree)' ' 0e51c00fcb93dffc755546f27593d511e1bdb46f'
+test_pretty head '% T' ' 0e51c00fcb93dffc755546f27593d511e1bdb46f'
+
+test_expect_success '% (unknown)' '
+	test_must_fail git for-each-ref --pretty="% (unknown)" refs/heads/master
+'
+
+test_pretty head '%<(20)%cn end' 'C O Mitter           end'
+test_pretty head '%>(20)%cn end' '          C O Mitter end'
+test_pretty head '%><(20)%cn end' '     C O Mitter      end'
+test_pretty head '%<(20)%(committername) end' 'C O Mitter           end'
+test_pretty head '%>(20)%(committername) end' '          C O Mitter end'
+test_pretty head '%><(20)%(committername) end' '     C O Mitter      end'
+
+test_pretty tag '%(refname)' refs/tags/testtag
+test_pretty tag '%(upstream)' ''
+test_pretty tag '%(objecttype)' tag
+test_pretty tag '%(objectsize)' 154
+test_pretty tag '%(objectname)' 98b46b1d36e5b07909de1b3886224e3e81e87322
+test_pretty tag '%(tree)' ''
+test_pretty tag '%(parent)' ''
+test_pretty tag '%(numparent)' ''
+test_pretty tag '%(object)' '67a36f10722846e891fbada1ba48ed035de75581'
+test_pretty tag '%(type)' 'commit'
+test_pretty tag '%(author)' ''
+test_pretty tag '%(authorname)' ''
+test_pretty tag '%(authoremail)' ''
+test_pretty tag '%(authordate)' ''
+test_pretty tag '%(committer)' ''
+test_pretty tag '%(committername)' ''
+test_pretty tag '%(committeremail)' ''
+test_pretty tag '%(committerdate)' ''
+test_pretty tag '%(tag)' 'testtag'
+test_pretty tag '%(tagger)' 'C O Mitter <committer@example.com> 1151939925 +0200'
+test_pretty tag '%(taggername)' 'C O Mitter'
+test_pretty tag '%(taggeremail)' '<committer@example.com>'
+test_pretty tag '%(taggerdate)' 'Mon Jul 3 17:18:45 2006 +0200'
+test_pretty tag '%(creator)' 'C O Mitter <committer@example.com> 1151939925 +0200'
+test_pretty tag '%(creatordate)' 'Mon Jul 3 17:18:45 2006 +0200'
+test_pretty tag '%(subject)' 'Tagging at 1151939927'
+test_pretty tag '%(contents:subject)' 'Tagging at 1151939927'
+test_pretty tag '%(body)' ''
+test_pretty tag '%(contents:body)' ''
+test_pretty tag '%(contents:signature)' ''
+test_pretty tag '%(contents)' 'Tagging at 1151939927
+'
+
+# make sure we don't segfault when non-commits are passed in
+# format_commit_message. Should be fixed so that some of these
+# placeholders produce something useful for non-commits.
+test_pretty tag '%H' '%H'
+test_pretty tag '%h' '%h'
+test_pretty tag '%T' '%T'
+test_pretty tag '%t' '%t'
+
 test_expect_success 'Check invalid atoms names are errors' '
 	test_must_fail git for-each-ref --format="%(INVALID)" refs/heads
 '
-- 
1.8.3.247.g485169c

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

* [PATCH v2 09/15] for-each-ref: teach verify_format() about pretty's syntax
  2013-06-09 17:54 [PATCH v2 00/15] Towards a more awesome git branch Ramkumar Ramachandra
                   ` (7 preceding siblings ...)
  2013-06-09 17:54 ` [PATCH v2 08/15] for-each-ref: get --pretty using format_commit_message() Ramkumar Ramachandra
@ 2013-06-09 17:54 ` Ramkumar Ramachandra
  2013-06-09 17:54 ` [PATCH v2 10/15] for-each-ref: introduce format specifier %>(*) and %<(*) Ramkumar Ramachandra
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Ramkumar Ramachandra @ 2013-06-09 17:54 UTC (permalink / raw)
  To: Git List; +Cc: Nguyễn Thái Ngọc Duy, Junio C Hamano

From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

Pretty format accepts either ' ', '+' or '-' after '%' and before the
placeholder name to modify certain behaviors. Teach verify_format()
about this so that it finds atom "upstream" in, for example,
'% (upstream)'. This is important because verify_format populates
used_atom, which get_value() and populate_value() later rely on.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 builtin/for-each-ref.c | 15 +++++++++------
 pretty.c               |  4 ++++
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 8611777..39454fb 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -150,7 +150,7 @@ static int parse_atom(const char *atom, const char *ep)
 /*
  * In a format string, find the next occurrence of %(atom).
  */
-static const char *find_next(const char *cp)
+static const char *find_next(const char *cp, int pretty)
 {
 	while (*cp) {
 		if (*cp == '%') {
@@ -160,6 +160,9 @@ static const char *find_next(const char *cp)
 			 */
 			if (cp[1] == '(')
 				return cp;
+			else if (pretty && cp[1] && cp[2] == '(' &&
+				 strchr(" +-", cp[1])) /* see format_commit_item() */
+				return cp + 1;
 			else if (cp[1] == '%')
 				cp++; /* skip over two % */
 			/* otherwise this is a singleton, literal % */
@@ -173,10 +176,10 @@ static const char *find_next(const char *cp)
  * Make sure the format string is well formed, and parse out
  * the used atoms.
  */
-static int verify_format(const char *format)
+static int verify_format(const char *format, int pretty)
 {
 	const char *cp, *sp;
-	for (cp = format; *cp && (sp = find_next(cp)); ) {
+	for (cp = format; *cp && (sp = find_next(cp, pretty)); ) {
 		const char *ep = strchr(sp, ')');
 		if (!ep)
 			return error("malformed format string %s", sp);
@@ -935,7 +938,7 @@ static void show_ref(struct strbuf *sb, struct refinfo *info,
 {
 	const char *cp, *sp, *ep;
 
-	for (cp = format; *cp && (sp = find_next(cp)); cp = ep + 1) {
+	for (cp = format; *cp && (sp = find_next(cp, 0)); cp = ep + 1) {
 		ep = strchr(sp, ')');
 		if (cp < sp)
 			emit(sb, cp, sp);
@@ -1098,8 +1101,8 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 	}
 	if (format != default_format && pretty)
 		die("--format and --pretty cannot be used together");
-	if ((pretty && verify_format(pretty)) ||
-	    (!pretty && verify_format(format)))
+	if ((pretty && verify_format(pretty, 1)) ||
+	    (!pretty && verify_format(format, 0)))
 		usage_with_options(for_each_ref_usage, opts);
 
 	if (!sort)
diff --git a/pretty.c b/pretty.c
index 816aa32..28c0a72 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1439,6 +1439,10 @@ static size_t format_commit_item(struct strbuf *sb, /* in UTF-8 */
 		ADD_SP_BEFORE_NON_EMPTY
 	} magic = NO_MAGIC;
 
+	/*
+	 * Note: any modification in what placeholder[0] contains
+	 * should be redone in builtin/for-each-ref.c:find_next().
+	 */
 	switch (placeholder[0]) {
 	case '-':
 		magic = DEL_LF_BEFORE_EMPTY;
-- 
1.8.3.247.g485169c

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

* [PATCH v2 10/15] for-each-ref: introduce format specifier %>(*) and %<(*)
  2013-06-09 17:54 [PATCH v2 00/15] Towards a more awesome git branch Ramkumar Ramachandra
                   ` (8 preceding siblings ...)
  2013-06-09 17:54 ` [PATCH v2 09/15] for-each-ref: teach verify_format() about pretty's syntax Ramkumar Ramachandra
@ 2013-06-09 17:54 ` Ramkumar Ramachandra
  2013-06-09 17:54 ` [PATCH v2 11/15] for-each-ref: introduce %(HEAD) marker Ramkumar Ramachandra
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Ramkumar Ramachandra @ 2013-06-09 17:54 UTC (permalink / raw)
  To: Git List; +Cc: Nguyễn Thái Ngọc Duy, Junio C Hamano

From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

Pretty placeholders %>(N) and %<(N) require a user provided width N,
which makes sense because the commit chain could be really long and the
user only needs to look at a few at the top, going to the end just to
calculate the best width wastes CPU cycles.

for-each-ref is different; the display set is small, and we display them
all at once. We even support sorting, which goes through all display
items anyway.  This patch introduces new %>(*) and %<(*), which are
supposed to be followed immediately by %(fieldname) (i.e. original
for-each-ref specifiers, not ones coming from pretty.c). They calculate
the best width for the %(fieldname), ignoring ansi escape sequences if
any.

[rr: documentation]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 Documentation/git-for-each-ref.txt |  7 +++++++
 builtin/for-each-ref.c             | 38 ++++++++++++++++++++++++++++++++++++++
 t/t6300-for-each-ref.sh            | 20 ++++++++++++++++++++
 3 files changed, 65 insertions(+)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index d8ad758..8cbc08c 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -47,6 +47,10 @@ OPTIONS
 	are hex digits interpolates to character with hex code
 	`xx`; for example `%00` interpolates to `\0` (NUL),
 	`%09` to `\t` (TAB) and `%0a` to `\n` (LF).
++
+Placeholders `%<(*)` and `%>(*)` work like `%<(<N>)` and `%>(<N>)`
+respectively, except that the width of the next placeholder is
+calculated.
 
 <pretty>::
 	A format string with supporting placeholders described in the
@@ -68,6 +72,9 @@ Caveats:
 3. Only the placeholders inherited from `<format>` will respect
    quoting settings.
 
+3. Only the placeholders inherited from `<format>` will work with the
+   alignment placeholders `%<(*)` and '%>(*)`.
+
 <pattern>...::
 	If one or more patterns are given, only refs are shown that
 	match against at least one pattern, either using fnmatch(3) or
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 39454fb..da479d1 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -9,6 +9,7 @@
 #include "quote.h"
 #include "parse-options.h"
 #include "remote.h"
+#include "utf8.h"
 
 /* Quoting styles */
 #define QUOTE_NONE 0
@@ -966,10 +967,30 @@ static void show_refs(struct refinfo **refs, int maxcount,
 }
 
 struct format_one_atom_context {
+	struct refinfo **refs;
+	int maxcount;
+
 	struct refinfo *info;
 	int quote_style;
 };
 
+static unsigned int get_atom_width(struct format_one_atom_context *ctx,
+				   const char *start, const char *end)
+{
+	struct strbuf sb = STRBUF_INIT;
+	int i, atom = parse_atom(start, end);
+	unsigned int len = 0, sb_len;
+	for (i = 0; i < ctx->maxcount; i++) {
+		print_value(&sb, ctx->refs[i], atom, ctx->quote_style);
+		sb_len = utf8_strnwidth(sb.buf, sb.len, 1);
+		if (sb_len > len)
+			len = sb_len;
+		strbuf_reset(&sb);
+	}
+	strbuf_release(&sb);
+	return len;
+}
+
 static size_t format_one_atom(struct strbuf *sb, const char *placeholder,
 			      void *format_context, void *user_data,
 			      struct strbuf *subst)
@@ -982,6 +1003,21 @@ static size_t format_one_atom(struct strbuf *sb, const char *placeholder,
 		return 1;
 	}
 
+	/*
+	 * Substitute %>(*)%(atom) and friends with real width.
+	 */
+	if (*placeholder == '>' || *placeholder == '<') {
+		const char *star = placeholder + 1;
+		if (!prefixcmp(star, "(*)%(") &&
+		    ((ep = strchr(star + strlen("(*)%("), ')')) != NULL)) {
+			star++;
+			strbuf_addf(subst, "%c(%u)",
+				    *placeholder,
+				    get_atom_width(ctx, star + strlen("*)%("), ep));
+			return 1 + strlen("(*)");
+		}
+	}
+
 	if (*placeholder != '(')
 		return 0;
 
@@ -1008,6 +1044,8 @@ static void show_pretty_refs(struct refinfo **refs, int maxcount,
 	ctx.abbrev = DEFAULT_ABBREV;
 	ctx.format = format_one_atom;
 	ctx.user_data = &fctx;
+	fctx.refs = refs;
+	fctx.maxcount = maxcount;
 	fctx.quote_style = quote_style;
 	for (i = 0; i < maxcount; i++) {
 		struct commit *commit = NULL;
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index d39e0b4..160018c 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -196,6 +196,26 @@ test_pretty head '%<(20)%(committername) end' 'C O Mitter           end'
 test_pretty head '%>(20)%(committername) end' '          C O Mitter end'
 test_pretty head '%><(20)%(committername) end' '     C O Mitter      end'
 
+test_expect_success '%<(*)%(refname) A' '
+	git for-each-ref --pretty="%<(*)%(refname) A" >actual &&
+	cat >expected <<EOF &&
+refs/heads/master          A
+refs/remotes/origin/master A
+refs/tags/testtag          A
+EOF
+	test_cmp expected actual
+'
+
+test_expect_success '%>(*)%(refname) A' '
+	git for-each-ref --pretty="%>(*)%(refname) A" >actual &&
+	qz_to_tab_space >expected <<EOF &&
+Z        refs/heads/master A
+refs/remotes/origin/master A
+Z        refs/tags/testtag A
+EOF
+	test_cmp expected actual
+'
+
 test_pretty tag '%(refname)' refs/tags/testtag
 test_pretty tag '%(upstream)' ''
 test_pretty tag '%(objecttype)' tag
-- 
1.8.3.247.g485169c

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

* [PATCH v2 11/15] for-each-ref: introduce %(HEAD) marker
  2013-06-09 17:54 [PATCH v2 00/15] Towards a more awesome git branch Ramkumar Ramachandra
                   ` (9 preceding siblings ...)
  2013-06-09 17:54 ` [PATCH v2 10/15] for-each-ref: introduce format specifier %>(*) and %<(*) Ramkumar Ramachandra
@ 2013-06-09 17:54 ` Ramkumar Ramachandra
  2013-06-09 17:54 ` [PATCH v2 12/15] for-each-ref: introduce %(upstream:track[short]) Ramkumar Ramachandra
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Ramkumar Ramachandra @ 2013-06-09 17:54 UTC (permalink / raw)
  To: Git List; +Cc: Nguyễn Thái Ngọc Duy, Junio C Hamano

'git branch' shows which branch you are currently on with an '*', but
'git for-each-ref' misses this feature.  So, extend the format with
%(HEAD) to do exactly the same thing.

Now you can use the following format in for-each-ref:

  %C(red)%(HEAD)%C(reset) %C(green)%(refname:short)%C(reset)

to display a red asterisk next to the current ref.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-for-each-ref.txt |  4 ++++
 builtin/for-each-ref.c             | 13 +++++++++++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 8cbc08c..8d982e3 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -121,6 +121,10 @@ upstream::
 	from the displayed ref. Respects `:short` in the same way as
 	`refname` above.
 
+HEAD::
+	Useful to indicate the currently checked out branch.  Is '*'
+	if HEAD points to the current ref, and ' ' otherwise.
+
 In addition to the above, for commit and tag objects, the header
 field names (`tree`, `parent`, `object`, `type`, and `tag`) can
 be used to specify the value in the header field.
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index da479d1..3d357a9 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -76,6 +76,7 @@ static struct {
 	{ "upstream" },
 	{ "symref" },
 	{ "flag" },
+	{ "HEAD" },
 };
 
 /*
@@ -679,8 +680,16 @@ static void populate_value(struct refinfo *ref)
 				v->s = xstrdup(buf + 1);
 			}
 			continue;
-		}
-		else
+		} else if (!strcmp(name, "HEAD")) {
+			const char *head;
+			unsigned char sha1[20];
+			head = resolve_ref_unsafe("HEAD", sha1, 1, NULL);
+			if (!strcmp(ref->refname, head))
+				v->s = "*";
+			else
+				v->s = " ";
+			continue;
+		} else
 			continue;
 
 		formatp = strchr(name, ':');
-- 
1.8.3.247.g485169c

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

* [PATCH v2 12/15] for-each-ref: introduce %(upstream:track[short])
  2013-06-09 17:54 [PATCH v2 00/15] Towards a more awesome git branch Ramkumar Ramachandra
                   ` (10 preceding siblings ...)
  2013-06-09 17:54 ` [PATCH v2 11/15] for-each-ref: introduce %(HEAD) marker Ramkumar Ramachandra
@ 2013-06-09 17:54 ` Ramkumar Ramachandra
  2013-06-09 17:54 ` [PATCH v2 13/15] for-each-ref: improve responsiveness of %(upstream:track) Ramkumar Ramachandra
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Ramkumar Ramachandra @ 2013-06-09 17:54 UTC (permalink / raw)
  To: Git List; +Cc: Nguyễn Thái Ngọc Duy, Junio C Hamano

Introduce %(upstream:track) to display "[ahead M, behind N]" and
%(upstream:trackshort) to display "=", ">", "<", or "<>"
appropriately (inspired by the contrib/completion/git-prompt.sh).

Now you can use the following format in for-each-ref:

  %C(green)%(refname:short)%C(reset)%(upstream:trackshort)

to display refs with terse tracking information.

Note that :track and :trackshort only work with upstream, and error out
when used with anything else.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-for-each-ref.txt |  6 +++++-
 builtin/for-each-ref.c             | 42 ++++++++++++++++++++++++++++++++++++--
 2 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 8d982e3..d666ebd 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -119,7 +119,11 @@ objectname::
 upstream::
 	The name of a local ref which can be considered ``upstream''
 	from the displayed ref. Respects `:short` in the same way as
-	`refname` above.
+	`refname` above.  Additionally respects `:track` to show
+	"[ahead N, behind M]" and `:trackshort` to show the terse
+	version (like the prompt) ">", "<", "<>", or "=".  Has no
+	effect if the ref does not have tracking information
+	associated with it.
 
 HEAD::
 	Useful to indicate the currently checked out branch.  Is '*'
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 3d357a9..72b33ee 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -628,6 +628,7 @@ static void populate_value(struct refinfo *ref)
 	int eaten, i;
 	unsigned long size;
 	const unsigned char *tagged;
+	int upstream_present = 0;
 
 	ref->value = xcalloc(sizeof(struct atom_value), used_atom_cnt);
 
@@ -645,6 +646,7 @@ static void populate_value(struct refinfo *ref)
 		int deref = 0;
 		const char *refname;
 		const char *formatp;
+		struct branch *branch;
 
 		if (*name == '*') {
 			deref = 1;
@@ -656,7 +658,6 @@ static void populate_value(struct refinfo *ref)
 		else if (!prefixcmp(name, "symref"))
 			refname = ref->symref ? ref->symref : "";
 		else if (!prefixcmp(name, "upstream")) {
-			struct branch *branch;
 			/* only local branches may have an upstream */
 			if (prefixcmp(ref->refname, "refs/heads/"))
 				continue;
@@ -666,6 +667,7 @@ static void populate_value(struct refinfo *ref)
 			    !branch->merge[0]->dst)
 				continue;
 			refname = branch->merge[0]->dst;
+			upstream_present = 1;
 		}
 		else if (!strcmp(name, "flag")) {
 			char buf[256], *cp = buf;
@@ -683,6 +685,7 @@ static void populate_value(struct refinfo *ref)
 		} else if (!strcmp(name, "HEAD")) {
 			const char *head;
 			unsigned char sha1[20];
+
 			head = resolve_ref_unsafe("HEAD", sha1, 1, NULL);
 			if (!strcmp(ref->refname, head))
 				v->s = "*";
@@ -695,11 +698,46 @@ static void populate_value(struct refinfo *ref)
 		formatp = strchr(name, ':');
 		/* look for "short" refname format */
 		if (formatp) {
+			int num_ours, num_theirs;
+
 			formatp++;
 			if (!strcmp(formatp, "short"))
 				refname = shorten_unambiguous_ref(refname,
 						      warn_ambiguous_refs);
-			else
+			else if (!strcmp(formatp, "track") &&
+				!prefixcmp(name, "upstream")) {
+				char buf[40];
+
+				if (!upstream_present)
+					continue;
+				if (!stat_tracking_info(branch, &num_ours, &num_theirs))
+					v->s = "";
+				else if (!num_ours) {
+					sprintf(buf, "[behind %d]", num_theirs);
+					v->s = xstrdup(buf);
+				} else if (!num_theirs) {
+					sprintf(buf, "[ahead %d]", num_ours);
+					v->s = xstrdup(buf);
+				} else {
+					sprintf(buf, "[ahead %d, behind %d]",
+						num_ours, num_theirs);
+					v->s = xstrdup(buf);
+				}
+				continue;
+			} else if (!strcmp(formatp, "trackshort") &&
+				!prefixcmp(name, "upstream")) {
+				if (!upstream_present)
+					continue;
+				if (!stat_tracking_info(branch, &num_ours, &num_theirs))
+					v->s = "=";
+				else if (!num_ours)
+					v->s = "<";
+				else if (!num_theirs)
+					v->s = ">";
+				else
+					v->s = "<>";
+				continue;
+			} else
 				die("unknown %.*s format %s",
 				    (int)(formatp - name), name, formatp);
 		}
-- 
1.8.3.247.g485169c

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

* [PATCH v2 13/15] for-each-ref: improve responsiveness of %(upstream:track)
  2013-06-09 17:54 [PATCH v2 00/15] Towards a more awesome git branch Ramkumar Ramachandra
                   ` (11 preceding siblings ...)
  2013-06-09 17:54 ` [PATCH v2 12/15] for-each-ref: introduce %(upstream:track[short]) Ramkumar Ramachandra
@ 2013-06-09 17:54 ` Ramkumar Ramachandra
  2013-06-09 17:54 ` [PATCH v2 14/15] pretty: introduce get_pretty_userformat Ramkumar Ramachandra
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Ramkumar Ramachandra @ 2013-06-09 17:54 UTC (permalink / raw)
  To: Git List; +Cc: Nguyễn Thái Ngọc Duy, Junio C Hamano

From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

Before anything is printed, for-each-ref sorts all refs first.  As
part of the sorting, populate_value() is called to fill the values in
for all atoms/placeholders per entry. By the time sort_refs() is done,
pretty much all data is already retrieved.

This works fine when data can be cheaply retrieved before
%(upstream:track) comes into the picture. It may take a noticeable
amount of time to process %(upstream:track) for each entry. All
entries add up and make --format='%(refname)%(upstream:track)' seem
hung for a few seconds, then display everything at once.

Improve the responsiveness by only processing the one atom (*) at a
time so that processing one atom for all entries (e.g. sorting) won't
cause much delay (unless you choose a "heavy" atom to process).

(*) This is not entirely correct. If you sort by an atom that needs
object database access, then it will fill all atoms that need odb.
Which is not a bad thing. We don't want to access odb once at sorting
phase and again at display phase.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 builtin/for-each-ref.c | 49 +++++++++++++++++++++++++------------------------
 1 file changed, 25 insertions(+), 24 deletions(-)

diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 72b33ee..25764aa 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -565,20 +565,6 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj
 }
 
 /*
- * We want to have empty print-string for field requests
- * that do not apply (e.g. "authordate" for a tag object)
- */
-static void fill_missing_values(struct atom_value *val)
-{
-	int i;
-	for (i = 0; i < used_atom_cnt; i++) {
-		struct atom_value *v = &val[i];
-		if (v->s == NULL)
-			v->s = "";
-	}
-}
-
-/*
  * val is a list of atom_value to hold returned values.  Extract
  * the values for atoms in used_atom array out of (obj, buf, sz).
  * when deref is false, (obj, buf, sz) is the object that is
@@ -621,7 +607,7 @@ static inline char *copy_advance(char *dst, const char *src)
 /*
  * Parse the object referred by ref, and grab needed value.
  */
-static void populate_value(struct refinfo *ref)
+static void populate_value(struct refinfo *ref, int only_this_atom)
 {
 	void *buf;
 	struct object *obj;
@@ -630,13 +616,15 @@ static void populate_value(struct refinfo *ref)
 	const unsigned char *tagged;
 	int upstream_present = 0;
 
-	ref->value = xcalloc(sizeof(struct atom_value), used_atom_cnt);
+	if (!ref->value) {
+		ref->value = xcalloc(sizeof(struct atom_value), used_atom_cnt);
 
-	if (need_symref && (ref->flag & REF_ISSYMREF) && !ref->symref) {
-		unsigned char unused1[20];
-		ref->symref = resolve_refdup(ref->refname, unused1, 1, NULL);
-		if (!ref->symref)
-			ref->symref = "";
+		if (need_symref && (ref->flag & REF_ISSYMREF) && !ref->symref) {
+			unsigned char unused1[20];
+			ref->symref = resolve_refdup(ref->refname, unused1, 1, NULL);
+			if (!ref->symref)
+				ref->symref = "";
+		}
 	}
 
 	/* Fill in specials first */
@@ -648,6 +636,9 @@ static void populate_value(struct refinfo *ref)
 		const char *formatp;
 		struct branch *branch;
 
+		if (only_this_atom != -1 && only_this_atom != i)
+			continue;
+
 		if (*name == '*') {
 			deref = 1;
 			name++;
@@ -754,6 +745,10 @@ static void populate_value(struct refinfo *ref)
 
 	for (i = 0; i < used_atom_cnt; i++) {
 		struct atom_value *v = &ref->value[i];
+
+		if (only_this_atom != -1 && only_this_atom != i)
+			continue;
+
 		if (v->s == NULL)
 			goto need_obj;
 	}
@@ -809,9 +804,15 @@ static void populate_value(struct refinfo *ref)
  */
 static void get_value(struct refinfo *ref, int atom, struct atom_value **v)
 {
-	if (!ref->value) {
-		populate_value(ref);
-		fill_missing_values(ref->value);
+	if (!ref->value || !ref->value[atom].s) {
+		populate_value(ref, atom);
+		/*
+		 * We want to have empty print-string for field
+		 * requests that do not apply (e.g. "authordate" for a
+		 * tag object)
+		 */
+		if (!ref->value[atom].s)
+			ref->value[atom].s = "";
 	}
 	*v = &ref->value[atom];
 }
-- 
1.8.3.247.g485169c

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

* [PATCH v2 14/15] pretty: introduce get_pretty_userformat
  2013-06-09 17:54 [PATCH v2 00/15] Towards a more awesome git branch Ramkumar Ramachandra
                   ` (12 preceding siblings ...)
  2013-06-09 17:54 ` [PATCH v2 13/15] for-each-ref: improve responsiveness of %(upstream:track) Ramkumar Ramachandra
@ 2013-06-09 17:54 ` Ramkumar Ramachandra
  2013-06-09 17:54 ` [PATCH v2 15/15] for-each-ref: use get_pretty_userformat in --pretty Ramkumar Ramachandra
  2013-06-10  1:25 ` [PATCH v2 00/15] Towards a more awesome git branch Duy Nguyen
  15 siblings, 0 replies; 19+ messages in thread
From: Ramkumar Ramachandra @ 2013-06-09 17:54 UTC (permalink / raw)
  To: Git List; +Cc: Nguyễn Thái Ngọc Duy, Junio C Hamano

This helper function is intended to be used by callers implementing
--pretty themselves; it parses pretty.* configuration variables
recursively and hands the user-defined format back to the caller.  No
builtins are supported, as CMT_FMT_* are really only useful when
displaying commits.  Callers might like to define their own builtins in
the future.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 commit.h |  1 +
 pretty.c | 25 +++++++++++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/commit.h b/commit.h
index 38dc0c0..7755282 100644
--- a/commit.h
+++ b/commit.h
@@ -113,6 +113,7 @@ extern char *logmsg_reencode(const struct commit *commit,
 			     const char *output_encoding);
 extern void logmsg_free(char *msg, const struct commit *commit);
 extern void get_commit_format(const char *arg, struct rev_info *);
+extern const char *get_pretty_userformat(const char *arg);
 extern const char *format_subject(struct strbuf *sb, const char *msg,
 				  const char *line_separator);
 extern void userformat_find_requirements(const char *fmt, struct userformat_want *w);
diff --git a/pretty.c b/pretty.c
index 28c0a72..70e4e44 100644
--- a/pretty.c
+++ b/pretty.c
@@ -174,6 +174,31 @@ void get_commit_format(const char *arg, struct rev_info *rev)
 }
 
 /*
+ * Function to parse --pretty string, lookup pretty.* configuration
+ * variables and return the format string, assuming no builtin
+ * formats.  Not limited to commits, unlike get_commit_format().
+ */
+const char *get_pretty_userformat(const char *arg)
+{
+	struct cmt_fmt_map *commit_format;
+
+	if (!arg || !*arg)
+		return NULL;
+
+	if (!prefixcmp(arg, "format:") || !prefixcmp(arg, "tformat:"))
+		return xstrdup(strchr(arg, ':' + 1));
+
+	if (strchr(arg, '%'))
+		return xstrdup(arg);
+
+	commit_format = find_commit_format(arg);
+	if (!commit_format || commit_format->format != CMIT_FMT_USERFORMAT)
+		die("invalid --pretty format: %s", arg);
+
+	return xstrdup(commit_format->user_format);
+}
+
+/*
  * Generic support for pretty-printing the header
  */
 static int get_one_line(const char *msg)
-- 
1.8.3.247.g485169c

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

* [PATCH v2 15/15] for-each-ref: use get_pretty_userformat in --pretty
  2013-06-09 17:54 [PATCH v2 00/15] Towards a more awesome git branch Ramkumar Ramachandra
                   ` (13 preceding siblings ...)
  2013-06-09 17:54 ` [PATCH v2 14/15] pretty: introduce get_pretty_userformat Ramkumar Ramachandra
@ 2013-06-09 17:54 ` Ramkumar Ramachandra
  2013-06-10  1:25 ` [PATCH v2 00/15] Towards a more awesome git branch Duy Nguyen
  15 siblings, 0 replies; 19+ messages in thread
From: Ramkumar Ramachandra @ 2013-06-09 17:54 UTC (permalink / raw)
  To: Git List; +Cc: Nguyễn Thái Ngọc Duy, Junio C Hamano

Use get_pretty_userformat() to interpret the --pretty string.  This
means that you can now reference a format specified in a pretty.*
configuration variable as an argument to 'git for-each-ref --pretty='.
There are two caveats:

1. A leading "format:" or "tformat:" is automatically stripped and
   ignored.  Separator semantics are not configurable (yet).

2. No built-in formats are available.  The ones specified in
   pretty-formats (oneline, short etc) don't make sense when displaying
   refs anyway.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-for-each-ref.txt |  3 +++
 builtin/for-each-ref.c             | 16 +++++++++-------
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index d666ebd..ef39f2a 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -60,6 +60,9 @@ calculated.
 +
 Caveats:
 
+0. No built-in formats from PRETTY FORMATS (like oneline, short) are
+   available.
+
 1. Many of the placeholders in "PRETTY FORMATS" are designed to work
    specifically on commit objects: when non-commit objects are
    supplied, those placeholders won't work (i.e. they will be emitted
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 25764aa..ed7bd7d 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -1151,7 +1151,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 	int num_refs;
 	const char *default_format = "%(objectname) %(objecttype)\t%(refname)";
 	const char *format = default_format;
-	const char *pretty = NULL;
+	const char *pretty_raw = NULL, *pretty_userformat = NULL;
 	struct ref_sort *sort = NULL, **sort_tail = &sort;
 	int maxcount = 0, quote_style = 0;
 	struct refinfo **refs;
@@ -1170,13 +1170,15 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 		OPT_GROUP(""),
 		OPT_INTEGER( 0 , "count", &maxcount, N_("show only <n> matched refs")),
 		OPT_STRING(  0 , "format", &format, N_("format"), N_("format to use for the output")),
-		OPT_STRING(  0 , "pretty", &pretty, N_("format"), N_("alternative format to use for the output")),
+		OPT_STRING(  0 , "pretty", &pretty_raw, N_("format"), N_("alternative format to use for the output")),
 		OPT_CALLBACK(0 , "sort", sort_tail, N_("key"),
 			    N_("field name to sort on"), &opt_parse_sort),
 		OPT_END(),
 	};
 
 	parse_options(argc, argv, prefix, opts, for_each_ref_usage, 0);
+	if (pretty_raw)
+		pretty_userformat = get_pretty_userformat(pretty_raw);
 	if (maxcount < 0) {
 		error("invalid --count argument: `%d'", maxcount);
 		usage_with_options(for_each_ref_usage, opts);
@@ -1185,10 +1187,10 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 		error("more than one quoting style?");
 		usage_with_options(for_each_ref_usage, opts);
 	}
-	if (format != default_format && pretty)
+	if (format != default_format && pretty_userformat)
 		die("--format and --pretty cannot be used together");
-	if ((pretty && verify_format(pretty, 1)) ||
-	    (!pretty && verify_format(format, 0)))
+	if ((pretty_userformat && verify_format(pretty_userformat, 1)) ||
+	    (!pretty_userformat && verify_format(format, 0)))
 		usage_with_options(for_each_ref_usage, opts);
 
 	if (!sort)
@@ -1209,8 +1211,8 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 	if (!maxcount || num_refs < maxcount)
 		maxcount = num_refs;
 
-	if (pretty)
-		show_pretty_refs(refs, maxcount, pretty, quote_style);
+	if (pretty_userformat)
+		show_pretty_refs(refs, maxcount, pretty_userformat, quote_style);
 	else
 		show_refs(refs, maxcount, format, quote_style);
 	return 0;
-- 
1.8.3.247.g485169c

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

* Re: [PATCH v2 00/15] Towards a more awesome git branch
  2013-06-09 17:54 [PATCH v2 00/15] Towards a more awesome git branch Ramkumar Ramachandra
                   ` (14 preceding siblings ...)
  2013-06-09 17:54 ` [PATCH v2 15/15] for-each-ref: use get_pretty_userformat in --pretty Ramkumar Ramachandra
@ 2013-06-10  1:25 ` Duy Nguyen
  2013-06-10 12:08   ` Ramkumar Ramachandra
  15 siblings, 1 reply; 19+ messages in thread
From: Duy Nguyen @ 2013-06-10  1:25 UTC (permalink / raw)
  To: Ramkumar Ramachandra; +Cc: Git List, Junio C Hamano

On Mon, Jun 10, 2013 at 12:54 AM, Ramkumar Ramachandra
<artagnon@gmail.com> wrote:
> Hi,
>
> This iteration contains some minor fixups (courtesy reviews by Eric
> Sunshine and Junio), and some tests from Duy squashed in.

I'm starting to think this is a half-baked solution. It hides
problems, for example commit placeholders should produce empty string,
not the literal placeholders. Doing that from a callback is really
ugly. There's also the problem with sorting and quoting (both only
work with for-each-ref atoms only). A better solution may be improving
pretty.c to the point where it can more or less replace f-e-r's
--format. Even more, I think pretty engine should be easily added to
cat-file (especially --batch), as a generic way to extract
information. But for the reason I will send shortly, I will not work
not work on this series or any others.
--
Duy

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

* Re: [PATCH v2 00/15] Towards a more awesome git branch
  2013-06-10  1:25 ` [PATCH v2 00/15] Towards a more awesome git branch Duy Nguyen
@ 2013-06-10 12:08   ` Ramkumar Ramachandra
  2013-06-10 16:02     ` Junio C Hamano
  0 siblings, 1 reply; 19+ messages in thread
From: Ramkumar Ramachandra @ 2013-06-10 12:08 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git List

[-CC: Duy, since he has left the community]

Junio: since Duy is no longer around to guide us, I will rely on your guidance.

Duy Nguyen wrote:
> I'm starting to think this is a half-baked solution. It hides
> problems, for example commit placeholders should produce empty string,
> not the literal placeholders.

Why should they produce empty strings?  Aren't they equivalent to
invalid placeholders?

> Doing that from a callback is really
> ugly.

Why is the callback ugly?  I thought it was a great way to extend
pretty-formats, without teaching pretty.c about every possible format
that callers could ever want.

> There's also the problem with sorting and quoting (both only
> work with for-each-ref atoms only).

Why would I want to sort by reflog-identity-name (or something) in
for-each-ref?  The sensible fields for sorting in for-each-ref are all
for-each-ref atoms.

On quoting, I agree.  We must move the quoting to pretty.c eventually,
but I don't think it is urgent.

> A better solution may be improving
> pretty.c to the point where it can more or less replace f-e-r's
> --format.

Why would you want to stuff everything into pretty.c?  If any callers
wants to implement one specialized format, the only way to do it is to
stuff it into the One True pretty-formats?

> Even more, I think pretty engine should be easily added to
> cat-file (especially --batch), as a generic way to extract
> information.

Cute theoretical exercise.  As usual, I'm not interested: this topic
is about making git-branch more awesome, not playing with
pretty-formats.

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

* Re: [PATCH v2 00/15] Towards a more awesome git branch
  2013-06-10 12:08   ` Ramkumar Ramachandra
@ 2013-06-10 16:02     ` Junio C Hamano
  0 siblings, 0 replies; 19+ messages in thread
From: Junio C Hamano @ 2013-06-10 16:02 UTC (permalink / raw)
  To: Ramkumar Ramachandra; +Cc: Git List

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> [-CC: Duy, since he has left the community]
>
> Junio: since Duy is no longer around to guide us, I will rely on your guidance.
>
> Duy Nguyen wrote:
>> I'm starting to think this is a half-baked solution. It hides
>> problems, for example commit placeholders should produce empty string,
>> not the literal placeholders.
>
> Why should they produce empty strings?  Aren't they equivalent to
> invalid placeholders?

I haven't even skimmed the series, so responding to a comment given
as a response to 00/15 is a bit difficult, but I _think_ this refers
to this issue: what should "%(cD)" produce when given to for-each-ref
and you have a tag that points at non committish?

http://thread.gmane.org/gmane.comp.version-control.git/226343/focus=226460

I may be guessing incorrectly what Duy's comment is about, though.

I'll hopefully have something more concrete and useful after I have
a chance to read the series, but not now.

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

end of thread, other threads:[~2013-06-10 16:02 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-09 17:54 [PATCH v2 00/15] Towards a more awesome git branch Ramkumar Ramachandra
2013-06-09 17:54 ` [PATCH v2 01/15] for-each-ref, quote: convert *_quote_print -> *_quote_buf Ramkumar Ramachandra
2013-06-09 17:54 ` [PATCH v2 02/15] for-each-ref: don't print out elements directly Ramkumar Ramachandra
2013-06-09 17:54 ` [PATCH v2 03/15] tar-tree: remove dependency on sq_quote_print() Ramkumar Ramachandra
2013-06-09 17:54 ` [PATCH v2 04/15] quote: remove sq_quote_print() Ramkumar Ramachandra
2013-06-09 17:54 ` [PATCH v2 05/15] pretty: extend pretty_print_context with callback Ramkumar Ramachandra
2013-06-09 17:54 ` [PATCH v2 06/15] pretty: limit recursion in format_commit_one() Ramkumar Ramachandra
2013-06-09 17:54 ` [PATCH v2 07/15] pretty: allow passing NULL commit to format_commit_message() Ramkumar Ramachandra
2013-06-09 17:54 ` [PATCH v2 08/15] for-each-ref: get --pretty using format_commit_message() Ramkumar Ramachandra
2013-06-09 17:54 ` [PATCH v2 09/15] for-each-ref: teach verify_format() about pretty's syntax Ramkumar Ramachandra
2013-06-09 17:54 ` [PATCH v2 10/15] for-each-ref: introduce format specifier %>(*) and %<(*) Ramkumar Ramachandra
2013-06-09 17:54 ` [PATCH v2 11/15] for-each-ref: introduce %(HEAD) marker Ramkumar Ramachandra
2013-06-09 17:54 ` [PATCH v2 12/15] for-each-ref: introduce %(upstream:track[short]) Ramkumar Ramachandra
2013-06-09 17:54 ` [PATCH v2 13/15] for-each-ref: improve responsiveness of %(upstream:track) Ramkumar Ramachandra
2013-06-09 17:54 ` [PATCH v2 14/15] pretty: introduce get_pretty_userformat Ramkumar Ramachandra
2013-06-09 17:54 ` [PATCH v2 15/15] for-each-ref: use get_pretty_userformat in --pretty Ramkumar Ramachandra
2013-06-10  1:25 ` [PATCH v2 00/15] Towards a more awesome git branch Duy Nguyen
2013-06-10 12:08   ` Ramkumar Ramachandra
2013-06-10 16:02     ` Junio C Hamano

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 NNTP newsgroup(s).