[PATCH 0/6] clean up parsing of maybe_bool

When we want to parse a boolean config item without dying on error, we
call git_config_maybe_bool() which takes two arguments: the value to be
parsed (obviously) and a `name` which is completely ignored. Junio has
suggested to drop `name` and rename the function [1]. That effort even
started shortly after that, by introducing git_parse_maybe_bool(). The
new function currently only has a single user outside config.c.

Patch 5 of this series deprecates the old function and updates all
callers to use git_parse_maybe_bool() instead. Patch 6 is a final
cleanup: one of the converted callers suddenly had an unused argument.

Patches 3 and 4 prepare for the switch. In particular, patch 4 ensures
that the two functions are actually equivalent. In doing so, it affects
`git push --signed=..` which was very slightly inconsistent to the rest
of Git.

Patch 2 adds a failing test in preparation for patch 4. Patch 1 corrects
the documentation not to say "git push --sign=.." to make it a bit more
obvious that the opposite typo is not being made in patches 2 and 4.

git_parse_maybe_bool is used in sb/diff-color-move, which is in "next".
This series makes --color-moved and diff.colormoved consistent with
other booleans. That should be a good thing, but cc Stefan to be sure.

Martin

[1] https://public-inbox.org/git/xmqq7fotd71o.fsf@gitster.dls.corp.google.com/

Martin Ågren (6):
  Doc/git-{push,send-pack}: correct --sign= to --signed=
  t5334: document that git push --signed=1 does not work
  config: introduce git_parse_maybe_bool_text
  config: make git_{config,parse}_maybe_bool equivalent
  treewide: deprecate git_config_maybe_bool, use git_parse_maybe_bool
  parse_decoration_style: drop unused argument `var`

 Documentation/git-push.txt             |  4 ++--
 Documentation/git-send-pack.txt        |  4 ++--
 Documentation/technical/api-config.txt |  4 ++++
 t/t5534-push-signed.sh                 |  7 +++++++
 builtin/log.c                          | 10 +++++-----
 builtin/merge.c                        |  4 ++--
 builtin/pull.c                         |  4 ++--
 builtin/push.c                         |  2 +-
 builtin/remote.c                       |  2 +-
 builtin/send-pack.c                    |  2 +-
 config.c                               | 15 ++++++++++-----
 pager.c                                |  2 +-
 submodule-config.c                     |  6 +++---
 13 files changed, 41 insertions(+), 25 deletions(-)

-- 
2.14.0.5.g0f7b1ed27

Since we're about to touch the behavior of --signed=, do this as a
preparatory step.

