Justin Tobler <jltobler@xxxxxxxxx> 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.
