Re: [PATCH 3/2] pretty: support right alignment

From: Nguyen Thai Ngoc Duy <pclouds@gmail.com>
Date: Sep 21, 2012, 13:03 UTC
Message-ID: <20120921130349.GA17999@lanh>
In-Reply-To: <7v392cd4vi.fsf@alter.siamese.dyndns.org>

On Thu, Sep 20, 2012 at 09:40:49AM -0700, Junio C Hamano wrote:

Show 15 quoted lines

>  - I am wondering if somebody ever want to do this with a follow-up
>    patch:
> 
> 	Left %h%|Center %cd%|Right %ad
> 
>    Is %| a sensible choice for "flush right"?  I am wondering if it
>    makes more sense to make %|, %< and %> as "multi-column
>    introducer" (the example defines output with three columns) that
>    also tells how text inside each column is flushed inside the
>    column, e.g.
> 
> 	%>col 1 right flushed%|col 2 centered%< col 3 left flushed
> 
>    or something like that (we may want explicit "column width"
>    specifiers if we were to do this kind of thing).

Instead of thinking of "columns", we could go back to "placeholders", or in printf terms, an "%s". In addition to plain %s, we need something similar to "%*s" and "%-*s" to pad right and left.

Conceptually it's simpler. We don't have to deal with a bunch of problems in your quotes that I cut out. Still it allows users to do flush right, flush left and so on within limits. They just need to think in terms of fixed-size cells. So...

%>(N)%? is transformed roughly to printf("%-*s", N, %?). Similarly %<(N)%? becomes printf("%*s", N, %?). We could have %|(N) to pad both %left and right (aka centered).

Better?

We might need a modifier or something to allow cutting (and maybe putting ellipsis in place) to keep oversized cells from breaking the layout.

The demonstration patch follows. You can't build because I don't post the whole series.

-- 8< --

diff --git a/pretty.c b/pretty.c
index b1cec71..543c309 100644
--- a/pretty.c
+++ b/pretty.c
@@ -617,6 +617,12 @@ struct chunk {
 	size_t len;
 };
 
+enum flush_type {
+	no_flush,
+	flush_right,
+	flush_left
+};
+
 struct format_commit_context {
 	const struct commit *commit;
 	const struct pretty_print_context *pretty_ctx;
@@ -624,13 +630,14 @@ struct format_commit_context {
 	unsigned commit_message_parsed:1;
 	unsigned commit_signature_parsed:1;
 	unsigned use_color:1;
+	enum flush_type flush_type;
 	struct {
 		char *gpg_output;
 		char good_bad;
 		char *signer;
 	} signature;
 	char *message;
-	size_t width, indent1, indent2;
+	size_t width, indent1, indent2, padding;
 
 	/* These offsets are relative to the start of the commit message. */
 	struct chunk author;
@@ -944,6 +951,24 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,
 			return end - placeholder + 1;
 		} else
 			return 0;
+
+	case '<':
+	case '>':
+		if (placeholder[1] == '(') {
+			const char *start = placeholder + 2;
+			const char *end = strchr(start, ')');
+			char *next;
+			int width;
+			if (!end || end == start)
+				return 0;
+			width = strtoul(start, &next, 10);
+			if (next == start || width == 0)
+				return 0;
+			c->padding = width;
+			c->flush_type = *placeholder == '<' ? flush_right : flush_left;
+			return end - placeholder + 1;
+		}
+		return 0;
 	}
 
 	/* these depend on the commit */
@@ -1102,6 +1127,8 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,
 static size_t format_commit_item(struct strbuf *sb, const char *placeholder,
 				 void *context)
 {
+	struct format_commit_context *c = context;
+	struct strbuf local_sb = STRBUF_INIT;
 	int consumed;
 	size_t orig_len;
 	enum {
@@ -1127,10 +1154,23 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder,
 	if (magic != NO_MAGIC)
 		placeholder++;
 
-	orig_len = sb->len;
-	consumed = format_commit_one(sb, placeholder, context);
-	if (magic == NO_MAGIC)
-		return consumed;
+	if (c->flush_type != no_flush) {
+		int len;
+		consumed = format_commit_one(&local_sb, placeholder, context);
+		/* the number of column, esc seq skipped */
+		len = utf8_strnwidth(local_sb.buf, -1, 1);
+		strbuf_addf(sb,
+			    c->flush_type == flush_right ? "%-*s" : "%*s",
+			    (int)(c->padding + (local_sb.len - len)),
+			    local_sb.buf);
+		strbuf_release(&local_sb);
+		c->flush_type = no_flush;
+	} else {
+		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')
-- 8< --