From: Junio C Hamano <gitster@pobox.com>
To: Justin Tobler <jltobler@gmail.com>
Cc: git@vger.kernel.org, peff@peff.net
Subject: Re: [PATCH v2 2/3] builtin: introduce diff-pairs command
Date: Wed, 19 Feb 2025 15:47:25 -0800 [thread overview]
Message-ID: <xmqqwmdlebua.fsf@gitster.g> (raw)
In-Reply-To: <xmqq1pvtfrq0.fsf@gitster.g> (Junio C. Hamano's message of "Wed, 19 Feb 2025 15:19:03 -0800")
Junio C Hamano <gitster@pobox.com> writes:
>> 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()).
Sorry, but I hit <SEND> too early before finishing the most
important part. We can move the features between upstream frontends
and downstream diff-pairs. Depending on our goals, the best
division of labor would be different. If we want to make it easy
for people to write their custom frontends, for example, it might
make sense to allow them to be as stupid and simple as possible and
make all the heavy lifting the responsibility of the diff-pairs
backend, which is the shared resource these frontends share and rely
on (so that they have incentive to help us make sure diff-pairs will
stay correct and performant). If on the other hand we want to allow
people to do fancy processing in their custom frontends, maybe keeping
diff-pairs as stupid and transparent would be a better option to give
the people who write upstream/frontends more predictable behaviour.
Where to do the pathspec limiting is one of these things. You could
make it responsibility for the frontends if we assume that frontends
must do their own limiting. Or you could make it an optional feature
of the backends, so that frontends that does not do its own limiting
can ask diff-pairs to limit. Which side to burden more really depends
on whose job we are trying to make it easier.
Thanks.
next prev parent reply other threads:[~2025-02-19 23:47 UTC|newest]
Thread overview: 78+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-12-13 4:23 [PATCH 0/3] batch blob diff generation Justin Tobler
2024-12-13 4:23 ` [PATCH 1/3] builtin: introduce diff-blob command Justin Tobler
2024-12-13 4:23 ` [PATCH 2/3] builtin/diff-blob: add "--stdin" option Justin Tobler
2024-12-13 4:23 ` [PATCH 3/3] builtin/diff-blob: Add "-z" option Justin Tobler
2024-12-13 8:12 ` [PATCH 0/3] batch blob diff generation Jeff King
2024-12-13 10:17 ` Junio C Hamano
2024-12-13 10:38 ` Jeff King
2024-12-15 2:07 ` Junio C Hamano
2024-12-15 2:17 ` Junio C Hamano
2024-12-16 11:11 ` Jeff King
2024-12-16 16:29 ` Junio C Hamano
2024-12-18 11:39 ` Jeff King
2024-12-18 14:53 ` Junio C Hamano
2024-12-20 9:09 ` Jeff King
2024-12-20 9:10 ` Jeff King
2024-12-13 16:41 ` Justin Tobler
2024-12-16 11:18 ` Jeff King
2024-12-13 22:34 ` Junio C Hamano
2024-12-15 23:24 ` Junio C Hamano
2024-12-16 11:30 ` Jeff King
2025-02-12 4:18 ` [PATCH v2 " Justin Tobler
2025-02-12 4:18 ` [PATCH v2 1/3] diff: return diff_filepair from diff queue helpers Justin Tobler
2025-02-12 9:06 ` Karthik Nayak
2025-02-12 17:35 ` Justin Tobler
2025-02-12 9:23 ` Patrick Steinhardt
2025-02-12 17:24 ` Justin Tobler
2025-02-13 5:45 ` Patrick Steinhardt
2025-02-12 4:18 ` [PATCH v2 2/3] builtin: introduce diff-pairs command Justin Tobler
2025-02-12 9:23 ` Patrick Steinhardt
2025-02-12 9:51 ` Karthik Nayak
2025-02-25 23:38 ` Justin Tobler
2025-02-12 11:40 ` Jean-Noël Avila
2025-02-12 16:50 ` Junio C Hamano
2025-02-19 22:19 ` Justin Tobler
2025-02-19 23:19 ` Junio C Hamano
2025-02-19 23:47 ` Junio C Hamano [this message]
2025-02-20 0:32 ` Justin Tobler
2025-02-20 14:56 ` Justin Tobler
2025-02-20 16:14 ` Junio C Hamano
2025-02-17 14:38 ` Phillip Wood
2025-02-19 20:51 ` Justin Tobler
2025-02-19 21:57 ` Junio C Hamano
2025-02-19 22:38 ` Justin Tobler
2025-02-26 14:47 ` Phillip Wood
2025-02-12 4:18 ` [PATCH v2 3/3] builtin/diff-pairs: allow explicit diff queue flush Justin Tobler
2025-02-12 9:23 ` Patrick Steinhardt
2025-02-17 14:38 ` Phillip Wood
2025-02-19 23:09 ` Justin Tobler
2025-02-25 23:39 ` [PATCH v3 0/3] batch blob diff generation Justin Tobler
2025-02-25 23:39 ` [PATCH v3 1/3] diff: return diff_filepair from diff queue helpers Justin Tobler
2025-02-26 18:04 ` Junio C Hamano
2025-02-25 23:39 ` [PATCH v3 2/3] builtin: introduce diff-pairs command Justin Tobler
2025-02-26 18:24 ` Junio C Hamano
2025-02-27 22:15 ` Justin Tobler
2025-02-27 9:35 ` Karthik Nayak
2025-02-27 22:36 ` Justin Tobler
2025-02-27 12:56 ` Patrick Steinhardt
2025-02-27 23:00 ` Justin Tobler
2025-02-25 23:39 ` [PATCH v3 3/3] builtin/diff-pairs: allow explicit diff queue flush Justin Tobler
2025-02-26 14:58 ` [PATCH v3 0/3] batch blob diff generation phillip.wood123
2025-02-27 22:04 ` Justin Tobler
2025-02-28 0:26 ` [PATCH v4 0/4] " Justin Tobler
2025-02-28 0:26 ` [PATCH v4 1/4] diff: return diff_filepair from diff queue helpers Justin Tobler
2025-02-28 0:26 ` [PATCH v4 2/4] diff: add option to skip resolving diff statuses Justin Tobler
2025-02-28 8:29 ` Patrick Steinhardt
2025-02-28 17:10 ` Justin Tobler
2025-02-28 0:26 ` [PATCH v4 3/4] builtin: introduce diff-pairs command Justin Tobler
2025-02-28 8:29 ` Patrick Steinhardt
2025-02-28 17:26 ` Justin Tobler
2025-02-28 0:26 ` [PATCH v4 4/4] builtin/diff-pairs: allow explicit diff queue flush Justin Tobler
2025-02-28 21:33 ` [PATCH v5 0/4] batch blob diff generation Justin Tobler
2025-02-28 21:33 ` [PATCH v5 1/4] diff: return diff_filepair from diff queue helpers Justin Tobler
2025-03-03 16:17 ` Junio C Hamano
2025-02-28 21:33 ` [PATCH v5 2/4] diff: add option to skip resolving diff statuses Justin Tobler
2025-03-03 16:19 ` Junio C Hamano
2025-02-28 21:33 ` [PATCH v5 3/4] builtin: introduce diff-pairs command Justin Tobler
2025-03-03 16:30 ` Junio C Hamano
2025-02-28 21:33 ` [PATCH v5 4/4] builtin/diff-pairs: allow explicit diff queue flush Justin Tobler
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqwmdlebua.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=jltobler@gmail.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).