Elijah Newren <newren@xxxxxxxxx> writes: >> I forgot to add that I am not happy with this "centralized tear >> down" step, even though I am reasonably happy with the "lazy set-up" >> step. I wonder why the remerge-diff related members have to exist >> in the rev_info structure in the first place, instead of being in >> the diffopt structure? Moving them to diffopt may make the end >> result much more pleasant to read. > > I'm not sure they need to exist in the rev_info structure. I was > probably thinking that e.g. log --remerge-diff would "need to do lots > of diffs" but I only needed the temporary store setup once, and once > in my mind matched better with rev_info. I wasn't aware of > diff_options.no_free or anything surrounding it. If we can do the > temporary store setup in diffopt and only do it once for all N diffs > in a `git log --remerge-diff` run, then we could move this stuff from > rev_info to diffopt. After staring at the data structure around this area for a while, I think these two patches are OK, especially within the current structure. rev_info structure has a set of bits that are marked as "Diff flags" (some of which are not about diff at all, though; for example, verbose_header and no_commit_id belong more to the "Format info" group), and those related to merges do belong there, as they need more than "here are two tree-like things I want to compare". So, I'll mark these two patches for 'next' now. Thanks.