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

[Junio C Hamano <gitster@pobox.com>]

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".

>  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;
>  	}