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

[Jeff King <peff@peff.net>]

On Tue, Sep 28, 2021 at 11:25:20PM -0700, Elijah Newren wrote:

> > Just brainstorming some alternatives:
> >
> >  - we have diff.orderFile, etc. Could we stuff this data into a less
> >    confusing name (even just "$filename.conflict_msg"), and then provide
> >    a custom ordering to the diff code? I think it could be done by
> >    generating a static ordering ahead of time, but it might even just be
> >    possible to tell diffcore_order() to take the ".conflict_msg"
> >    extension into account in its comparison function.
> 
> I can't just go on the ".conflict_msg" extension.  As you noted above,
> this scheme is not sufficient for avoiding collisions.  So I need to
> append extra "cruft" to the name in the case of collisions -- meaning
> we can't special case on just that extension.

Sure, but we can call it filename.conflict_msg.1, etc, and the sort code
can pattern-match. It can never be fully robust (if you really did have
a foo.conflict_msg, we'd sort it differently), but I think it's OK if
the worst-case is that pathological trees get ordered slightly
sub-optimally).

That said, I think it could also make sense to annotate the conflict
files by giving them an unusual set of mode bits. That would be easier
to recognize (and while real trees _could_ have silly modes, we do
complain about them in fsck, so it shouldn't happen in practice).

> 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, or put
> up with the fact that the files with non-content conflicts will be
> listed very first in the output, even if their name is
> 'zee-last-file.c' -- surprising users at the output ordering.
> 
> This also means that if the user had their own ordering defined, then
> I'm overriding it and messing up their ordering, which might be
> problematic.

Agreed. I do think it may be too horrible unless you teach
diffcore_order() to actually understand your annotations in code.  But
that wouldn't be too hard if it's done in the mode bits.

But I do think anything that avoids these pseudo-files is going to be a
lot cleaner overall.

> >  - 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...

I think Junio already indicated that we can pretty much make this look
like whatever we want. As soon as any reader sees "conflict", it should
happily ignore the rest as something it doesn't know about. And my short
example here was just meant to be illustrative. I agree it probably
needs more details (and the whole CONFLICT line that usually goes to
stderr is probably the best thing).

-Peff