Re: [PATCH] Generate a warning message if we find an unrecognized option.

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 9, 2010, 00:59 UTC
Message-ID: <7vvde7z0kf.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20100209004514.GB4065@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 22 quoted lines

> On Mon, Feb 08, 2010 at 04:33:35PM -0600, Jeremy White wrote:
>
>> diff --git a/imap-send.c b/imap-send.c
>> index 51f371b..885da22 100644
>> --- a/imap-send.c
>> +++ b/imap-send.c
>> @@ -1360,6 +1360,7 @@ static int git_imap_config(const char *key, const char *val, void *cb)
>>  		server.ssl_verify = git_config_bool(key, val);
>>  	else if (!strcmp("preformattedHTML", key))
>>  		server.use_html = git_config_bool(key, val);
>> +        else imap_info("Unknown imap configuration option '%s'\n", key);
>>  	return 0;
>>  }
>
> Slight NAK from me on this. When we later add new options, it makes
> using the same config for multiple versions of git difficult (the old
> versions will complain about the unknown option).
>
> And obviously that is weighed against the ability to notice things like
> typos. But if we are going to start complaining about unknown config, we
> would probably do better to complain about _all_ unknown config, and not
> just this one subsection.

We would probably want something like:

	static int do_warn_unknown_config;

	void warn_unknown_config(const char *key)
        {
		if (do_warn_unknown_config)
                	warn("Unknown configuration variable %s", key);
        }

and sprinkle that everywhere.

An interesting issue is where to flip do_warn_unknown_config. A naïve and obvious implementation would do:

        static int git_default_core_config(const char *var, const char *value)
        {
		...
		if (!strcmp(var, "core.warnunknownconfig")) {
			do_warn_unknown_config = git_config_bool(var, value);
			return 0;
		}
		...
	}

but that means the definition of this variable has to come very early in the configuration file to be effective.