Commit dates on conflict markers

Commit dates on conflict markers

[Roger Light]

Hello,

When I carry out a merge with conflicts, it's not always clear when
resolving the conflicts which is the correct part of code to use. I
sometimes use git blame to guide me as to the age of the different
chunks of code and hence what to choose.

I was wondering if there might be a way to help include that sort of
information directly into the conflict.

If you had a single line conflict it would be straightforward to
display by including the date the line was last modified alongside the
conflict marker:

<<<<<<< HEAD date:yesterday
print("please")
======= date:10 years ago
print("help")
>>>>>>> main

With a more realistic change with multiple lines and context from
different commits, it's not immediately obvious to me that it's
possible to do in a way that isn't completely horrible.

I am in no way a git expert though, so I thought I'd ask and see what
you thought.

Regards,

Roger

Re: Commit dates on conflict markers

[Junio C Hamano]

Roger Light <roger@atchoo.org> writes:

> When I carry out a merge with conflicts, it's not always clear when
> resolving the conflicts which is the correct part of code to use. I
> sometimes use git blame to guide me as to the age of the different
> chunks of code and hence what to choose.
>
> I was wondering if there might be a way to help include that sort of
> information directly into the conflict.
>
> If you had a single line conflict it would be straightforward to
> display by including the date the line was last modified alongside the
> conflict marker:
>
> <<<<<<< HEAD date:yesterday
> print("please")
> ======= date:10 years ago
> print("help")
> >>>>>>> main
>
> With a more realistic change with multiple lines and context from
> different commits, it's not immediately obvious to me that it's
> possible to do in a way that isn't completely horrible.

Our conflict marker lines do get human readable labels but the
format used by merge_3way() both in merge-ort and merge-recursive
backends is hardcoded to be <branchname> ':' <pathname> and it is
sufficient to let you tell which commit involved in the merge and
which path in that commit the contents came from.

A change that only shows the commit date without allowing end user
configuration will *not* be worth doing, but allowing them to use
placeholders like '%h %s' in "git log --format='%h %s'" (check
pretty.c for the catalog) would be a good exercise; it should not
take somebody with an ultra-deep knowledge of how the code works.

Re: Commit dates on conflict markers

[Jeff King]

On Mon, Sep 11, 2023 at 04:31:06PM -0700, Junio C Hamano wrote:

> A change that only shows the commit date without allowing end user
> configuration will *not* be worth doing, but allowing them to use
> placeholders like '%h %s' in "git log --format='%h %s'" (check
> pretty.c for the catalog) would be a good exercise; it should not
> take somebody with an ultra-deep knowledge of how the code works.

FWIW, I had the same thought. But I wasn't quite sure where we even set
these strings, so here are a few thoughts for anybody who wants to work
on it:

  - the relevant strings are passed in to ll_merge(); you can grep for
    callers and see that there are a number of strings which end up here
    (based on what info we actually have).

    The most interesting one to start with is probably the call in
    merge_3way() of merge-ort.c.

  - there we have three object_ids, "o", "a", and "b" representing the
    three sides. But these are just blobs. We have strings "ancestor",
    "branch1", and "branch2" in merge_options, but we would probably not
    want to re-resolve those names. So probably some extra fields need
    to go into merge_options.

  - I'm not sure if each of those endpoints is always a commit. For a
    regular merge, they would be. But in a recursive merge, we'd
    sometimes create an intermediate virtual tree. So we'd need to
    handle the case that there is no commit (and either fall back to a
    more vanilla string, or make a fake commit with reasonable details).

  - The current labels are based on the "original" ref names (which I
    think are really "what was handed to merge"; so it might be
    "master~13" etc) along with the blob path (if it is not the same for
    both endpoints). So you'd want more than just passing the format
    string to format_commit_message(). You would want an extra
    placeholder to represent those values (either ref and pathname
    individually, or possibly a single placeholder for "ref and maybe
    pathname if it's interesting").

    The least-bad way to do that is perhaps to expand the format twice:
    once for the special placeholder (quoting any "%" found in the
    filename, etc), and then feeding the whole result to the
    pretty-print formatter.

I was hoping this might be only a few-line change, but I actually think
it's a bit more complicated than that. But still maybe not _too_ bad.

-Peff

Re: Commit dates on conflict markers

[Elijah Newren]

On Mon, Sep 11, 2023 at 4:31 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Roger Light <roger@atchoo.org> writes:
>
> > When I carry out a merge with conflicts, it's not always clear when
> > resolving the conflicts which is the correct part of code to use. I
> > sometimes use git blame to guide me as to the age of the different
> > chunks of code and hence what to choose.
> >
> > I was wondering if there might be a way to help include that sort of
> > information directly into the conflict.
> >
> > If you had a single line conflict it would be straightforward to
> > display by including the date the line was last modified alongside the
> > conflict marker:
> >
> > <<<<<<< HEAD date:yesterday
> > print("please")
> > ======= date:10 years ago
> > print("help")
> > >>>>>>> main
> >
> > With a more realistic change with multiple lines and context from
> > different commits, it's not immediately obvious to me that it's
> > possible to do in a way that isn't completely horrible.
>
> Our conflict marker lines do get human readable labels but the
> format used by merge_3way() both in merge-ort and merge-recursive
> backends is hardcoded to be <branchname> ':' <pathname> and it is
> sufficient to let you tell which commit involved in the merge and
> which path in that commit the contents came from.
>
> A change that only shows the commit date without allowing end user
> configuration will *not* be worth doing, but allowing them to use
> placeholders like '%h %s' in "git log --format='%h %s'" (check
> pretty.c for the catalog) would be a good exercise; it should not
> take somebody with an ultra-deep knowledge of how the code works.

Generalizing conflict marker annotations to use other hints may make
sense, but I am not sure that "date" is a good example or reason to
generalize it, for three reasons:

  * [No date] Virtual merge bases from recursive merges do not have a
date to associate with them.  Do we just make one up?  Average the
range of dates of the commits being merged to create the virtual merge
base?
  * [Wrong date(s)] The date Roger probably wants is the date when the
conflicting text was introduced to the given side of history, not the
date of the tip of that branch.  merge-ort is pretty fundamentally a
three-way merge algorithm, meaning it only ever uses the merge-base
and the two branch tips.  I don't want to see that changed either;
other than for computing the merge base, I'm very skeptical of any
movement to make merge-ort depend upon any intermediate commit(s).
Such changes would likely be better placed in an entirely new merge
algorithm, but then you have to write an entirely new merge algorithm,
which is decidedly non-trivial.
  * [Too many dates] What if the conflict region had two lines on one
side and each line was added on different dates -- what then to
display for the date for that side of history?  The earliest?  The
latest?  Some kind of weighted average?  Feels like a bug report
generator to me.

So, if we generalize conflict marker annotations, we probably ought to
omit "date" from the new possibilities.