Re: [PATCH v5 3/4] builtin: introduce diff-pairs command

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

Justin Tobler <jltobler@gmail.com> writes:

> +static unsigned parse_mode_or_die(const char *mode, const char **end)

A minor naming issue, but the previous round called this endp, which
is probably a better name; making it explicit that it is a pointer
to receive the discovered end of converted string is in line with
how a similar parameter to strtol() and friends is named as
"endptr".

Not worth a reroll to rename this one alone, though.

> +	while (1) {
> +		struct object_id oid_a, oid_b;
> +		struct diff_filepair *pair;
> +		unsigned mode_a, mode_b;
> +		const char *p;
> +		char status;
> +
> +		if (strbuf_getwholeline(&meta, stdin, line_term) == EOF)
> +			break;

Nice.

> diff --git a/git.c b/git.c
> index 450d6aaa86..77c4359522 100644
> --- a/git.c
> +++ b/git.c
> @@ -541,6 +541,7 @@ static struct cmd_struct commands[] = {
>  	{ "diff", cmd_diff, NO_PARSEOPT },
>  	{ "diff-files", cmd_diff_files, RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT },
>  	{ "diff-index", cmd_diff_index, RUN_SETUP | NO_PARSEOPT },
> +	{ "diff-pairs", cmd_diff_pairs, RUN_SETUP | NO_PARSEOPT },

OK.  We need a repository to find objects named in our input, but we
do not need working tree.  Makes sense.