git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Will Palmer <wmpalmer@gmail.com>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>, Jeff King <peff@peff.net>,
	Will Palmer <wmpalmer@gmail.com>
Subject: [PATCH/RFC 3/9] interpret %C(invalid) as we would %%C(invalid)
Date: Tue, 29 Mar 2011 00:17:25 +0100	[thread overview]
Message-ID: <1301354251-23380-4-git-send-email-wmpalmer@gmail.com> (raw)
In-Reply-To: <1301354251-23380-1-git-send-email-wmpalmer@gmail.com>

%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

  parent reply	other threads:[~2011-03-28 23:18 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1301354251-23380-4-git-send-email-wmpalmer@gmail.com \
    --to=wmpalmer@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).