Re: git config --help not functional on bad config

From: Stefan Beller <sbeller@google.com>
Date: Jul 11, 2017, 19:05 UTC
Message-ID: <CAGZ79kbQsRQaHjRccs-0jkYkfhBwU-gX-M3A9sP4hqTtqqpFYw@mail.gmail.com>
In-Reply-To: <20170711175342.mllx7efdsqlzv5lo@sigill.intra.peff.net>

On Tue, Jul 11, 2017 at 10:53 AM, Jeff King <peff@peff.net> wrote:

Show 32 quoted lines

> On Tue, Jul 11, 2017 at 03:49:21PM +0100, Peter Krefting wrote:
>
>> That's fine. However, when trying to look for help, it is not that useful:
>>
>>   $ git config --help
>>   error: malformed value for branch.autosetuprebase
>>   fatal: bad config variable 'branch.autosetuprebase' in file '.git/config' at line 24
>>
>> Perhaps it should allow "--help" to go through even if the configuration is
>> bad?
>
> Yes, I agree the current behavior is poor. What's happening under the
> hood is that "--help" for any command runs "git help config", which in
> turn looks at the config to pick up things like help.format.
>
> But it also loads git_default_config(), which I suspect isn't actually
> useful. It goes all the way back to 70087cdbd (git-help: add
> "help.format" config variable., 2007-12-15), and it looks like it was
> probably added just to match other config callbacks.
>
> So I think we could probably just do this:
>
> diff --git a/builtin/help.c b/builtin/help.c
> index 334a8494a..c42dfc9e9 100644
> --- a/builtin/help.c
> +++ b/builtin/help.c
> @@ -273,7 +273,7 @@ static int git_help_config(const char *var, const char *value, void *cb)
>         if (starts_with(var, "man."))
>                 return add_man_viewer_info(var, value);
>
> -       return git_default_config(var, value, cb);
> +       return 0;

Instead of ignoring any default config, we could do

    git_default_config(var, value, cb);
    /* ignore broken config, we may be the help call for config */
    return 0;

I looked through git_default_config which seems to only die with useful error messages for compression related settings, but these we may want to convert to errors instead of dies, when going this way.

Thanks, Stefan