[PATCH 0/14] combine-diff cleanups
From: Jeff King <hidden>
Date: 2025-01-09 08:27:26
Since Wink successfully nerd-sniped me into digging into the
combine-diff code, and since I had such a hard time figuring out some of
its logic, I spent a little time trying to put that puzzling to good use
to make it more readable.
Aside from a minor leak fix in the first patch, I didn't find any bugs.
So arguably this whole thing could be discarded as churn. But I hope at
least some of it is worthwhile, and I tried to order it to keep the less
controversial bits near the top.
The series can be split into a few sections:
[01/14]: run_diff_files(): delay allocation of combine_diff_path
[02/14]: combine-diff: add combine_diff_path_new()
[03/14]: tree-diff: clear parent array in path_appendnew()
[04/14]: combine-diff: use pointer for parent paths
[05/14]: diff: add a comment about combine_diff_path.parent.path
[06/14]: run_diff_files(): de-mystify the size of combine_diff_path struct
These first six clean up most of the allocation and initialization
confusion that started this thread. They can't go all the way
because of the scariness in path_appendnew().
[07/14]: tree-diff: drop path_appendnew() alloc optimization
[08/14]: tree-diff: pass whole path string to path_appendnew()
[09/14]: tree-diff: inline path_appendnew()
[10/14]: combine-diff: drop public declaration of combine_diff_path_size()
And these ones take it further, but at the cost of losing an
optimization in patch 07. I don't think it was doing much (and I
gave some timings there). But it's a judgement call on whether the
cleaner code is worthwhile.
[11/14]: tree-diff: drop list-tail argument to diff_tree_paths()
[12/14]: tree-diff: use the name "tail" to refer to list tail
[13/14]: tree-diff: simplify emit_path() list management
[14/14]: tree-diff: make list tail-passing more explicit
And these last four fix some confusion I had while reading the
functions. I think they _could_ be done independent of 7-14,
but there'd be some kinks to work out in emit_path().
The final one is probably a matter of taste, and I'm not sure if
people find it easier to understand than the original or not. If
not, it can easily be dropped.
combine-diff.c | 80 +++++++++++++-------------
diff-lib.c | 36 ++++--------
diff.h | 18 ++++--
tree-diff.c | 152 ++++++++++++-------------------------------------
4 files changed, 102 insertions(+), 184 deletions(-)
-Peff