Re: [PATCH 2/3] diff: fix filtering of additional headers under --remerge-diff
From: Elijah Newren <hidden>
Date: 2022-09-01 03:38:19
On Wed, Aug 31, 2022 at 3:26 PM Junio C Hamano [off-list ref] wrote:
"Elijah Newren via GitGitGadget" [off-list ref] writes:quoted
From: Elijah Newren <redacted> Commit 95433eeed9 ("diff: add ability to insert additional headers for paths", 2022-02-02) introduced the possibility of additional headers. Because there could be conflicts with no content differences (e.g. a modify/delete conflict resolved in favor of taking the modified file as-is), that commit also modified the diff_queue_is_empty() and diff_flush_patch() logic to ensure these headers were included even if there was no associated content diff.In the longer term, I think we may have to redo the way additional headers are inserted to the diff_queue. All the diff code I am familiar with (read: written before this hack was introduced) trusts that diff_queue.nr is the number of paths that are returned by the diff frontend, and unless there is diffcore_break involved, there will be at most one diff_filepair that is about a path. Why do these need to be separate entries in the queue, not a new attribute added to an existing filepair? Are we inserting pieces of information that are not about any paths that are involved in the diff?
They usually are added to an existing filepair, but as your last question alludes to, there isn't always one that exists. One example where this can happen is with a modify/delete conflict. If someone got one of those and decided to keep the modified file as-is when resolving the conflict, then there'd be no content diff to show for that file when investigating the resulting merge commit with --remerge-diff. But since the point of --remerge-diff is to show whatever work the user did to resolve conflicts, and that file was conflicted, we want to show _something_ for that path. Thus the reason for creating a new "phoney" filepair just for attaching the "remerge CONFLICT (modify/delete) ..." notice.