On Thu, Sep 30, 2021 at 12:58 AM Jeff King <peff@xxxxxxxx> wrote: > > 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 tried giving things unusual mode bits in early versions of merge-ort for other reasons. It doesn't work: canon_mode() will "fix" the unusualness so there's no way for the reader to know that they had unusual bits. But, as you said later, avoiding these pseudo-files is going to be cleaner anyway, so let's just do that. > > 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
