Re: [PATCH] Sanity-ckeck config variable names

From: Libor Pechacek <lpechacek@suse.cz>
Date: Jan 21, 2011, 10:02 UTC
Message-ID: <20110121100212.GE19715@fm.suse.cz>
In-Reply-To: <20110121000629.GA7773@sigill.intra.peff.net>

On Thu 20-01-11 18:22:32, Jeff King wrote:

Show 5 quoted lines

> > + * Returns 0 on success, 1 when there is an invalid character in the key and 2
> > + * if there is no section name in the key.
> 
> Please switch these to -1 and -2, as we generally use negative integers
> to indicate errors in library-ish function.

Done.

> Other than that, the code looks OK to me. However, it does cause
> t1300.85 to fail.

Thanks for catching it, added your changes to the patch and tested. I didn't notice the test suite. :)

On Thu 20-01-11 19:06:29, Jeff King wrote:

Show 18 quoted lines

> Doesn't this hunk:
> 
> > @@ -168,10 +167,6 @@ static int get_value(const char *key_, const char *regex_)
> >       }
> >  
> >       key = xstrdup(key_);
> > -     for (tl=key+strlen(key)-1; tl >= key && *tl != '.'; --tl)
> > -             *tl = tolower(*tl);
> > -     for (tl=key; *tl && *tl != '.'; ++tl)
> > -             *tl = tolower(*tl);
> 
> Mean that regexp keys no longer get downcased properly? I.e.,
> 
>   git config Foo.value true
>   git config --get-regexp 'foo.*'
>   git config --get-regexp 'Foo.*'
> 
> used to work for both lookups, but now fails for the second one?

After thinking about it I added the code back. More on it in the "documentation fix" thread.

Thanks for your kind guidance.

Libor

-- 
Libor Pechacek
SUSE L3 Team, Prague