Re: [PATCH v2] config: preserve case for one-shot config on the command line

[Stefan Beller]

On Tue, Feb 21, 2017 at 9:50 AM, Jeff King  wrote:
> On Tue, Feb 21, 2017 at 09:17:07AM -0800, Junio C Hamano wrote:
>
>>  * Changes from v1:
>>
>>- update the comment before the second loop to find the last
>>  dot.
>>
>>- move two new tests into existing t1300 (at least for now).
>>
>>- make it clear that the second of the new tests check two
>>  aspects of "three level vars" by setting up the expectation for
>>  the first half near the actual tests and adding comments on
>>  what it tests before the second half.
>
> Thanks, this addresses all of my (admittedly minor) concerns.
>
> -Peff

This patch looks different than what I last looked at. I like it.
Though the manual search of the last dot instead of using
strrchr seems to be over-optimizing to me.
Anyway it is still very understandable.

Thanks,
Stefan

Re: [PATCH v2] config: preserve case for one-shot config on the command line

[Jeff King]

On Tue, Feb 21, 2017 at 09:17:07AM -0800, Junio C Hamano wrote:

>  * Changes from v1:
> 
>- update the comment before the second loop to find the last
>  dot.
> 
>- move two new tests into existing t1300 (at least for now).
> 
>- make it clear that the second of the new tests check two
>  aspects of "three level vars" by setting up the expectation for
>  the first half near the actual tests and adding comments on
>  what it tests before the second half.

Thanks, this addresses all of my (admittedly minor) concerns.

-Peff

[PATCH v2] config: preserve case for one-shot config on the command line

[Junio C Hamano]

The "git -c = cmd" mechanism is to pretend that a
configuration variable  is set to  while the cmd is
running.  The code to do so however downcased  in its entirety,
which is wrong for a three-level ...

The  part needs to stay as-is.

Reported-by: Lars Schneider 
Diagnosed-by: Jonathan Tan 
Helped-by: Jeff King 
Signed-off-by: Junio C Hamano 
---

 * Changes from v1:

   - update the comment before the second loop to find the last
 dot.

   - move two new tests into existing t1300 (at least for now).

   - make it clear that the second of the new tests check two
 aspects of "three level vars" by setting up the expectation for
 the first half near the actual tests and adding comments on
 what it tests before the second half.

 config.c   | 30 +-
 t/t1300-repo-config.sh | 46 ++
 2 files changed, 75 insertions(+), 1 deletion(-)

diff --git a/config.c b/config.c
index 0dfed682b8..4128debc71 100644
--- a/config.c
+++ b/config.c
@@ -199,6 +199,34 @@ void git_config_push_parameter(const char *text)
strbuf_release();
 }
 
+/*
+ * downcase the  and  in . or
+ * .. and do so in place.  
+ * is left intact.
+ */
+static void canonicalize_config_variable_name(char *varname)
+{
+   char *cp, *last_dot;
+
+   /* downcase the first segment */
+   for (cp = varname; *cp; cp++) {
+   if (*cp == '.')
+   break;
+   *cp = tolower(*cp);
+   }
+   if (!*cp)
+   return;
+
+   /* find the last dot (we start from the first dot we just found) */
+   for (last_dot = cp; *cp; cp++)
+   if (*cp == '.')
+   last_dot = cp;
+
+   /* downcase the last segment */
+   for (cp = last_dot; *cp; cp++)
+   *cp = tolower(*cp);
+}
+
 int git_config_parse_parameter(const char *text,
   config_fn_t fn, void *data)
 {
@@ -221,7 +249,7 @@ int git_config_parse_parameter(const char *text,
strbuf_list_free(pair);
return error("bogus config parameter: %s", text);
}
-   strbuf_tolower(pair[0]);
+   canonicalize_config_variable_name(pair[0]->buf);
if (fn(pair[0]->buf, value, data) < 0) {
strbuf_list_free(pair);
return -1;
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 923bfc5a26..7a16f66a9d 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -1097,6 +1097,52 @@ test_expect_success 'multiple git -c appends config' '
test_cmp expect actual
 '
 
+test_expect_success 'last one wins: two level vars' '
+
+   # sec.var and sec.VAR are the same variable, as the first
+   # and the last level of a configuration variable name is
+   # case insensitive.
+
+   echo VAL >expect &&
+
+   git -c sec.var=val -c sec.VAR=VAL config --get sec.var >actual &&
+   test_cmp expect actual &&
+   git -c SEC.var=val -c sec.var=VAL config --get sec.var >actual &&
+   test_cmp expect actual &&
+
+   git -c sec.var=val -c sec.VAR=VAL config --get SEC.var >actual &&
+   test_cmp expect actual &&
+   git -c SEC.var=val -c sec.var=VAL config --get sec.VAR >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'last one wins: three level vars' '
+
+   # v.a.r and v.A.r are not the same variable, as the middle
+   # level of a three-level configuration variable name is
+   # case sensitive.
+
+   echo val >expect &&
+   git -c v.a.r=val -c v.A.r=VAL config --get v.a.r >actual &&
+   test_cmp expect actual &&
+   git -c v.a.r=val -c v.A.r=VAL config --get V.a.R >actual &&
+   test_cmp expect actual &&
+
+   # v.a.r and V.a.R are the same variable, as the first
+   # and the last level of a configuration variable name is
+   # case insensitive.
+
+   echo VAL >expect &&
+   git -c v.a.r=val -c v.a.R=VAL config --get v.a.r >actual &&
+   test_cmp expect actual &&
+   git -c v.a.r=val -c V.a.r=VAL config --get v.a.r >actual &&
+   test_cmp expect actual &&
+   git -c v.a.r=val -c v.a.R=VAL config --get V.a.R >actual &&
+   test_cmp expect actual &&
+   git -c v.a.r=val -c V.a.r=VAL config --get V.a.R >actual &&
+   test_cmp expect actual
+'
+
 test_expect_success 'git -c is not confused by empty environment' '
GIT_CONFIG_PARAMETERS="" git -c x.one=1 config --list
 '
-- 
2.12.0-rc2-221-ga92f6f5816