Re: [PATCH] Add `log.decorate' configuration variable.

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 17, 2010, 03:16 UTC
Message-ID: <7v635wimac.fsf@alter.siamese.dyndns.org>
In-Reply-To: <alpine.LNX.2.00.1002171427080.3414@vqena.qenxr.bet.am>

Steven Drake <sdrake@xnet.co.nz> writes:

> Good point, and looking at the code "log.decorate" only has an affect after
> cmd_log_init() is called, which is call by cmd_whatchanged(), cmd_show(), 
> cmd_log_reflog() and cmd_log() so only those command are affected
> (notably not format-patch).

I was not worried about what your change does. I am worried about protecting what the code after your change currently does from future changes done by other people while you are not actively watching the patches in flight on this list.

Show 14 quoted lines

> While on the subject of git_config I think die_bad_config() should be an
> extern (i.e. decleared in cache.h and a static function) so that it could
> be used in git_XXX_config functions for handling error.  Something like:
>
> diff --git a/builtin-log.c b/builtin-log.c
> index f096eea..a41a7bb 100644
> --- a/builtin-log.c
> +++ b/builtin-log.c
> @@ -264,6 +264,8 @@ static int git_log_config(const char *var, const char *value, void *cb)
>  			decoration_style = DECORATE_FULL_REFS;
>  		else if (!strcmp(value, "short"))
>  			decoration_style = DECORATE_SHORT_REFS;
> +		else
> +			die_bad_config(var);

We generally avoid doing this, as we may later want to add different values to "log.decorate", and keep the older git working as if nothing is specified, rather than barfing, so that people can access the same repository, perhaps over NFS, from different machines with varying vintage of git.