Re: [PATCH v2 2/3] builtin: introduce diff-pairs command

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 12, 2025, 16:50 UTC
Message-ID: <xmqqldubumxf.fsf@gitster.g>
In-Reply-To: <20250212041825.2455031-3-jltobler@gmail.com>

Justin Tobler <jltobler@gmail.com> writes:

Show 17 quoted lines

> +NOTES
> +----
> +
> +`diff-pairs` should handle any input generated by `diff-tree --raw -z`.
> +It may choke or otherwise misbehave on output from `diff-files`, etc.
> +
> +Here's an incomplete list of things that `diff-pairs` could do, but
> +doesn't (mostly in the name of simplicity):
> +
> + - Only `-z` input is accepted, not normal `--raw` input.
> +
> + - Abbreviated sha1s are rejected in the input from `diff-tree`; if you
> +   want to abbreviate the output, you can pass `--abbrev` to
> +   `diff-pairs`.
> +
> + - Pathspecs are not handled by `diff-pairs`; you can limit the diff via
> +   the initial `diff-tree` invocation.

Which of the above limitations are fundamental, and which are merely due to incomplete implementation that could be improved in the future iterations? Without reading the code deeply, a lot of them look like merely due to this iteration being at a WIP state and not quite ready for the general public.

What is especially curious is the reason why it is limited to diff-tree (by the way, don't you require '-r' if you are fed 'diff-tree' output, or are you prepared to expand tree objects in the input yourself?).

I can guess that the 0{40} object names in the postimage to signal paths with working tree changes unadded to the index is something this fundamentally cannot work with, but you should be able to grok 'diff-index --cached', which does not have that issue, just fine.

Show 12 quoted lines

> diff --git a/Documentation/meson.build b/Documentation/meson.build
> index ead8e48213..e5ee177022 100644
> --- a/Documentation/meson.build
> +++ b/Documentation/meson.build
> @@ -41,6 +41,7 @@ manpages = {
>    'git-diagnose.adoc' : 1,
>    'git-diff-files.adoc' : 1,
>    'git-diff-index.adoc' : 1,
> +  'git-diff-pairs.adoc' : 1,
>    'git-difftool.adoc' : 1,
>    'git-diff-tree.adoc' : 1,
>    'git-diff.adoc' : 1,

This apparently does not apply to 'master' and the base at least needs to contain 1f010d6b (doc: use .adoc extension for AsciiDoc files, 2025-01-20). Please clearly mark the series as such in the cover letter if the series is not built on top of recent 'master' (or 'maint' if it is a series to fix breakage, but it does not apply to this series).

Thanks.