Thread (2 messages) 2 messages, 2 authors, 2022-09-01

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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help