{"thread":{"id":"60223","subject":"Commit dates on conflict markers","startedAt":"2023-09-11T21:39:22Z","lastAt":"2023-09-12T14:33:58Z","messageCount":4,"participants":["Roger Light","Junio C Hamano","Jeff King","Elijah Newren"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"481703","messageId":"CAH7zdydYgSf+21GB70=gRhEcupv4e1ix==7LWCeQYgpD-1Rcmw@mail.gmail.com","threadId":"60223","inReplyTo":null,"subject":"Commit dates on conflict markers","fromName":"Roger Light","fromEmail":"roger@atchoo.org","sentAt":"2023-09-11T21:02:47Z","receivedAt":"2023-09-11T21:39:22Z","isPatch":false,"sender":{"key":"roger@atchoo.org","avatar":null},"body":"Hello,\n\nWhen I carry out a merge with conflicts, it's not always clear when\nresolving the conflicts which is the correct part of code to use. I\nsometimes use git blame to guide me as to the age of the different\nchunks of code and hence what to choose.\n\nI was wondering if there might be a way to help include that sort of\ninformation directly into the conflict.\n\nIf you had a single line conflict it would be straightforward to\ndisplay by including the date the line was last modified alongside the\nconflict marker:\n\n<<<<<<< HEAD date:yesterday\nprint(\"please\")\n======= date:10 years ago\nprint(\"help\")\n>>>>>>> main\n\nWith a more realistic change with multiple lines and context from\ndifferent commits, it's not immediately obvious to me that it's\npossible to do in a way that isn't completely horrible.\n\nI am in no way a git expert though, so I thought I'd ask and see what\nyou thought.\n\nRegards,\n\nRoger\n"},{"id":"481720","messageId":"xmqq8r9cs2x1.fsf@gitster.g","threadId":"60223","inReplyTo":"CAH7zdydYgSf+21GB70=gRhEcupv4e1ix==7LWCeQYgpD-1Rcmw@mail.gmail.com","subject":"Re: Commit dates on conflict markers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-09-11T23:31:06Z","receivedAt":"2023-09-12T00:28:55Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Roger Light <roger@atchoo.org> writes:\n\n> When I carry out a merge with conflicts, it's not always clear when\n> resolving the conflicts which is the correct part of code to use. I\n> sometimes use git blame to guide me as to the age of the different\n> chunks of code and hence what to choose.\n>\n> I was wondering if there might be a way to help include that sort of\n> information directly into the conflict.\n>\n> If you had a single line conflict it would be straightforward to\n> display by including the date the line was last modified alongside the\n> conflict marker:\n>\n> <<<<<<< HEAD date:yesterday\n> print(\"please\")\n> ======= date:10 years ago\n> print(\"help\")\n> >>>>>>> main\n>\n> With a more realistic change with multiple lines and context from\n> different commits, it's not immediately obvious to me that it's\n> possible to do in a way that isn't completely horrible.\n\nOur conflict marker lines do get human readable labels but the\nformat used by merge_3way() both in merge-ort and merge-recursive\nbackends is hardcoded to be <branchname> ':' <pathname> and it is\nsufficient to let you tell which commit involved in the merge and\nwhich path in that commit the contents came from.\n\nA change that only shows the commit date without allowing end user\nconfiguration will *not* be worth doing, but allowing them to use\nplaceholders like '%h %s' in \"git log --format='%h %s'\" (check\npretty.c for the catalog) would be a good exercise; it should not\ntake somebody with an ultra-deep knowledge of how the code works.\n"},{"id":"481751","messageId":"20230912085720.GE1630538@coredump.intra.peff.net","threadId":"60223","inReplyTo":"xmqq8r9cs2x1.fsf@gitster.g","subject":"Re: Commit dates on conflict markers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-09-12T08:57:20Z","receivedAt":"2023-09-12T08:57:24Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Sep 11, 2023 at 04:31:06PM -0700, Junio C Hamano wrote:\n\n> A change that only shows the commit date without allowing end user\n> configuration will *not* be worth doing, but allowing them to use\n> placeholders like '%h %s' in \"git log --format='%h %s'\" (check\n> pretty.c for the catalog) would be a good exercise; it should not\n> take somebody with an ultra-deep knowledge of how the code works.\n\nFWIW, I had the same thought. But I wasn't quite sure where we even set\nthese strings, so here are a few thoughts for anybody who wants to work\non it:\n\n  - the relevant strings are passed in to ll_merge(); you can grep for\n    callers and see that there are a number of strings which end up here\n    (based on what info we actually have).\n\n    The most interesting one to start with is probably the call in\n    merge_3way() of merge-ort.c.\n\n  - there we have three object_ids, \"o\", \"a\", and \"b\" representing the\n    three sides. But these are just blobs. We have strings \"ancestor\",\n    \"branch1\", and \"branch2\" in merge_options, but we would probably not\n    want to re-resolve those names. So probably some extra fields need\n    to go into merge_options.\n\n  - I'm not sure if each of those endpoints is always a commit. For a\n    regular merge, they would be. But in a recursive merge, we'd\n    sometimes create an intermediate virtual tree. So we'd need to\n    handle the case that there is no commit (and either fall back to a\n    more vanilla string, or make a fake commit with reasonable details).\n\n  - The current labels are based on the \"original\" ref names (which I\n    think are really \"what was handed to merge\"; so it might be\n    \"master~13\" etc) along with the blob path (if it is not the same for\n    both endpoints). So you'd want more than just passing the format\n    string to format_commit_message(). You would want an extra\n    placeholder to represent those values (either ref and pathname\n    individually, or possibly a single placeholder for \"ref and maybe\n    pathname if it's interesting\").\n\n    The least-bad way to do that is perhaps to expand the format twice:\n    once for the special placeholder (quoting any \"%\" found in the\n    filename, etc), and then feeding the whole result to the\n    pretty-print formatter.\n\nI was hoping this might be only a few-line change, but I actually think\nit's a bit more complicated than that. But still maybe not _too_ bad.\n\n-Peff\n"},{"id":"481765","messageId":"CABPp-BE-4ewkDjkR++iyY2K=-T5cBGTk5cRbCqBE3F02tFkxug@mail.gmail.com","threadId":"60223","inReplyTo":"xmqq8r9cs2x1.fsf@gitster.g","subject":"Re: Commit dates on conflict markers","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2023-09-12T14:33:39Z","receivedAt":"2023-09-12T14:33:58Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mon, Sep 11, 2023 at 4:31 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Roger Light <roger@atchoo.org> writes:\n>\n> > When I carry out a merge with conflicts, it's not always clear when\n> > resolving the conflicts which is the correct part of code to use. I\n> > sometimes use git blame to guide me as to the age of the different\n> > chunks of code and hence what to choose.\n> >\n> > I was wondering if there might be a way to help include that sort of\n> > information directly into the conflict.\n> >\n> > If you had a single line conflict it would be straightforward to\n> > display by including the date the line was last modified alongside the\n> > conflict marker:\n> >\n> > <<<<<<< HEAD date:yesterday\n> > print(\"please\")\n> > ======= date:10 years ago\n> > print(\"help\")\n> > >>>>>>> main\n> >\n> > With a more realistic change with multiple lines and context from\n> > different commits, it's not immediately obvious to me that it's\n> > possible to do in a way that isn't completely horrible.\n>\n> Our conflict marker lines do get human readable labels but the\n> format used by merge_3way() both in merge-ort and merge-recursive\n> backends is hardcoded to be <branchname> ':' <pathname> and it is\n> sufficient to let you tell which commit involved in the merge and\n> which path in that commit the contents came from.\n>\n> A change that only shows the commit date without allowing end user\n> configuration will *not* be worth doing, but allowing them to use\n> placeholders like '%h %s' in \"git log --format='%h %s'\" (check\n> pretty.c for the catalog) would be a good exercise; it should not\n> take somebody with an ultra-deep knowledge of how the code works.\n\nGeneralizing conflict marker annotations to use other hints may make\nsense, but I am not sure that \"date\" is a good example or reason to\ngeneralize it, for three reasons:\n\n  * [No date] Virtual merge bases from recursive merges do not have a\ndate to associate with them.  Do we just make one up?  Average the\nrange of dates of the commits being merged to create the virtual merge\nbase?\n  * [Wrong date(s)] The date Roger probably wants is the date when the\nconflicting text was introduced to the given side of history, not the\ndate of the tip of that branch.  merge-ort is pretty fundamentally a\nthree-way merge algorithm, meaning it only ever uses the merge-base\nand the two branch tips.  I don't want to see that changed either;\nother than for computing the merge base, I'm very skeptical of any\nmovement to make merge-ort depend upon any intermediate commit(s).\nSuch changes would likely be better placed in an entirely new merge\nalgorithm, but then you have to write an entirely new merge algorithm,\nwhich is decidedly non-trivial.\n  * [Too many dates] What if the conflict region had two lines on one\nside and each line was added on different dates -- what then to\ndisplay for the date for that side of history?  The earliest?  The\nlatest?  Some kind of weighted average?  Feels like a bug report\ngenerator to me.\n\nSo, if we generalize conflict marker annotations, we probably ought to\nomit \"date\" from the new possibilities.\n"}]}