git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>
Subject: [PATCH 1/2] log.decorate: accept 0/1 bool values
Date: Wed, 17 Nov 2010 12:00:45 -0500	[thread overview]
Message-ID: <20101117170045.GA4108@sigill.intra.peff.net> (raw)

We explicitly document "0" and "1" as synonyms for "false"
and "true" in boolean config options. However, we don't
actually handle those values in git_config_maybe_bool.

In most cases this works fine, as we call git_config_bool,
which in turn calls git_config_bool_or_int, which in turn
calls git_config_maybe_bool. Values of 0/1 are considered
"not bool", but their integer values end up being converted
to the corresponding boolean values.

However, the log.decorate code looks for maybe_bool
explicitly, so that it can fall back to the "short" and
"full" strings. It does not handle 0/1 at all, and considers
them invalid values.

We cannot simply add 0/1 support to git_config_maybe_bool.
That would confuse git_config_bool_or_int, which may want to
distinguish the integer values "0" and "1" from bools.

Signed-off-by: Jeff King <peff@peff.net>
---
I don't know that anyone explicitly cares about distinguishing "0" from
"false", but we do expose this interface via "git config --bool-or-int",
and we test for it explicitly in t1300. So I kept the existing behavior
intact in that instance.

Note that another way to solve this would be to check explicitly for
"short" or "full" and then just call git_config_bool itself. That works
in this case, but not for an open-ended case (i.e., where you can take
either a bool, or any arbitrary string). And I needed that case for my
patch 2/2. :)

 config.c       |   16 ++++++++++++++--
 t/t4202-log.sh |    9 +++++++++
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/config.c b/config.c
index 4b0a820..9918b93 100644
--- a/config.c
+++ b/config.c
@@ -410,7 +410,7 @@ unsigned long git_config_ulong(const char *name, const char *value)
 	return ret;
 }
 
-int git_config_maybe_bool(const char *name, const char *value)
+static int git_config_maybe_bool_text(const char *name, const char *value)
 {
 	if (!value)
 		return 1;
@@ -427,9 +427,21 @@ int git_config_maybe_bool(const char *name, const char *value)
 	return -1;
 }
 
+int git_config_maybe_bool(const char *name, const char *value)
+{
+	int v = git_config_maybe_bool_text(name, value);
+	if (0 <= v)
+		return v;
+	if (!strcmp(value, "0"))
+		return 0;
+	if (!strcmp(value, "1"))
+		return 1;
+	return -1;
+}
+
 int git_config_bool_or_int(const char *name, const char *value, int *is_bool)
 {
-	int v = git_config_maybe_bool(name, value);
+	int v = git_config_maybe_bool_text(name, value);
 	if (0 <= v) {
 		*is_bool = 1;
 		return v;
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 2e51356..2043bb8 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -422,6 +422,15 @@ test_expect_success 'log.decorate configuration' '
 	test_cmp expect.full actual &&
 
 	git config --unset-all log.decorate &&
+	git config log.decorate 1 &&
+	git log --oneline >actual &&
+	test_cmp expect.short actual &&
+	git log --oneline --decorate=full >actual &&
+	test_cmp expect.full actual &&
+	git log --oneline --decorate=no >actual &&
+	test_cmp expect.none actual &&
+
+	git config --unset-all log.decorate &&
 	git config log.decorate short &&
 	git log --oneline >actual &&
 	test_cmp expect.short actual &&
-- 
1.7.3.2.362.g308e9

             reply	other threads:[~2010-11-17 17:00 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-17 17:00 Jeff King [this message]
2010-11-17 17:04 ` [PATCH 2/2] allow command-specific pagers in pager.<cmd> Jeff King
2010-11-17 19:36 ` [PATCH 1/2] log.decorate: accept 0/1 bool values Junio C Hamano
2010-11-17 19:52   ` Jeff King
2010-11-18 17:00     ` Junio C Hamano
2010-11-18 17:14       ` Jeff King

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=20101117170045.GA4108@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /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).