The documentation mentions --sign=, and it works. But that's just
because it's an unambiguous abbreviation of --signed, which is how it is
actually implemented. This was added in commit 30261094 ("push: support
signing pushes iff the server supports it", 2015-08-19). Back when that
series was developed [1] [2], there were suggestions about both --sign=
and --signed=. The final implementation settled on --signed=, but some
of the documentation and commit messages ended up using --sign=.

The option is referred to as --signed= in Documentation/config.txt
(under push.gpgSign).

One could argue that we have promised --sign for two years now, so we
should implement it as an alias for --signed. (Then we might also
deprecate the latter, something which was considered already then.) That
would be a slightly more intrusive change.

This minor issue would only be a problem once we want to implement some
other option --signfoo, but the earlier we do this step, the better.

[1] v1-thread:
https://public-inbox.org/git/1439492451-11233-1-git-send-email-dborowitz@google.com/T/#u

[2] v2-thread:
https://public-inbox.org/git/1439998007-28719-1-git-send-email-dborowitz@google.com/T/#m6533a6c4707a30b0d81e86169ff8559460cbf6eb

Signed-off-by: Martin Ågren <redacted>
---
 Documentation/git-push.txt      | 4 ++--
 Documentation/git-send-pack.txt | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 0a639664f..3e76e99f3 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt

@@ -12,7 +12,7 @@ SYNOPSIS
 'git push' [--all | --mirror | --tags] [--follow-tags] [--atomic] [-n | --dry-run] [--receive-pack=<git-receive-pack>]
 	   [--repo=<repository>] [-f | --force] [-d | --delete] [--prune] [-v | --verbose]
 	   [-u | --set-upstream] [--push-option=<string>]
-	   [--[no-]signed|--sign=(true|false|if-asked)]
+	   [--[no-]signed|--signed=(true|false|if-asked)]
 	   [--force-with-lease[=<refname>[:<expect>]]]
 	   [--no-verify] [<repository> [<refspec>...]]
 

@@ -141,7 +141,7 @@ already exists on the remote side.
 	information, see `push.followTags` in linkgit:git-config[1].
 
 --[no-]signed::
---sign=(true|false|if-asked)::
+--signed=(true|false|if-asked)::
 	GPG-sign the push request to update refs on the receiving
 	side, to allow it to be checked by the hooks and/or be
 	logged.  If `false` or `--no-signed`, no signing will be

diff --git a/Documentation/git-send-pack.txt b/Documentation/git-send-pack.txt
index 966abb0df..f51c64939 100644
--- a/Documentation/git-send-pack.txt
+++ b/Documentation/git-send-pack.txt

@@ -11,7 +11,7 @@ SYNOPSIS
 [verse]
 'git send-pack' [--all] [--dry-run] [--force] [--receive-pack=<git-receive-pack>]
 		[--verbose] [--thin] [--atomic]
-		[--[no-]signed|--sign=(true|false|if-asked)]
+		[--[no-]signed|--signed=(true|false|if-asked)]
 		[<host>:]<directory> [<ref>...]
 
 DESCRIPTION

@@ -71,7 +71,7 @@ be in a separate packet, and the list must end with a flush packet.
 	refs.
 
 --[no-]signed::
---sign=(true|false|if-asked)::
+--signed=(true|false|if-asked)::
 	GPG-sign the push request to update refs on the receiving
 	side, to allow it to be checked by the hooks and/or be
 	logged.  If `false` or `--no-signed`, no signing will be

-- 
2.14.0.5.g0f7b1ed27

When accepting booleans as command-line or config options throughout
Git, there are several documented synonyms for true and false.
However, one particular user is slightly broken: `git push --signed=..`
does not understand the integer synonyms for true and false.

This is hardly wanted. The --signed option has a different notion of
boolean than all other arguments and config options, including the
config option corresponding to it, push.gpgSign.

Add a test documenting the failure to handle --signed=1.

Signed-off-by: Martin Ågren <redacted>
---
 t/t5534-push-signed.sh | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/t/t5534-push-signed.sh b/t/t5534-push-signed.sh
index 464ffdd14..5dce55e1d 100755
--- a/t/t5534-push-signed.sh
+++ b/t/t5534-push-signed.sh

@@ -71,6 +71,13 @@ test_expect_success 'push --signed fails with a receiver without push certificat
 	test_i18ngrep "the receiving end does not support" err
 '
 
+test_expect_failure 'push --signed=1 is accepted' '
+	prepare_dst &&
+	mkdir -p dst/.git/hooks &&
+	test_must_fail git push --signed=1 dst noop ff +noff 2>err &&
+	test_i18ngrep "the receiving end does not support" err
+'
+
 test_expect_success GPG 'no certificate for a signed push with no update' '
 	prepare_dst &&
 	mkdir -p dst/.git/hooks &&

-- 
2.14.0.5.g0f7b1ed27

Martin Ågren [off-list ref] writes:

When accepting booleans as command-line or config options throughout
Git, there are several documented synonyms for true and false.
However, one particular user is slightly broken: `git push --signed=..`
does not understand the integer synonyms for true and false.

This is hardly wanted. The --signed option has a different notion of
boolean than all other arguments and config options, including the
config option corresponding to it, push.gpgSign.

Add a test documenting the failure to handle --signed=1.

My knee-jerk reaction is that parse_maybe_bool() is broken in that
it should do everything that config_maybe_bool() does.  I wonder why
we do not call git_parse_int() like git_config_maybe_bool() does?

... Ahh, that is because bool_or_int() wants to know that "1" is an
int and not a bool, and parse_maybe_bool() is designed to cater to
the need of that thing, in addition to config_maybe_bool().  And the
"--signed=" thing wants parse_bool_or_string(), not bool_or_int(),
so we should treat "1" as true and "0" as false.  There is no way to
do so in the current code and that is why you have the new _text()
thing in patches 3-4/6.

Makes sense.

n> +	mkdir -p dst/.git/hooks &&

+	test_must_fail git push --signed=1 dst noop ff +noff 2>err &&
+	test_i18ngrep "the receiving end does not support" err
+'
+
 test_expect_success GPG 'no certificate for a signed push with no update' '
 	prepare_dst &&
 	mkdir -p dst/.git/hooks &&

Commit 9a549d43 ("config.c: rename git_config_maybe_bool_text and export
it as git_parse_maybe_bool", 2015-08-19) intended git_parse_maybe_bool
to be a replacement for git_config_maybe_bool, which could then be
retired. That is not obvious from the commit message, but that is what
the background on the mailing list suggests [1].

However, git_{config,parse}_maybe_bool do not handle all input the same.
Before the rename, that was by design and there is a caller in config.c
which requires git_parse_maybe_bool to behave exactly as it does.

Prepare for the next patch by renaming git_parse_maybe_bool to ..._text
and reimplementing the first one as a simple call to the second one. Let
the existing users in config.c use ..._text, since it does what they
need.

[1] https://public-inbox.org/git/xmqq7fotd71o.fsf@gitster.dls.corp.google.com/

Signed-off-by: Martin Ågren <redacted>
---
 config.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/config.c b/config.c
index 231f9a750..7df57cec0 100644
--- a/config.c
+++ b/config.c

@@ -928,7 +928,7 @@ ssize_t git_config_ssize_t(const char *name, const char *value)
 	return ret;
 }
 
-int git_parse_maybe_bool(const char *value)
+static int git_parse_maybe_bool_text(const char *value)
 {
 	if (!value)
 		return 1;

@@ -945,9 +945,14 @@ int git_parse_maybe_bool(const char *value)
 	return -1;
 }
 
+int git_parse_maybe_bool(const char *value)
+{
+	return git_parse_maybe_bool_text(value);
+}
+
 int git_config_maybe_bool(const char *name, const char *value)
 {
-	int v = git_parse_maybe_bool(value);
+	int v = git_parse_maybe_bool_text(value);
 	if (0 <= v)
 		return v;
 	if (git_parse_int(value, &v))

@@ -957,7 +962,7 @@ int git_config_maybe_bool(const char *name, const char *value)
 
 int git_config_bool_or_int(const char *name, const char *value, int *is_bool)
 {
-	int v = git_parse_maybe_bool(value);
+	int v = git_parse_maybe_bool_text(value);
 	if (0 <= v) {
 		*is_bool = 1;
 		return v;

-- 
2.14.0.5.g0f7b1ed27

Both of these act on a string `value` which they parse as a boolean. The
"parse"-variant was introduced as a replacement for the "config"-variant
which for historical reasons takes an unused argument `name`. That it
was intended as a replacement is not obvious from commit 9a549d43
("config.c: rename git_config_maybe_bool_text and export it as
git_parse_maybe_bool", 2015-08-19), but that is what the background on
the mailing list suggests [1].

However, these two functions do not parse `value` in exactly the same
way. In particular, git_config_maybe_bool accepts integers (0 for false,
non-0 for true). This means there are two slightly different definitions
of "maybe_bool" in the code-base, and that every time a call to
git_config_maybe_bool is changed to use git_parse_maybe_bool, it risks
breaking someone's workflow.

Move the implementation of "config" into "parse" and make the latter a
trivial wrapper.

This also fixes the only user of git_parse_maybe_bool, `git push
--signed=..`.

[1] https://public-inbox.org/git/xmqq7fotd71o.fsf@gitster.dls.corp.google.com/

Signed-off-by: Martin Ågren <redacted>
---
 t/t5534-push-signed.sh |  2 +-
 config.c               | 10 +++++-----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/t/t5534-push-signed.sh b/t/t5534-push-signed.sh
index 5dce55e1d..1cea758f7 100755
--- a/t/t5534-push-signed.sh
+++ b/t/t5534-push-signed.sh

@@ -71,7 +71,7 @@ test_expect_success 'push --signed fails with a receiver without push certificat
 	test_i18ngrep "the receiving end does not support" err
 '
 
-test_expect_failure 'push --signed=1 is accepted' '
+test_expect_success 'push --signed=1 is accepted' '
 	prepare_dst &&
 	mkdir -p dst/.git/hooks &&
 	test_must_fail git push --signed=1 dst noop ff +noff 2>err &&

diff --git a/config.c b/config.c
index 7df57cec0..d87376a5d 100644
--- a/config.c
+++ b/config.c

@@ -946,11 +946,6 @@ static int git_parse_maybe_bool_text(const char *value)
 }
 
 int git_parse_maybe_bool(const char *value)
-{
-	return git_parse_maybe_bool_text(value);
-}
-
-int git_config_maybe_bool(const char *name, const char *value)
 {
 	int v = git_parse_maybe_bool_text(value);
 	if (0 <= v)

@@ -960,6 +955,11 @@ int git_config_maybe_bool(const char *name, const char *value)
 	return -1;
 }
 
+int git_config_maybe_bool(const char *name, const char *value)
+{
+	return git_parse_maybe_bool(value);
+}
+
 int git_config_bool_or_int(const char *name, const char *value, int *is_bool)
 {
 	int v = git_parse_maybe_bool_text(value);

-- 
2.14.0.5.g0f7b1ed27

The only difference between these is that the former takes an argument
`name` which it ignores completely. Still, the callers are quite careful
to provide reasonable values for it.

Once in-flight topics have landed, we should be able to remove
git_config_maybe_bool. In the meantime, document it as deprecated in the
technical documentation. While at it, document git_parse_maybe_bool.

Signed-off-by: Martin Ågren <redacted>
---
 Documentation/technical/api-config.txt | 4 ++++
 builtin/log.c                          | 4 ++--
 builtin/merge.c                        | 4 ++--
 builtin/pull.c                         | 4 ++--
 builtin/push.c                         | 2 +-
 builtin/remote.c                       | 2 +-
 builtin/send-pack.c                    | 2 +-
 config.c                               | 2 +-
 pager.c                                | 2 +-
 submodule-config.c                     | 6 +++---
 10 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/Documentation/technical/api-config.txt b/Documentation/technical/api-config.txt
index 20741f345..7a83a3a6e 100644
--- a/Documentation/technical/api-config.txt
+++ b/Documentation/technical/api-config.txt

@@ -187,6 +187,10 @@ Same as `git_config_bool`, except that integers are returned as-is, and
 an `is_bool` flag is unset.
 
 `git_config_maybe_bool`::
+Deprecated. Use `git_parse_maybe_bool` instead. They are exactly the
+same, except this function takes an unused argument `name`.
+
+`git_parse_maybe_bool`::
 Same as `git_config_bool`, except that it returns -1 on error rather
 than dying.
 

diff --git a/builtin/log.c b/builtin/log.c
index c6362cf92..9182f0ee3 100644
--- a/builtin/log.c
+++ b/builtin/log.c

@@ -60,7 +60,7 @@ static int auto_decoration_style(void)
 
 static int parse_decoration_style(const char *var, const char *value)
 {
-	switch (git_config_maybe_bool(var, value)) {
+	switch (git_parse_maybe_bool(value)) {
 	case 1:
 		return DECORATE_SHORT_REFS;
 	case 0:

@@ -821,7 +821,7 @@ static int git_format_config(const char *var, const char *value, void *cb)
 		return 0;
 	}
 	if (!strcmp(var, "format.from")) {
-		int b = git_config_maybe_bool(var, value);
+		int b = git_parse_maybe_bool(value);
 		free(from);
 		if (b < 0)
 			from = xstrdup(value);

diff --git a/builtin/merge.c b/builtin/merge.c
index 900bafdb4..6a5122921 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c

@@ -566,7 +566,7 @@ static int git_merge_config(const char *k, const char *v, void *cb)
 	else if (!strcmp(k, "merge.renormalize"))
 		option_renormalize = git_config_bool(k, v);
 	else if (!strcmp(k, "merge.ff")) {
-		int boolval = git_config_maybe_bool(k, v);
+		int boolval = git_parse_maybe_bool(v);
 		if (0 <= boolval) {
 			fast_forward = boolval ? FF_ALLOW : FF_NO;
 		} else if (v && !strcmp(v, "only")) {

@@ -940,7 +940,7 @@ static int default_edit_option(void)
 		return 0;
 
 	if (e) {
-		int v = git_config_maybe_bool(name, e);
+		int v = git_parse_maybe_bool(e);
 		if (v < 0)
 			die(_("Bad value '%s' in environment '%s'"), e, name);
 		return v;

diff --git a/builtin/pull.c b/builtin/pull.c
index 9b86e519b..7fe281414 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c

@@ -39,7 +39,7 @@ enum rebase_type {
 static enum rebase_type parse_config_rebase(const char *key, const char *value,
 		int fatal)
 {
-	int v = git_config_maybe_bool("pull.rebase", value);
+	int v = git_parse_maybe_bool(value);
 
 	if (!v)
 		return REBASE_FALSE;

@@ -274,7 +274,7 @@ static const char *config_get_ff(void)
 	if (git_config_get_value("pull.ff", &value))
 		return NULL;
 
-	switch (git_config_maybe_bool("pull.ff", value)) {
+	switch (git_parse_maybe_bool(value)) {
 	case 0:
 		return "--no-ff";
 	case 1:

diff --git a/builtin/push.c b/builtin/push.c
index 03846e837..2ac810422 100644
--- a/builtin/push.c
+++ b/builtin/push.c

@@ -481,7 +481,7 @@ static int git_push_config(const char *k, const char *v, void *cb)
 	} else if (!strcmp(k, "push.gpgsign")) {
 		const char *value;
 		if (!git_config_get_value("push.gpgsign", &value)) {
-			switch (git_config_maybe_bool("push.gpgsign", value)) {
+			switch (git_parse_maybe_bool(value)) {
 			case 0:
 				set_push_cert_flags(flags, SEND_PACK_PUSH_CERT_NEVER);
 				break;

diff --git a/builtin/remote.c b/builtin/remote.c
index 6273c0c23..a995ea86c 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c

@@ -301,7 +301,7 @@ static int config_read_branches(const char *key, const char *value, void *cb)
 			}
 			string_list_append(&info->merge, xstrdup(value));
 		} else {
-			int v = git_config_maybe_bool(orig_key, value);
+			int v = git_parse_maybe_bool(value);
 			if (v >= 0)
 				info->rebase = v;
 			else if (!strcmp(value, "preserve"))

diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 633e0c3cd..fc4f0bb5f 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c

@@ -105,7 +105,7 @@ static int send_pack_config(const char *k, const char *v, void *cb)
 	if (!strcmp(k, "push.gpgsign")) {
 		const char *value;
 		if (!git_config_get_value("push.gpgsign", &value)) {
-			switch (git_config_maybe_bool("push.gpgsign", value)) {
+			switch (git_parse_maybe_bool(value)) {
 			case 0:
 				args.push_cert = SEND_PACK_PUSH_CERT_NEVER;
 				break;

diff --git a/config.c b/config.c
index d87376a5d..9b474d9a3 100644
--- a/config.c
+++ b/config.c

@@ -1851,7 +1851,7 @@ int git_configset_get_maybe_bool(struct config_set *cs, const char *key, int *de
 {
 	const char *value;
 	if (!git_configset_get_value(cs, key, &value)) {
-		*dest = git_config_maybe_bool(key, value);
+		*dest = git_parse_maybe_bool(value);
 		if (*dest == -1)
 			return -1;
 		return 0;

diff --git a/pager.c b/pager.c
index 4dd9e1b26..92b23e6cd 100644
--- a/pager.c
+++ b/pager.c

@@ -194,7 +194,7 @@ static int pager_command_config(const char *var, const char *value, void *vdata)
 	const char *cmd;
 
 	if (skip_prefix(var, "pager.", &cmd) && !strcmp(cmd, data->cmd)) {
-		int b = git_config_maybe_bool(var, value);
+		int b = git_parse_maybe_bool(value);
 		if (b >= 0)
 			data->want = b;
 		else {

diff --git a/submodule-config.c b/submodule-config.c
index 5fe2d0787..11a32584d 100644
--- a/submodule-config.c
+++ b/submodule-config.c

@@ -232,7 +232,7 @@ static struct submodule *lookup_or_create_by_name(struct submodule_cache *cache,
 static int parse_fetch_recurse(const char *opt, const char *arg,
 			       int die_on_error)
 {
-	switch (git_config_maybe_bool(opt, arg)) {
+	switch (git_parse_maybe_bool(arg)) {
 	case 1:
 		return RECURSE_SUBMODULES_ON;
 	case 0:

@@ -277,7 +277,7 @@ int option_fetch_parse_recurse_submodules(const struct option *opt,
 static int parse_update_recurse(const char *opt, const char *arg,
 				int die_on_error)
 {
-	switch (git_config_maybe_bool(opt, arg)) {
+	switch (git_parse_maybe_bool(arg)) {
 	case 1:
 		return RECURSE_SUBMODULES_ON;
 	case 0:

@@ -297,7 +297,7 @@ int parse_update_recurse_submodules_arg(const char *opt, const char *arg)
 static int parse_push_recurse(const char *opt, const char *arg,
 			       int die_on_error)
 {
-	switch (git_config_maybe_bool(opt, arg)) {
+	switch (git_parse_maybe_bool(arg)) {
 	case 1:
 		/* There's no simple "on" value when pushing */
 		if (die_on_error)

-- 
2.14.0.5.g0f7b1ed27

The previous commit left it unused.

Signed-off-by: Martin Ågren <redacted>
---
 builtin/log.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 9182f0ee3..483d15a94 100644
--- a/builtin/log.c
+++ b/builtin/log.c

@@ -58,7 +58,7 @@ static int auto_decoration_style(void)
 	return (isatty(1) || pager_in_use()) ? DECORATE_SHORT_REFS : 0;
 }
 
-static int parse_decoration_style(const char *var, const char *value)
+static int parse_decoration_style(const char *value)
 {
 	switch (git_parse_maybe_bool(value)) {
 	case 1:

@@ -82,7 +82,7 @@ static int decorate_callback(const struct option *opt, const char *arg, int unse
 	if (unset)
 		decoration_style = 0;
 	else if (arg)
-		decoration_style = parse_decoration_style("command line", arg);
+		decoration_style = parse_decoration_style(arg);
 	else
 		decoration_style = DECORATE_SHORT_REFS;
 

@@ -409,7 +409,7 @@ static int git_log_config(const char *var, const char *value, void *cb)
 	if (!strcmp(var, "log.date"))
 		return git_config_string(&default_date_mode, var, value);
 	if (!strcmp(var, "log.decorate")) {
-		decoration_style = parse_decoration_style(var, value);
+		decoration_style = parse_decoration_style(value);
 		if (decoration_style < 0)
 			decoration_style = 0; /* maybe warn? */
 		return 0;

-- 
2.14.0.5.g0f7b1ed27

On Mon, Aug 7, 2017 at 11:20 AM, Martin Ågren [off-list ref] wrote:

When we want to parse a boolean config item without dying on error, we
call git_config_maybe_bool() which takes two arguments: the value to be
parsed (obviously) and a `name` which is completely ignored. Junio has
suggested to drop `name` and rename the function [1]. That effort even
started shortly after that, by introducing git_parse_maybe_bool(). The
new function currently only has a single user outside config.c.

Patch 5 of this series deprecates the old function and updates all
callers to use git_parse_maybe_bool() instead. Patch 6 is a final
cleanup: one of the converted callers suddenly had an unused argument.

Patches 3 and 4 prepare for the switch. In particular, patch 4 ensures
that the two functions are actually equivalent. In doing so, it affects
`git push --signed=..` which was very slightly inconsistent to the rest
of Git.

Patch 2 adds a failing test in preparation for patch 4. Patch 1 corrects
the documentation not to say "git push --sign=.." to make it a bit more
obvious that the opposite typo is not being made in patches 2 and 4.

git_parse_maybe_bool is used in sb/diff-color-move, which is in "next".
This series makes --color-moved and diff.colormoved consistent with
other booleans. That should be a good thing, but cc Stefan to be sure.

The series looks fine to me overall, though patch 5 is overly gentle IMHO.
We could have removed it right there as Junio is very good at resolving
conflicts or producing dirty merges for such a situation.
But delaying it until no other series' are in flight is fine with me, too.

Looking back at sb/diff-color-move and the code where
git_parse_maybe_bool is used, I do not think this will be an issue.

Thanks,
Stefan