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

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

Justin Tobler <jltobler@gmail.com> writes:

> Thinking about this some more, I'm a bit unsure whether
> git-diff-pairs(1) should support "normal" `--raw` input. Furthermore, if
> we do want to support it, maybe it should be the default?
>
> From my perspective, ultimately I don't think there is much additional
> value provided by supporting multiple input options for
> git-diff-pairs(1) since the end result would be the same and its just an
> intermediate format. As I see it, the benefit of the NUL delimited raw
> diff ouput format is that it is a bit simpler to parse and likely a bit
> more efficient as it wouldn't have to deal with unquoting paths with
> special characters. The benefit of the "normal" raw format is probably
> that it is the more intuitive default option.
>
> I'm certainly interested in what folks think about this :)

FWIW, in our toolset, "-z" is not the default primarily because text
format were chosen to help debuggability, which used to really matter
in the early days.

> For abbreviated object IDs, supporting them would make the input format
> more flexible, but it would be simpler to just require the full OID be
> provided thus making the input format more explicit. My current thinking
> is to leave this unless others think it would be useful to support.

Abbreviated object names would only be at "might be nice to have"
level, I would think.  We are talking about tools-to-tools
communication after all.

> Regarding pathspec support, being that git-diff-pairs(1) operates solely
> on the provided set of file pairs produced via some other Git operation,
> I don't think further limiting would provide much additional value
> either. If we do want this though, I think support could be added in the
> future.

Another consideration is which side of the pipeline should take the
responsibility to invoke the diffcore machinery.  We certainly could
make it the job for the upstream/frontend, in which case diff-pairs
does not have to call into diffcore-rename, BUT it also means the
downstream/backend needs to be able to parse two paths (renamed from
and renamed to).  Or we could make it the job for the downstream,
and forbid the upstream/frontend from feeding renamed pairs (i.e.
any input with status letter R or C are invalid), in which case
diff-pairs can choose to invoke rename detection or not by paying
attention to the -M option and invoking diffcore_rename() itself
(which should be at no-cost from coding point of view, as it should
be just the matter of calling diffcore_std()).

> The tree objects in the input are not expanded. With `git diff-pairs
> --raw` these objects are just printed again. With the `--patch` option,
> they are just ommitted.

Instead of getting expanded into its subpaths?

>> 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).
>
> Will do

No longer needed, as the tip of 'master' now lives in the .adoc
world.  Hurrah!