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

From: Jeff King <peff@peff.net>
Date: Sep 28, 2021, 22:29 UTC
Message-ID: <YVOXPTjsp9lrxmS6@coredump.intra.peff.net>
In-Reply-To: <ed71913886e19ccc276b382de707b4bab7834d12.1630376800.git.gitgitgadget@gmail.com>

On Tue, Aug 31, 2021 at 02:26:35AM +0000, Elijah Newren via GitGitGadget wrote:

Show 15 quoted lines

> There are several considerations here:
>   * We have to pick file(s) where we write these conflict messages too
>   * We want to make it as clear as possible that it's not a real file
>     but a set of messages about another file
>   * We want conflict messages about a file to appear near the file in
>     question in a diff, preferably immediately preceding the file in
>     question
>   * Related to the above, per-file conflict messages are preferred
>     over lumping all conflict messages into one big file
> 
> To achive the above:
>   * We put the conflict messages for $filename in
>       $filename[0:-1] + " " + $filename[-1] + ".conflict_msg"
>     or, in words, we insert a space before the final character of
>     the filename and then also add ".conflict_msg" at the end.

It took me a minute to understand the space thing. I thought at first it was about avoiding conflicts with existing names (and while it might help in practice, it's not a guarantee). But I think it's about the "appear preceding the file" goal. The space sorts before any other printable character in the final position.

That's...simultaneously clever and gross. My biggest complaint is that the space looks like a bug in the output.

Using another character like "." might not be too bad, as it's also fairly early in the ascii table. But it's really this "do it before the last character" thing that is key to getting the ordering right.

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.

 - 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/foo.c
     +++ b/foo.c
     @@ some actual diff starts here @@

Obviously such a thing can't really be applied. But then you wouldn't want to apply the addition of "my.file e.conflict_msg" either.

I dunno. The latter especially is definitely more work, and requires a bit more cooperation between the merge and diff code. In particular, you can't just feed a straight tree to the diff anymore. We have to hold back the annotations, and then apply them to the resulting diff. But I think the output is much more pleasing to the eye.

-Peff