From: Jonathan Nieder <jrnieder@gmail.com>
Date: Fri, 09 Dec 2011 20:47:15 GMT
Subject: Re: [PATCH 8/9] revert: report fine-grained error messages from insn parser
Message-ID: <20111209204715.GM20913@elie.hsd1.il.comcast.net>
In-Reply-To: <1323445326-24637-9-git-send-email-artagnon@gmail.com>

Ramkumar Ramachandra wrote:

> --- a/builtin/revert.c
> +++ b/builtin/revert.c
> @@ -719,8 +719,10 @@ static int format_todo(struct strbuf *buf, struct replay_insn_list *todo_list)
>  	return 0;
>  }
>  
> -static int parse_insn_line(char *bol, char *eol, struct replay_insn_list *item)
> +static int parse_insn_line(char *bol, char *eol,
> +			struct replay_insn_list *item, int lineno)
>  {
> +	const char *todo_file = git_path(SEQ_TODO_FILE);
>  	unsigned char commit_sha1[20];

I know that this function does not call git_path() again before the
value is used, so this is safe today, but I do not trust people in the
future to preserve that property (for example, maybe someone will want
to call get_sha1() earlier).  Why not wait to call git_path() when it
is time to use the value it returns?