Re: [PATCH 2/7] merge-ort: add ability to record conflict messages in a file

2 messages, 2 authors, 2021-09-29 · open the first message on its own page

Re: [PATCH 2/7] merge-ort: add ability to record conflict messages in a file

From: Junio C Hamano <hidden>
Date: 2021-09-29 16:14:53

Elijah Newren [off-list ref] writes:
I also don't like how diff.orderFile provides a global ordering of the
files listed, rather than providing some scheme for relative
orderings.  That'd either force me to precompute the diff to determine
all the files that were different so I can list _all_ of them,...
Don't we determine all the files that would be listed in the diff
anyway?  The diffcore pipeline collects all the different filepairs,
matches them up for rename/copy detection, and finally do the output
formatting for each individual filepair.
So, I'm not so sure about this solution; it feels like it introduces
bigger holes than the ugly space character it is fixing.
quoted
 - there can be other non-diff data between the individual segments. For
   example, "patch" will skip over non-diff lines. And certainly in Git
   we have our own custom headers. I'm wondering if we could attach
   these annotations to the diff-pair somehow, and then show something
   like:

     diff --git a/foo.c b/foo.c
     index 1234abcd..5678cdef 100644
     conflict modify/delete foo.c
A couple things here...

First, I'm not so sure I like the abbreviation here.  Just knowing
"modify/delete" might be enough in some cases, but I'd rather have the
full messages that would have been printed to the console...

Second, what about when there are multiple ...

Third, what about the cases where there is no diff, ...
None of the above seems like a problem to me at least from the
presentation and consumption sides.  There is no rule that extended
diff headers has to fit on a 72-char line, cannot use line folding
by inserting LF-SP in the middle of a logical line, and there
already is at least one case we give an extended diff header without
a single line of content change (namely, "chmod +x README").

Thanks.

Re: [PATCH 2/7] merge-ort: add ability to record conflict messages in a file

From: Elijah Newren <hidden>
Date: 2021-09-29 16:32:03

On Wed, Sep 29, 2021 at 9:14 AM Junio C Hamano [off-list ref] wrote:
Elijah Newren [off-list ref] writes:
quoted
I also don't like how diff.orderFile provides a global ordering of the
files listed, rather than providing some scheme for relative
orderings.  That'd either force me to precompute the diff to determine
all the files that were different so I can list _all_ of them,...
Don't we determine all the files that would be listed in the diff
anyway?  The diffcore pipeline collects all the different filepairs,
matches them up for rename/copy detection, and finally do the output
formatting for each individual filepair.
merge-ort is missing one side of the diff.  It only has the parents of
the merge commit, and their merge base.  So, merge-ort would now need
to know about an additional commit (the original merge), and compute
the diff between that and the merge-result it is creating, in advance
of an external caller (log) calling diff to compute the result between
those two trees.
quoted
So, I'm not so sure about this solution; it feels like it introduces
bigger holes than the ugly space character it is fixing.
quoted
 - there can be other non-diff data between the individual segments. For
   example, "patch" will skip over non-diff lines. And certainly in Git
   we have our own custom headers. I'm wondering if we could attach
   these annotations to the diff-pair somehow, and then show something
   like:

     diff --git a/foo.c b/foo.c
     index 1234abcd..5678cdef 100644
     conflict modify/delete foo.c
A couple things here...

First, I'm not so sure I like the abbreviation here.  Just knowing
"modify/delete" might be enough in some cases, but I'd rather have the
full messages that would have been printed to the console...

Second, what about when there are multiple ...

Third, what about the cases where there is no diff, ...
None of the above seems like a problem to me at least from the
presentation and consumption sides.  There is no rule that extended
diff headers has to fit on a 72-char line, cannot use line folding
by inserting LF-SP in the middle of a logical line, and there
already is at least one case we give an extended diff header without
a single line of content change (namely, "chmod +x README").
Cool, that's good to hear.  I'll look into it.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help