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

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 3, 2025, 16:30 UTC
Message-ID: <xmqq7c56xeja.fsf@gitster.g>
In-Reply-To: <20250228213346.1335224-4-jltobler@gmail.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.

Show 9 quoted lines

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

Show 9 quoted lines

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