Re: [PATCH] commit: clarify "could not read 'template'" message

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 19, 2011, 06:04 UTC
Message-ID: <7v4o1e544m.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20110819050714.GA21697@elie.gateway.2wire.net>

Jonathan Nieder <jrnieder@gmail.com> writes:

> At least the problem could have been easier to debug.  How about this
> patch, to start?

It is a good start, but at this point do we know where the template_file came from? It does not seem rocket surgery to update git_commit_config() so that it stores the value taken from configuration file in a separate variable so that we can tell the difference in order to tweak the message.

Are there only these three kinds of "pathname" variables that can be configured?

 - Those to be always used (it is an error if missing);
 - Those to be always used (the calling code creat(2)s if missing); and
 - Those to be used only if already exist (otherwise ignored).

If so, a good way forward might be to update git_config_pathname() and tell it which kind of path we are talking about. For the first and the third kind, it would (after ~ expansion and other magic) stat the string and error out for the first kind and return NULL for the third kind. For the second kind, we do not need to do anything special. We may need to distinguish between files and dirs, so we might need 6 variations, not just 3, but that is a minor implementation detail.

This is one of the reasons that I tend to resist applying patches that add useless configuration variables "just because it seems useful".

Show 17 quoted lines

>  builtin/commit.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index cb738574..d029ec03 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -703,7 +703,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
>  		hook_arg1 = "squash";
>  	} else if (template_file) {
>  		if (strbuf_read_file(&sb, template_file, 0) < 0)
> -			die_errno(_("could not read '%s'"), template_file);
> +			die_errno(_("could not read commit message template '%s'"),
> +				  template_file);
>  		hook_arg1 = "template";
>  		clean_message_contents = 0;
>  	}