Re: [RESEND][PATCH 00/15] Towards a more awesome git branch

[Duy Nguyen <pclouds@gmail.com>]

On Tue, Jul 09, 2013 at 04:02:11PM +0530, Ramkumar Ramachandra wrote:
> Hi,
> 
> I'm sending this out in the hope of attracting some reviews.  It's an
> unedited resend, and there were zero conflicts from the rebase.

I still don't like my callback idea. How about this? It's refactoring
a bit so that you can now extract the color parsing and alignment code
from pretty.c and add them to for-each-ref as new atoms in --format.
As this is simply extending --format, the syntax unification idea is
put back on the shelf and won't hinder your work.

parse_color() can be used to generate color escape sequences. For
alignment, you can do something like this:

pad_an_item();
process_following_atom_after_the_alignment_atom();
align_an_item();

I'm afraid it needs a bit more work on the for-each-ref side and may
even need some fixups (I tested t4205 only and it passed, but I may
have left some bugs)

-- 8< --
diff --git a/pretty.c b/pretty.c
index 9e43154..40a30a4 100644
--- a/pretty.c
+++ b/pretty.c
@@ -766,19 +766,23 @@ enum trunc_type {
 	trunc_right
 };
 
+struct alignment {
+	enum flush_type flush_type;
+	enum trunc_type truncate;
+	int padding;
+};
+
 struct format_commit_context {
 	const struct commit *commit;
 	const struct pretty_print_context *pretty_ctx;
 	unsigned commit_header_parsed:1;
 	unsigned commit_message_parsed:1;
 	struct signature_check signature_check;
-	enum flush_type flush_type;
-	enum trunc_type truncate;
 	char *message;
 	char *commit_encoding;
 	size_t width, indent1, indent2;
 	int auto_color;
-	int padding;
+	struct alignment alignment;
 
 	/* These offsets are relative to the start of the commit message. */
 	struct chunk author;
@@ -957,7 +961,7 @@ static int format_reflog_person(struct strbuf *sb,
 
 static size_t parse_color(struct strbuf *sb, /* in UTF-8 */
 			  const char *placeholder,
-			  struct format_commit_context *c)
+			  int color_setting)
 {
 	if (placeholder[1] == '(') {
 		const char *begin = placeholder + 2;
@@ -967,7 +971,7 @@ static size_t parse_color(struct strbuf *sb, /* in UTF-8 */
 		if (!end)
 			return 0;
 		if (!prefixcmp(begin, "auto,")) {
-			if (!want_color(c->pretty_ctx->color))
+			if (!want_color(color_setting))
 				return end - placeholder + 1;
 			begin += 5;
 		}
@@ -995,7 +999,7 @@ static size_t parse_color(struct strbuf *sb, /* in UTF-8 */
 
 static size_t parse_padding_placeholder(struct strbuf *sb,
 					const char *placeholder,
-					struct format_commit_context *c)
+					struct alignment *c)
 {
 	const char *ch = placeholder;
 	enum flush_type flush_type;
@@ -1076,7 +1080,8 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
 			c->auto_color = 1;
 			return 7; /* consumed 7 bytes, "C(auto)" */
 		} else {
-			int ret = parse_color(sb, placeholder, c);
+			int ret = parse_color(sb, placeholder,
+					      c->pretty_ctx->color);
 			if (ret)
 				c->auto_color = 0;
 			/*
@@ -1126,7 +1131,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
 
 	case '<':
 	case '>':
-		return parse_padding_placeholder(sb, placeholder, c);
+		return parse_padding_placeholder(sb, placeholder, &c->alignment);
 	}
 
 	/* these depend on the commit */
@@ -1291,12 +1296,8 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
 	return 0;	/* unknown placeholder */
 }
 
-static size_t format_and_pad_commit(struct strbuf *sb, /* in UTF-8 */
-				    const char *placeholder,
-				    struct format_commit_context *c)
+static int pad_an_item(struct strbuf *sb, int padding)
 {
-	struct strbuf local_sb = STRBUF_INIT;
-	int total_consumed = 0, len, padding = c->padding;
 	if (padding < 0) {
 		const char *start = strrchr(sb->buf, '\n');
 		int occupied;
@@ -1305,9 +1306,17 @@ static size_t format_and_pad_commit(struct strbuf *sb, /* in UTF-8 */
 		occupied = utf8_strnwidth(start, -1, 1);
 		padding = (-padding) - occupied;
 	}
+	return padding;
+}
+
+static int format_an_item(const char *placeholder,
+			  struct strbuf *local_sb,
+			  struct format_commit_context *c)
+{
+	int total_consumed = 0;
 	while (1) {
 		int modifier = *placeholder == 'C';
-		int consumed = format_commit_one(&local_sb, placeholder, c);
+		int consumed = format_commit_one(local_sb, placeholder, c);
 		total_consumed += consumed;
 
 		if (!modifier)
@@ -1319,9 +1328,19 @@ static size_t format_and_pad_commit(struct strbuf *sb, /* in UTF-8 */
 		placeholder++;
 		total_consumed++;
 	}
-	len = utf8_strnwidth(local_sb.buf, -1, 1);
+	return total_consumed;
+}
 
-	if (c->flush_type == flush_left_and_steal) {
+static void align_an_item(struct strbuf *sb, /* in UTF-8 */
+			  struct strbuf *item,
+			  enum flush_type flush_type,
+			  enum trunc_type truncate,
+			  int padding)
+{
+	int len;
+
+	len = utf8_strnwidth(item->buf, -1, 1);
+	if (flush_type == flush_left_and_steal) {
 		const char *ch = sb->buf + sb->len - 1;
 		while (len > padding && ch > sb->buf) {
 			const char *p;
@@ -1343,55 +1362,68 @@ static size_t format_and_pad_commit(struct strbuf *sb, /* in UTF-8 */
 			 * got a good ansi sequence, put it back to
 			 * local_sb as we're cutting sb
 			 */
-			strbuf_insert(&local_sb, 0, p, ch + 1 - p);
+			strbuf_insert(item, 0, p, ch + 1 - p);
 			ch = p - 1;
 		}
 		strbuf_setlen(sb, ch + 1 - sb->buf);
-		c->flush_type = flush_left;
+		flush_type = flush_left;
 	}
 
 	if (len > padding) {
-		switch (c->truncate) {
+		switch (truncate) {
 		case trunc_left:
-			strbuf_utf8_replace(&local_sb,
+			strbuf_utf8_replace(item,
 					    0, len - (padding - 2),
 					    "..");
 			break;
 		case trunc_middle:
-			strbuf_utf8_replace(&local_sb,
+			strbuf_utf8_replace(item,
 					    padding / 2 - 1,
 					    len - (padding - 2),
 					    "..");
 			break;
 		case trunc_right:
-			strbuf_utf8_replace(&local_sb,
+			strbuf_utf8_replace(item,
 					    padding - 2, len - (padding - 2),
 					    "..");
 			break;
 		case trunc_none:
 			break;
 		}
-		strbuf_addstr(sb, local_sb.buf);
+		strbuf_addstr(sb, item->buf);
 	} else {
 		int sb_len = sb->len, offset = 0;
-		if (c->flush_type == flush_left)
+		if (flush_type == flush_left)
 			offset = padding - len;
-		else if (c->flush_type == flush_both)
+		else if (flush_type == flush_both)
 			offset = (padding - len) / 2;
 		/*
 		 * we calculate padding in columns, now
 		 * convert it back to chars
 		 */
-		padding = padding - len + local_sb.len;
+		padding = padding - len + item->len;
 		strbuf_grow(sb, padding);
 		strbuf_setlen(sb, sb_len + padding);
 		memset(sb->buf + sb_len, ' ', sb->len - sb_len);
-		memcpy(sb->buf + sb_len + offset, local_sb.buf,
-		       local_sb.len);
+		memcpy(sb->buf + sb_len + offset, item->buf,
+		       item->len);
 	}
+}
+
+static size_t format_and_pad_commit(struct strbuf *sb, /* in UTF-8 */
+				    const char *placeholder,
+				    struct format_commit_context *c)
+{
+	int consumed, padding;
+	struct strbuf local_sb = STRBUF_INIT;
+
+	padding = pad_an_item(sb, c->alignment.padding);
+	consumed = format_an_item(placeholder, &local_sb, c);
+	align_an_item(sb, &local_sb, c->alignment.flush_type,
+		      c->alignment.truncate, padding);
 	strbuf_release(&local_sb);
-	c->flush_type = no_flush;
-	return total_consumed;
+	c->alignment.flush_type = no_flush;
+	return consumed;
 }
 
 static size_t format_commit_item(struct strbuf *sb, /* in UTF-8 */
@@ -1424,7 +1456,7 @@ static size_t format_commit_item(struct strbuf *sb, /* in UTF-8 */
 		placeholder++;
 
 	orig_len = sb->len;
-	if (((struct format_commit_context *)context)->flush_type != no_flush)
+	if (((struct format_commit_context *)context)->alignment.flush_type != no_flush)
 		consumed = format_and_pad_commit(sb, placeholder, context);
 	else
 		consumed = format_commit_one(sb, placeholder, context);
-- 8< --