Subject: Re: [PATCH v2 1/3] diff: return diff_filepair from diff queue helpers

From: Patrick Steinhardt <[email-masked]>
Date: Wed, 12 Feb 2025 10:23:36 +0100

On Tue, Feb 11, 2025 at 10:18:23PM -0600, Justin Tobler wrote:
> The `diff_addremove()` and `diff_change()` functions setup and queue
> diffs, but do not return the `diff_filepair` added to the queue. In a
> subsequent commit, modifications to `diff_filepair` need to take place
> in certain cases after being queued.
> 
> Split out the queuing operations into `diff_filepair_addremove()` and
> `diff_filepair_change()` which also return a handle to the queued
> `diff_filepair`.

One of the things that puzzled me a bit is that we keep the old-style
functions, where the only difference is the return value. Wouldn't it
make more sense to instead adapt these existing functions to reduce the
amount of duplication?

At the same time, while we're already at it, do we maybe also want to
adapt the functions so that they get the `diff_queue` as input instead
of relying on the global queue? That would make them more generally
useful and be a step into the right direction regarding libification. If
so, it would indeed make sense to also rename the function into e.g.
`diff_queue_addremove()`.

Patrick