Re: [PATCH v5 1/4] diff: return diff_filepair from diff queue helpers

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

Justin Tobler <jltobler@gmail.com> writes:

> The `diff_addremove()` and `diff_change()` functions set up and queue
> diffs, but do not return the `diff_filepair` added to the queue. In a
> subsequent commit, modifications to `diff_filepair` need to occur in
> certain cases after being queued.
>
> Since the existing `diff_addremove()` and `diff_change()` are also used
> for callbacks in `diff_options` as types `add_remove_fn_t` and
> `change_fn_t`, modifying the existing function signatures requires
> further changes.

Sensible.  The patch presented below looks a sane and safe no-op for
the existing code paths, which is what we want to see in a preliminary
refactoring step like this one.

> The diff options for pruning use `file_add_remove()`
> and `file_change()` where file pairs do not even get queued. Thus,
> separate functions are implemented instead.

This looked a bit confusing, but it is an explanation for the reason
why we simply do not change the function signature of the callback
members of diff_options structure.  These addremove/change callbacks
are designed to be a general way to allow applications to react to
discovered changes to paths, and I agree that it makes sense for
them to be usable to perform something that has nothing to do with
the diff_queue structure.

> Split out the queuing operations into `diff_queue_addremove()` and
> `diff_queue_change()` which also return a handle to the queued
> `diff_filepair`. Both `diff_addremove()` and `diff_change()` are
> reimplemented as thin wrappers around the new functions.

Nice.