From: Junio C Hamano Date: Tue, 09 Feb 2010 00:59:12 GMT Subject: Re: [PATCH] Generate a warning message if we find an unrecognized option. Message-ID: <7vvde7z0kf.fsf@alter.siamese.dyndns.org> In-Reply-To: <20100209004514.GB4065@coredump.intra.peff.net> Jeff King writes: > 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.