{"thread":{"id":"54871","subject":"[PATCH v5 0/1] mergetool: remove unconflicted lines","startedAt":"2020-12-23T04:54:57Z","lastAt":"2021-03-13T23:38:42Z","messageCount":80,"participants":["Felipe Contreras","Junio C Hamano","Seth House","Johannes Sixt","Johannes Schindelin","Jonathan Nieder"],"isPatch":true,"patchVersion":5,"patchTotal":1},"messages":[{"id":"412907","messageId":"20201223045358.100754-1-felipe.contreras@gmail.com","threadId":"54871","inReplyTo":null,"subject":"[PATCH v5 0/1] mergetool: remove unconflicted lines","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-23T04:53:57Z","receivedAt":"2020-12-23T04:54:57Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"There's not much to say other that what the commit message of the patch says.\n\nNote: no feedback has been ignored; I replied to all the feedback, I didn't hear anything back.\n\nChanges since v4:\n\n * Improved commit message with suggestions from Phillip Wood.\n\nFelipe Contreras (1):\n mergetool: add automerge configuration\n\n Documentation/config/mergetool.txt | 3 +++\n git-mergetool.sh | 17 +++++++++++++++++\n t/t7610-mergetool.sh | 18 ++++++++++++++++++\n 3 files changed, 38 insertions(+)\n\nRange-diff:\n1: 776c1fbb97 ! 1: 2dc53f4dda mergetool: add automerge configuration\n @@ Metadata\n ## Commit message ##\n mergetool: add automerge configuration\n \n - It doesn't make sense to display lines without conflicts in the\n - different views of all mergetools.\n + The purpose of mergetools is to resolve conflicts when git cannot\n + automatically do so.\n \n - Only the lines that warrant conflict markers should be displayed.\n + In order to do that git has added markers in the specific areas that\n + need resolving, which the user must manually fix. The tool is supposed\n + to help with that.\n \n - Most people would want this behavior on, but in case some don't; add a\n - new configuration: mergetool.autoMerge.\n + However, by passing the original BASE, LOCAL, and REMOTE files, many\n + changes without conflict are presented to the user when in fact nothing\n + needs to be done for those.\n +\n + We can fix that by propagating the final version of the file with the\n + automatic merge to all the panes of the mergetool (BASE, LOCAL, and\n + REMOTE), and only make them differ on the places where there are actual\n + conflicts.\n +\n + As most people will want the new behavior, we enable it by default.\n + Users that do not want the new behavior can set the new configuration\n + mergetool.autoMerge to false.\n \n See Seth House's blog post [1] for the idea, and the rationale.\n \n-- \n2.30.0.rc1\n\n"},{"id":"412908","messageId":"20201223045358.100754-2-felipe.contreras@gmail.com","threadId":"54871","inReplyTo":"20201223045358.100754-1-felipe.contreras@gmail.com","subject":"[PATCH v5 1/1] mergetool: add automerge configuration","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-23T04:53:58Z","receivedAt":"2020-12-23T04:54:58Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"The purpose of mergetools is to resolve conflicts when git cannot\nautomatically do so.\n\nIn order to do that git has added markers in the specific areas that\nneed resolving, which the user must manually fix. The tool is supposed\nto help with that.\n\nHowever, by passing the original BASE, LOCAL, and REMOTE files, many\nchanges without conflict are presented to the user when in fact nothing\nneeds to be done for those.\n\nWe can fix that by propagating the final version of the file with the\nautomatic merge to all the panes of the mergetool (BASE, LOCAL, and\nREMOTE), and only make them differ on the places where there are actual\nconflicts.\n\nAs most people will want the new behavior, we enable it by default.\nUsers that do not want the new behavior can set the new configuration\nmergetool.autoMerge to false.\n\nSee Seth House's blog post [1] for the idea, and the rationale.\n\n[1] https://www.eseth.org/2020/mergetools.html\n\nOriginal-idea-by: Seth House \nSigned-off-by: Felipe Contreras \n---\n Documentation/config/mergetool.txt | 3 +++\n git-mergetool.sh | 17 +++++++++++++++++\n t/t7610-mergetool.sh | 18 ++++++++++++++++++\n 3 files changed, 38 insertions(+)\n\ndiff --git a/Documentation/config/mergetool.txt b/Documentation/config/mergetool.txt\nindex 16a27443a3..7ce6d0d3ac 100644\n--- a/Documentation/config/mergetool.txt\n+++ b/Documentation/config/mergetool.txt\n@@ -61,3 +61,6 @@ mergetool.writeToTemp::\n \n mergetool.prompt::\n \tPrompt before each invocation of the merge resolution program.\n+\n+mergetool.autoMerge::\n+\tRemove lines without conflicts from all the files. Defaults to `true`.\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex e3f6d543fb..f4db0cac8d 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -239,6 +239,17 @@ checkout_staged_file () {\n \tfi\n }\n \n+auto_merge () {\n+\tgit merge-file --diff3 --marker-size=7 -q -p \"$LOCAL\" \"$BASE\" \"$REMOTE\" >\"$DIFF3\"\n+\tif test -s \"$DIFF3\"\n+\tthen\n+\t\tsed -e '/^<<<<<<< /,/^||||||| /d' -e '/^=======\\r\\?$/,/^>>>>>>> /d' \"$DIFF3\" >\"$BASE\"\n+\t\tsed -e '/^||||||| /,/^>>>>>>> /d' -e '/^<<<<<<< /d' \"$DIFF3\" >\"$LOCAL\"\n+\t\tsed -e '/^<<<<<<< /,/^=======\\r\\?$/d' -e '/^>>>>>>> /d' \"$DIFF3\" >\"$REMOTE\"\n+\tfi\n+\trm -- \"$DIFF3\"\n+}\n+\n merge_file () {\n \tMERGED=\"$1\"\n \n@@ -274,6 +285,7 @@ merge_file () {\n \t\tBASE=${BASE##*/}\n \tfi\n \n+\tDIFF3=\"$MERGETOOL_TMPDIR/${BASE}_DIFF3_$$$ext\"\n \tBACKUP=\"$MERGETOOL_TMPDIR/${BASE}_BACKUP_$$$ext\"\n \tLOCAL=\"$MERGETOOL_TMPDIR/${BASE}_LOCAL_$$$ext\"\n \tREMOTE=\"$MERGETOOL_TMPDIR/${BASE}_REMOTE_$$$ext\"\n@@ -322,6 +334,11 @@ merge_file () {\n \tcheckout_staged_file 2 \"$MERGED\" \"$LOCAL\"\n \tcheckout_staged_file 3 \"$MERGED\" \"$REMOTE\"\n \n+\tif test \"$(git config --bool mergetool.autoMerge)\" != \"false\"\n+\tthen\n+\t\tauto_merge\n+\tfi\n+\n \tif test -z \"$local_mode\" || test -z \"$remote_mode\"\n \tthen\n \t\techo \"Deleted merge conflict for '$MERGED':\"\ndiff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh\nindex 70afdd06fa..ccabd04823 100755\n--- a/t/t7610-mergetool.sh\n+++ b/t/t7610-mergetool.sh\n@@ -828,4 +828,22 @@ test_expect_success 'mergetool -Oorder-file is honored' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'mergetool automerge' '\n+\ttest_config mergetool.automerge true &&\n+\ttest_when_finished \"git reset --hard\" &&\n+\tgit checkout -b test${test_count}_b master &&\n+\ttest_write_lines >file1 base \"\" a &&\n+\tgit commit -a -m \"base\" &&\n+\ttest_write_lines >file1 base \"\" c &&\n+\tgit commit -a -m \"remote update\" &&\n+\tgit checkout -b test${test_count}_a HEAD~ &&\n+\ttest_write_lines >file1 local \"\" b &&\n+\tgit commit -a -m \"local update\" &&\n+\ttest_must_fail git merge test${test_count}_b &&\n+\tyes \"\" | git mergetool file1 &&\n+\ttest_write_lines >expect local \"\" c &&\n+\ttest_cmp expect file1 &&\n+\tgit commit -m \"test resolved with mergetool\"\n+'\n+\n test_done\n-- \n2.30.0.rc1\n\n"},{"id":"412920","messageId":"xmqqblekabof.fsf@gitster.c.googlers.com","threadId":"54871","inReplyTo":"20201223045358.100754-2-felipe.contreras@gmail.com","subject":"Re: [PATCH v5 1/1] mergetool: add automerge configuration","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-12-23T13:34:24Z","receivedAt":"2020-12-23T13:35:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> +auto_merge () {\n> +\tgit merge-file --diff3 --marker-size=7 -q -p \"$LOCAL\" \"$BASE\" \"$REMOTE\" >\"$DIFF3\"\n> +\tif test -s \"$DIFF3\"\n> +\tthen\n\nWe do not want to ignore the exit status from the command. IOW, I\nthink the above wants to be rather\n\n\tif git merge-file ... >\"$DIFF3\" &&\n\t test -s \"$DIFF3\"\n\tthen\n\t\t...\n\nto catch a merge-file that writes halfway and then crashes (doing\nthe same check in different ways are probably possible, of course)\n\n> +\t\tsed -e '/^<<<<<<< /,/^||||||| /d' -e '/^=======\\r\\?$/,/^>>>>>>> /d' \"$DIFF3\" >\"$BASE\"\n\nDoes everybody's sed take \"\\?\" and interprets it as zero-or-one?\nPOSIX uses BRE and it doesn't like \\? as far as I recall, and \"-E\"\nto force ERE is a GNUism.\n\n> +\t\tsed -e '/^||||||| /,/^>>>>>>> /d' -e '/^<<<<<<< /d' \"$DIFF3\" >\"$LOCAL\"\n> +\t\tsed -e '/^<<<<<<< /,/^=======\\r\\?$/d' -e '/^>>>>>>> /d' \"$DIFF3\" >\"$REMOTE\"\n\nI'd feel safer if these resulting $BASE, $LOCAL and $REMOTE are\nvalidated to be conflict-marker free (i.e. '^\\([<|=>]\\)\\1\\1\\1\\1\\1\\1'\ndoes not appear) to make sure there was no funny virtual ancestor\nthat records a conflicted recursive merge result confused our logic.\n\nWhen we see an unfortunate sign that it happened, we can revert the\nautomerge and let the tool handle the original input.\n\n> +\tfi\n> +\trm -- \"$DIFF3\"\n> +}\n> +\n\n\"$DIFF3\" is always created (unless shell redirection into it fails),\nso \"rm --\" would be fine in practice, I guess, but \"rm -f --\" would\nnot hurt.\n\n> +\tDIFF3=\"$MERGETOOL_TMPDIR/${BASE}_DIFF3_$$$ext\"\n\n$MERGETOOL_TMPDIR is either \"mktemp -d -t \"git-mergetool-XXXXXX\" or\n\".\". Also, we liberally pass \"$DIFF3\" to \"sed\" as an argument and\nassume that the command would take it as a filename and not an\noption.\n\nFor the above reason, \"rm --\", while it is not wrong per-se, can be\njust a simple \"rm\", as there is no funny leading letters in \"$DIFF3\"\nthat requires disambiguation.\n\n"},{"id":"412926","messageId":"5fe352e3968f6_198be2083@natae.notmuch","threadId":"54871","inReplyTo":"xmqqblekabof.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v5 1/1] mergetool: add automerge configuration","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-23T14:23:31Z","receivedAt":"2020-12-23T14:24:15Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Junio C Hamano wrote:\n> Felipe Contreras writes:\n> \n> > +auto_merge () {\n> > +\tgit merge-file --diff3 --marker-size=7 -q -p \"$LOCAL\" \"$BASE\" \"$REMOTE\" >\"$DIFF3\"\n> > +\tif test -s \"$DIFF3\"\n> > +\tthen\n> \n> We do not want to ignore the exit status from the command. IOW, I\n> think the above wants to be rather\n> \n> \tif git merge-file ... >\"$DIFF3\" &&\n> \t test -s \"$DIFF3\"\n> \tthen\n> \t\t...\n\nThat doesn't work.\n\n\"git merge-file\" always returns non-zero status when it succeeds (it's\nthe number of conflicts generated).\n\n> > +\t\tsed -e '/^<<<<<<< /,/^||||||| /d' -e '/^=======\\r\\?$/,/^>>>>>>> /d' \"$DIFF3\" >\"$BASE\"\n> \n> Does everybody's sed take \"\\?\" and interprets it as zero-or-one?\n\nI don't know.\n\n> POSIX uses BRE and it doesn't like \\? as far as I recall, and \"-E\"\n> to force ERE is a GNUism.\n\nAnother possibility is \\s\\*. It's less specific though.\n\n> > +\t\tsed -e '/^||||||| /,/^>>>>>>> /d' -e '/^<<<<<<< /d' \"$DIFF3\" >\"$LOCAL\"\n> > +\t\tsed -e '/^<<<<<<< /,/^=======\\r\\?$/d' -e '/^>>>>>>> /d' \"$DIFF3\" >\"$REMOTE\"\n> \n> I'd feel safer if these resulting $BASE, $LOCAL and $REMOTE are\n> validated to be conflict-marker free (i.e. '^\\([<|=>]\\)\\1\\1\\1\\1\\1\\1'\n> does not appear) to make sure there was no funny virtual ancestor\n> that records a conflicted recursive merge result confused our logic.\n> \n> When we see an unfortunate sign that it happened, we can revert the\n> automerge and let the tool handle the original input.\n\nWhat if the original file does have these markers?\n\nWhich is probably something we should be checking beforehand and not\nattempt an automerge in those cases.\n\nOr we could add the --base option to \"git merge-file\" so we don't have\nto do that work by hand.\n\n> > +\tfi\n> > +\trm -- \"$DIFF3\"\n> > +}\n> > +\n> \n> \"$DIFF3\" is always created (unless shell redirection into it fails),\n> so \"rm --\" would be fine in practice, I guess, but \"rm -f --\" would\n> not hurt.\n\nI just did the same as below:\n\n rm -- \"$BACKUP\"\n\n> > +\tDIFF3=\"$MERGETOOL_TMPDIR/${BASE}_DIFF3_$$$ext\"\n> \n> $MERGETOOL_TMPDIR is either \"mktemp -d -t \"git-mergetool-XXXXXX\" or\n> \".\". Also, we liberally pass \"$DIFF3\" to \"sed\" as an argument and\n> assume that the command would take it as a filename and not an\n> option.\n> \n> For the above reason, \"rm --\", while it is not wrong per-se, can be\n> just a simple \"rm\", as there is no funny leading letters in \"$DIFF3\"\n> that requires disambiguation.\n\nOther parts of the file do this:\n\n rm -f -- \"$LOCAL\" \"$REMOTE\" \"$BASE\" \"$BACKUP\"\n\nI'm just following what the script already does.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"412945","messageId":"xmqqblek8e94.fsf@gitster.c.googlers.com","threadId":"54871","inReplyTo":"5fe352e3968f6_198be2083@natae.notmuch","subject":"Re: [PATCH v5 1/1] mergetool: add automerge configuration","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-12-23T20:21:43Z","receivedAt":"2020-12-23T20:22:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> Junio C Hamano wrote:\n>> Felipe Contreras writes:\n>> \n>> > +auto_merge () {\n>> > +\tgit merge-file --diff3 --marker-size=7 -q -p \"$LOCAL\" \"$BASE\" \"$REMOTE\" >\"$DIFF3\"\n>> > +\tif test -s \"$DIFF3\"\n>> > +\tthen\n>> \n>> We do not want to ignore the exit status from the command. IOW, I\n>> think the above wants to be rather\n>> \n>> \tif git merge-file ... >\"$DIFF3\" &&\n>> \t test -s \"$DIFF3\"\n>> \tthen\n>> \t\t...\n>\n> That doesn't work.\n>\n> \"git merge-file\" always returns non-zero status when it succeeds (it's\n> the number of conflicts generated).\n\nAh, I forgot about that one. I think \"the number of conflicts\" was\na UI mistake (the original that it mimics is \"merge\" from RCS suite,\nwhich uses 1 and 2 for \"conflicts\" and \"trouble\") but we know we\nwill get conflicts, so it is wrong to expect success from the\ncommand. Deliberately ignoring the return status is the right thing\nto do.\n\n> What if the original file does have these markers?\n>\n> Which is probably something we should be checking beforehand and not\n> attempt an automerge in those cases.\n\nYes, that is a much better approach to avoid unnecessary work.\n\nWhen we made the conflict marker length configurable, we were hoping\nthat we no longer have to worry about the cases where payload files\n(original or ours or theirs) have lines that are confusingly similar\nto the conflict markers, but because we are interfacing external tools\nthat are unaware of the facility, it probably would not help us in\nthis case all that much.\n\nFWIW, we use a fiarly large size for our own files in t/ and\nDocumentation/ directories ourselves, and it does help topic branch\nmerges somewhat frequently.\n"},{"id":"412969","messageId":"5fe3dd62e12f8_7855a2081f@natae.notmuch","threadId":"54871","inReplyTo":"xmqqblek8e94.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v5 1/1] mergetool: add automerge configuration","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-24T00:14:26Z","receivedAt":"2020-12-24T00:15:26Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Junio C Hamano wrote:\n> Felipe Contreras writes:\n> \n> > Junio C Hamano wrote:\n> >> Felipe Contreras writes:\n> >> \n> >> > +auto_merge () {\n> >> > +\tgit merge-file --diff3 --marker-size=7 -q -p \"$LOCAL\" \"$BASE\" \"$REMOTE\" >\"$DIFF3\"\n> >> > +\tif test -s \"$DIFF3\"\n> >> > +\tthen\n> >> \n> >> We do not want to ignore the exit status from the command. IOW, I\n> >> think the above wants to be rather\n> >> \n> >> \tif git merge-file ... >\"$DIFF3\" &&\n> >> \t test -s \"$DIFF3\"\n> >> \tthen\n> >> \t\t...\n> >\n> > That doesn't work.\n> >\n> > \"git merge-file\" always returns non-zero status when it succeeds (it's\n> > the number of conflicts generated).\n> \n> Ah, I forgot about that one. I think \"the number of conflicts\" was\n> a UI mistake (the original that it mimics is \"merge\" from RCS suite,\n> which uses 1 and 2 for \"conflicts\" and \"trouble\") but we know we\n> will get conflicts, so it is wrong to expect success from the\n> command. Deliberately ignoring the return status is the right thing\n> to do.\n\nI agree. My bet is that nobody is checking the return status of \"git\nmerge-file\" to find out the number of conflicts. Plus, how can you check\nthe difference between 255 conflicts and error -1?\n\nBut that's the situation we are in now.\n\n> > What if the original file does have these markers?\n> >\n> > Which is probably something we should be checking beforehand and not\n> > attempt an automerge in those cases.\n> \n> Yes, that is a much better approach to avoid unnecessary work.\n> \n> When we made the conflict marker length configurable, we were hoping\n> that we no longer have to worry about the cases where payload files\n> (original or ours or theirs) have lines that are confusingly similar\n> to the conflict markers, but because we are interfacing external tools\n> that are unaware of the facility, it probably would not help us in\n> this case all that much.\n> \n> FWIW, we use a fiarly large size for our own files in t/ and\n> Documentation/ directories ourselves, and it does help topic branch\n> merges somewhat frequently.\n\nWe could do something like --marker-size=13 to minimize the chances of\nthat happening.\n\nIn that case I would prefer '/^<\\{13\\} /' (to avoid too many\ncharacters). I see those regexes used elsewhere in git, but I don't know\nhow portable that is.\n\nIf we wanted to make sure none of those markers remain it's not enough\nto check for '^[<|=>]{13}', what follows up should be a space, or some\ndelimiter, not another < for example. So maybe '^[<|=>]{13}[^<|=>]'?\n\nSo, do we want those three things?\n\n 1. A non-standard marker-size\n 2. Check beforehand the existence of those markers and disable\n automerge\n 3. Check afterwards the existence of those markers and disable\n automerge\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"412973","messageId":"xmqqv9cs3uxo.fsf@gitster.c.googlers.com","threadId":"54871","inReplyTo":"5fe3dd62e12f8_7855a2081f@natae.notmuch","subject":"Re: [PATCH v5 1/1] mergetool: add automerge configuration","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-12-24T00:32:35Z","receivedAt":"2020-12-24T00:33:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n>> Ah, I forgot about that one. I think \"the number of conflicts\" was\n>> a UI mistake (the original that it mimics is \"merge\" from RCS suite,\n>> which uses 1 and 2 for \"conflicts\" and \"trouble\") but we know we\n>> will get conflicts, so it is wrong to expect success from the\n>> command. Deliberately ignoring the return status is the right thing\n>> to do.\n>\n> I agree. My bet is that nobody is checking the return status of \"git\n> merge-file\" to find out the number of conflicts. Plus, how can you check\n> the difference between 255 conflicts and error -1?\n\nYup, I already mentioned UI mistake so you do not have to repeat it\nto consume more bandwidth. We're in agreement already.\n\n> We could do something like --marker-size=13 to minimize the chances of\n> that happening.\n>\n> In that case I would prefer '/^<\\{13\\} /' (to avoid too many\n> characters). I see those regexes used elsewhere in git, but I don't know\n> how portable that is.\n\nIf it is used elsewhere with \"sed\", then that would be OK, but if it\nis not with \"sed\" but with \"grep\", that's quite a different story.\n\n> So, do we want those three things?\n>\n> 1. A non-standard marker-size\n> 2. Check beforehand the existence of those markers and disable\n> automerge\n> 3. Check afterwards the existence of those markers and disable\n> automerge\n\nI do not think 3 is needed if we do 2 and I do not think 1 would\nparticularly be useful *UNLESS* the code consults with the attribute\nsystem to see what marker size the path uses to avoid crashing with\nthe non-standard marker-size the path already uses.\n\nSo the easiest would be not to do anything for now, with a note\nabout known limitations in the doc. The second easiest would be to\ndo 2. alone. We could do 1. to be more complete but I tend to think\nthat it is better to leave it as #leftoverbits.\n\n\n\n"},{"id":"412977","messageId":"5fe3f083f27cd_7855a20885@natae.notmuch","threadId":"54871","inReplyTo":"xmqqv9cs3uxo.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v5 1/1] mergetool: add automerge configuration","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-24T01:36:03Z","receivedAt":"2020-12-24T01:37:03Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Junio C Hamano wrote:\n> Felipe Contreras writes:\n> \n> >> Ah, I forgot about that one. I think \"the number of conflicts\" was\n> >> a UI mistake (the original that it mimics is \"merge\" from RCS suite,\n> >> which uses 1 and 2 for \"conflicts\" and \"trouble\") but we know we\n> >> will get conflicts, so it is wrong to expect success from the\n> >> command. Deliberately ignoring the return status is the right thing\n> >> to do.\n> >\n> > I agree. My bet is that nobody is checking the return status of \"git\n> > merge-file\" to find out the number of conflicts. Plus, how can you check\n> > the difference between 255 conflicts and error -1?\n> \n> Yup, I already mentioned UI mistake so you do not have to repeat\n\nYou said it was a UI mistake, not me. I am a different mind than yours.\n\nThis [1] is the first time *you* communicated it was a UI mistake.\n\nThis [2] is the first time *I* communicated it was a UI mistake.\n\nI communicated that fact after you, so I did not repeat anything,\nbecause I hadn't said that before. *You* did, not *me*.\n\n> it to consume more bandwidth.\n\nThis is what is consuming bandwidth.\n\nNot me stating *for the first time* that I agree what you just stated.\n\nYou could have skipped what I said *for the first time*, if you didn't\nfind it particularly interesting, and that would have saved bandwidth.\n\n> > We could do something like --marker-size=13 to minimize the chances of\n> > that happening.\n> >\n> > In that case I would prefer '/^<\\{13\\} /' (to avoid too many\n> > characters). I see those regexes used elsewhere in git, but I don't know\n> > how portable that is.\n> \n> If it is used elsewhere with \"sed\", then that would be OK, but if it\n> is not with \"sed\" but with \"grep\", that's quite a different story.\n\nIn t/t3427-rebase-subtree.sh there is:\n\n sed -e \"s%\\([0-9a-f]\\{40\\} \\)files_subtree/%\\1%\"\n\nNot sure if that counts. There's other places in the tests.\n\nHowever, I don't see the point if the marker-size is a low enough number, like 7.\n\n> > So, do we want those three things?\n> >\n> > 1. A non-standard marker-size\n> > 2. Check beforehand the existence of those markers and disable\n> > automerge\n> > 3. Check afterwards the existence of those markers and disable\n> > automerge\n> \n> I do not think 3 is needed if we do 2 and I do not think 1 would\n> particularly be useful *UNLESS* the code consults with the attribute\n> system to see what marker size the path uses to avoid crashing with\n> the non-standard marker-size the path already uses.\n\nBut what is more likely? a) That the marker-size is 7 (the default), or\nb) that the marker-size is not the default, but that there's a\nmarker-size attribute *and* the value is precisely 13?\n\nI think a) is way more likely than b).\n\n> So the easiest would be not to do anything for now, with a note\n> about known limitations in the doc. The second easiest would be to\n> do 2. alone. We could do 1. to be more complete but I tend to think\n> that it is better to leave it as #leftoverbits.\n\nOK. I think 1. is low-hanging fruit, but I'm fine with not doing\nanything, or trying 2.\n\nI don't think 2. would be that hard, so I will try that before\nre-rolling the series.\n\n(unless somebody replies to my other pending arguments)\n\nCheers.\n\n[1] https://lore.kernel.org/git/xmqqblek8e94.fsf@gitster.c.googlers.com/\n[2] https://lore.kernel.org/git/5fe3dd62e12f8_7855a2081f@natae.notmuch/\n\n-- \nFelipe Contreras\n"},{"id":"412986","messageId":"xmqqim8r4tjh.fsf@gitster.c.googlers.com","threadId":"54871","inReplyTo":"5fe3f083f27cd_7855a20885@natae.notmuch","subject":"Re: [PATCH v5 1/1] mergetool: add automerge configuration","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-12-24T06:17:22Z","receivedAt":"2020-12-24T06:18:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n>> Yup, I already mentioned UI mistake so you do not have to repeat\n>\n> You said it was a UI mistake, not me. I am a different mind than yours.\n\nYes, but the point is that I do not need to nor particularly want to\nhear your opinion on the behaviour of \"git merge-file\". I know (and\nothers reading the thread on the list also know) that the exit code\nof the command is misdesigned already.\n\n> I communicated that fact after you, so I did not repeat anything,\n> because I hadn't said that before. *You* did, not *me*.\n\nAgain, please realize that on list discussion is a team effort to\ncome up together a better design of a shared solution. And if you\nalready know that (I don't read your mind ;-), please act like you\ndo, too.\n\nThanks.\n\n"},{"id":"412994","messageId":"xmqqim8r36ba.fsf@gitster.c.googlers.com","threadId":"54871","inReplyTo":"20201223045358.100754-1-felipe.contreras@gmail.com","subject":"Re: [PATCH v5 0/1] mergetool: remove unconflicted lines","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-12-24T09:24:25Z","receivedAt":"2020-12-24T09:25:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> There's not much to say other that what the commit message of the patch says.\n>\n> Note: no feedback has been ignored; I replied to all the feedback, I didn't hear anything back.\n>\n> Changes since v4:\n>\n> * Improved commit message with suggestions from Phillip Wood.\n>\n> Felipe Contreras (1):\n> mergetool: add automerge configuration\n\nThis breakage is possibly a fallout from either this patch or\n1e2ae142 (t7[5-9]*: adjust the references to the default branch name\n\"main\", 2020-11-18).\n\n https://github.com/git/git/runs/1602803804#step:7:10358\n\nI cannot quite tell how the two strings compared with 'test' on\noutput line 10355 are different in the output, though.\n\nThanks.\n"},{"id":"413003","messageId":"5fe4baed206cc_19c9208e8@natae.notmuch","threadId":"54871","inReplyTo":"xmqqim8r4tjh.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v5 1/1] mergetool: add automerge configuration","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-24T15:59:41Z","receivedAt":"2020-12-24T16:00:40Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Junio C Hamano wrote:\n> Felipe Contreras writes:\n> \n> >> Yup, I already mentioned UI mistake so you do not have to repeat\n> >\n> > You said it was a UI mistake, not me. I am a different mind than yours.\n> \n> Yes, but the point is that I do not need to nor particularly want to\n> hear your opinion on the behaviour of \"git merge-file\".\n\nThen disregard the comment.\n\n> I know (and others reading the thread on the list also know) that the\n> exit code of the command is misdesigned already.\n\nUnless you can read minds, you don't know that.\n\nAnd even if you do, I don't know what you know. I can't read your mind.\n\nPlus, they can disregard the comment as well.\n\n> > I communicated that fact after you, so I did not repeat anything,\n> > because I hadn't said that before. *You* did, not *me*.\n> \n> Again, please realize that on list discussion is a team effort to\n> come up together a better design of a shared solution.\n\nWhich is why agreement in a team with different minds and different\nviewpoints is important.\n\nJust to show a few instances of Jeff King telling you he agrees with\nyou:\n\n 1. \"I agree it's not all that useful in that example\" [1]. 16 Dec 2020\n 2. \"I agree with the current definition\" [2]. 18 Dec 2020 (same thread)\n 3. \"I agree the two should behave the same\" [3]. 18 Dec 2020 (same\n mail)\n\nThe fact that you value Jeff King's agreement and don't care what some\nother members in the community think, is a personal vale judgement, and\ndoesn't necessarily mean the viewpoints of such community members are\nobjectively worthless.\n\nCheers.\n\n[1] https://lore.kernel.org/git/X9pUc2HXUr3+WHbR@coredump.intra.peff.net/\n[2] https://lore.kernel.org/git/X9xJ6BHM9VY0%2FyLs@coredump.intra.peff.net/\n[3] https://lore.kernel.org/git/X9xJ6BHM9VY0%2FyLs@coredump.intra.peff.net/\n\n-- \nFelipe Contreras\n"},{"id":"413004","messageId":"5fe4bec2da21a_19c92085f@natae.notmuch","threadId":"54871","inReplyTo":"xmqqim8r36ba.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v5 0/1] mergetool: remove unconflicted lines","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-24T16:16:02Z","receivedAt":"2020-12-24T16:18:05Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Junio C Hamano wrote:\n> Felipe Contreras writes:\n> \n> > There's not much to say other that what the commit message of the patch says.\n> >\n> > Note: no feedback has been ignored; I replied to all the feedback, I didn't hear anything back.\n> >\n> > Changes since v4:\n> >\n> > * Improved commit message with suggestions from Phillip Wood.\n> >\n> > Felipe Contreras (1):\n> > mergetool: add automerge configuration\n> \n> This breakage is possibly a fallout from either this patch or\n> 1e2ae142 (t7[5-9]*: adjust the references to the default branch name\n> \"main\", 2020-11-18).\n> \n> https://github.com/git/git/runs/1602803804#step:7:10358\n\nIt seems likely it's the mergetool patch.\n\nThis regex '/^=======\\r\\?$/' is supposed to handle the crlf situation.\n\nI can't imagine what would be different in Windows regarding that\nsituation.\n\n-- \nFelipe Contreras\n"},{"id":"413009","messageId":"xmqq4kka3ke8.fsf@gitster.c.googlers.com","threadId":"54871","inReplyTo":"5fe4baed206cc_19c9208e8@natae.notmuch","subject":"Re: [PATCH v5 1/1] mergetool: add automerge configuration","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-12-24T22:32:31Z","receivedAt":"2020-12-24T22:33:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> Junio C Hamano wrote:\n>> Felipe Contreras writes:\n>> \n>> >> Yup, I already mentioned UI mistake so you do not have to repeat\n>> >\n>> > You said it was a UI mistake, not me. I am a different mind than yours.\n>> \n>> Yes, but the point is that I do not need to nor particularly want to\n>> hear your opinion on the behaviour of \"git merge-file\".\n>\n>> I know (and others reading the thread on the list also know) that the\n>> exit code of the command is misdesigned already.\n>\n> Unless you can read minds, you don't know that.\n\nActually I do, because they heard from me already ;-). If this were\nthe case where our messages crossed, perhaps, but in this case yours\nwas a response to my message.\n\n>> Again, please realize that on list discussion is a team effort to\n>> come up together a better design of a shared solution.\n>\n> Which is why agreement in a team with different minds and different\n> viewpoints is important.\n\nIt is not like opinions on all points are important. Whether the\nexit code from merge-file is or is not a UI mistake does NOT have\nany influence on what we were discussing. My opinion is that exit\ncode from merge-file is a UI mistake, but even if you disagree with\nthat, that would not change the conclusion we already reached that\nthe code should ignore its exit status, like you originally wrote.\n\nI am already trying to ignore your opinions on things that do not\nmatter in the context of this project, as you told me earlier ;-)\nBut just like patches, messages are written only once but read by\nmany people, so I'd always aim for reducing noise at the source.\n\nAnyway, happy holidays and pleasant new year to you and to\neverybody.\n"},{"id":"413027","messageId":"5fe8cc0998fc0_e22d208c5@natae.notmuch","threadId":"54871","inReplyTo":"xmqq4kka3ke8.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v5 1/1] mergetool: add automerge configuration","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-27T18:01:45Z","receivedAt":"2020-12-27T18:13:57Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Junio C Hamano wrote:\n> Felipe Contreras writes:\n> \n> > Junio C Hamano wrote:\n> >> Felipe Contreras writes:\n> >> \n> >> >> Yup, I already mentioned UI mistake so you do not have to repeat\n> >> >\n> >> > You said it was a UI mistake, not me. I am a different mind than yours.\n> >> \n> >> Yes, but the point is that I do not need to nor particularly want to\n> >> hear your opinion on the behaviour of \"git merge-file\".\n> >\n> >> I know (and others reading the thread on the list also know) that the\n> >> exit code of the command is misdesigned already.\n> >\n> > Unless you can read minds, you don't know that.\n> \n> Actually I do, because they heard from me already ;-).\n\nThey heard that you *think* it's a UI mistake.\n\nThe fact that you think something is a mistake doesn't necessarily mean\nit's actually a mistake, and other community members might think\notherwise.\n\nYou do not dictate what others on the list know.\n\n> >> Again, please realize that on list discussion is a team effort to\n> >> come up together a better design of a shared solution.\n> >\n> > Which is why agreement in a team with different minds and different\n> > viewpoints is important.\n> \n> It is not like opinions on all points are important. Whether the\n> exit code from merge-file is or is not a UI mistake does NOT have\n> any influence on what we were discussing.\n\nWhich is why I initially did not express such an opinion.\n\nBut you did, presumably you had some reason to do so, so I simply\ndid the same and expressed mine.\n\n> I am already trying to ignore your opinions on things that do not\n> matter in the context of this project, as you told me earlier ;-)\n> But just like patches, messages are written only once but read by\n> many people, so I'd always aim for reducing noise at the source.\n\nWhat you consider noise others might not.\n\nGood writers say you should not assume what your readers know.\n\nYes, some readers might think exactly like you do, and they don't need\nwhat you consider obvious information. But for every person that\nthinks exactly like you, there are dozens that don't, and it's those you\nshould keep in mind.\n\nMost people err on the side of not providing enough information to the\nminds dissimilar to theirs.\n\nThis is called the curse of knowledge [1].\n\nI try not to do that.\n\n> Anyway, happy holidays and pleasant new year to you and to\n> everybody.\n\nSame to you.\n\nCheers.\n\n[1] https://en.wikipedia.org/wiki/Curse_of_knowledge\n\n-- \nFelipe Contreras\n"},{"id":"413030","messageId":"20201227205835.502556-3-seth@eseth.com","threadId":"54871","inReplyTo":"20201227205835.502556-1-seth@eseth.com","subject":"[PATCH v6 2/2] mergetool: Add per-tool support for the autoMerge flag","fromName":"Seth House","fromEmail":"seth@eseth.com","sentAt":"2020-12-27T20:58:35Z","receivedAt":"2020-12-27T21:00:01Z","isPatch":true,"sender":{"key":"seth@eseth.com","avatar":"https://avatars.githubusercontent.com/u/91293?v=4"},"body":"Keep the global mergetool flag and add a per-tool override flag so that\nusers may enable the flag for one tool and disable it for another.\n\nSigned-off-by: Seth House \n---\n Documentation/config/mergetool.txt | 3 +++\n git-mergetool.sh | 5 ++++-\n 2 files changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config/mergetool.txt b/Documentation/config/mergetool.txt\nindex 43af7a96f9..7f32281a61 100644\n--- a/Documentation/config/mergetool.txt\n+++ b/Documentation/config/mergetool.txt\n@@ -21,6 +21,9 @@ mergetool..trustExitCode::\n \tif the file has been updated, otherwise the user is prompted to\n \tindicate the success of the merge.\n \n+mergetool..autoMerge::\n+\tAutomatically resolve conflicts that don't require user intervention.\n+\n mergetool.meld.hasOutput::\n \tOlder versions of `meld` do not support the `--output` option.\n \tGit will attempt to detect whether `meld` supports `--output`\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex 6e86d3b492..81df301734 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -323,7 +323,10 @@ merge_file () {\n \tcheckout_staged_file 2 \"$MERGED\" \"$LOCAL\"\n \tcheckout_staged_file 3 \"$MERGED\" \"$REMOTE\"\n \n-\tif test \"$(git config --bool mergetool.autoMerge)\" = \"true\"\n+\tif test \"$(\n+\t\tgit config --get --bool \"mergetool.$merge_tool.automerge\" ||\n+\t\tgit config --get --bool \"mergetool.automerge\" ||\n+\t\techo true)\" = true\n \tthen\n \t\tgit merge-file --diff3 -q -p \"$LOCAL\" \"$BASE\" \"$REMOTE\" >\"$DIFF3\"\n \t\tsed -e '/^<<<<<<< /,/^||||||| /d' -e '/^=======\\r\\?$/,/^>>>>>>> /d' \"$DIFF3\" >\"$BASE\"\n-- \n2.29.2\n\n\n"},{"id":"413031","messageId":"20201227205835.502556-2-seth@eseth.com","threadId":"54871","inReplyTo":"20201227205835.502556-1-seth@eseth.com","subject":"[PATCH v6 1/2] mergetool: add automerge configuration","fromName":"Seth House","fromEmail":"seth@eseth.com","sentAt":"2020-12-27T20:58:34Z","receivedAt":"2020-12-27T21:00:01Z","isPatch":true,"sender":{"key":"seth@eseth.com","avatar":"https://avatars.githubusercontent.com/u/91293?v=4"},"body":"From: Felipe Contreras \n\nIt doesn't make sense to display easily-solvable conflicts in the\ndifferent views of all mergetools.\n\nOnly the chunks that warrant conflict markers should be displayed.\n\nIn order to unobtrusively do this, add a new configuration:\nmergetool.autoMerge.\n\nSee Seth House's blog post [1] for the idea, and the rationale.\n\n[1] https://www.eseth.org/2020/mergetools.html\n\nOriginal-idea-by: Seth House \nSigned-off-by: Felipe Contreras \n---\n Documentation/config/mergetool.txt | 3 +++\n git-mergetool.sh | 10 ++++++++++\n t/t7610-mergetool.sh | 18 ++++++++++++++++++\n 3 files changed, 31 insertions(+)\n\ndiff --git a/Documentation/config/mergetool.txt b/Documentation/config/mergetool.txt\nindex 16a27443a3..43af7a96f9 100644\n--- a/Documentation/config/mergetool.txt\n+++ b/Documentation/config/mergetool.txt\n@@ -61,3 +61,6 @@ mergetool.writeToTemp::\n \n mergetool.prompt::\n \tPrompt before each invocation of the merge resolution program.\n+\n+mergetool.autoMerge::\n+\tAutomatically resolve conflicts that don't require user intervention.\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex e3f6d543fb..6e86d3b492 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -274,6 +274,7 @@ merge_file () {\n \t\tBASE=${BASE##*/}\n \tfi\n \n+\tDIFF3=\"$MERGETOOL_TMPDIR/${BASE}_DIFF3_$$$ext\"\n \tBACKUP=\"$MERGETOOL_TMPDIR/${BASE}_BACKUP_$$$ext\"\n \tLOCAL=\"$MERGETOOL_TMPDIR/${BASE}_LOCAL_$$$ext\"\n \tREMOTE=\"$MERGETOOL_TMPDIR/${BASE}_REMOTE_$$$ext\"\n@@ -322,6 +323,15 @@ merge_file () {\n \tcheckout_staged_file 2 \"$MERGED\" \"$LOCAL\"\n \tcheckout_staged_file 3 \"$MERGED\" \"$REMOTE\"\n \n+\tif test \"$(git config --bool mergetool.autoMerge)\" = \"true\"\n+\tthen\n+\t\tgit merge-file --diff3 -q -p \"$LOCAL\" \"$BASE\" \"$REMOTE\" >\"$DIFF3\"\n+\t\tsed -e '/^<<<<<<< /,/^||||||| /d' -e '/^=======\\r\\?$/,/^>>>>>>> /d' \"$DIFF3\" >\"$BASE\"\n+\t\tsed -e '/^||||||| /,/^>>>>>>> /d' -e '/^<<<<<<< /d' \"$DIFF3\" >\"$LOCAL\"\n+\t\tsed -e '/^<<<<<<< /,/^=======\\r\\?$/d' -e '/^>>>>>>> /d' \"$DIFF3\" >\"$REMOTE\"\n+\t\trm -- \"$DIFF3\"\n+\tfi\n+\n \tif test -z \"$local_mode\" || test -z \"$remote_mode\"\n \tthen\n \t\techo \"Deleted merge conflict for '$MERGED':\"\ndiff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh\nindex 70afdd06fa..b75c91199b 100755\n--- a/t/t7610-mergetool.sh\n+++ b/t/t7610-mergetool.sh\n@@ -828,4 +828,22 @@ test_expect_success 'mergetool -Oorder-file is honored' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'mergetool automerge' '\n+\ttest_config mergetool.automerge true &&\n+\ttest_when_finished \"git reset --hard\" &&\n+\tgit checkout -b test${test_count}_b master &&\n+\techo -e \"base\\n\\na\" >file1 &&\n+\tgit commit -a -m \"base\" &&\n+\techo -e \"base\\n\\nc\" >file1 &&\n+\tgit commit -a -m \"remote update\" &&\n+\tgit checkout -b test${test_count}_a HEAD~ &&\n+\techo -e \"local\\n\\nb\" >file1 &&\n+\tgit commit -a -m \"local update\" &&\n+\ttest_must_fail git merge test${test_count}_b &&\n+\tyes \"\" | git mergetool file1 &&\n+\techo -e \"local\\n\\nc\" >expect &&\n+\ttest_cmp expect file1 &&\n+\tgit commit -m \"test resolved with mergetool\"\n+'\n+\n test_done\n-- \n2.29.2\n\n\n"},{"id":"413032","messageId":"20201227205835.502556-1-seth@eseth.com","threadId":"54871","inReplyTo":"20201223045358.100754-1-felipe.contreras@gmail.com","subject":"[PATCH v6 0/1] mergetool: add automerge configuration","fromName":"Seth House","fromEmail":"seth@eseth.com","sentAt":"2020-12-27T20:58:33Z","receivedAt":"2020-12-27T21:00:01Z","isPatch":true,"sender":{"key":"seth@eseth.com","avatar":"https://avatars.githubusercontent.com/u/91293?v=4"},"body":"Sorry for the slow turnaround on this. I haven't used Git via email\npatches before now so it took me quite a few hours to read through\ntutorials, configure git-send-email and fight missing Perl libs. Please\nlet me know if I did anything incorrectly! I should be able to\ncontribute more quickly from now on.\n\nChanges since v5:\n\n * Add per-tool configuration that Felipe has a \"deep philosophical\"\n opposition to adding.\n\nFelipe Contreras (1):\n mergetool: add automerge configuration\n\nSeth House (1):\n mergetool: Add per-tool support for the autoMerge flag\n\n Documentation/config/mergetool.txt | 6 ++++++\n git-mergetool.sh | 13 +++++++++++++\n t/t7610-mergetool.sh | 18 ++++++++++++++++++\n 3 files changed, 37 insertions(+)\n\n-- \n2.29.2\n\n"},{"id":"413033","messageId":"xmqq1rfarji5.fsf@gitster.c.googlers.com","threadId":"54871","inReplyTo":"20201227205835.502556-2-seth@eseth.com","subject":"Re: [PATCH v6 1/2] mergetool: add automerge configuration","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-12-27T22:06:58Z","receivedAt":"2020-12-27T22:07:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Seth House writes:\n\n> ...\n> See Seth House's blog post [1] for the idea, and the rationale.\n>\n> [1] https://www.eseth.org/2020/mergetools.html\n>\n> Original-idea-by: Seth House \n> Signed-off-by: Felipe Contreras \n> ---\n\nMissing Sign-off as a relayer.\n\n> diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh\n> index 70afdd06fa..b75c91199b 100755\n> --- a/t/t7610-mergetool.sh\n> +++ b/t/t7610-mergetool.sh\n> @@ -828,4 +828,22 @@ test_expect_success 'mergetool -Oorder-file is honored' '\n> \ttest_cmp expect actual\n> '\n> \n> +test_expect_success 'mergetool automerge' '\n> +\ttest_config mergetool.automerge true &&\n> +\ttest_when_finished \"git reset --hard\" &&\n> +\tgit checkout -b test${test_count}_b master &&\n> +\techo -e \"base\\n\\na\" >file1 &&\n\nThese do not seem to be taken from the version that has been\nimproved by reviwer comments after v3.\n\n> +\tgit commit -a -m \"base\" &&\n> +\techo -e \"base\\n\\nc\" >file1 &&\n> +\tgit commit -a -m \"remote update\" &&\n> +\tgit checkout -b test${test_count}_a HEAD~ &&\n> +\techo -e \"local\\n\\nb\" >file1 &&\n> +\tgit commit -a -m \"local update\" &&\n> +\ttest_must_fail git merge test${test_count}_b &&\n> +\tyes \"\" | git mergetool file1 &&\n> +\techo -e \"local\\n\\nc\" >expect &&\n> +\ttest_cmp expect file1 &&\n> +\tgit commit -m \"test resolved with mergetool\"\n> +'\n> +\n> test_done\n"},{"id":"413034","messageId":"20201227222922.GA509599@ellen","threadId":"54871","inReplyTo":"xmqq1rfarji5.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v6 1/2] mergetool: add automerge configuration","fromName":"Seth House","fromEmail":"seth@eseth.com","sentAt":"2020-12-27T22:29:22Z","receivedAt":"2020-12-27T22:30:23Z","isPatch":true,"sender":{"key":"seth@eseth.com","avatar":"https://avatars.githubusercontent.com/u/91293?v=4"},"body":"On Sun, Dec 27, 2020 at 02:06:58PM -0800, Junio C Hamano wrote:\n> Missing Sign-off as a relayer.\n\nI haven't come across that in the docs on contributing to Git and my\nGoogle searches aren't helping. Do you mind pointing me to what to add?\n\n> These do not seem to be taken from the version that has been\n> improved by reviwer comments after v3.\n\nWhoops! Thanks for the catch. Seems I fished the wrong version out of my\nemail. Created a new v7 based off the correct v5.\n\n"},{"id":"413035","messageId":"xmqqsg7qop94.fsf@gitster.c.googlers.com","threadId":"54871","inReplyTo":"20201227205835.502556-3-seth@eseth.com","subject":"Re: [PATCH v6 2/2] mergetool: Add per-tool support for the autoMerge flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-12-27T22:31:03Z","receivedAt":"2020-12-27T22:32:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Seth House writes:\n\n> Keep the global mergetool flag and add a per-tool override flag so that\n> users may enable the flag for one tool and disable it for another.\n>\n> Signed-off-by: Seth House \n> ---\n> Documentation/config/mergetool.txt | 3 +++\n> git-mergetool.sh | 5 ++++-\n> 2 files changed, 7 insertions(+), 1 deletion(-)\n>\n> diff --git a/Documentation/config/mergetool.txt b/Documentation/config/mergetool.txt\n> index 43af7a96f9..7f32281a61 100644\n> --- a/Documentation/config/mergetool.txt\n> +++ b/Documentation/config/mergetool.txt\n> @@ -21,6 +21,9 @@ mergetool..trustExitCode::\n> \tif the file has been updated, otherwise the user is prompted to\n> \tindicate the success of the merge.\n> \n> +mergetool..autoMerge::\n> +\tAutomatically resolve conflicts that don't require user intervention.\n> +\n> mergetool.meld.hasOutput::\n> \tOlder versions of `meld` do not support the `--output` option.\n> \tGit will attempt to detect whether `meld` supports `--output`\n> diff --git a/git-mergetool.sh b/git-mergetool.sh\n> index 6e86d3b492..81df301734 100755\n> --- a/git-mergetool.sh\n> +++ b/git-mergetool.sh\n> @@ -323,7 +323,10 @@ merge_file () {\n> \tcheckout_staged_file 2 \"$MERGED\" \"$LOCAL\"\n> \tcheckout_staged_file 3 \"$MERGED\" \"$REMOTE\"\n> \n> -\tif test \"$(git config --bool mergetool.autoMerge)\" = \"true\"\n> +\tif test \"$(\n> +\t\tgit config --get --bool \"mergetool.$merge_tool.automerge\" ||\n> +\t\tgit config --get --bool \"mergetool.automerge\" ||\n> +\t\techo true)\" = true\n\nYour [v6 1/2] that you build this step on does not enable the\nfeature by default, but this step does; it deserves to be documented\nand mentioned in the proposed log message.\n\nBut I think you'd want to build this step on top of newer one, if\nonly to take the portability fix to the tests, and that patch\nenables the feature by default, so ...\n\n> \tthen\n> \t\tgit merge-file --diff3 -q -p \"$LOCAL\" \"$BASE\" \"$REMOTE\" >\"$DIFF3\"\n> \t\tsed -e '/^<<<<<<< /,/^||||||| /d' -e '/^=======\\r\\?$/,/^>>>>>>> /d' \"$DIFF3\" >\"$BASE\"\n\nThanks.\n"},{"id":"413036","messageId":"xmqqzh1yn9cs.fsf@gitster.c.googlers.com","threadId":"54871","inReplyTo":"20201227222922.GA509599@ellen","subject":"Re: [PATCH v6 1/2] mergetool: add automerge configuration","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-12-27T22:59:47Z","receivedAt":"2020-12-27T23:01:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Seth House writes:\n\n> On Sun, Dec 27, 2020 at 02:06:58PM -0800, Junio C Hamano wrote:\n>> Missing Sign-off as a relayer.\n>\n> I haven't come across that in the docs on contributing to Git and my\n> Google searches aren't helping. Do you mind pointing me to what to add?\n\nDocumentation/SubmittingPatches#sign-off\n\n === Certify your work by adding your `Signed-off-by` trailer\n\n To improve tracking of who did what, we ask you to certify that you\n wrote the patch or have the right to pass it on under the same license\n as ours, by \"signing off\" your patch. Without sign-off, we cannot\n accept your patches.\n\n If you can certify the below D-C-O:\n\n [[dco]]\n .Developer's Certificate of Origin 1.1\n ____\n By making a contribution to this project, I certify that:\n\n a. The contribution was created in whole or in part by me and I\n have the right to submit it under the open source license\n indicated in the file; or\n\n b. The contribution is based upon previous work that, to the best\n of my knowledge, is covered under an appropriate open source\n license and I have the right under that license to submit that\n work with modifications, whether created in whole or in part\n by me, under the same open source license (unless I am\n permitted to submit under a different license), as indicated\n in the file; or\n\n c. The contribution was provided directly to me by some other\n person who certified (a), (b) or (c) and I have not modified\n it.\n\n d. I understand and agree that this project and the contribution\n are public and that a record of the contribution (including all\n personal information I submit with it, including my sign-off) is\n maintained indefinitely and may be redistributed consistent with\n this project or the open source license(s) involved.\n ____\n\n you add a \"Signed-off-by\" trailer to your commit, that looks like\n this:\n\n ....\n Signed-off-by: Random J Developer \n ....\n\n\nSo, you'd add your own signed-off-by trailer at the end of the\ntrailer list.\n\nhttps://lore.kernel.org/git/pull.805.git.1607091741254.gitgitgadget@gmail.com/\n\nfor an example where Johannes Schindelin picked up a patch written\nby Dennis Ameling and relayed it to the list.\n\nThanks.\n"},{"id":"413039","messageId":"20201228004152.522421-3-seth@eseth.com","threadId":"54871","inReplyTo":"20201228004152.522421-1-seth@eseth.com","subject":"[PATCH v7 2/2] mergetool: Add per-tool support for the autoMerge flag","fromName":"Seth House","fromEmail":"seth@eseth.com","sentAt":"2020-12-28T00:41:52Z","receivedAt":"2020-12-28T00:43:12Z","isPatch":true,"sender":{"key":"seth@eseth.com","avatar":"https://avatars.githubusercontent.com/u/91293?v=4"},"body":"Keep the global mergetool flag and add a per-tool override flag so that\nusers may enable the flag for one tool and disable it for another.\n\nSigned-off-by: Seth House \n---\n Documentation/config/mergetool.txt | 3 +++\n git-mergetool.sh | 5 ++++-\n 2 files changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config/mergetool.txt b/Documentation/config/mergetool.txt\nindex 7ce6d0d3ac..ef147fc118 100644\n--- a/Documentation/config/mergetool.txt\n+++ b/Documentation/config/mergetool.txt\n@@ -21,6 +21,9 @@ mergetool..trustExitCode::\n \tif the file has been updated, otherwise the user is prompted to\n \tindicate the success of the merge.\n \n+mergetool..autoMerge::\n+\tRemove lines without conflicts from all the files. Defaults to `true`.\n+\n mergetool.meld.hasOutput::\n \tOlder versions of `meld` do not support the `--output` option.\n \tGit will attempt to detect whether `meld` supports `--output`\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex f4db0cac8d..e3c7d78d1d 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -334,7 +334,10 @@ merge_file () {\n \tcheckout_staged_file 2 \"$MERGED\" \"$LOCAL\"\n \tcheckout_staged_file 3 \"$MERGED\" \"$REMOTE\"\n \n-\tif test \"$(git config --bool mergetool.autoMerge)\" != \"false\"\n+\tif test \"$(\n+\t\tgit config --get --bool \"mergetool.$merge_tool.automerge\" ||\n+\t\tgit config --get --bool \"mergetool.automerge\" ||\n+\t\techo true)\" = true\n \tthen\n \t\tauto_merge\n \tfi\n-- \n2.29.2\n\n\n"},{"id":"413040","messageId":"20201228004152.522421-2-seth@eseth.com","threadId":"54871","inReplyTo":"20201228004152.522421-1-seth@eseth.com","subject":"[PATCH v7 1/2] mergetool: add automerge configuration","fromName":"Seth House","fromEmail":"seth@eseth.com","sentAt":"2020-12-28T00:41:51Z","receivedAt":"2020-12-28T00:43:12Z","isPatch":true,"sender":{"key":"seth@eseth.com","avatar":"https://avatars.githubusercontent.com/u/91293?v=4"},"body":"From: Felipe Contreras \n\nThe purpose of mergetools is to resolve conflicts when git cannot\nautomatically do so.\n\nIn order to do that git has added markers in the specific areas that\nneed resolving, which the user must manually fix. The tool is supposed\nto help with that.\n\nHowever, by passing the original BASE, LOCAL, and REMOTE files, many\nchanges without conflict are presented to the user when in fact nothing\nneeds to be done for those.\n\nWe can fix that by propagating the final version of the file with the\nautomatic merge to all the panes of the mergetool (BASE, LOCAL, and\nREMOTE), and only make them differ on the places where there are actual\nconflicts.\n\nAs most people will want the new behavior, we enable it by default.\nUsers that do not want the new behavior can set the new configuration\nmergetool.autoMerge to false.\n\nSee Seth House's blog post [1] for the idea, and the rationale.\n\n[1] https://www.eseth.org/2020/mergetools.html\n\nOriginal-idea-by: Seth House \nSigned-off-by: Felipe Contreras \nSigned-off-by: Seth House \n---\n Documentation/config/mergetool.txt | 3 +++\n git-mergetool.sh | 17 +++++++++++++++++\n t/t7610-mergetool.sh | 18 ++++++++++++++++++\n 3 files changed, 38 insertions(+)\n\ndiff --git a/Documentation/config/mergetool.txt b/Documentation/config/mergetool.txt\nindex 16a27443a3..7ce6d0d3ac 100644\n--- a/Documentation/config/mergetool.txt\n+++ b/Documentation/config/mergetool.txt\n@@ -61,3 +61,6 @@ mergetool.writeToTemp::\n \n mergetool.prompt::\n \tPrompt before each invocation of the merge resolution program.\n+\n+mergetool.autoMerge::\n+\tRemove lines without conflicts from all the files. Defaults to `true`.\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex e3f6d543fb..f4db0cac8d 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -239,6 +239,17 @@ checkout_staged_file () {\n \tfi\n }\n \n+auto_merge () {\n+\tgit merge-file --diff3 --marker-size=7 -q -p \"$LOCAL\" \"$BASE\" \"$REMOTE\" >\"$DIFF3\"\n+\tif test -s \"$DIFF3\"\n+\tthen\n+\t\tsed -e '/^<<<<<<< /,/^||||||| /d' -e '/^=======\\r\\?$/,/^>>>>>>> /d' \"$DIFF3\" >\"$BASE\"\n+\t\tsed -e '/^||||||| /,/^>>>>>>> /d' -e '/^<<<<<<< /d' \"$DIFF3\" >\"$LOCAL\"\n+\t\tsed -e '/^<<<<<<< /,/^=======\\r\\?$/d' -e '/^>>>>>>> /d' \"$DIFF3\" >\"$REMOTE\"\n+\tfi\n+\trm -- \"$DIFF3\"\n+}\n+\n merge_file () {\n \tMERGED=\"$1\"\n \n@@ -274,6 +285,7 @@ merge_file () {\n \t\tBASE=${BASE##*/}\n \tfi\n \n+\tDIFF3=\"$MERGETOOL_TMPDIR/${BASE}_DIFF3_$$$ext\"\n \tBACKUP=\"$MERGETOOL_TMPDIR/${BASE}_BACKUP_$$$ext\"\n \tLOCAL=\"$MERGETOOL_TMPDIR/${BASE}_LOCAL_$$$ext\"\n \tREMOTE=\"$MERGETOOL_TMPDIR/${BASE}_REMOTE_$$$ext\"\n@@ -322,6 +334,11 @@ merge_file () {\n \tcheckout_staged_file 2 \"$MERGED\" \"$LOCAL\"\n \tcheckout_staged_file 3 \"$MERGED\" \"$REMOTE\"\n \n+\tif test \"$(git config --bool mergetool.autoMerge)\" != \"false\"\n+\tthen\n+\t\tauto_merge\n+\tfi\n+\n \tif test -z \"$local_mode\" || test -z \"$remote_mode\"\n \tthen\n \t\techo \"Deleted merge conflict for '$MERGED':\"\ndiff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh\nindex 70afdd06fa..ccabd04823 100755\n--- a/t/t7610-mergetool.sh\n+++ b/t/t7610-mergetool.sh\n@@ -828,4 +828,22 @@ test_expect_success 'mergetool -Oorder-file is honored' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'mergetool automerge' '\n+\ttest_config mergetool.automerge true &&\n+\ttest_when_finished \"git reset --hard\" &&\n+\tgit checkout -b test${test_count}_b master &&\n+\ttest_write_lines >file1 base \"\" a &&\n+\tgit commit -a -m \"base\" &&\n+\ttest_write_lines >file1 base \"\" c &&\n+\tgit commit -a -m \"remote update\" &&\n+\tgit checkout -b test${test_count}_a HEAD~ &&\n+\ttest_write_lines >file1 local \"\" b &&\n+\tgit commit -a -m \"local update\" &&\n+\ttest_must_fail git merge test${test_count}_b &&\n+\tyes \"\" | git mergetool file1 &&\n+\ttest_write_lines >expect local \"\" c &&\n+\ttest_cmp expect file1 &&\n+\tgit commit -m \"test resolved with mergetool\"\n+'\n+\n test_done\n-- \n2.29.2\n\n\n"},{"id":"413041","messageId":"20201228004152.522421-1-seth@eseth.com","threadId":"54871","inReplyTo":"20201227205835.502556-1-seth@eseth.com","subject":"[PATCH v7 0/2] mergetool: add automerge configuration","fromName":"Seth House","fromEmail":"seth@eseth.com","sentAt":"2020-12-28T00:41:50Z","receivedAt":"2020-12-28T00:43:13Z","isPatch":true,"sender":{"key":"seth@eseth.com","avatar":"https://avatars.githubusercontent.com/u/91293?v=4"},"body":"Changes since v6:\n\n * Incorporated Junio's help and advice:\n\n * Rebased v7 off of the correct v5 version.\n * Signed off on Felipe's commit. (Although I have minor qualms with\n Felipe's various wording and even the name of the flag it is\n decidedly not worth burdening the list with bike-shedding.)\n\nChanges since v5:\n\n * Add per-tool configuration that Felipe has a \"deep philosophical\"\n opposition to adding.\n\nFelipe Contreras (1):\n mergetool: add automerge configuration\n\nSeth House (1):\n mergetool: Add per-tool support for the autoMerge flag\n\n Documentation/config/mergetool.txt | 6 ++++++\n git-mergetool.sh | 20 ++++++++++++++++++++\n t/t7610-mergetool.sh | 18 ++++++++++++++++++\n 3 files changed, 44 insertions(+)\n\n-- \n2.29.2\n\n"},{"id":"413042","messageId":"5fe92e9dec09e_10e65208de@natae.notmuch","threadId":"54871","inReplyTo":"20201227205835.502556-1-seth@eseth.com","subject":"RE: [PATCH v6 0/1] mergetool: add automerge configuration","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-28T01:02:21Z","receivedAt":"2020-12-28T01:05:15Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Seth House wrote:\n> Sorry for the slow turnaround on this. I haven't used Git via email\n> patches before now so it took me quite a few hours to read through\n> tutorials, configure git-send-email and fight missing Perl libs.\n\nWhat distribution are you using?\n\n> Changes since v5:\n\nThis is not v6 of my patch series; it's v1 of yours, which I think\nshould have a different title.\n\nWhat happens when I want to do v6?\n\nOther than that (and the fact that you initially used the wrong version\nas a baseline), I'm fine with your approach of doing a patch on top of\nmine.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"413043","messageId":"5fe93275963c_10e652085f@natae.notmuch","threadId":"54871","inReplyTo":"20201228004152.522421-3-seth@eseth.com","subject":"RE: [PATCH v7 2/2] mergetool: Add per-tool support for the autoMerge flag","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-28T01:18:45Z","receivedAt":"2020-12-28T01:23:42Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Seth House wrote:\n> diff --git a/git-mergetool.sh b/git-mergetool.sh\n> index f4db0cac8d..e3c7d78d1d 100755\n> --- a/git-mergetool.sh\n> +++ b/git-mergetool.sh\n> @@ -334,7 +334,10 @@ merge_file () {\n> \tcheckout_staged_file 2 \"$MERGED\" \"$LOCAL\"\n> \tcheckout_staged_file 3 \"$MERGED\" \"$REMOTE\"\n> \n> -\tif test \"$(git config --bool mergetool.autoMerge)\" != \"false\"\n> +\tif test \"$(\n> +\t\tgit config --get --bool \"mergetool.$merge_tool.automerge\" ||\n> +\t\tgit config --get --bool \"mergetool.automerge\" ||\n> +\t\techo true)\" = true\n\nThis is a per-tool user configuration.\n\nWasn't your argument that some tools would want to disable this flag?\nThat is; the tool, not the user.\n\nFor example, the author of diffconflicts might want to disable this flag\nfor all its users, or at least disable it by default.\n\nHow can the winmerge difftool disable this flag?\n\n> \tthen\n> \t\tauto_merge\n> \tfi\n\n-- \nFelipe Contreras\n"},{"id":"413044","messageId":"20201228045427.1166911-1-seth@eseth.com","threadId":"54871","inReplyTo":"20201228004152.522421-1-seth@eseth.com","subject":"[PATCH v8 0/4] mergetool: add automerge configuration","fromName":"Seth House","fromEmail":"seth@eseth.com","sentAt":"2020-12-28T04:54:23Z","receivedAt":"2020-12-28T04:55:59Z","isPatch":true,"sender":{"key":"seth@eseth.com","avatar":"https://avatars.githubusercontent.com/u/91293?v=4"},"body":"Changes since v7:\n\n * Add a tool-specific override function to setup_tool based on Junio's\n original patch feedback.\n\n The implementation of initialize_merge_tool is very much not set in\n stone. Suggestions are very welcome for alternate approaches that are\n less invasive.\n\nFelipe Contreras (1):\n mergetool: add automerge configuration\n\nSeth House (3):\n mergetool: Add per-tool support for the autoMerge flag\n mergetool: Break setup_tool out into separate initialization function\n mergetool: Add automerge_enabled tool-specific override function\n\n Documentation/config/mergetool.txt | 6 ++++++\n Documentation/git-mergetool--lib.txt | 4 ++++\n git-difftool--helper.sh | 2 ++\n git-mergetool--lib.sh | 11 ++++++++---\n git-mergetool.sh | 22 ++++++++++++++++++++++\n t/t7610-mergetool.sh | 18 ++++++++++++++++++\n 6 files changed, 60 insertions(+), 3 deletions(-)\n\n-- \n2.29.2\n\n\n"},{"id":"413045","messageId":"20201228045427.1166911-2-seth@eseth.com","threadId":"54871","inReplyTo":"20201228045427.1166911-1-seth@eseth.com","subject":"[PATCH v8 1/4] mergetool: add automerge configuration","fromName":"Seth House","fromEmail":"seth@eseth.com","sentAt":"2020-12-28T04:54:24Z","receivedAt":"2020-12-28T04:55:59Z","isPatch":true,"sender":{"key":"seth@eseth.com","avatar":"https://avatars.githubusercontent.com/u/91293?v=4"},"body":"From: Felipe Contreras \n\nThe purpose of mergetools is to resolve conflicts when git cannot\nautomatically do so.\n\nIn order to do that git has added markers in the specific areas that\nneed resolving, which the user must manually fix. The tool is supposed\nto help with that.\n\nHowever, by passing the original BASE, LOCAL, and REMOTE files, many\nchanges without conflict are presented to the user when in fact nothing\nneeds to be done for those.\n\nWe can fix that by propagating the final version of the file with the\nautomatic merge to all the panes of the mergetool (BASE, LOCAL, and\nREMOTE), and only make them differ on the places where there are actual\nconflicts.\n\nAs most people will want the new behavior, we enable it by default.\nUsers that do not want the new behavior can set the new configuration\nmergetool.autoMerge to false.\n\nSee Seth House's blog post [1] for the idea, and the rationale.\n\n[1] https://www.eseth.org/2020/mergetools.html\n\nOriginal-idea-by: Seth House \nSigned-off-by: Felipe Contreras \nSigned-off-by: Seth House \n---\n Documentation/config/mergetool.txt | 3 +++\n git-mergetool.sh | 17 +++++++++++++++++\n t/t7610-mergetool.sh | 18 ++++++++++++++++++\n 3 files changed, 38 insertions(+)\n\ndiff --git a/Documentation/config/mergetool.txt b/Documentation/config/mergetool.txt\nindex 16a27443a3..7ce6d0d3ac 100644\n--- a/Documentation/config/mergetool.txt\n+++ b/Documentation/config/mergetool.txt\n@@ -61,3 +61,6 @@ mergetool.writeToTemp::\n \n mergetool.prompt::\n \tPrompt before each invocation of the merge resolution program.\n+\n+mergetool.autoMerge::\n+\tRemove lines without conflicts from all the files. Defaults to `true`.\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex e3f6d543fb..f4db0cac8d 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -239,6 +239,17 @@ checkout_staged_file () {\n \tfi\n }\n \n+auto_merge () {\n+\tgit merge-file --diff3 --marker-size=7 -q -p \"$LOCAL\" \"$BASE\" \"$REMOTE\" >\"$DIFF3\"\n+\tif test -s \"$DIFF3\"\n+\tthen\n+\t\tsed -e '/^<<<<<<< /,/^||||||| /d' -e '/^=======\\r\\?$/,/^>>>>>>> /d' \"$DIFF3\" >\"$BASE\"\n+\t\tsed -e '/^||||||| /,/^>>>>>>> /d' -e '/^<<<<<<< /d' \"$DIFF3\" >\"$LOCAL\"\n+\t\tsed -e '/^<<<<<<< /,/^=======\\r\\?$/d' -e '/^>>>>>>> /d' \"$DIFF3\" >\"$REMOTE\"\n+\tfi\n+\trm -- \"$DIFF3\"\n+}\n+\n merge_file () {\n \tMERGED=\"$1\"\n \n@@ -274,6 +285,7 @@ merge_file () {\n \t\tBASE=${BASE##*/}\n \tfi\n \n+\tDIFF3=\"$MERGETOOL_TMPDIR/${BASE}_DIFF3_$$$ext\"\n \tBACKUP=\"$MERGETOOL_TMPDIR/${BASE}_BACKUP_$$$ext\"\n \tLOCAL=\"$MERGETOOL_TMPDIR/${BASE}_LOCAL_$$$ext\"\n \tREMOTE=\"$MERGETOOL_TMPDIR/${BASE}_REMOTE_$$$ext\"\n@@ -322,6 +334,11 @@ merge_file () {\n \tcheckout_staged_file 2 \"$MERGED\" \"$LOCAL\"\n \tcheckout_staged_file 3 \"$MERGED\" \"$REMOTE\"\n \n+\tif test \"$(git config --bool mergetool.autoMerge)\" != \"false\"\n+\tthen\n+\t\tauto_merge\n+\tfi\n+\n \tif test -z \"$local_mode\" || test -z \"$remote_mode\"\n \tthen\n \t\techo \"Deleted merge conflict for '$MERGED':\"\ndiff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh\nindex 70afdd06fa..ccabd04823 100755\n--- a/t/t7610-mergetool.sh\n+++ b/t/t7610-mergetool.sh\n@@ -828,4 +828,22 @@ test_expect_success 'mergetool -Oorder-file is honored' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'mergetool automerge' '\n+\ttest_config mergetool.automerge true &&\n+\ttest_when_finished \"git reset --hard\" &&\n+\tgit checkout -b test${test_count}_b master &&\n+\ttest_write_lines >file1 base \"\" a &&\n+\tgit commit -a -m \"base\" &&\n+\ttest_write_lines >file1 base \"\" c &&\n+\tgit commit -a -m \"remote update\" &&\n+\tgit checkout -b test${test_count}_a HEAD~ &&\n+\ttest_write_lines >file1 local \"\" b &&\n+\tgit commit -a -m \"local update\" &&\n+\ttest_must_fail git merge test${test_count}_b &&\n+\tyes \"\" | git mergetool file1 &&\n+\ttest_write_lines >expect local \"\" c &&\n+\ttest_cmp expect file1 &&\n+\tgit commit -m \"test resolved with mergetool\"\n+'\n+\n test_done\n-- \n2.29.2\n\n\n"},{"id":"413047","messageId":"20201228045427.1166911-3-seth@eseth.com","threadId":"54871","inReplyTo":"20201228045427.1166911-1-seth@eseth.com","subject":"[PATCH v8 2/4] mergetool: Add per-tool support for the autoMerge flag","fromName":"Seth House","fromEmail":"seth@eseth.com","sentAt":"2020-12-28T04:54:25Z","receivedAt":"2020-12-28T04:55:59Z","isPatch":true,"sender":{"key":"seth@eseth.com","avatar":"https://avatars.githubusercontent.com/u/91293?v=4"},"body":"Keep the global mergetool flag and add a per-tool override flag so that\nusers may enable the flag for one tool and disable it for another.\n\nSigned-off-by: Seth House \n---\n Documentation/config/mergetool.txt | 3 +++\n git-mergetool.sh | 5 ++++-\n 2 files changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config/mergetool.txt b/Documentation/config/mergetool.txt\nindex 7ce6d0d3ac..ef147fc118 100644\n--- a/Documentation/config/mergetool.txt\n+++ b/Documentation/config/mergetool.txt\n@@ -21,6 +21,9 @@ mergetool..trustExitCode::\n \tif the file has been updated, otherwise the user is prompted to\n \tindicate the success of the merge.\n \n+mergetool..autoMerge::\n+\tRemove lines without conflicts from all the files. Defaults to `true`.\n+\n mergetool.meld.hasOutput::\n \tOlder versions of `meld` do not support the `--output` option.\n \tGit will attempt to detect whether `meld` supports `--output`\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex f4db0cac8d..e3c7d78d1d 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -334,7 +334,10 @@ merge_file () {\n \tcheckout_staged_file 2 \"$MERGED\" \"$LOCAL\"\n \tcheckout_staged_file 3 \"$MERGED\" \"$REMOTE\"\n \n-\tif test \"$(git config --bool mergetool.autoMerge)\" != \"false\"\n+\tif test \"$(\n+\t\tgit config --get --bool \"mergetool.$merge_tool.automerge\" ||\n+\t\tgit config --get --bool \"mergetool.automerge\" ||\n+\t\techo true)\" = true\n \tthen\n \t\tauto_merge\n \tfi\n-- \n2.29.2\n\n\n"},{"id":"413046","messageId":"20201228045427.1166911-5-seth@eseth.com","threadId":"54871","inReplyTo":"20201228045427.1166911-1-seth@eseth.com","subject":"[PATCH v8 4/4] mergetool: Add automerge_enabled tool-specific override function","fromName":"Seth House","fromEmail":"seth@eseth.com","sentAt":"2020-12-28T04:54:27Z","receivedAt":"2020-12-28T04:56:00Z","isPatch":true,"sender":{"key":"seth@eseth.com","avatar":"https://avatars.githubusercontent.com/u/91293?v=4"},"body":"Hat-tip to Junio C Hamano for the implementation.\n\nSigned-off-by: Seth House \n---\n git-mergetool--lib.sh | 4 ++++\n git-mergetool.sh | 2 +-\n 2 files changed, 5 insertions(+), 1 deletion(-)\n\ndiff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh\nindex e059b3559e..5084ceffeb 100644\n--- a/git-mergetool--lib.sh\n+++ b/git-mergetool--lib.sh\n@@ -164,6 +164,10 @@ setup_tool () {\n \t\treturn 1\n \t}\n \n+\tautomerge_enabled () {\n+\t\ttrue\n+\t}\n+\n \ttranslate_merge_tool_path () {\n \t\techo \"$1\"\n \t}\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex 929192d0f8..a44afd3822 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -336,7 +336,7 @@ merge_file () {\n \n \tinitialize_merge_tool \"$merge_tool\"\n \n-\tif test \"$(\n+\tif automerge_enabled && test \"$(\n \t\tgit config --get --bool \"mergetool.$merge_tool.automerge\" ||\n \t\tgit config --get --bool \"mergetool.automerge\" ||\n \t\techo true)\" = true\n-- \n2.29.2\n\n\n"},{"id":"413048","messageId":"20201228045427.1166911-4-seth@eseth.com","threadId":"54871","inReplyTo":"20201228045427.1166911-1-seth@eseth.com","subject":"[PATCH v8 3/4] mergetool: Break setup_tool out into separate initialization function","fromName":"Seth House","fromEmail":"seth@eseth.com","sentAt":"2020-12-28T04:54:26Z","receivedAt":"2020-12-28T04:56:00Z","isPatch":true,"sender":{"key":"seth@eseth.com","avatar":"https://avatars.githubusercontent.com/u/91293?v=4"},"body":"The tool-specific functions are sometimes needed in scope earlier than\nwhen run_merge_tool is called.\n\nSigned-off-by: Seth House \n---\n Documentation/git-mergetool--lib.txt | 4 ++++\n git-difftool--helper.sh | 2 ++\n git-mergetool--lib.sh | 7 ++++---\n git-mergetool.sh | 2 ++\n 4 files changed, 12 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-mergetool--lib.txt b/Documentation/git-mergetool--lib.txt\nindex 4da9d24096..3e8f59ac0e 100644\n--- a/Documentation/git-mergetool--lib.txt\n+++ b/Documentation/git-mergetool--lib.txt\n@@ -38,6 +38,10 @@ get_merge_tool_cmd::\n get_merge_tool_path::\n \treturns the custom path for a merge tool.\n \n+initialize_merge_tool::\n+\tbring merge tool specific functions into scope so they can be used or\n+\toverridden.\n+\n run_merge_tool::\n \tlaunches a merge tool given the tool name and a true/false\n \tflag to indicate whether a merge base is present.\ndiff --git a/git-difftool--helper.sh b/git-difftool--helper.sh\nindex 46af3e60b7..c47a6d4253 100755\n--- a/git-difftool--helper.sh\n+++ b/git-difftool--helper.sh\n@@ -61,6 +61,7 @@ launch_merge_tool () {\n \t\texport BASE\n \t\teval $GIT_DIFFTOOL_EXTCMD '\"$LOCAL\"' '\"$REMOTE\"'\n \telse\n+\t\tinitialize_merge_tool \"$merge_tool\"\n \t\trun_merge_tool \"$merge_tool\"\n \tfi\n }\n@@ -79,6 +80,7 @@ if test -n \"$GIT_DIFFTOOL_DIRDIFF\"\n then\n \tLOCAL=\"$1\"\n \tREMOTE=\"$2\"\n+\tinitialize_merge_tool \"$merge_tool\"\n \trun_merge_tool \"$merge_tool\" false\n else\n \t# Launch the merge tool on each path provided by 'git diff'\ndiff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh\nindex 7225abd811..e059b3559e 100644\n--- a/git-mergetool--lib.sh\n+++ b/git-mergetool--lib.sh\n@@ -248,6 +248,10 @@ trust_exit_code () {\n \tfi\n }\n \n+initialize_merge_tool () {\n+\t# Bring tool-specific functions into scope\n+\tsetup_tool \"$1\" || return 1\n+}\n \n # Entry point for running tools\n run_merge_tool () {\n@@ -259,9 +263,6 @@ run_merge_tool () {\n \tmerge_tool_path=$(get_merge_tool_path \"$1\") || exit\n \tbase_present=\"$2\"\n \n-\t# Bring tool-specific functions into scope\n-\tsetup_tool \"$1\" || return 1\n-\n \tif merge_mode\n \tthen\n \t\trun_merge_cmd \"$1\"\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex e3c7d78d1d..929192d0f8 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -334,6 +334,8 @@ merge_file () {\n \tcheckout_staged_file 2 \"$MERGED\" \"$LOCAL\"\n \tcheckout_staged_file 3 \"$MERGED\" \"$REMOTE\"\n \n+\tinitialize_merge_tool \"$merge_tool\"\n+\n \tif test \"$(\n \t\tgit config --get --bool \"mergetool.$merge_tool.automerge\" ||\n \t\tgit config --get --bool \"mergetool.automerge\" ||\n-- \n2.29.2\n\n\n"},{"id":"413049","messageId":"xmqqsg7qjk9q.fsf@gitster.c.googlers.com","threadId":"54871","inReplyTo":"20201228004152.522421-1-seth@eseth.com","subject":"Re: [PATCH v7 0/2] mergetool: add automerge configuration","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-12-28T10:29:53Z","receivedAt":"2020-12-28T10:31:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Seth House writes:\n\n> * Signed off on Felipe's commit. (Although I have minor qualms with\n> Felipe's various wording and even the name of the flag it is\n> decidedly not worth burdening the list with bike-shedding.)\n\nEven when the original is a horrible patch in your opinion that is\nladen with bugs, as long as the original author signed it off\n(which means that the original author certifies that it can be\nincluded in and distributed by the project under our licensing\nterms, and agrees to the fact that the original author did so will\nbe recorded in perpetuity), you can relay such a patch as-is, and\nyou are required (i.e. SubmittingPatches is pretty clear that\nwithout your sign-off we cannot accept) to sign it off to record\nthe provenance of the code.\n\nThe other side of the above coin is that you are not endorsing or\nvounching for the patch when you sign it off, so your name is not\nsmudged by wording and flag name chosen in a way that you may\nconsider poor. So \"Although...\" part is not a good objection\nagainst signing it off.\n\nIn other words, sign-off is not about assuring quality.\n\nAlso, instead of relaying as-is, you can relay a patch with your\nimprovements rolled into the same patch (i.e. not as follow-up\nfixes). Some (or major) parts of the original patch may still\nremain in the edited result and you'd need to keep original author's\nsign-off as-is [*1*].\n\nIn this topic's case, 2/2 would be a feature enhancement on top of\n1/2, so relaying 1/2 as-is would be OK, but in a case where an\npromising patch was sent with sign-off and bugs, then gets abandoned\nby the original author, fixing the bug in the patch you relay in\nplace (i.e. not as follow-up patches) may even be necessary to keep\nbisectability. When you do so, you'd typically do:\n\n\tSubject: [PATCH] title of the patch\n\n\t... original author's log message, possibly copyedited\n\t... by you\n\n+\t\n\n\tSigned-off-by: Original Author \n+\t[or brief comment here]\n+\tSigned-off-by: Your Name \n\n (1) add your sign-off at the end\n (2) explain what you changed relative to the original, either\n inside [] on the line before your sign-off, or at the end\n of the log message proper.\n\nto indicate that it is not relayed as-is; this allows you to take\nresponsibility of an unintended breakage your \"fixes\" might have\ncaused.\n\n\n[Footnote]\n\n*1* The result may become something that no longer aligns the\noriginal author's opinion, but that is OK. The sign-off by the\noriginal author just says that the original author has the right to\ncontribute (the remaining part of) the patch and the original author\nagrees that the record of author's involvement in the patch\n(including sign-off) will be kept. \n\nIt is not about assuring quality of the final work by the original\nauthor, either.\n"},{"id":"413065","messageId":"f9837d51-a0d4-0476-bc5e-8aa8cdb96b8e@kdbg.org","threadId":"54871","inReplyTo":"20201228045427.1166911-2-seth@eseth.com","subject":"Re: [PATCH v8 1/4] mergetool: add automerge configuration","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2020-12-28T11:30:18Z","receivedAt":"2020-12-28T11:31:04Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 28.12.20 um 05:54 schrieb Seth House:\n> diff --git a/Documentation/config/mergetool.txt b/Documentation/config/mergetool.txt\n> index 16a27443a3..7ce6d0d3ac 100644\n> --- a/Documentation/config/mergetool.txt\n> +++ b/Documentation/config/mergetool.txt\n> @@ -61,3 +61,6 @@ mergetool.writeToTemp::\n> \n> mergetool.prompt::\n> \tPrompt before each invocation of the merge resolution program.\n> +\n> +mergetool.autoMerge::\n> +\tRemove lines without conflicts from all the files. Defaults to `true`.\n\nThis text, starting with \"Remove lines\", sounds alarming. Isn't it more\nalong the lines of:\n\n\tConsolidate non-conflicting parts, so that only conflicting\n\tparts are presented to the merge tool. Defaults to `true`.\n\nIt would be great to keep the list of config entries sorted. I know that\nmergetool.prompt is not at the correct location, but that shouldn't be\nan excuse to make the situation worse.\n\n-- Hannes\n"},{"id":"413066","messageId":"cc8c5d6e-eee3-afdb-55cc-633455a5dcfa@kdbg.org","threadId":"54871","inReplyTo":"20201228045427.1166911-4-seth@eseth.com","subject":"Re: [PATCH v8 3/4] mergetool: Break setup_tool out into separate initialization function","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2020-12-28T11:48:29Z","receivedAt":"2020-12-28T11:49:13Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 28.12.20 um 05:54 schrieb Seth House:\n> The tool-specific functions are sometimes needed in scope earlier than\n> when run_merge_tool is called.\n\nYou should answer why this change is needed. \"are sometimes needed in\nscope earlier\" cannot be true, else we would have a bug that is fixed by\nthis change. But this isn't a bug fix, is it?\n\nIt would be ok to say \"We are going to add another thing that we will\nneed before run_merge_tool; this is a preparation\" or something.\n\nWhich brings me to another point: I do not see that something is added\nto initialize_merge_tool in a later patch. You are only replacing\nsetup_tool calls by initialize_merge_tool, which forwards to setup_tool.\nWhy do we need a new function?\n\n> \n> Signed-off-by: Seth House \n> ---\n> Documentation/git-mergetool--lib.txt | 4 ++++\n> git-difftool--helper.sh | 2 ++\n> git-mergetool--lib.sh | 7 ++++---\n> git-mergetool.sh | 2 ++\n> 4 files changed, 12 insertions(+), 3 deletions(-)\n> \n> diff --git a/Documentation/git-mergetool--lib.txt b/Documentation/git-mergetool--lib.txt\n> index 4da9d24096..3e8f59ac0e 100644\n> --- a/Documentation/git-mergetool--lib.txt\n> +++ b/Documentation/git-mergetool--lib.txt\n> @@ -38,6 +38,10 @@ get_merge_tool_cmd::\n> get_merge_tool_path::\n> \treturns the custom path for a merge tool.\n> \n> +initialize_merge_tool::\n> +\tbring merge tool specific functions into scope so they can be used or\n> +\toverridden.\n> +\n> run_merge_tool::\n> \tlaunches a merge tool given the tool name and a true/false\n> \tflag to indicate whether a merge base is present.\n\n[ swapped hunks for better sentence structure ]\n\n> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh\n> index 7225abd811..e059b3559e 100644\n> --- a/git-mergetool--lib.sh\n> +++ b/git-mergetool--lib.sh\n> @@ -248,6 +248,10 @@ trust_exit_code () {\n> \tfi\n> }\n>\n> +initialize_merge_tool () {\n> +\t# Bring tool-specific functions into scope\n> +\tsetup_tool \"$1\" || return 1\n> +}\n>\n> # Entry point for running tools\n> run_merge_tool () {\n> @@ -259,9 +263,6 @@ run_merge_tool () {\n> \tmerge_tool_path=$(get_merge_tool_path \"$1\") || exit\n> \tbase_present=\"$2\"\n>\n> -\t# Bring tool-specific functions into scope\n> -\tsetup_tool \"$1\" || return 1\n> -\n\nBefore this change, run_merge_tool would exit here when there was an\nerror during setup_tool. But...\n\n> diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh\n> index 46af3e60b7..c47a6d4253 100755\n> --- a/git-difftool--helper.sh\n> +++ b/git-difftool--helper.sh\n> @@ -61,6 +61,7 @@ launch_merge_tool () {\n> \t\texport BASE\n> \t\teval $GIT_DIFFTOOL_EXTCMD '\"$LOCAL\"' '\"$REMOTE\"'\n> \telse\n> +\t\tinitialize_merge_tool \"$merge_tool\"\n> \t\trun_merge_tool \"$merge_tool\"\n> \tfi\n\n... after the change we do not exit anymore. Does it matter?\n\nPerhaps\n\n\t\tinitialize_merge_tool \"$merge_tool\" &&\n \t\trun_merge_tool \"$merge_tool\"\n\n> }\n> @@ -79,6 +80,7 @@ if test -n \"$GIT_DIFFTOOL_DIRDIFF\"\n> then\n> \tLOCAL=\"$1\"\n> \tREMOTE=\"$2\"\n> +\tinitialize_merge_tool \"$merge_tool\"\n> \trun_merge_tool \"$merge_tool\" false\n> else\n> \t# Launch the merge tool on each path provided by 'git diff'\n\n\n> diff --git a/git-mergetool.sh b/git-mergetool.sh\n> index e3c7d78d1d..929192d0f8 100755\n> --- a/git-mergetool.sh\n> +++ b/git-mergetool.sh\n> @@ -334,6 +334,8 @@ merge_file () {\n> \tcheckout_staged_file 2 \"$MERGED\" \"$LOCAL\"\n> \tcheckout_staged_file 3 \"$MERGED\" \"$REMOTE\"\n> \n> +\tinitialize_merge_tool \"$merge_tool\"\n> +\n> \tif test \"$(\n> \t\tgit config --get --bool \"mergetool.$merge_tool.automerge\" ||\n> \t\tgit config --get --bool \"mergetool.automerge\" ||\n> \n\n"},{"id":"413067","messageId":"01f6951b-69e0-01c6-6b77-950ea9d76e66@kdbg.org","threadId":"54871","inReplyTo":"20201228045427.1166911-5-seth@eseth.com","subject":"Re: [PATCH v8 4/4] mergetool: Add automerge_enabled tool-specific override function","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2020-12-28T11:57:38Z","receivedAt":"2020-12-28T11:58:22Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 28.12.20 um 05:54 schrieb Seth House:\n> Hat-tip to Junio C Hamano for the implementation.\n\nWe usually write this as\n\nHelped-by: Junio C Hamano \n\n> \n> Signed-off-by: Seth House \n> ---\n> git-mergetool--lib.sh | 4 ++++\n> git-mergetool.sh | 2 +-\n> 2 files changed, 5 insertions(+), 1 deletion(-)\n> \n> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh\n> index e059b3559e..5084ceffeb 100644\n> --- a/git-mergetool--lib.sh\n> +++ b/git-mergetool--lib.sh\n> @@ -164,6 +164,10 @@ setup_tool () {\n> \t\treturn 1\n> \t}\n> \n> +\tautomerge_enabled () {\n> +\t\ttrue\n\nI would have written this as `return 0` instead of `true` like some of\nthe functions above this hunk.\n\n> +\t}\n> +\n> \ttranslate_merge_tool_path () {\n> \t\techo \"$1\"\n> \t}\n> diff --git a/git-mergetool.sh b/git-mergetool.sh\n> index 929192d0f8..a44afd3822 100755\n> --- a/git-mergetool.sh\n> +++ b/git-mergetool.sh\n> @@ -336,7 +336,7 @@ merge_file () {\n> \n> \tinitialize_merge_tool \"$merge_tool\"\n> \n> -\tif test \"$(\n> +\tif automerge_enabled && test \"$(\n> \t\tgit config --get --bool \"mergetool.$merge_tool.automerge\" ||\n> \t\tgit config --get --bool \"mergetool.automerge\" ||\n> \t\techo true)\" = true\n> \n\n-- Hannes\n"},{"id":"413069","messageId":"xmqqzh1yhyam.fsf@gitster.c.googlers.com","threadId":"54871","inReplyTo":"20201228045427.1166911-3-seth@eseth.com","subject":"Re: [PATCH v8 2/4] mergetool: Add per-tool support for the autoMerge flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-12-28T13:09:53Z","receivedAt":"2020-12-28T13:10:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\n> Subject: Re: [PATCH v8 2/4] mergetool: Add per-tool support for the autoMerge flag\n\n\"git shortlog --no-merges --since=2.months\" may tell you this but our\nconvention is not to capitalize the word after \":\" on the\ntitle.\n\n> Keep the global mergetool flag and add a per-tool override flag so that\n> users may enable the flag for one tool and disable it for another.\n>\n> Signed-off-by: Seth House \n> ---\n> Documentation/config/mergetool.txt | 3 +++\n> git-mergetool.sh | 5 ++++-\n> 2 files changed, 7 insertions(+), 1 deletion(-)\n>\n> diff --git a/Documentation/config/mergetool.txt b/Documentation/config/mergetool.txt\n> index 7ce6d0d3ac..ef147fc118 100644\n> --- a/Documentation/config/mergetool.txt\n> +++ b/Documentation/config/mergetool.txt\n> @@ -21,6 +21,9 @@ mergetool..trustExitCode::\n> \tif the file has been updated, otherwise the user is prompted to\n> \tindicate the success of the merge.\n> \n> +mergetool..autoMerge::\n> +\tRemove lines without conflicts from all the files. Defaults to `true`.\n> +\n\nThis entry needs to mention how it relates to the big red button\nmergetool.autoMerge and vice versa. E.g.\n\n mergetool.autoMerge::\n-\tRemove lines without conflicts from all the files. Defaults to `true`.\n+\tRemove lines without conflicts from all the files. Can be\n+\toverriden per-tool via `mergetool..autoMerge` configuration\n+\tvariable. Defaults to `true`.\n\nwould be a good update to the documentation introduced by the\nprevious step. It is somewhat misleading for the per-tool entry\nadded in this patch to say \"Defaults to `true`\", as the value of the\nbig red button configuration would be the real default.\n\n\tRemove ... the files when the mergetool '' is in use.\n\tSee also `mergetool.autoMerge`.\n\nor something like that, perhaps.\n"},{"id":"413071","messageId":"5fe9ee6d58c2c_209d20891@natae.notmuch","threadId":"54871","inReplyTo":"xmqqsg7qjk9q.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v7 0/2] mergetool: add automerge configuration","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-28T14:40:45Z","receivedAt":"2020-12-28T14:41:39Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Junio C Hamano wrote:\n> Also, instead of relaying as-is, you can relay a patch with your\n> improvements rolled into the same patch (i.e. not as follow-up\n> fixes). Some (or major) parts of the original patch may still\n> remain in the edited result and you'd need to keep original author's\n> sign-off as-is [*1*].\n\nYes, you *can*, but doing so in this case would be against the author's\nwishes, and a violation of the Developer Certificate of Origin.\n\n> In this topic's case, 2/2 would be a feature enhancement on top of\n> 1/2, so relaying 1/2 as-is would be OK, but in a case where an\n> promising patch was sent with sign-off and bugs, then gets abandoned\n> by the original author, fixing the bug in the patch you relay in\n> place (i.e. not as follow-up patches) may even be necessary to keep\n> bisectability. When you do so, you'd typically do:\n\nIf the author doesn't object (which is usually the case), this makes\nsense.\n\nBut if the author objects, you would be violating clause (d) of the DCO.\n\nJust because the only way to do X is to violate laws, terms, or\nagreements doesn't mean that's what you should do. You can simply not do\nX.\n\n-- \nFelipe Contreras\n"},{"id":"413075","messageId":"xmqqtus6hxty.fsf@gitster.c.googlers.com","threadId":"54871","inReplyTo":"20201228045427.1166911-5-seth@eseth.com","subject":"Re: [PATCH v8 4/4] mergetool: Add automerge_enabled tool-specific override function","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-12-28T13:19:53Z","receivedAt":"2020-12-28T16:11:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Seth House writes:\n\n> Hat-tip to Junio C Hamano for the implementation.\n\nThat is the least interesting thing the log message for this commit\ncan talk about. Instead readers should be able to learn things like\nthese from the log message (I am not saying the log message should\nhave these in an enumerated list; I am just enumerating these as\nsamples):\n\n - Why does this exist? \n\n - What does a tool author want to use the mechanism for, and how\n does the tool author use it? \n\n - This mechanism allows tool authors to say \"never allow autoMerge\n for this tool\", but there is no provision to let them say \"always\n use autoMerge without allowing users to turn it off\".\n\nIt also needs a bit of documentation update to mention that\nindividual mergetool backend can choose not to trigger the feature\nat all, even if the user configures it with mergetool.autoMerge and\nmergetool..autoMerge options.\n\nThanks.\n\n> Signed-off-by: Seth House \n> ---\n> git-mergetool--lib.sh | 4 ++++\n> git-mergetool.sh | 2 +-\n> 2 files changed, 5 insertions(+), 1 deletion(-)\n>\n> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh\n> index e059b3559e..5084ceffeb 100644\n> --- a/git-mergetool--lib.sh\n> +++ b/git-mergetool--lib.sh\n> @@ -164,6 +164,10 @@ setup_tool () {\n> \t\treturn 1\n> \t}\n> \n> +\tautomerge_enabled () {\n> +\t\ttrue\n> +\t}\n> +\n> \ttranslate_merge_tool_path () {\n> \t\techo \"$1\"\n> \t}\n> diff --git a/git-mergetool.sh b/git-mergetool.sh\n> index 929192d0f8..a44afd3822 100755\n> --- a/git-mergetool.sh\n> +++ b/git-mergetool.sh\n> @@ -336,7 +336,7 @@ merge_file () {\n> \n> \tinitialize_merge_tool \"$merge_tool\"\n> \n> -\tif test \"$(\n> +\tif automerge_enabled && test \"$(\n> \t\tgit config --get --bool \"mergetool.$merge_tool.automerge\" ||\n> \t\tgit config --get --bool \"mergetool.automerge\" ||\n> \t\techo true)\" = true\n"},{"id":"413102","messageId":"20201228192919.1195211-6-seth@eseth.com","threadId":"54871","inReplyTo":"20201228192919.1195211-1-seth@eseth.com","subject":"[PATCH v9 5/5] mergetool: add automerge_enabled tool-specific override function","fromName":"Seth House","fromEmail":"seth@eseth.com","sentAt":"2020-12-28T19:29:19Z","receivedAt":"2020-12-28T23:23:04Z","isPatch":true,"sender":{"key":"seth@eseth.com","avatar":"https://avatars.githubusercontent.com/u/91293?v=4"},"body":"The author or maintainer of a mergetool may optionally elect disable (or\nenable) the `autoMerge` feature for that mergetool even if the user has\nchosen differently using the `mergetool.autoMerge` and\n`mergetool..autoMerge` options.\n\nTo add a tool-specific override, edit the `mergetools/` shell\nscript for that tool and add an `automerge_enabled` function:\n\n automerge_enabled () {\n return 1\n }\n\nDisabling may be desirable if the mergetool wants or needs access to the\noriginal, unmodified 'LOCAL', 'REMOTE', and 'BASE' versions of the\nconflicted file. For example:\n\n- A tool may use a custom conflict resolution algorithm and prefer to\n ignore the results of Git's conflict resolution.\n- A tool may want to visually compare/constrast the version of the file\n from before the merge (saved to 'LOCAL', 'REMOTE', and 'BASE') with\n Git's conflict resolution results (saved to 'MERGED').\n- A student or researcher working on a new algorithm may want to\n directly compare the result of that algorithm with the result of Git's\n algorithm.\n\nHelped-by: Junio C Hamano \nSigned-off-by: Seth House \n---\n git-mergetool--lib.sh | 4 ++++\n git-mergetool.sh | 2 +-\n 2 files changed, 5 insertions(+), 1 deletion(-)\n\ndiff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh\nindex e059b3559e..567991abbc 100644\n--- a/git-mergetool--lib.sh\n+++ b/git-mergetool--lib.sh\n@@ -164,6 +164,10 @@ setup_tool () {\n \t\treturn 1\n \t}\n \n+\tautomerge_enabled () {\n+\t\treturn 0\n+\t}\n+\n \ttranslate_merge_tool_path () {\n \t\techo \"$1\"\n \t}\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex 929192d0f8..a44afd3822 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -336,7 +336,7 @@ merge_file () {\n \n \tinitialize_merge_tool \"$merge_tool\"\n \n-\tif test \"$(\n+\tif automerge_enabled && test \"$(\n \t\tgit config --get --bool \"mergetool.$merge_tool.automerge\" ||\n \t\tgit config --get --bool \"mergetool.automerge\" ||\n \t\techo true)\" = true\n-- \n2.29.2\n\n\n"},{"id":"413104","messageId":"20201228192919.1195211-4-seth@eseth.com","threadId":"54871","inReplyTo":"20201228192919.1195211-1-seth@eseth.com","subject":"[PATCH v9 3/5] mergetool: add per-tool support for the autoMerge flag","fromName":"Seth House","fromEmail":"seth@eseth.com","sentAt":"2020-12-28T19:29:17Z","receivedAt":"2020-12-28T23:23:05Z","isPatch":true,"sender":{"key":"seth@eseth.com","avatar":"https://avatars.githubusercontent.com/u/91293?v=4"},"body":"Keep the global mergetool flag and add a per-tool override flag so that\nusers may enable the flag for one tool and disable it for another.\n\nHelped-by: Junio C Hamano \nHelped-by: Johannes Sixt \nSigned-off-by: Seth House \n---\n Documentation/config/mergetool.txt | 15 ++++++++++++++-\n git-mergetool.sh | 5 ++++-\n 2 files changed, 18 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config/mergetool.txt b/Documentation/config/mergetool.txt\nindex 3291fa7102..bde472d49a 100644\n--- a/Documentation/config/mergetool.txt\n+++ b/Documentation/config/mergetool.txt\n@@ -1,3 +1,9 @@\n+mergetool..autoMerge::\n+\tA mergetool-specific override for the global `mergetool.autoMerge`\n+\tconfiguration flag. This allows individual mergetools to enable or\n+\tdisable the flag regardless of the global setting. See\n+\t`mergetool.autoMerge` for the full description.\n+\n mergetool..cmd::\n \tSpecify the command to invoke the specified merge tool. The\n \tspecified command is evaluated in shell with the following\n@@ -41,7 +47,14 @@ mergetool.meld.useAutoMerge::\n \tdefault value.\n \n mergetool.autoMerge::\n-\tRemove lines without conflicts from all the files. Defaults to `true`.\n+\tDuring a merge Git will automatically resolve as many conflicts as\n+\tpossible and then wrap conflict markers around any conflicts that it\n+\tcannot resolve. This flag consolidates the non-conflicting parts into\n+\tthe corresponding 'LOCAL' and 'REMOTE' files so that only the\n+\tunresolved conflicts are presented to the merge tool. Can be overriden\n+\tper-tool via the `mergetool..autoMerge` configuration variable.\n+\tNote: individual mergetool scripts can elect to ignore user preferences\n+\tentirely. Defaults to `true`.\n \n mergetool.keepBackup::\n \tAfter performing a merge, the original file with conflict markers\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex f4db0cac8d..e3c7d78d1d 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -334,7 +334,10 @@ merge_file () {\n \tcheckout_staged_file 2 \"$MERGED\" \"$LOCAL\"\n \tcheckout_staged_file 3 \"$MERGED\" \"$REMOTE\"\n \n-\tif test \"$(git config --bool mergetool.autoMerge)\" != \"false\"\n+\tif test \"$(\n+\t\tgit config --get --bool \"mergetool.$merge_tool.automerge\" ||\n+\t\tgit config --get --bool \"mergetool.automerge\" ||\n+\t\techo true)\" = true\n \tthen\n \t\tauto_merge\n \tfi\n-- \n2.29.2\n\n\n"},{"id":"413106","messageId":"20201228192919.1195211-1-seth@eseth.com","threadId":"54871","inReplyTo":"20201228045427.1166911-1-seth@eseth.com","subject":"[PATCH v9 0/5] mergetool: add automerge configuration","fromName":"Seth House","fromEmail":"seth@eseth.com","sentAt":"2020-12-28T19:29:14Z","receivedAt":"2020-12-28T23:23:05Z","isPatch":true,"sender":{"key":"seth@eseth.com","avatar":"https://avatars.githubusercontent.com/u/91293?v=4"},"body":"Thank you Junio and Johannes for patiently pointing out all my newbie\nmistakes.\n\nChanges since v8:\n\n- Improve documentation of what the autoMerge flag does.\n- Add documentation note for the `automerge_enabled` override.\n\n - Junio, is there another place this should also live?\n\n- Cross-reference merge.autoMerge and merge..autoMerge docs.\n- Sort the list of mergetool options. Added as a standalone commit in\n case this change doesn't belong as part of this patchset. I'd be happy\n to move this to a standalone patch if that's preferable.\n- Improve all commit messages with full explanations and rationale:\n\n - Johannes, I didn't directly respond to your `initialize_merge_tool`\n question because you're right that explanation should be part of the\n commit message. Please let me know if that now answers your question\n and if not we can discuss further in a thread.\n\n- Fix omitted exit codes when running `initialize_merge_tool`.\n- Fix `automerge_enabled` return value for consistency.\n- Update commit message capitalization to conform to repo norms.\n- Rephrase commit message 'thanks' as Helped-by markers.\n\nFelipe Contreras (1):\n mergetool: add automerge configuration\n\nSeth House (4):\n mergetool: alphabetize the mergetool config docs\n mergetool: add per-tool support for the autoMerge flag\n mergetool: break setup_tool out into separate initialization function\n mergetool: add automerge_enabled tool-specific override function\n\n Documentation/config/mergetool.txt | 28 ++++++++++++++++++++++------\n Documentation/git-mergetool--lib.txt | 4 ++++\n git-difftool--helper.sh | 2 ++\n git-mergetool--lib.sh | 11 ++++++++---\n git-mergetool.sh | 22 ++++++++++++++++++++++\n t/t7610-mergetool.sh | 18 ++++++++++++++++++\n 6 files changed, 76 insertions(+), 9 deletions(-)\n\n-- \n2.29.2\n\n\n"},{"id":"413110","messageId":"20201228192919.1195211-5-seth@eseth.com","threadId":"54871","inReplyTo":"20201228192919.1195211-1-seth@eseth.com","subject":"[PATCH v9 4/5] mergetool: break setup_tool out into separate initialization function","fromName":"Seth House","fromEmail":"seth@eseth.com","sentAt":"2020-12-28T19:29:18Z","receivedAt":"2020-12-28T23:23:05Z","isPatch":true,"sender":{"key":"seth@eseth.com","avatar":"https://avatars.githubusercontent.com/u/91293?v=4"},"body":"This is preparation for the following commit where we need to source the\nmergetool shell script to look for overrides before `run_merge_tool` is\ncalled. Previously `run_merge_tool` both sourced that script and invoked\nthe mergetool.\n\nIn the case of the following commit, we need the result of the\n`automerge_enabled` override, if it exists, well before we actually run\n`run_merge_tool`.\n\nA new function `initialize_merge_tool` was chosen for consistency with\n`run_merge_tool` since `setup_tool` and `setup_user_tool` are not\nexposed or called directly.\n\nSigned-off-by: Seth House \n---\n Documentation/git-mergetool--lib.txt | 4 ++++\n git-difftool--helper.sh | 2 ++\n git-mergetool--lib.sh | 7 ++++---\n git-mergetool.sh | 2 ++\n 4 files changed, 12 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-mergetool--lib.txt b/Documentation/git-mergetool--lib.txt\nindex 4da9d24096..3e8f59ac0e 100644\n--- a/Documentation/git-mergetool--lib.txt\n+++ b/Documentation/git-mergetool--lib.txt\n@@ -38,6 +38,10 @@ get_merge_tool_cmd::\n get_merge_tool_path::\n \treturns the custom path for a merge tool.\n \n+initialize_merge_tool::\n+\tbring merge tool specific functions into scope so they can be used or\n+\toverridden.\n+\n run_merge_tool::\n \tlaunches a merge tool given the tool name and a true/false\n \tflag to indicate whether a merge base is present.\ndiff --git a/git-difftool--helper.sh b/git-difftool--helper.sh\nindex 46af3e60b7..234dd6944e 100755\n--- a/git-difftool--helper.sh\n+++ b/git-difftool--helper.sh\n@@ -61,6 +61,7 @@ launch_merge_tool () {\n \t\texport BASE\n \t\teval $GIT_DIFFTOOL_EXTCMD '\"$LOCAL\"' '\"$REMOTE\"'\n \telse\n+\t\tinitialize_merge_tool \"$merge_tool\" &&\n \t\trun_merge_tool \"$merge_tool\"\n \tfi\n }\n@@ -79,6 +80,7 @@ if test -n \"$GIT_DIFFTOOL_DIRDIFF\"\n then\n \tLOCAL=\"$1\"\n \tREMOTE=\"$2\"\n+\tinitialize_merge_tool \"$merge_tool\" &&\n \trun_merge_tool \"$merge_tool\" false\n else\n \t# Launch the merge tool on each path provided by 'git diff'\ndiff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh\nindex 7225abd811..e059b3559e 100644\n--- a/git-mergetool--lib.sh\n+++ b/git-mergetool--lib.sh\n@@ -248,6 +248,10 @@ trust_exit_code () {\n \tfi\n }\n \n+initialize_merge_tool () {\n+\t# Bring tool-specific functions into scope\n+\tsetup_tool \"$1\" || return 1\n+}\n \n # Entry point for running tools\n run_merge_tool () {\n@@ -259,9 +263,6 @@ run_merge_tool () {\n \tmerge_tool_path=$(get_merge_tool_path \"$1\") || exit\n \tbase_present=\"$2\"\n \n-\t# Bring tool-specific functions into scope\n-\tsetup_tool \"$1\" || return 1\n-\n \tif merge_mode\n \tthen\n \t\trun_merge_cmd \"$1\"\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex e3c7d78d1d..929192d0f8 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -334,6 +334,8 @@ merge_file () {\n \tcheckout_staged_file 2 \"$MERGED\" \"$LOCAL\"\n \tcheckout_staged_file 3 \"$MERGED\" \"$REMOTE\"\n \n+\tinitialize_merge_tool \"$merge_tool\"\n+\n \tif test \"$(\n \t\tgit config --get --bool \"mergetool.$merge_tool.automerge\" ||\n \t\tgit config --get --bool \"mergetool.automerge\" ||\n-- \n2.29.2\n\n\n"},{"id":"413105","messageId":"20201228192919.1195211-3-seth@eseth.com","threadId":"54871","inReplyTo":"20201228192919.1195211-1-seth@eseth.com","subject":"[PATCH v9 2/5] mergetool: alphabetize the mergetool config docs","fromName":"Seth House","fromEmail":"seth@eseth.com","sentAt":"2020-12-28T19:29:16Z","receivedAt":"2020-12-28T23:23:06Z","isPatch":true,"sender":{"key":"seth@eseth.com","avatar":"https://avatars.githubusercontent.com/u/91293?v=4"},"body":"The ordering in this file has drifted a little. Let's make things better\nwhile we're adding new entres. :)\n\nSigned-off-by: Seth House \n---\n Documentation/config/mergetool.txt | 20 ++++++++++----------\n 1 file changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/config/mergetool.txt b/Documentation/config/mergetool.txt\nindex 7ce6d0d3ac..3291fa7102 100644\n--- a/Documentation/config/mergetool.txt\n+++ b/Documentation/config/mergetool.txt\n@@ -1,7 +1,3 @@\n-mergetool..path::\n-\tOverride the path for the given tool. This is useful in case\n-\tyour tool is not in the PATH.\n-\n mergetool..cmd::\n \tSpecify the command to invoke the specified merge tool. The\n \tspecified command is evaluated in shell with the following\n@@ -13,6 +9,10 @@ mergetool..cmd::\n \tmerged; 'MERGED' contains the name of the file to which the merge\n \ttool should write the results of a successful merge.\n \n+mergetool..path::\n+\tOverride the path for the given tool. This is useful in case\n+\tyour tool is not in the PATH.\n+\n mergetool..trustExitCode::\n \tFor a custom merge command, specify whether the exit code of\n \tthe merge command can be used to determine whether the merge was\n@@ -40,6 +40,9 @@ mergetool.meld.useAutoMerge::\n \tvalue of `false` avoids using `--auto-merge` altogether, and is the\n \tdefault value.\n \n+mergetool.autoMerge::\n+\tRemove lines without conflicts from all the files. Defaults to `true`.\n+\n mergetool.keepBackup::\n \tAfter performing a merge, the original file with conflict markers\n \tcan be saved as a file with a `.orig` extension. If this variable\n@@ -53,14 +56,11 @@ mergetool.keepTemporaries::\n \tpreserved, otherwise they will be removed after the tool has\n \texited. Defaults to `false`.\n \n+mergetool.prompt::\n+\tPrompt before each invocation of the merge resolution program.\n+\n mergetool.writeToTemp::\n \tGit writes temporary 'BASE', 'LOCAL', and 'REMOTE' versions of\n \tconflicting files in the worktree by default. Git will attempt\n \tto use a temporary directory for these files when set `true`.\n \tDefaults to `false`.\n-\n-mergetool.prompt::\n-\tPrompt before each invocation of the merge resolution program.\n-\n-mergetool.autoMerge::\n-\tRemove lines without conflicts from all the files. Defaults to `true`.\n-- \n2.29.2\n\n\n"},{"id":"413112","messageId":"20201228192919.1195211-2-seth@eseth.com","threadId":"54871","inReplyTo":"20201228192919.1195211-1-seth@eseth.com","subject":"[PATCH v9 1/5] mergetool: add automerge configuration","fromName":"Seth House","fromEmail":"seth@eseth.com","sentAt":"2020-12-28T19:29:15Z","receivedAt":"2020-12-28T23:23:06Z","isPatch":true,"sender":{"key":"seth@eseth.com","avatar":"https://avatars.githubusercontent.com/u/91293?v=4"},"body":"From: Felipe Contreras \n\nThe purpose of mergetools is to resolve conflicts when git cannot\nautomatically do so.\n\nIn order to do that git has added markers in the specific areas that\nneed resolving, which the user must manually fix. The tool is supposed\nto help with that.\n\nHowever, by passing the original BASE, LOCAL, and REMOTE files, many\nchanges without conflict are presented to the user when in fact nothing\nneeds to be done for those.\n\nWe can fix that by propagating the final version of the file with the\nautomatic merge to all the panes of the mergetool (BASE, LOCAL, and\nREMOTE), and only make them differ on the places where there are actual\nconflicts.\n\nAs most people will want the new behavior, we enable it by default.\nUsers that do not want the new behavior can set the new configuration\nmergetool.autoMerge to false.\n\nSee Seth House's blog post [1] for the idea, and the rationale.\n\n[1] https://www.eseth.org/2020/mergetools.html\n\nOriginal-idea-by: Seth House \nSigned-off-by: Felipe Contreras \nSigned-off-by: Seth House \n---\n Documentation/config/mergetool.txt | 3 +++\n git-mergetool.sh | 17 +++++++++++++++++\n t/t7610-mergetool.sh | 18 ++++++++++++++++++\n 3 files changed, 38 insertions(+)\n\ndiff --git a/Documentation/config/mergetool.txt b/Documentation/config/mergetool.txt\nindex 16a27443a3..7ce6d0d3ac 100644\n--- a/Documentation/config/mergetool.txt\n+++ b/Documentation/config/mergetool.txt\n@@ -61,3 +61,6 @@ mergetool.writeToTemp::\n \n mergetool.prompt::\n \tPrompt before each invocation of the merge resolution program.\n+\n+mergetool.autoMerge::\n+\tRemove lines without conflicts from all the files. Defaults to `true`.\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex e3f6d543fb..f4db0cac8d 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -239,6 +239,17 @@ checkout_staged_file () {\n \tfi\n }\n \n+auto_merge () {\n+\tgit merge-file --diff3 --marker-size=7 -q -p \"$LOCAL\" \"$BASE\" \"$REMOTE\" >\"$DIFF3\"\n+\tif test -s \"$DIFF3\"\n+\tthen\n+\t\tsed -e '/^<<<<<<< /,/^||||||| /d' -e '/^=======\\r\\?$/,/^>>>>>>> /d' \"$DIFF3\" >\"$BASE\"\n+\t\tsed -e '/^||||||| /,/^>>>>>>> /d' -e '/^<<<<<<< /d' \"$DIFF3\" >\"$LOCAL\"\n+\t\tsed -e '/^<<<<<<< /,/^=======\\r\\?$/d' -e '/^>>>>>>> /d' \"$DIFF3\" >\"$REMOTE\"\n+\tfi\n+\trm -- \"$DIFF3\"\n+}\n+\n merge_file () {\n \tMERGED=\"$1\"\n \n@@ -274,6 +285,7 @@ merge_file () {\n \t\tBASE=${BASE##*/}\n \tfi\n \n+\tDIFF3=\"$MERGETOOL_TMPDIR/${BASE}_DIFF3_$$$ext\"\n \tBACKUP=\"$MERGETOOL_TMPDIR/${BASE}_BACKUP_$$$ext\"\n \tLOCAL=\"$MERGETOOL_TMPDIR/${BASE}_LOCAL_$$$ext\"\n \tREMOTE=\"$MERGETOOL_TMPDIR/${BASE}_REMOTE_$$$ext\"\n@@ -322,6 +334,11 @@ merge_file () {\n \tcheckout_staged_file 2 \"$MERGED\" \"$LOCAL\"\n \tcheckout_staged_file 3 \"$MERGED\" \"$REMOTE\"\n \n+\tif test \"$(git config --bool mergetool.autoMerge)\" != \"false\"\n+\tthen\n+\t\tauto_merge\n+\tfi\n+\n \tif test -z \"$local_mode\" || test -z \"$remote_mode\"\n \tthen\n \t\techo \"Deleted merge conflict for '$MERGED':\"\ndiff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh\nindex 70afdd06fa..ccabd04823 100755\n--- a/t/t7610-mergetool.sh\n+++ b/t/t7610-mergetool.sh\n@@ -828,4 +828,22 @@ test_expect_success 'mergetool -Oorder-file is honored' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'mergetool automerge' '\n+\ttest_config mergetool.automerge true &&\n+\ttest_when_finished \"git reset --hard\" &&\n+\tgit checkout -b test${test_count}_b master &&\n+\ttest_write_lines >file1 base \"\" a &&\n+\tgit commit -a -m \"base\" &&\n+\ttest_write_lines >file1 base \"\" c &&\n+\tgit commit -a -m \"remote update\" &&\n+\tgit checkout -b test${test_count}_a HEAD~ &&\n+\ttest_write_lines >file1 local \"\" b &&\n+\tgit commit -a -m \"local update\" &&\n+\ttest_must_fail git merge test${test_count}_b &&\n+\tyes \"\" | git mergetool file1 &&\n+\ttest_write_lines >expect local \"\" c &&\n+\ttest_cmp expect file1 &&\n+\tgit commit -m \"test resolved with mergetool\"\n+'\n+\n test_done\n-- \n2.29.2\n\n\n"},{"id":"413117","messageId":"5fea8e001e8a2_27555208fd@natae.notmuch","threadId":"54871","inReplyTo":"20201228192919.1195211-6-seth@eseth.com","subject":"RE: [PATCH v9 5/5] mergetool: add automerge_enabled tool-specific override function","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-29T02:01:36Z","receivedAt":"2020-12-29T02:02:28Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Seth House wrote:\n> Disabling may be desirable if the mergetool wants or needs access to the\n> original, unmodified 'LOCAL', 'REMOTE', and 'BASE' versions of the\n> conflicted file. For example:\n> \n> - A tool may use a custom conflict resolution algorithm and prefer to\n> ignore the results of Git's conflict resolution.\n\nIf git's conflict resolution decides there are no conflicts, how would\nsuch tool \"ignore\" that?\n\n> - A tool may want to visually compare/constrast the version of the file\n> from before the merge (saved to 'LOCAL', 'REMOTE', and 'BASE') with\n> Git's conflict resolution results (saved to 'MERGED').\n\nCan't such tool use \"git checkout-index\" for that?\n\n> - A student or researcher working on a new algorithm may want to\n> directly compare the result of that algorithm with the result of Git's\n> algorithm.\n\n 1. If git's algorithm decides there are no conflicts, and the new\n algorithm decides there are conflicts, how would such researcher\n find that out?\n\n 2. Can't such researcher simply do:\n git -c mergetool.automerge=false mergetool?\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"413124","messageId":"3eb8c8d9-5b56-82ff-21d8-725a6c269f66@kdbg.org","threadId":"54871","inReplyTo":"20201228192919.1195211-5-seth@eseth.com","subject":"Re: [PATCH v9 4/5] mergetool: break setup_tool out into separate initialization function","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2020-12-29T08:50:44Z","receivedAt":"2020-12-29T08:51:32Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 28.12.20 um 20:29 schrieb Seth House:\n> diff --git a/git-mergetool.sh b/git-mergetool.sh\n> index e3c7d78d1d..929192d0f8 100755\n> --- a/git-mergetool.sh\n> +++ b/git-mergetool.sh\n> @@ -334,6 +334,8 @@ merge_file () {\n> \tcheckout_staged_file 2 \"$MERGED\" \"$LOCAL\"\n> \tcheckout_staged_file 3 \"$MERGED\" \"$REMOTE\"\n> \n> +\tinitialize_merge_tool \"$merge_tool\"\n\nIn my earlier review, I was not explicit about the lack of error\nhandling of this invocation, because I hoped that you would notice it\nyourself. `initialize_merge_tool` does have a few failure modes via\n`setup_tool` that are not really unlikely; ignoring errors would be\nwrong, I think.\n\nBefore this change, the errors would be handled as part of the failing\n`run_merge_tool` call at the end of function `merge_file`. But now\nerrors are ignored. Just appending `|| return` would not be appropriate\nat this point because a lot has already happened before the call that\nhas to be rewound. Would it be possible to move the call above the\n`mergetool_tmpdir_init` call, so that nothing has to be rewound?\n\n> +\n> \tif test \"$(\n> \t\tgit config --get --bool \"mergetool.$merge_tool.automerge\" ||\n> \t\tgit config --get --bool \"mergetool.automerge\" ||\n> \n\n-- Hannes\n"},{"id":"413134","messageId":"20201229172349.GA17517@ellen","threadId":"54871","inReplyTo":"3eb8c8d9-5b56-82ff-21d8-725a6c269f66@kdbg.org","subject":"Re: [PATCH v9 4/5] mergetool: break setup_tool out into separate initialization function","fromName":"Seth House","fromEmail":"seth@eseth.com","sentAt":"2020-12-29T17:23:49Z","receivedAt":"2020-12-29T17:24:51Z","isPatch":true,"sender":{"key":"seth@eseth.com","avatar":"https://avatars.githubusercontent.com/u/91293?v=4"},"body":"On Tue, Dec 29, 2020 at 09:50:44AM +0100, Johannes Sixt wrote:\n> Would it be possible to move the call above the\n> `mergetool_tmpdir_init` call, so that nothing has to be rewound?\n\nAh, I see. Good suggestion. Yes, moving that call higher works just\nfine. E.g.:\n\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex a44afd3822..36c1920dd6 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -276,6 +276,8 @@ merge_file () {\n \t\text=\n \tesac\n \n+\tinitialize_merge_tool \"$merge_tool\" || return\n+\n \tmergetool_tmpdir_init\n \n \tif test \"$MERGETOOL_TMPDIR\" != \".\"\n@@ -334,8 +336,6 @@ merge_file () {\n \tcheckout_staged_file 2 \"$MERGED\" \"$LOCAL\"\n \tcheckout_staged_file 3 \"$MERGED\" \"$REMOTE\"\n \n-\tinitialize_merge_tool \"$merge_tool\"\n-\n \tif automerge_enabled && test \"$(\n \t\tgit config --get --bool \"mergetool.$merge_tool.automerge\" ||\n \t\tgit config --get --bool \"mergetool.automerge\" ||\n\nThanks. I'll roll that change into a v10 patch series later today or\ntomorrow to give a little more time for any other feedback.\n\n"},{"id":"413202","messageId":"nycvar.QRO.7.76.6.2012300645400.56@tvgsbejvaqbjf.bet","threadId":"54871","inReplyTo":"xmqqim8r36ba.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v5 0/1] mergetool: remove unconflicted lines","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-12-30T05:47:47Z","receivedAt":"2020-12-30T22:15:33Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Thu, 24 Dec 2020, Junio C Hamano wrote:\n\n> Felipe Contreras writes:\n>\n> > There's not much to say other that what the commit message of the patch says.\n> >\n> > Note: no feedback has been ignored; I replied to all the feedback, I didn't hear anything back.\n> >\n> > Changes since v4:\n> >\n> > * Improved commit message with suggestions from Phillip Wood.\n> >\n> > Felipe Contreras (1):\n> > mergetool: add automerge configuration\n>\n> This breakage is possibly a fallout from either this patch or\n> 1e2ae142 (t7[5-9]*: adjust the references to the default branch name\n> \"main\", 2020-11-18).\n>\n> https://github.com/git/git/runs/1602803804#step:7:10358\n>\n> I cannot quite tell how the two strings compared with 'test' on\n> output line 10355 are different in the output, though.\n\nI spent more time than I cared to spend on this, and still have not quite\nfigured out what is the fault, but I can state with conviction that the\nproblem is not even introduced by any merge into `seen`. The\n`fc/mergetool-automerge` branch itself is already broken:\nhttps://github.com/gitgitgadget/git/actions/runs/441233234\n\nCiao,\nDscho\n"},{"id":"413203","messageId":"5fed0053494bd_8cde92081e@natae.notmuch","threadId":"54871","inReplyTo":"nycvar.QRO.7.76.6.2012300645400.56@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v5 0/1] mergetool: remove unconflicted lines","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-30T22:33:55Z","receivedAt":"2020-12-30T22:35:42Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Johannes Schindelin wrote:\n> On Thu, 24 Dec 2020, Junio C Hamano wrote:\n> > This breakage is possibly a fallout from either this patch or\n> > 1e2ae142 (t7[5-9]*: adjust the references to the default branch name\n> > \"main\", 2020-11-18).\n> >\n> > https://github.com/git/git/runs/1602803804#step:7:10358\n> >\n> > I cannot quite tell how the two strings compared with 'test' on\n> > output line 10355 are different in the output, though.\n> \n> I spent more time than I cared to spend on this, and still have not quite\n> figured out what is the fault, but I can state with conviction that the\n> problem is not even introduced by any merge into `seen`. The\n> `fc/mergetool-automerge` branch itself is already broken:\n> https://github.com/gitgitgadget/git/actions/runs/441233234\n\nYes, if you didn't have me blocked and read what I said a week ago in\n[1], you would have saved yourself that time.\n\nSeth House has claimed the patch series though.\n\n[1] https://lore.kernel.org/git/5fe4bec2da21a_19c92085f@natae.notmuch/\n\n-- \nFelipe Contreras\n"},{"id":"413531","messageId":"xmqqpn2ivcc1.fsf@gitster.c.googlers.com","threadId":"54871","inReplyTo":"20201228192919.1195211-6-seth@eseth.com","subject":"Re: [PATCH v9 5/5] mergetool: add automerge_enabled tool-specific override function","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-06T05:55:26Z","receivedAt":"2021-01-06T05:56:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Seth House writes:\n\n> diff --git a/git-mergetool.sh b/git-mergetool.sh\n> index 929192d0f8..a44afd3822 100755\n> --- a/git-mergetool.sh\n> +++ b/git-mergetool.sh\n> @@ -336,7 +336,7 @@ merge_file () {\n> \n> \tinitialize_merge_tool \"$merge_tool\"\n> \n> -\tif test \"$(\n> +\tif automerge_enabled && test \"$(\n> \t\tgit config --get --bool \"mergetool.$merge_tool.automerge\" ||\n> \t\tgit config --get --bool \"mergetool.automerge\" ||\n> \t\techo true)\" = true\n\nThis allows the tool author to say \"nobody ever is allowed to use my\ntool with the automerge feature\". I know I may have suggested\nsomething like that, but I am not sure if we want to be all that\ndraconian.\n\nIf the user explicitly says \"I want the new behaviour enabled for\nthis particular merge tool\", we are better off letting the user use\nit and take responsibility for the possible breakage.\n\nMy preference would probably be\n\n - if \"mergetool.$merge_tool.automerge\" is set to 'true' or 'false',\n that's final.\n\n - Your automerge_enabled helper that is by default 'true' (but\n allows individual merge_tool to return 'false') is asked, and if\n it says 'false', that's final. But 'true' from automerge_enabled\n is not final at this step.\n\n - if \"mergetool.automerge\" is set to 'true' or 'false', that's\n final.\n\n - otherwise, your automerge_enabled helper's answer (either 'true'\n or 'false') gives the final answer.\n\nThat way, those who use a broad \"mergetool.automerge = true/false\"\nwould still honor what automerge_enabled yields (which is \"enabled\nby default but individual merge_tool can set its default to be\ndisabled\"), while individual mergetool.$merge_tool.automerge\nconfiguration would always win.\n\nHmm?\n"},{"id":"413640","messageId":"20210107035806.GA530261@ellen","threadId":"54871","inReplyTo":"xmqqpn2ivcc1.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v9 5/5] mergetool: add automerge_enabled tool-specific override function","fromName":"Seth House","fromEmail":"seth@eseth.com","sentAt":"2021-01-07T03:58:06Z","receivedAt":"2021-01-07T03:59:08Z","isPatch":true,"sender":{"key":"seth@eseth.com","avatar":"https://avatars.githubusercontent.com/u/91293?v=4"},"body":"On Tue, Jan 05, 2021 at 09:55:26PM -0800, Junio C Hamano wrote:\n> If the user explicitly says \"I want the new behaviour enabled for\n> this particular merge tool\", we are better off letting the user use\n> it and take responsibility for the possible breakage.\n\nGood suggestion. Agreed on all counts. I'll roll that preference\nhierarchy into the v10 patch set.\n\n"},{"id":"413655","messageId":"xmqqy2h5meum.fsf@gitster.c.googlers.com","threadId":"54871","inReplyTo":"20210107035806.GA530261@ellen","subject":"Re: [PATCH v9 5/5] mergetool: add automerge_enabled tool-specific override function","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-07T06:38:09Z","receivedAt":"2021-01-07T06:39:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Seth House writes:\n\n> On Tue, Jan 05, 2021 at 09:55:26PM -0800, Junio C Hamano wrote:\n>> If the user explicitly says \"I want the new behaviour enabled for\n>> this particular merge tool\", we are better off letting the user use\n>> it and take responsibility for the possible breakage.\n>\n> Good suggestion. Agreed on all counts. I'll roll that preference\n> hierarchy into the v10 patch set.\n\nBy the way, do you have any idea why we see test breakages only on\nmacos when this topic is merged to 'seen'?\n\nhttps://github.com/git/git/runs/1659807735?check_suite_focus=true#step:4:1641\nhttps://github.com/git/git/runs/1659807735?check_suite_focus=true#step:5:2641\n\nThanks.\n"},{"id":"413663","messageId":"20210107092716.GA548935@ellen","threadId":"54871","inReplyTo":"xmqqy2h5meum.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v9 5/5] mergetool: add automerge_enabled tool-specific override function","fromName":"Seth House","fromEmail":"seth@eseth.com","sentAt":"2021-01-07T09:27:16Z","receivedAt":"2021-01-07T09:28:05Z","isPatch":true,"sender":{"key":"seth@eseth.com","avatar":"https://avatars.githubusercontent.com/u/91293?v=4"},"body":"On Wed, Jan 06, 2021 at 10:38:09PM -0800, Junio C Hamano wrote:\n> By the way, do you have any idea why we see test breakages only on\n> macos when this topic is merged to 'seen'?\n\nThanks for those links. I have an OSX machine nearby and will\ninvestigate tomorrow.\n\nRelated: are the Windows tests affected by this patch? I wanted to check\nfor myself but I've been struggling with getting Git-for-Windows\ninstalled in a VM.\n\n"},{"id":"413746","messageId":"xmqq1rewl9qe.fsf@gitster.c.googlers.com","threadId":"54871","inReplyTo":"20210107092716.GA548935@ellen","subject":"Re: [PATCH v9 5/5] mergetool: add automerge_enabled tool-specific override function","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-07T21:26:17Z","receivedAt":"2021-01-07T21:27:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Seth House writes:\n\n> On Wed, Jan 06, 2021 at 10:38:09PM -0800, Junio C Hamano wrote:\n>> By the way, do you have any idea why we see test breakages only on\n>> macos when this topic is merged to 'seen'?\n>\n> Thanks for those links. I have an OSX machine nearby and will\n> investigate tomorrow.\n>\n> Related: are the Windows tests affected by this patch? I wanted to check\n> for myself but I've been struggling with getting Git-for-Windows\n> installed in a VM.\n\nOn the left hand side of the page I gave the links to, it shows that\n'windows-build' job is failing (and windows-test jobs are not run as\na consequence). I am not sure why it failed, but I have a feeling\nthat the build machinery hasn't even seen the code being built when\nit errored out.\n\n cf. https://github.com/git/git/runs/1659807855?check_suite_focus=true#step:3:40\n\nSo we cannot tell (yet).\n"},{"id":"413823","messageId":"nycvar.QRO.7.76.6.2101081602410.2213@tvgsbejvaqbjf.bet","threadId":"54871","inReplyTo":"xmqq1rewl9qe.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v9 5/5] mergetool: add automerge_enabled tool-specific override function","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-01-08T15:04:12Z","receivedAt":"2021-01-08T15:06:21Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Thu, 7 Jan 2021, Junio C Hamano wrote:\n\n> Seth House writes:\n>\n> > On Wed, Jan 06, 2021 at 10:38:09PM -0800, Junio C Hamano wrote:\n> >> By the way, do you have any idea why we see test breakages only on\n> >> macos when this topic is merged to 'seen'?\n> >\n> > Thanks for those links. I have an OSX machine nearby and will\n> > investigate tomorrow.\n> >\n> > Related: are the Windows tests affected by this patch? I wanted to check\n> > for myself but I've been struggling with getting Git-for-Windows\n> > installed in a VM.\n>\n> On the left hand side of the page I gave the links to, it shows that\n> 'windows-build' job is failing (and windows-test jobs are not run as\n> a consequence). I am not sure why it failed, but I have a feeling\n> that the build machinery hasn't even seen the code being built when\n> it errored out.\n>\n> cf. https://github.com/git/git/runs/1659807855?check_suite_focus=true#step:3:40\n>\n> So we cannot tell (yet).\n\nThere are unfortunately intermittent failures while downloading\ngit-sdk-64-minimal; That's what this job is seeing. I restarted that build:\nhttps://github.com/git/git/runs/1659807855?check_suite_focus=true#step:3:40\n\nCiao,\nDscho\n"},{"id":"415632","messageId":"20210130054655.48237-1-seth@eseth.com","threadId":"54871","inReplyTo":"20201228192919.1195211-1-seth@eseth.com","subject":"[PATCH v10 0/3] mergetool: add hideResolved configuration (was automerge)","fromName":"Seth House","fromEmail":"seth@eseth.com","sentAt":"2021-01-30T05:46:52Z","receivedAt":"2021-01-30T05:49:24Z","isPatch":true,"sender":{"key":"seth@eseth.com","avatar":"https://avatars.githubusercontent.com/u/91293?v=4"},"body":"Changes since v9:\n\n- Rename automerge to hideResolved.\n\n Several mergetools have a feature that they call \"automerge\", \"auto\n merge\", or \"auto solve\" and Git has a `mergetool.meld.useAutoMerge`\n flag so I think it's better to avoid potential confusion. Plus Git\n already performed the merge and this doesn't do any additional\n merging, instead it just \"hides\" conflicts that Git already resolved.\n\n- Reworked and consolidated commits and commit messages.\n\n- Add preference hierarchy.\n\n Followed Junio's suggestions:\n https://lore.kernel.org/git/xmqqpn2ivcc1.fsf@gitster.c.googlers.com/\n\n- Switch from sed to two merge-file calls.\n\n Thanks to everyone who helped with all the suggestions and fixup!s to\n get sed working cross-platform. Unfortunately there's not a great,\n portable method to preserve carriage returns when using both autocrlf\n and MSYS2: https://lore.kernel.org/git/20210120232447.GA35105@ellen/\n\n Although calling merge-file twice is (much) less efficient than sed\n it's still fairly quick for small files. For large files it's likely\n opening those files in a mergetool will have a higher overhead than\n the merge-file invocations:\n https://lore.kernel.org/git/20210122010902.GA48178@ellen/\n\n A potential future optimisation could be to augment the\n C implementation (xmerge.c ?) with a flag to write two files as the\n merge is being performed instead of writing conflict markers.\n\n- Kept `initialize_merge_tool` wrapper.\n\n I updated the commit message where `initialize_merge_tool` is\n introduced to try and better explain my thinking for not simply\n exposing `setup_tool` instead. I'm happy to switch that if anyone\n still feels it should be switched.\n\nSeth House (3):\n mergetool: add hideResolved configuration\n mergetool: break setup_tool out into separate initialization function\n mergetool: add per-tool support and overrides for the hideResolved\n flag\n\n Documentation/config/mergetool.txt | 15 +++++++++++++++\n Documentation/git-mergetool--lib.txt | 4 ++++\n git-difftool--helper.sh | 6 ++++++\n git-mergetool--lib.sh | 11 ++++++++---\n git-mergetool.sh | 26 ++++++++++++++++++++++++++\n t/t7610-mergetool.sh | 18 ++++++++++++++++++\n 6 files changed, 77 insertions(+), 3 deletions(-)\n\n-- \n2.29.2\n\n\n"},{"id":"415633","messageId":"20210130054655.48237-4-seth@eseth.com","threadId":"54871","inReplyTo":"20210130054655.48237-1-seth@eseth.com","subject":"[PATCH v10 3/3] mergetool: add per-tool support and overrides for the hideResolved flag","fromName":"Seth House","fromEmail":"seth@eseth.com","sentAt":"2021-01-30T05:46:55Z","receivedAt":"2021-01-30T05:52:04Z","isPatch":true,"sender":{"key":"seth@eseth.com","avatar":"https://avatars.githubusercontent.com/u/91293?v=4"},"body":"Keep the global mergetool flag and add a per-tool override flag so that\nusers may enable the flag for one tool and disable it for another. In\naddition, the author or maintainer of a mergetool may optionally elect\nto set the default `hideResolved` value for that mergetool.\n\nTo disable the feature for a specific tool, edit the `mergetools/`\nshell script for that tool and add a `hide_resolved_enabled` function:\n\n hide_resolved_enabled () {\n return 1\n }\n\nDisabling may be desirable if the mergetool wants or needs access to the\noriginal, unmodified 'LOCAL' and 'REMOTE' versions of the conflicted\nfile. For example:\n\n- A tool may use a custom conflict resolution algorithm and prefer to\n ignore the results of Git's conflict resolution.\n- A tool may want to visually compare/constrast the version of the file\n from before the merge (saved to 'LOCAL', 'REMOTE', and 'BASE') with\n Git's conflict resolution results (saved to 'MERGED').\n\nHelped-by: Johannes Sixt \nHelped-by: Junio C Hamano \nSigned-off-by: Seth House \n---\n Documentation/config/mergetool.txt | 6 ++++++\n git-mergetool--lib.sh | 4 ++++\n git-mergetool.sh | 14 ++++++++++++--\n 3 files changed, 22 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config/mergetool.txt b/Documentation/config/mergetool.txt\nindex 3171bacf91..046816fb07 100644\n--- a/Documentation/config/mergetool.txt\n+++ b/Documentation/config/mergetool.txt\n@@ -13,6 +13,12 @@ mergetool..cmd::\n \tmerged; 'MERGED' contains the name of the file to which the merge\n \ttool should write the results of a successful merge.\n \n+mergetool..hideResolved::\n+\tA mergetool-specific override for the global `mergetool.hideResolved`\n+\tconfiguration flag. This allows individual mergetools to enable or\n+\tdisable the flag regardless of the global setting. See\n+\t`mergetool.hideResolved` for the full description.\n+\n mergetool..trustExitCode::\n \tFor a custom merge command, specify whether the exit code of\n \tthe merge command can be used to determine whether the merge was\ndiff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh\nindex e059b3559e..11f00dde41 100644\n--- a/git-mergetool--lib.sh\n+++ b/git-mergetool--lib.sh\n@@ -164,6 +164,10 @@ setup_tool () {\n \t\treturn 1\n \t}\n \n+\thide_resolved_enabled () {\n+\t\treturn 0\n+\t}\n+\n \ttranslate_merge_tool_path () {\n \t\techo \"$1\"\n \t}\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex 865f12551a..6cf3884277 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -333,9 +333,19 @@ merge_file () {\n \tcheckout_staged_file 2 \"$MERGED\" \"$LOCAL\"\n \tcheckout_staged_file 3 \"$MERGED\" \"$REMOTE\"\n \n-\tif test \"$(git config --get mergetool.hideResolved)\" != \"false\"\n+\t# hideResolved preferences hierarchy:\n+\t# First respect user's tool-specific configuration if exists.\n+\tif test \"$(git config --get \"mergetool.$merge_tool.hideResolved\")\" != \"false\"\n \tthen\n-\t\thide_resolved\n+\t\t# Next respect tool-specified configuration.\n+\t\tif hide_resolved_enabled\n+\t\tthen\n+\t\t\t# Finally respect if user has a global disable.\n+\t\t\tif test \"$(git config --get \"mergetool.hideResolved\")\" != \"false\"\n+\t\t\tthen\n+\t\t\t\thide_resolved\n+\t\t\tfi\n+\t\tfi\n \tfi\n \n \tif test -z \"$local_mode\" || test -z \"$remote_mode\"\n-- \n2.29.2\n\n\n"},{"id":"415634","messageId":"20210130054655.48237-3-seth@eseth.com","threadId":"54871","inReplyTo":"20210130054655.48237-1-seth@eseth.com","subject":"[PATCH v10 2/3] mergetool: break setup_tool out into separate initialization function","fromName":"Seth House","fromEmail":"seth@eseth.com","sentAt":"2021-01-30T05:46:54Z","receivedAt":"2021-01-30T05:52:25Z","isPatch":true,"sender":{"key":"seth@eseth.com","avatar":"https://avatars.githubusercontent.com/u/91293?v=4"},"body":"This is preparation for the following commit where we need to source the\nmergetool shell script to look for overrides before `run_merge_tool` is\ncalled. Previously `run_merge_tool` both sourced that script and invoked\nthe mergetool.\n\nIn the case of the following commit, we need the result of the\n`hide_resolved` override, if present, before we actually run\n`run_merge_tool`.\n\nThe new `initialize_merge_tool` wrapper is exposed and documented as\na public interface for consistency with the existing `run_merge_tool`\nwhich is also public. Although `setup_tool` could instead be exposed\ndirectly, the related `setup_user_tool` would probably also want to be\nelevated to match and this felt the cleanest to me.\n\nSigned-off-by: Seth House \n---\n Documentation/git-mergetool--lib.txt | 4 ++++\n git-difftool--helper.sh | 6 ++++++\n git-mergetool--lib.sh | 7 ++++---\n git-mergetool.sh | 2 ++\n 4 files changed, 16 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-mergetool--lib.txt b/Documentation/git-mergetool--lib.txt\nindex 4da9d24096..3e8f59ac0e 100644\n--- a/Documentation/git-mergetool--lib.txt\n+++ b/Documentation/git-mergetool--lib.txt\n@@ -38,6 +38,10 @@ get_merge_tool_cmd::\n get_merge_tool_path::\n \treturns the custom path for a merge tool.\n \n+initialize_merge_tool::\n+\tbring merge tool specific functions into scope so they can be used or\n+\toverridden.\n+\n run_merge_tool::\n \tlaunches a merge tool given the tool name and a true/false\n \tflag to indicate whether a merge base is present.\ndiff --git a/git-difftool--helper.sh b/git-difftool--helper.sh\nindex 46af3e60b7..992124cc67 100755\n--- a/git-difftool--helper.sh\n+++ b/git-difftool--helper.sh\n@@ -61,6 +61,9 @@ launch_merge_tool () {\n \t\texport BASE\n \t\teval $GIT_DIFFTOOL_EXTCMD '\"$LOCAL\"' '\"$REMOTE\"'\n \telse\n+\t\tinitialize_merge_tool \"$merge_tool\"\n+\t\t# ignore the error from the above --- run_merge_tool\n+\t\t# will diagnose unusable tool by itself\n \t\trun_merge_tool \"$merge_tool\"\n \tfi\n }\n@@ -79,6 +82,9 @@ if test -n \"$GIT_DIFFTOOL_DIRDIFF\"\n then\n \tLOCAL=\"$1\"\n \tREMOTE=\"$2\"\n+\tinitialize_merge_tool \"$merge_tool\"\n+\t# ignore the error from the above --- run_merge_tool\n+\t# will diagnose unusable tool by itself\n \trun_merge_tool \"$merge_tool\" false\n else\n \t# Launch the merge tool on each path provided by 'git diff'\ndiff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh\nindex 7225abd811..e059b3559e 100644\n--- a/git-mergetool--lib.sh\n+++ b/git-mergetool--lib.sh\n@@ -248,6 +248,10 @@ trust_exit_code () {\n \tfi\n }\n \n+initialize_merge_tool () {\n+\t# Bring tool-specific functions into scope\n+\tsetup_tool \"$1\" || return 1\n+}\n \n # Entry point for running tools\n run_merge_tool () {\n@@ -259,9 +263,6 @@ run_merge_tool () {\n \tmerge_tool_path=$(get_merge_tool_path \"$1\") || exit\n \tbase_present=\"$2\"\n \n-\t# Bring tool-specific functions into scope\n-\tsetup_tool \"$1\" || return 1\n-\n \tif merge_mode\n \tthen\n \t\trun_merge_cmd \"$1\"\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex 5b0d15ed89..865f12551a 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -272,6 +272,8 @@ merge_file () {\n \t\text=\n \tesac\n \n+\tinitialize_merge_tool \"$merge_tool\" || return\n+\n \tmergetool_tmpdir_init\n \n \tif test \"$MERGETOOL_TMPDIR\" != \".\"\n-- \n2.29.2\n\n\n"},{"id":"415635","messageId":"20210130054655.48237-2-seth@eseth.com","threadId":"54871","inReplyTo":"20210130054655.48237-1-seth@eseth.com","subject":"[PATCH v10 1/3] mergetool: add hideResolved configuration","fromName":"Seth House","fromEmail":"seth@eseth.com","sentAt":"2021-01-30T05:46:53Z","receivedAt":"2021-01-30T05:52:36Z","isPatch":true,"sender":{"key":"seth@eseth.com","avatar":"https://avatars.githubusercontent.com/u/91293?v=4"},"body":"The purpose of a mergetool is to help the user resolve any conflicts\nthat Git cannot automatically resolve. If there is a conflict that must\nbe resolved manually Git will write a file named MERGED which contains\neverything Git was able to resolve by itself and also everything that it\nwas not able to resolve wrapped in conflict markers.\n\nOne way to think of MERGED is as a two- or three-way diff. If each\n\"side\" of the conflict markers is separately extracted an external tool\ncan represent those conflicts as a side-by-side diff.\n\nHowever many mergetools instead diff LOCAL and REMOTE both of which\ncontain versions of the file from before the merge. Since the conflicts\nGit resolved automatically are not present it forces the user to\nmanually re-resolve those conflicts. Some mergetools also show MERGED\nbut often only for reference and not as the focal point to resolve the\nconflicts.\n\nThis adds a `mergetool.hideResolved` flag that will overwrite LOCAL and\nREMOTE with each corresponding \"side\" of a conflicted file and thus hide\nall conflicts that Git was able to resolve itself. Overwriting these\nfiles will immediately benefit any mergetool that uses them without\nrequiring any changes.\n\nNo adverse effects were noted in a small survey of popular mergetools[1]\nso this behavior defaults to `true`. However it can be globally disabled\nby setting `mergetool.hideResolved` to `false`.\n\n[1] https://www.eseth.org/2020/mergetools.html\n https://github.com/whiteinge/eseth/blob/c884424769fffb05d87afb33b2cf80cecb4044c3/2020/mergetools.md\n\nOriginal-implementation-by: Felipe Contreras \nHelped-by: Felipe Contreras \nSigned-off-by: Seth House \n---\n Documentation/config/mergetool.txt | 9 +++++++++\n git-mergetool.sh | 14 ++++++++++++++\n t/t7610-mergetool.sh | 18 ++++++++++++++++++\n 3 files changed, 41 insertions(+)\n\ndiff --git a/Documentation/config/mergetool.txt b/Documentation/config/mergetool.txt\nindex 16a27443a3..3171bacf91 100644\n--- a/Documentation/config/mergetool.txt\n+++ b/Documentation/config/mergetool.txt\n@@ -40,6 +40,15 @@ mergetool.meld.useAutoMerge::\n \tvalue of `false` avoids using `--auto-merge` altogether, and is the\n \tdefault value.\n \n+mergetool.hideResolved::\n+\tDuring a merge Git will automatically resolve as many conflicts as\n+\tpossible and then wrap conflict markers around any conflicts that it\n+\tcannot resolve. This flag writes the non-conflicting parts into the\n+\tcorresponding 'LOCAL' and 'REMOTE' files so that only the unresolved\n+\tconflicts are presented to the merge tool. Can be overriden per-tool\n+\tvia the `mergetool..hideResolved` configuration variable.\n+\tDefaults to `true`.\n+\n mergetool.keepBackup::\n \tAfter performing a merge, the original file with conflict markers\n \tcan be saved as a file with a `.orig` extension. If this variable\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex e3f6d543fb..5b0d15ed89 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -239,6 +239,13 @@ checkout_staged_file () {\n \tfi\n }\n \n+hide_resolved () {\n+\tgit merge-file --ours -q -p \"$LOCAL\" \"$BASE\" \"$REMOTE\" >\"$LCONFL\"\n+\tgit merge-file --theirs -q -p \"$LOCAL\" \"$BASE\" \"$REMOTE\" >\"$RCONFL\"\n+\tmv -- \"$LCONFL\" \"$LOCAL\"\n+\tmv -- \"$RCONFL\" \"$REMOTE\"\n+}\n+\n merge_file () {\n \tMERGED=\"$1\"\n \n@@ -276,7 +283,9 @@ merge_file () {\n \n \tBACKUP=\"$MERGETOOL_TMPDIR/${BASE}_BACKUP_$$$ext\"\n \tLOCAL=\"$MERGETOOL_TMPDIR/${BASE}_LOCAL_$$$ext\"\n+\tLCONFL=\"$MERGETOOL_TMPDIR/${BASE}_LOCAL_LCONFL_$$$ext\"\n \tREMOTE=\"$MERGETOOL_TMPDIR/${BASE}_REMOTE_$$$ext\"\n+\tRCONFL=\"$MERGETOOL_TMPDIR/${BASE}_REMOTE_RCONFL_$$$ext\"\n \tBASE=\"$MERGETOOL_TMPDIR/${BASE}_BASE_$$$ext\"\n \n \tbase_mode= local_mode= remote_mode=\n@@ -322,6 +331,11 @@ merge_file () {\n \tcheckout_staged_file 2 \"$MERGED\" \"$LOCAL\"\n \tcheckout_staged_file 3 \"$MERGED\" \"$REMOTE\"\n \n+\tif test \"$(git config --get mergetool.hideResolved)\" != \"false\"\n+\tthen\n+\t\thide_resolved\n+\tfi\n+\n \tif test -z \"$local_mode\" || test -z \"$remote_mode\"\n \tthen\n \t\techo \"Deleted merge conflict for '$MERGED':\"\ndiff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh\nindex 70afdd06fa..0e34b87e37 100755\n--- a/t/t7610-mergetool.sh\n+++ b/t/t7610-mergetool.sh\n@@ -828,4 +828,22 @@ test_expect_success 'mergetool -Oorder-file is honored' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'mergetool hideResolved' '\n+\ttest_config mergetool.hideResolved true &&\n+\ttest_when_finished \"git reset --hard\" &&\n+\tgit checkout -b test${test_count}_b master &&\n+\ttest_write_lines >file1 base \"\" a &&\n+\tgit commit -a -m \"base\" &&\n+\ttest_write_lines >file1 base \"\" c &&\n+\tgit commit -a -m \"remote update\" &&\n+\tgit checkout -b test${test_count}_a HEAD~ &&\n+\ttest_write_lines >file1 local \"\" b &&\n+\tgit commit -a -m \"local update\" &&\n+\ttest_must_fail git merge test${test_count}_b &&\n+\tyes \"\" | git mergetool file1 &&\n+\ttest_write_lines >expect local \"\" c &&\n+\ttest_cmp expect file1 &&\n+\tgit commit -m \"test resolved with mergetool\"\n+'\n+\n test_done\n-- \n2.29.2\n\n\n"},{"id":"415644","messageId":"xmqqsg6iq23d.fsf@gitster.c.googlers.com","threadId":"54871","inReplyTo":"20210130054655.48237-4-seth@eseth.com","subject":"Re: [PATCH v10 3/3] mergetool: add per-tool support and overrides for the hideResolved flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-30T08:08:06Z","receivedAt":"2021-01-30T09:15:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Seth House writes:\n\n> Keep the global mergetool flag and add a per-tool override flag so that\n> users may enable the flag for one tool and disable it for another. In\n> addition, the author or maintainer of a mergetool may optionally elect\n> to set the default `hideResolved` value for that mergetool.\n\nOK.\n\n> To disable the feature for a specific tool, edit the `mergetools/`\n> shell script for that tool and add a `hide_resolved_enabled` function:\n>\n> hide_resolved_enabled () {\n> return 1\n> }\n>\n> Disabling may be desirable if the mergetool wants or needs access to the\n> original, unmodified 'LOCAL' and 'REMOTE' versions of the conflicted\n> file.\n\nThe above sounds as if it is a hint/help for end users, but it is\nunreasonable to expect all end users of a particular to edit\npart of their Git installation. I suspect that you didn't mean it\nthat way, and instead it is meant to advise (new) tool authors who\nwill add mergetools/ for their own tool, and when read with\nthat in mind, it does make sort-of sense (except that when you are\nauthor of this thing, you won't \"edit\" as if you are modifying\nsomething that already exists---you'd be the one who is adding the\n under mergetools/ directory).\n\nFor an end-user, to disable the feature for a tool, you'd just\nconfigure mergetool..hideResolved to 'false', right?\n\n> For example:\n>\n> - A tool may use a custom conflict resolution algorithm and prefer to\n> ignore the results of Git's conflict resolution.\n> - A tool may want to visually compare/constrast the version of the file\n> from before the merge (saved to 'LOCAL', 'REMOTE', and 'BASE') with\n> Git's conflict resolution results (saved to 'MERGED').\n>\n> Helped-by: Johannes Sixt \n> Helped-by: Junio C Hamano \n> Signed-off-by: Seth House \n> ---\n> Documentation/config/mergetool.txt | 6 ++++++\n> git-mergetool--lib.sh | 4 ++++\n> git-mergetool.sh | 14 ++++++++++++--\n> 3 files changed, 22 insertions(+), 2 deletions(-)\n>\n> diff --git a/Documentation/config/mergetool.txt b/Documentation/config/mergetool.txt\n> index 3171bacf91..046816fb07 100644\n> --- a/Documentation/config/mergetool.txt\n> +++ b/Documentation/config/mergetool.txt\n> @@ -13,6 +13,12 @@ mergetool..cmd::\n> \tmerged; 'MERGED' contains the name of the file to which the merge\n> \ttool should write the results of a successful merge.\n> \n> +mergetool..hideResolved::\n> +\tA mergetool-specific override for the global `mergetool.hideResolved`\n> +\tconfiguration flag. This allows individual mergetools to enable or\n> +\tdisable the flag regardless of the global setting. See\n> +\t`mergetool.hideResolved` for the full description.\n\nThis description is iffy. \n\nThe configuration allows \"users\" to enable or disable the feature\nfor individual mergetools, overriding the 'mergetool.hideResolved'\nglobal setting, no? The above paragraph reads as if the tool author\nis enabling/disabling it.\n\n> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh\n> index e059b3559e..11f00dde41 100644\n> --- a/git-mergetool--lib.sh\n> +++ b/git-mergetool--lib.sh\n> @@ -164,6 +164,10 @@ setup_tool () {\n> \t\treturn 1\n> \t}\n> \n> +\thide_resolved_enabled () {\n> +\t\treturn 0\n> +\t}\n> +\n> \ttranslate_merge_tool_path () {\n> \t\techo \"$1\"\n> \t}\n> diff --git a/git-mergetool.sh b/git-mergetool.sh\n> index 865f12551a..6cf3884277 100755\n> --- a/git-mergetool.sh\n> +++ b/git-mergetool.sh\n> @@ -333,9 +333,19 @@ merge_file () {\n> \tcheckout_staged_file 2 \"$MERGED\" \"$LOCAL\"\n> \tcheckout_staged_file 3 \"$MERGED\" \"$REMOTE\"\n> \n> -\tif test \"$(git config --get mergetool.hideResolved)\" != \"false\"\n> +\t# hideResolved preferences hierarchy:\n> +\t# First respect user's tool-specific configuration if exists.\n> +\tif test \"$(git config --get \"mergetool.$merge_tool.hideResolved\")\" != \"false\"\n\nThe same \"--type=bool\" comment applies to this step, too.\n\n> \tthen\n> +\t\t# Next respect tool-specified configuration.\n> +\t\tif hide_resolved_enabled\n> +\t\tthen\n> +\t\t\t# Finally respect if user has a global disable.\n> +\t\t\tif test \"$(git config --get \"mergetool.hideResolved\")\" != \"false\"\n> +\t\t\tthen\n> +\t\t\t\thide_resolved\n> +\t\t\tfi\n> +\t\tfi\n> \tfi\n\nI am not sure if I understand this logic.\n\nIf the user says \"for the tool , set hideresolved to\ntrue/false\" explicitly, I think it should be final. Even if the\ntool's author expresses that s/he prefers not to have to work on a\npre-munged input by setting hide_resolved_enabled to false, if the\nend-user says s/he wants to use it on that tool, we do not want to\nhelp the tool to override the user's wish.\n\nIf the user says \"use hideresolved feature, as I like it in general\"\nby setting mergetool.hideResolved, on the other hand, it may be also\nreasonable to heed \"no, I recommend against it for this tool\" for\nindividual tool whose hide_resolved_enabled returns false. And if\nthe global is set to 'false', the user says \"I do not want it\", and\nit may be iffy to let individual tool to countermand it.\n\nWHen dealing with either of these variables, therefore, you'd need\nto know if the variable is not set at all, or if the variable is set\nto true, or to false. Even if we default to enabled, we need to be\nable to tell if the user didn't say anything (and we enabled the\nfeature for the user because of our default choice), or if the user\nexplicitly said s/he wants it.\n\nIn other words, you'd need to treat mergetool.hideResolved and\nmergetool.$merge_tool.hideResolved as tristates.\n\nHere is how \"git config --type=bool\" can be used to normalize\nvarious ways to spell true/false and tell between \"not set\" and \"set\nto some value\":\n\n $ git -c a.b config --type=bool a.b; echo $?\n true\n 0\n $ git -c a.b=yes config --type=bool a.b; echo $?\n true\n 0\n $ git -c a.b=0 config --type=bool a.b; echo $?\n false\n 0\n $ git config --type=bool a.b; echo $?\n 1\n\nIOW, if \"git config --type=bool\" fails, the user does not have the\nvariable set. If it succeeds, you'd get normalized 'true/false'\nstring on its standard output.\n\nUsing that technique, here is my attempt to rewrite the above logic,\nwith commentary.\n\n global_config=mergetool.hideResolved\n tool_config=mergetool.$merge_tool.hideResolved\n\n if enabled=$(git config --type=bool \"$tool_config\")\n then\n\t# The user explicitly says true or false, so there\n\t# is no point in asking any other source of preferences\n\t;\n elif enabled=$(git config --type=bool \"$global_config\")\n then\n\t# There is a blanket preference for all tools, and 'true'\n\t# means \"I like the hide-resolved in general, so use it\n\t# when appropriate\" by the user. We can let the tool\n\t# author to override and disable, though.\n\t#\n\t# On the other hand, when set to 'false', it is \"I really\n\t# don't like the feature in general, so do not use it\n\t# anywhere\", which we take it as final, without letting\n\t# the tool override it.\n if test \"$enabled\" = true && hide_resolved_enabled\n\tthen\n\t\tenabled=true\n\telse\n\t\tenabled=false\n\tfi\n else\n\t# The user does not have preference. Ask the tool\n\tif hide_resolved_enabled\n\tthen\n\t\tenabled=true\n\telse\n\t\tenabled=false\n\tfi\n fi\n\n # Now we know if the feature should be used.\n if test \"$enabled\" = true\n then\n\thide_resolved\n fi\n\n"},{"id":"415645","messageId":"xmqqmtwqq239.fsf@gitster.c.googlers.com","threadId":"54871","inReplyTo":"20210130054655.48237-2-seth@eseth.com","subject":"Re: [PATCH v10 1/3] mergetool: add hideResolved configuration","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-30T08:08:10Z","receivedAt":"2021-01-30T09:16:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Seth House writes:\n\n> +mergetool.hideResolved::\n> +\tDuring a merge Git will automatically resolve as many conflicts as\n> +\tpossible and then wrap conflict markers around any conflicts that it\n> +\tcannot resolve. This flag writes the non-conflicting parts into the\n> +\tcorresponding 'LOCAL' and 'REMOTE' files so that only the unresolved\n> +\tconflicts are presented to the merge tool. Can be overriden per-tool\n> +\tvia the `mergetool..hideResolved` configuration variable.\n> +\tDefaults to `true`.\n\nThis description makes the readers expect that the configuration\nvariable is a boolean, and setting it to 'no' would disable the\nfeature, but ...\n\n> @@ -322,6 +331,11 @@ merge_file () {\n> \tcheckout_staged_file 2 \"$MERGED\" \"$LOCAL\"\n> \tcheckout_staged_file 3 \"$MERGED\" \"$REMOTE\"\n> \n> +\tif test \"$(git config --get mergetool.hideResolved)\" != \"false\"\n\n... without --type=bool, any boolean 'false' value that is not\nexactly spelled 'false' won't be normalized and fail this test.\n\nI haven't read the remaining 2 patches, so I cannot yet tell if I\ncan just insert \"--type=bool\" here and everything would be fine,\nor if there are other fallouts for doing so.\n\n> +\tthen\n> +\t\thide_resolved\n> +\tfi\n> +\n> \tif test -z \"$local_mode\" || test -z \"$remote_mode\"\n> \tthen\n> \t\techo \"Deleted merge conflict for '$MERGED':\"\n> diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh\n> index 70afdd06fa..0e34b87e37 100755\n> --- a/t/t7610-mergetool.sh\n> +++ b/t/t7610-mergetool.sh\n> @@ -828,4 +828,22 @@ test_expect_success 'mergetool -Oorder-file is honored' '\n> \ttest_cmp expect actual\n> '\n> \n> +test_expect_success 'mergetool hideResolved' '\n> +\ttest_config mergetool.hideResolved true &&\n> +\ttest_when_finished \"git reset --hard\" &&\n> +\tgit checkout -b test${test_count}_b master &&\n\nAs a new feature, this should work with the tip of 'master', but I\nthink the t7610 test forces the initial branch name to be 'main'.\n\nI'll tweak locally while queuing.\n\n> +\ttest_write_lines >file1 base \"\" a &&\n> +\tgit commit -a -m \"base\" &&\n> +\ttest_write_lines >file1 base \"\" c &&\n> +\tgit commit -a -m \"remote update\" &&\n> +\tgit checkout -b test${test_count}_a HEAD~ &&\n> +\ttest_write_lines >file1 local \"\" b &&\n> +\tgit commit -a -m \"local update\" &&\n> +\ttest_must_fail git merge test${test_count}_b &&\n> +\tyes \"\" | git mergetool file1 &&\n> +\ttest_write_lines >expect local \"\" c &&\n> +\ttest_cmp expect file1 &&\n> +\tgit commit -m \"test resolved with mergetool\"\n> +'\n> +\n> test_done\n"},{"id":"416553","messageId":"20210209200712.156540-3-seth@eseth.com","threadId":"54871","inReplyTo":"20210209200712.156540-1-seth@eseth.com","subject":"[PATCH v11 2/3] mergetool: break setup_tool out into separate initialization function","fromName":"Seth House","fromEmail":"seth@eseth.com","sentAt":"2021-02-09T20:07:11Z","receivedAt":"2021-02-09T20:54:58Z","isPatch":true,"sender":{"key":"seth@eseth.com","avatar":"https://avatars.githubusercontent.com/u/91293?v=4"},"body":"This is preparation for the following commit where we need to source the\nmergetool shell script to look for overrides before `run_merge_tool` is\ncalled. Previously `run_merge_tool` both sourced that script and invoked\nthe mergetool.\n\nIn the case of the following commit, we need the result of the\n`hide_resolved` override, if present, before we actually run\n`run_merge_tool`.\n\nThe new `initialize_merge_tool` wrapper is exposed and documented as\na public interface for consistency with the existing `run_merge_tool`\nwhich is also public. Although `setup_tool` could instead be exposed\ndirectly, the related `setup_user_tool` would probably also want to be\nelevated to match and this felt the cleanest to me.\n\nSigned-off-by: Seth House \n---\n Documentation/git-mergetool--lib.txt | 4 ++++\n git-difftool--helper.sh | 6 ++++++\n git-mergetool--lib.sh | 7 ++++---\n git-mergetool.sh | 2 ++\n 4 files changed, 16 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-mergetool--lib.txt b/Documentation/git-mergetool--lib.txt\nindex 4da9d24096..3e8f59ac0e 100644\n--- a/Documentation/git-mergetool--lib.txt\n+++ b/Documentation/git-mergetool--lib.txt\n@@ -38,6 +38,10 @@ get_merge_tool_cmd::\n get_merge_tool_path::\n \treturns the custom path for a merge tool.\n \n+initialize_merge_tool::\n+\tbring merge tool specific functions into scope so they can be used or\n+\toverridden.\n+\n run_merge_tool::\n \tlaunches a merge tool given the tool name and a true/false\n \tflag to indicate whether a merge base is present.\ndiff --git a/git-difftool--helper.sh b/git-difftool--helper.sh\nindex 46af3e60b7..992124cc67 100755\n--- a/git-difftool--helper.sh\n+++ b/git-difftool--helper.sh\n@@ -61,6 +61,9 @@ launch_merge_tool () {\n \t\texport BASE\n \t\teval $GIT_DIFFTOOL_EXTCMD '\"$LOCAL\"' '\"$REMOTE\"'\n \telse\n+\t\tinitialize_merge_tool \"$merge_tool\"\n+\t\t# ignore the error from the above --- run_merge_tool\n+\t\t# will diagnose unusable tool by itself\n \t\trun_merge_tool \"$merge_tool\"\n \tfi\n }\n@@ -79,6 +82,9 @@ if test -n \"$GIT_DIFFTOOL_DIRDIFF\"\n then\n \tLOCAL=\"$1\"\n \tREMOTE=\"$2\"\n+\tinitialize_merge_tool \"$merge_tool\"\n+\t# ignore the error from the above --- run_merge_tool\n+\t# will diagnose unusable tool by itself\n \trun_merge_tool \"$merge_tool\" false\n else\n \t# Launch the merge tool on each path provided by 'git diff'\ndiff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh\nindex 78f3647ed9..4a8e36c792 100644\n--- a/git-mergetool--lib.sh\n+++ b/git-mergetool--lib.sh\n@@ -250,6 +250,10 @@ trust_exit_code () {\n \tfi\n }\n \n+initialize_merge_tool () {\n+\t# Bring tool-specific functions into scope\n+\tsetup_tool \"$1\" || return 1\n+}\n \n # Entry point for running tools\n run_merge_tool () {\n@@ -261,9 +265,6 @@ run_merge_tool () {\n \tmerge_tool_path=$(get_merge_tool_path \"$1\") || exit\n \tbase_present=\"$2\"\n \n-\t# Bring tool-specific functions into scope\n-\tsetup_tool \"$1\" || return 1\n-\n \tif merge_mode\n \tthen\n \t\trun_merge_cmd \"$1\"\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex 40a103443d..e5eac935f3 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -272,6 +272,8 @@ merge_file () {\n \t\text=\n \tesac\n \n+\tinitialize_merge_tool \"$merge_tool\" || return\n+\n \tmergetool_tmpdir_init\n \n \tif test \"$MERGETOOL_TMPDIR\" != \".\"\n-- \n2.29.2\n\n\n"},{"id":"416554","messageId":"20210209200712.156540-1-seth@eseth.com","threadId":"54871","inReplyTo":"20210130054655.48237-1-seth@eseth.com","subject":"[PATCH v11 0/3] mergetool: add hideResolved configuration (was automerge)","fromName":"Seth House","fromEmail":"seth@eseth.com","sentAt":"2021-02-09T20:07:09Z","receivedAt":"2021-02-09T20:54:59Z","isPatch":true,"sender":{"key":"seth@eseth.com","avatar":"https://avatars.githubusercontent.com/u/91293?v=4"},"body":"Changes since v10:\n\n- Update the calls to `git config` to return normalized strings.\n\n Junio, thank you for explaining the existence/omission and normalized\n strings tristate. I missed that in the docs and that's perfect.\n\n- Adopt Junio's replacement preference hierarchy conditionals to respect\n opt-ins and not just opt-outs.\n\n Your suggested code worked out-of-box in all the scenarios I could\n think to test. \\o/\n\n- Tweak the mergetool.hideResolved docs to call out the role of LOCAL\n and REMOTE.\n\n- Reword commit messages and docs to better differentiate between config\n flags users set and code that merge tool maintainers write.\n\nSeth House (3):\n mergetool: add hideResolved configuration\n mergetool: break setup_tool out into separate initialization function\n mergetool: add per-tool support and overrides for the hideResolved\n flag\n\n Documentation/config/mergetool.txt | 14 ++++++++\n Documentation/git-mergetool--lib.txt | 4 +++\n git-difftool--helper.sh | 6 ++++\n git-mergetool--lib.sh | 11 ++++--\n git-mergetool.sh | 52 ++++++++++++++++++++++++++++\n t/t7610-mergetool.sh | 18 ++++++++++\n 6 files changed, 102 insertions(+), 3 deletions(-)\n\n-- \n2.29.2\n\n\n"},{"id":"416555","messageId":"20210209200712.156540-2-seth@eseth.com","threadId":"54871","inReplyTo":"20210209200712.156540-1-seth@eseth.com","subject":"[PATCH v11 1/3] mergetool: add hideResolved configuration","fromName":"Seth House","fromEmail":"seth@eseth.com","sentAt":"2021-02-09T20:07:10Z","receivedAt":"2021-02-09T20:54:59Z","isPatch":true,"sender":{"key":"seth@eseth.com","avatar":"https://avatars.githubusercontent.com/u/91293?v=4"},"body":"The purpose of a mergetool is to help the user resolve any conflicts\nthat Git cannot automatically resolve. If there is a conflict that must\nbe resolved manually Git will write a file named MERGED which contains\neverything Git was able to resolve by itself and also everything that it\nwas not able to resolve wrapped in conflict markers.\n\nOne way to think of MERGED is as a two- or three-way diff. If each\n\"side\" of the conflict markers is separately extracted an external tool\ncan represent those conflicts as a side-by-side diff.\n\nHowever many mergetools instead diff LOCAL and REMOTE both of which\ncontain versions of the file from before the merge. Since the conflicts\nGit resolved automatically are not present it forces the user to\nmanually re-resolve those conflicts. Some mergetools also show MERGED\nbut often only for reference and not as the focal point to resolve the\nconflicts.\n\nThis adds a `mergetool.hideResolved` flag that will overwrite LOCAL and\nREMOTE with each corresponding \"side\" of a conflicted file and thus hide\nall conflicts that Git was able to resolve itself. Overwriting these\nfiles will immediately benefit any mergetool that uses them without\nrequiring any changes to the tool.\n\nNo adverse effects were noted in a small survey of popular mergetools[1]\nso this behavior defaults to `true`. However it can be globally disabled\nby setting `mergetool.hideResolved` to `false`.\n\n[1] https://www.eseth.org/2020/mergetools.html\n https://github.com/whiteinge/eseth/blob/c884424769fffb05d87afb33b2cf80cecb4044c3/2020/mergetools.md\n\nOriginal-implementation-by: Felipe Contreras \nSigned-off-by: Seth House \n---\n Documentation/config/mergetool.txt | 10 ++++++++++\n git-mergetool.sh | 14 ++++++++++++++\n t/t7610-mergetool.sh | 18 ++++++++++++++++++\n 3 files changed, 42 insertions(+)\n\ndiff --git a/Documentation/config/mergetool.txt b/Documentation/config/mergetool.txt\nindex 16a27443a3..b858191970 100644\n--- a/Documentation/config/mergetool.txt\n+++ b/Documentation/config/mergetool.txt\n@@ -40,6 +40,16 @@ mergetool.meld.useAutoMerge::\n \tvalue of `false` avoids using `--auto-merge` altogether, and is the\n \tdefault value.\n \n+mergetool.hideResolved::\n+\tDuring a merge Git will automatically resolve as many conflicts as\n+\tpossible and write the 'MERGED' file containing conflict markers around\n+\tany conflicts that it cannot resolve; 'LOCAL' and 'REMOTE' normally\n+\trepresent the versions of the file from before Git's conflict\n+\tresolution. This flag causes 'LOCAL' and 'REMOTE' to be overwriten so\n+\tthat only the unresolved conflicts are presented to the merge tool. Can\n+\tbe configured per-tool via the `mergetool..hideResolved`\n+\tconfiguration variable. Defaults to `true`.\n+\n mergetool.keepBackup::\n \tAfter performing a merge, the original file with conflict markers\n \tcan be saved as a file with a `.orig` extension. If this variable\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex e3f6d543fb..40a103443d 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -239,6 +239,13 @@ checkout_staged_file () {\n \tfi\n }\n \n+hide_resolved () {\n+\tgit merge-file --ours -q -p \"$LOCAL\" \"$BASE\" \"$REMOTE\" >\"$LCONFL\"\n+\tgit merge-file --theirs -q -p \"$LOCAL\" \"$BASE\" \"$REMOTE\" >\"$RCONFL\"\n+\tmv -- \"$LCONFL\" \"$LOCAL\"\n+\tmv -- \"$RCONFL\" \"$REMOTE\"\n+}\n+\n merge_file () {\n \tMERGED=\"$1\"\n \n@@ -276,7 +283,9 @@ merge_file () {\n \n \tBACKUP=\"$MERGETOOL_TMPDIR/${BASE}_BACKUP_$$$ext\"\n \tLOCAL=\"$MERGETOOL_TMPDIR/${BASE}_LOCAL_$$$ext\"\n+\tLCONFL=\"$MERGETOOL_TMPDIR/${BASE}_LOCAL_LCONFL_$$$ext\"\n \tREMOTE=\"$MERGETOOL_TMPDIR/${BASE}_REMOTE_$$$ext\"\n+\tRCONFL=\"$MERGETOOL_TMPDIR/${BASE}_REMOTE_RCONFL_$$$ext\"\n \tBASE=\"$MERGETOOL_TMPDIR/${BASE}_BASE_$$$ext\"\n \n \tbase_mode= local_mode= remote_mode=\n@@ -322,6 +331,11 @@ merge_file () {\n \tcheckout_staged_file 2 \"$MERGED\" \"$LOCAL\"\n \tcheckout_staged_file 3 \"$MERGED\" \"$REMOTE\"\n \n+\tif test \"$(git config --type=bool mergetool.hideResolved)\" != \"false\"\n+\tthen\n+\t\thide_resolved\n+\tfi\n+\n \tif test -z \"$local_mode\" || test -z \"$remote_mode\"\n \tthen\n \t\techo \"Deleted merge conflict for '$MERGED':\"\ndiff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh\nindex 04b0095072..cec4a860ef 100755\n--- a/t/t7610-mergetool.sh\n+++ b/t/t7610-mergetool.sh\n@@ -842,4 +842,22 @@ test_expect_success 'mergetool --tool-help shows recognized tools' '\n \tgrep meld mergetools\n '\n \n+test_expect_success 'mergetool hideResolved' '\n+\ttest_config mergetool.hideResolved true &&\n+\ttest_when_finished \"git reset --hard\" &&\n+\tgit checkout -b test${test_count}_b master &&\n+\ttest_write_lines >file1 base \"\" a &&\n+\tgit commit -a -m \"base\" &&\n+\ttest_write_lines >file1 base \"\" c &&\n+\tgit commit -a -m \"remote update\" &&\n+\tgit checkout -b test${test_count}_a HEAD~ &&\n+\ttest_write_lines >file1 local \"\" b &&\n+\tgit commit -a -m \"local update\" &&\n+\ttest_must_fail git merge test${test_count}_b &&\n+\tyes \"\" | git mergetool file1 &&\n+\ttest_write_lines >expect local \"\" c &&\n+\ttest_cmp expect file1 &&\n+\tgit commit -m \"test resolved with mergetool\"\n+'\n+\n test_done\n-- \n2.29.2\n\n\n"},{"id":"416560","messageId":"20210209200712.156540-4-seth@eseth.com","threadId":"54871","inReplyTo":"20210209200712.156540-1-seth@eseth.com","subject":"[PATCH v11 3/3] mergetool: add per-tool support and overrides for the hideResolved flag","fromName":"Seth House","fromEmail":"seth@eseth.com","sentAt":"2021-02-09T20:07:12Z","receivedAt":"2021-02-09T21:44:14Z","isPatch":true,"sender":{"key":"seth@eseth.com","avatar":"https://avatars.githubusercontent.com/u/91293?v=4"},"body":"Add a per-tool override flag so that users may enable the flag for one\ntool and disable it for another by setting\n`mergetool..hideResolved` to `false`.\n\nIn addition, the author or maintainer of a mergetool may optionally\noverride the default `hideResolved` value for that mergetool. If the\n`mergetools/` shell script contains a `hide_resolved_enabled`\nfunction it will be called when the mergetool is invoked and the return\nvalue will be used as the default for the `hideResolved` flag.\n\n hide_resolved_enabled () {\n return 1\n }\n\nDisabling may be desirable if the mergetool wants or needs access to the\noriginal, unmodified 'LOCAL' and 'REMOTE' versions of the conflicted\nfile. For example:\n\n- A tool may use a custom conflict resolution algorithm and prefer to\n ignore the results of Git's conflict resolution.\n- A tool may want to visually compare/constrast the version of the file\n from before the merge (saved to 'LOCAL', 'REMOTE', and 'BASE') with\n Git's conflict resolution results (saved to 'MERGED').\n\nHelped-by: Johannes Sixt \nHelped-by: Junio C Hamano \nSigned-off-by: Seth House \n---\n Documentation/config/mergetool.txt | 5 +++++\n git-mergetool--lib.sh | 4 ++++\n git-mergetool.sh | 36 +++++++++++++++++++++++++++++-\n 3 files changed, 44 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config/mergetool.txt b/Documentation/config/mergetool.txt\nindex b858191970..90f76f5b9b 100644\n--- a/Documentation/config/mergetool.txt\n+++ b/Documentation/config/mergetool.txt\n@@ -13,6 +13,11 @@ mergetool..cmd::\n \tmerged; 'MERGED' contains the name of the file to which the merge\n \ttool should write the results of a successful merge.\n \n+mergetool..hideResolved::\n+\tAllows the user to override the global `mergetool.hideResolved` value\n+\tfor a specific tool. See `mergetool.hideResolved` for the full\n+\tdescription.\n+\n mergetool..trustExitCode::\n \tFor a custom merge command, specify whether the exit code of\n \tthe merge command can be used to determine whether the merge was\ndiff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh\nindex 4a8e36c792..542a6a75eb 100644\n--- a/git-mergetool--lib.sh\n+++ b/git-mergetool--lib.sh\n@@ -166,6 +166,10 @@ setup_tool () {\n \t\treturn 1\n \t}\n \n+\thide_resolved_enabled () {\n+\t\treturn 0\n+\t}\n+\n \ttranslate_merge_tool_path () {\n \t\techo \"$1\"\n \t}\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex e5eac935f3..911470a5b2 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -333,7 +333,41 @@ merge_file () {\n \tcheckout_staged_file 2 \"$MERGED\" \"$LOCAL\"\n \tcheckout_staged_file 3 \"$MERGED\" \"$REMOTE\"\n \n-\tif test \"$(git config --type=bool mergetool.hideResolved)\" != \"false\"\n+\t# hideResolved preferences hierarchy.\n+\tglobal_config=\"mergetool.hideResolved\"\n+\ttool_config=\"mergetool.${merge_tool}.hideResolved\"\n+\n+\tif enabled=$(git config --type=bool \"$tool_config\")\n+\tthen\n+\t\t# The user has a specific preference for a specific tool and no\n+\t\t# other preferences should override that.\n+\t\t: ;\n+\telif enabled=$(git config --type=bool \"$global_config\")\n+\tthen\n+\t\t# The user has a general preference for all tools.\n+\t\t#\n+\t\t# 'true' means the user likes the feature so we should use it\n+\t\t# where possible but tool authors can still override.\n+\t\t#\n+\t\t# 'false' means the user doesn't like the feature so we should\n+\t\t# not use it anywhere.\n+\t\tif test \"$enabled\" = true && hide_resolved_enabled\n+\t\tthen\n+\t\t enabled=true\n+\t\telse\n+\t\t enabled=false\n+\t\tfi\n+\telse\n+\t\t# The user does not have a preference. Ask the tool.\n+\t\tif hide_resolved_enabled\n+\t\tthen\n+\t\t enabled=true\n+\t\telse\n+\t\t enabled=false\n+\t\tfi\n+\tfi\n+\n+\tif test \"$enabled\" = true\n \tthen\n \t\thide_resolved\n \tfi\n-- \n2.29.2\n\n\n"},{"id":"416566","messageId":"xmqqeehozybq.fsf@gitster.c.googlers.com","threadId":"54871","inReplyTo":"20210209200712.156540-1-seth@eseth.com","subject":"Re: [PATCH v11 0/3] mergetool: add hideResolved configuration (was automerge)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-09T22:11:05Z","receivedAt":"2021-02-09T22:26:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Seth House writes:\n\n> Changes since v10:\n> Seth House (3):\n> mergetool: add hideResolved configuration\n> mergetool: break setup_tool out into separate initialization function\n> mergetool: add per-tool support and overrides for the hideResolved\n> flag\n\nThanks for all these iterations. The resuling series looks good to\nme.\n"},{"id":"416592","messageId":"20210209232714.GA172268@ellen.lan","threadId":"54871","inReplyTo":"xmqqeehozybq.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v11 0/3] mergetool: add hideResolved configuration (was automerge)","fromName":"Seth House","fromEmail":"seth@eseth.com","sentAt":"2021-02-09T23:27:14Z","receivedAt":"2021-02-10T00:11:02Z","isPatch":true,"sender":{"key":"seth@eseth.com","avatar":"https://avatars.githubusercontent.com/u/91293?v=4"},"body":"On Tue, Feb 09, 2021 at 02:11:05PM -0800, Junio C Hamano wrote:\n> Thanks for all these iterations. The resuling series looks good to\n> me.\n\nWoot! Thanks for all the great feedback and for walking me through the\nprocess. I'm really happy with where this ended up.\n\n"},{"id":"418565","messageId":"YEbdj27CmjNKSWf4@google.com","threadId":"54871","inReplyTo":"20210209200712.156540-2-seth@eseth.com","subject":"[PATCH] mergetool: do not enable hideResolved by default","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2021-03-09T02:29:35Z","receivedAt":"2021-03-09T02:30:26Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"A typical mergetool uses four panes, showing the content of the file\nbeing resolved from MERGE_BASE ('BASE'), HEAD ('LOCAL'), MERGE_HEAD\n('REMOTE'), and the working copy. This allows understanding the\nconflicts in context: by seeing the entire content of the file from\nMERGE_HEAD, say, we can see the full intent of the code we are pulling\nin and understand what they were trying to do that conflicted with our\nown changes.\n\nSometimes, though, the exact content of these three competing versions\nof a file is not so important. Especially if the mergetool supports\nfolding unchanged lines, the new 'mergetool.hideResolved' feature can\nbe helpful for allowing a person resolving a merge to focus on the\nportion with conflicts. For sections of the file where BASE matched\nLOCAL or REMOTE, this feature makes all three versions match the\nresolved version, so that the user resolving can focus exclusively on\nthe portions with conflicts. In other words, hideResolved makes a\nmulti-pane merge tool show a similar amount of information to the file\nwith conflict markers with conflictstyle=diff3, saving the operator\nfrom having to pay attention to parts that resolved cleanly.\n\n98ea309b3f (mergetool: add hideResolved configuration, 2021-02-09)\nwhich introduced this setting enabled it by default, explaining:\n\n No adverse effects were noted in a small survey of popular mergetools[1]\n so this behavior defaults to `true`. However it can be globally disabled\n by setting `mergetool.hideResolved` to `false`.\n\nIn practice, however, this has proved confusing for users. No\nindication is shown in the UI that the base, local, and remote\nversions shown have been modified by additional resolution.\nEspecially in cases where conflicts involve elements beyond textual\nconflict, it has resulted in incorrect resolutions and wasted work to\nfigure out what happened. Flip the default back to the traditional\nbehavior of `false`: although the old behavior involves slightly\nslower merges in the only-textual-conflicts case, it prevents this\nkind of painful moment of betrayal by one's tools, which is more\nimportant.\n\nShould we want to migrate to hideResolved=true in the future, we still\ncan. It just requires a more careful migration, including a period\nwhere \"git mergetool\" shows a warning or errors out in affected cases.\n\nReported-by: Dana Dahlstrom \nSigned-off-by: Jonathan Nieder \n---\nHi,\n\nSeth House wrote:\n\n> No adverse effects were noted in a small survey of popular mergetools[1]\n> so this behavior defaults to `true`. However it can be globally disabled\n> by setting `mergetool.hideResolved` to `false`.\n\nThanks much for protecting this by a flag. We tried this out\ninternally at Google when it hit \"next\" and not too long later\nrealized that the new default of \"true\" is not workable for us. I\ndon't believe it's the right default for Git, either, hence this\npatch.\n\nThanks for working on the merge resolution workflow; it's much\nappreciated.\n\nSincerely,\nJonathan\n\n Documentation/config/mergetool.txt | 2 +-\n git-mergetool.sh | 9 ++-------\n 2 files changed, 3 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/config/mergetool.txt b/Documentation/config/mergetool.txt\nindex 90f76f5b9b..cafbbef46a 100644\n--- a/Documentation/config/mergetool.txt\n+++ b/Documentation/config/mergetool.txt\n@@ -53,7 +53,7 @@ mergetool.hideResolved::\n \tresolution. This flag causes 'LOCAL' and 'REMOTE' to be overwriten so\n \tthat only the unresolved conflicts are presented to the merge tool. Can\n \tbe configured per-tool via the `mergetool..hideResolved`\n-\tconfiguration variable. Defaults to `true`.\n+\tconfiguration variable. Defaults to `false`.\n \n mergetool.keepBackup::\n \tAfter performing a merge, the original file with conflict markers\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex 911470a5b2..f751d9cfe2 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -358,13 +358,8 @@ merge_file () {\n \t\t enabled=false\n \t\tfi\n \telse\n-\t\t# The user does not have a preference. Ask the tool.\n-\t\tif hide_resolved_enabled\n-\t\tthen\n-\t\t enabled=true\n-\t\telse\n-\t\t enabled=false\n-\t\tfi\n+\t\t# The user does not have a preference. Default to disabled.\n+\t\tenabled=false\n \tfi\n \n \tif test \"$enabled\" = true\n-- \n2.31.0.rc1.246.gcd05c9c855\n\n"},{"id":"418567","messageId":"YEcKy83ZmvGTAfxq@ellen.lan","threadId":"54871","inReplyTo":"YEbdj27CmjNKSWf4@google.com","subject":"Re: [PATCH] mergetool: do not enable hideResolved by default","fromName":"Seth House","fromEmail":"seth@eseth.com","sentAt":"2021-03-09T05:42:35Z","receivedAt":"2021-03-09T05:43:47Z","isPatch":true,"sender":{"key":"seth@eseth.com","avatar":"https://avatars.githubusercontent.com/u/91293?v=4"},"body":"On Mon, Mar 08, 2021 at 06:29:35PM -0800, Jonathan Nieder wrote:\n> A typical mergetool uses four panes, showing the content of the file\n> being resolved from MERGE_BASE ('BASE'), HEAD ('LOCAL'), MERGE_HEAD\n> ('REMOTE'), and the working copy. This allows understanding the\n> conflicts in context: by seeing the entire content of the file from\n> MERGE_HEAD, say, we can see the full intent of the code we are pulling\n> in and understand what they were trying to do that conflicted with our\n> own changes.\n\nWell said. Agreed on all counts.\n\nThe very early days of these patch sets touched on this exact discussion\npoint. (I'd link to it but that early discussion was a tad...unfocused.)\nI make semi-frequent reference of those versions of the conflicted file\nin the way you describe and have disabled hideResolved for a merge tool\nI maintain for that reason.\n\n> No adverse effects were noted in a small survey of popular mergetools[1]\n> so this behavior defaults to `true`. However it can be globally disabled\n> by setting `mergetool.hideResolved` to `false`.\n> \n> In practice, however, this has proved confusing for users. No\n> indication is shown in the UI that the base, local, and remote\n> versions shown have been modified by additional resolution.\n\nCompelling point. This flag drastically changes what LOCAL and REMOTE\nrepresent with little to no explanation.\n\nThere are three options to achieve the same end-goal of hideResolved\nthat I've thought of:\n\n1. Individual merge tools should do this work, not Git.\n\n A merge tool already has all the information needed to hide\n already-resolved conflicts since that is what MERGED represents.\n Conflict markers *are* a two-way diff and a merge tool should\n display them as such, rather than display the textual markers\n verbatim.\n\n In many ways this is the ideal approach -- all merge tools could be\n doing this with existing Git right now but none have seemingly\n thought of doing so yet.\n\n2. Git could pass six versions of the conflicted file to a merge tool,\n rather than the current four.\n\n Merge tools could accept LOCAL, REMOTE, BASE, MERGED (as most\n currently do), and also LCONFL and RCONFL files. The latter two\n being copies of MERGED but \"pre split\" by Git into the left\n conflicts and the right conflicts.\n\n This would spare the merge tool the work of splitting MERGED. It may\n encourage them to continue displaying LOCAL and REMOTE as useful\n context but also make it easy to diff LCONFL with RCONFL and use\n that diff to actually resolve the conflict. It could also make\n things worse, as many tools simply diff _every_ file Git gives them\n regardless if that makes sense or not (>_<).\n\n3. Git could overwrite LOCAL and REMOTE to display only unresolved\n conflicts.\n\n (The current hideResolved addition.) This has the pragmatic benefit\n of requiring the least amount of change for all merge tools, but to\n your point above, *destroys* valuable data -- the additional context\n to help understand where the conflicts came from -- and that data\n can't be viewd without running additional Git commands to fetch it.\n\nDefaulting hideResolved to off is a fine change IMO. We don't have a way\nto communicate to the end-user that LOCAL and REMOTE represent something\nmarkedly different than what they have traditionally represented, so\nhaving this be an opt-in will force the user to read the docs and\nunderstand the ramifications.\n\nI really appreciate your thoughts that accompanied this patch. Sorry for\nthe long response but your email made me want to ask the question:\n\nDoes the need to default hideResolved to off mean that it is the wrong\napproach?\n\nThinking through an end-user's workflow: would a user want to configure\ntwo copies of the same merge tool -- one with hideResolved and one\nwithout? An easy conflict could benefit from the former but if it's\na tricky conflict the user would have to exit the tool and reopen the\nsame tool without the flag. That sounds like an annoying workflow, and\nalthough the user would now have that extra, valuable context it would\nalso put them squarely back into the current state of viewing\nalready-resolved conflicts.\n\nI know the Option 3, hideResolved, is merged and has that momentum and\nthis patch looks good to me -- but perhaps Option 2 is more \"correct\",\nor Option 1, or yet another option I haven't thought of. Thoughts?\n\n"},{"id":"418674","messageId":"YEgfhYSz7VaCtvH1@google.com","threadId":"54871","inReplyTo":"YEcKy83ZmvGTAfxq@ellen.lan","subject":"Re: [PATCH] mergetool: do not enable hideResolved by default","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2021-03-10T01:23:17Z","receivedAt":"2021-03-10T01:24:14Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nSeth House wrote:\n\n> The very early days of these patch sets touched on this exact discussion\n> point. (I'd link to it but that early discussion was a tad...unfocused.)\n> I make semi-frequent reference of those versions of the conflicted file\n> in the way you describe and have disabled hideResolved for a merge tool\n> I maintain for that reason.\n\nThanks. Do you have a public example of a merge that was produced in\nsuch a way? It might help focus the discussion.\n\nFor concreteness' sake: in the repository that Dana mentioned, one can\nsee some merges from before hideResolved at\nhttps://android.googlesource.com/platform/tools/idea/+log/mirror-goog-studio-master-dev/build.txt.\n\nThe xml files there (I'm not sure these are the right ones for me to\nfocus on, just commenting as I observe) remind me of other routine\nconflicts with xml I've had to resolve in the past, e.g. at\nhttps://git.eclipse.org/r/c/jgit/jgit/+/134451/3. Having information\nfrom each side of the merge and not a mixture can be very helpful in\nthis kind of case. That's especially true when the three-way merge\nalgorithm didn't end up lining up the files correctly, which has\nhappened from time to time to me in files with repetitive structure.\n\n[...]\n> There are three options to achieve the same end-goal of hideResolved\n> that I've thought of:\n>\n> 1. Individual merge tools should do this work, not Git.\n>\n> A merge tool already has all the information needed to hide\n> already-resolved conflicts since that is what MERGED represents.\n> Conflict markers *are* a two-way diff and a merge tool should\n> display them as such, rather than display the textual markers\n> verbatim.\n>\n> In many ways this is the ideal approach -- all merge tools could be\n> doing this with existing Git right now but none have seemingly\n> thought of doing so yet.\n\nOne obstacle to this is that a merge tool can't count on the file in\nthe worktree containing pristine conflict markers, because the user\nmay have already started to work on the merge resolution.\n\n> 2. Git could pass six versions of the conflicted file to a merge tool,\n> rather than the current four.\n>\n> Merge tools could accept LOCAL, REMOTE, BASE, MERGED (as most\n> currently do), and also LCONFL and RCONFL files. The latter two\n> being copies of MERGED but \"pre split\" by Git into the left\n> conflicts and the right conflicts.\n>\n> This would spare the merge tool the work of splitting MERGED. It may\n> encourage them to continue displaying LOCAL and REMOTE as useful\n> context but also make it easy to diff LCONFL with RCONFL and use\n> that diff to actually resolve the conflict. It could also make\n> things worse, as many tools simply diff _every_ file Git gives them\n> regardless if that makes sense or not (>_<).\n\nInteresting! I kind of like this, especially if it were something the\ntool could opt in to. That said, I'm not the best person to ask, since\nI never ended up finding a good workflow using mergetool for my own use;\ninstead, I tend to do the work of a merge tool \"by hand\":\n\n- gradually resolving the merge in each diff3-style conflict hunk by\n removing common lines from base+local and base+remote until there is\n nothing left in base\n\n- in harder cases, making the worktree match the local version,\n putting the diff from base to remote in a temporary file, and then\n hunk by hunk applying it\n\n- in even harder cases, using git-imerge\n \n\n[...]\n> 3. Git could overwrite LOCAL and REMOTE to display only unresolved\n> conflicts.\n>\n> (The current hideResolved addition.) This has the pragmatic benefit\n> of requiring the least amount of change for all merge tools,\n\nThat's a good argument for having the option available, *as long as\nthe user explicitly turns it on*.\n\n[...]\n> Does the need to default hideResolved to off mean that it is the wrong\n> approach?\n\nOne disadvantage relative to (1) is that the mergetool has no way to\nvisually distinguish the automatically resolved portion. For that\nreason, I suspect this will never be something we can make the\ndefault. But in principle I'm not against it existing.\n\nThe implementation is concise and maintainable. The documentation\nadds a little user-facing complexity; I think as long as we're able\nto keep it clear and well maintained, that should be okay.\n\ngit-mergetool.txt probably ought to mention the hideResolved setting.\nOtherwise, users can have a confusing experience if they set the\nconfig once and forget about it later.\n\n[...]\n> Thinking through an end-user's workflow: would a user want to configure\n> two copies of the same merge tool -- one with hideResolved and one\n> without? An easy conflict could benefit from the former but if it's\n> a tricky conflict the user would have to exit the tool and reopen the\n> same tool without the flag. That sounds like an annoying workflow, and\n> although the user would now have that extra, valuable context it would\n> also put them squarely back into the current state of viewing\n> already-resolved conflicts.\n>\n> I know the Option 3, hideResolved, is merged and has that momentum and\n> this patch looks good to me -- but perhaps Option 2 is more \"correct\",\n> or Option 1, or yet another option I haven't thought of. Thoughts?\n\nI suspect option 1 is indeed more correct. Dana mentions that some\nmergetools (p4merge?) use different colors to highlight the\n'automatically resolved' portions, something that isn't possible using\noption 3.\n\nThanks,\nJonathan\n"},{"id":"418682","messageId":"xmqqmtvbjuvl.fsf@gitster.g","threadId":"54871","inReplyTo":"YEbdj27CmjNKSWf4@google.com","subject":"Re: [PATCH] mergetool: do not enable hideResolved by default","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-10T08:06:06Z","receivedAt":"2021-03-10T08:06:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> diff --git a/git-mergetool.sh b/git-mergetool.sh\n> index 911470a5b2..f751d9cfe2 100755\n> --- a/git-mergetool.sh\n> +++ b/git-mergetool.sh\n> @@ -358,13 +358,8 @@ merge_file () {\n> \t\t enabled=false\n> \t\tfi\n> \telse\n> -\t\t# The user does not have a preference. Ask the tool.\n> -\t\tif hide_resolved_enabled\n> -\t\tthen\n> -\t\t enabled=true\n> -\t\telse\n> -\t\t enabled=false\n> -\t\tfi\n> +\t\t# The user does not have a preference. Default to disabled.\n> +\t\tenabled=false\n\nOK. So the logic used to be\n\n - If the user has preference for a specific backend, use it;\n\n - If the user says the feature is unwanted, that is final;\n\n - If the user says it generally is OK to use the feature, let each\n backend set the preference;\n\n - If there is no preference, let each backend set the preference.\n\nAs we want to disable the feature for any backend when the user does\nnot explicitly say the feature is wanted (either in general, or for\na specific backend), the change in the above hunk is exactly want we\nwant to see.\n\nLooking good. Let's not revert the series and disable by default.\n\nShould I expect an updated log message, though? What was in the\nproposed log message sounded more unsubstantiated complaint than\ngiving readable reasons why the feature is unwanted, but both the\nresponse by Seth and your response to Seth's response had material\nthat made it more convincing why we would want to disable this by\ndefault, e.g. \"with little to no explanation\", \"We don't have a way\nto communicate to the end-user\" (both by Seth), \"when ... didn't end\nup lining up the files correctly\", \"no way to visually distinguish\"\n(yours) are all good ingredients to explain why this feature is\nprone to subtly and silently give wrong information to the\nend-users.\n\nThanks.\n"},{"id":"418779","messageId":"xmqqzgzafo5o.fsf@gitster.g","threadId":"54871","inReplyTo":"xmqqmtvbjuvl.fsf@gitster.g","subject":"Re: [PATCH] mergetool: do not enable hideResolved by default","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-11T01:57:07Z","receivedAt":"2021-03-11T01:58:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> As we want to disable the feature for any backend when the user does\n> not explicitly say the feature is wanted (either in general, or for\n> a specific backend), the change in the above hunk is exactly want we\n> want to see.\n>\n> Looking good. Let's not revert the series and disable by default.\n>\n> Should I expect an updated log message, though? What was in the\n> proposed log message sounded more unsubstantiated complaint than\n> giving readable reasons why the feature is unwanted, but both the\n> response by Seth and your response to Seth's response had material\n> that made it more convincing why we would want to disable this by\n> default, e.g. \"with little to no explanation\", \"We don't have a way\n> to communicate to the end-user\" (both by Seth), \"when ... didn't end\n> up lining up the files correctly\", \"no way to visually distinguish\"\n> (yours) are all good ingredients to explain why this feature is\n> prone to subtly and silently give wrong information to the\n> end-users.\n\nFor tonight's pushout, I'll use the patch as-is and merge it in\n'seen'.\n\nThanks.\n"},{"id":"419014","messageId":"xmqqlfas55mk.fsf@gitster.g","threadId":"54871","inReplyTo":"xmqqzgzafo5o.fsf@gitster.g","subject":"Re: [PATCH] mergetool: do not enable hideResolved by default","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-12T23:12:03Z","receivedAt":"2021-03-12T23:13:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Junio C Hamano writes:\n>\n>> As we want to disable the feature for any backend when the user does\n>> not explicitly say the feature is wanted (either in general, or for\n>> a specific backend), the change in the above hunk is exactly want we\n>> want to see.\n>>\n>> Looking good. Let's not revert the series and disable by default.\n>>\n>> Should I expect an updated log message, though? What was in the\n>> proposed log message sounded more unsubstantiated complaint than\n>> giving readable reasons why the feature is unwanted, but both the\n>> response by Seth and your response to Seth's response had material\n>> that made it more convincing why we would want to disable this by\n>> default, e.g. \"with little to no explanation\", \"We don't have a way\n>> to communicate to the end-user\" (both by Seth), \"when ... didn't end\n>> up lining up the files correctly\", \"no way to visually distinguish\"\n>> (yours) are all good ingredients to explain why this feature is\n>> prone to subtly and silently give wrong information to the\n>> end-users.\n>\n> For tonight's pushout, I'll use the patch as-is and merge it in\n> 'seen'.\n\nAny progress here?\n"},{"id":"419017","messageId":"YEv5d0pGvEVpepoY@google.com","threadId":"54871","inReplyTo":"xmqqlfas55mk.fsf@gitster.g","subject":"Re: [PATCH] mergetool: do not enable hideResolved by default","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2021-03-12T23:29:59Z","receivedAt":"2021-03-12T23:30:46Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n> Junio C Hamano writes:\n> > Junio C Hamano writes:\n\n>>> As we want to disable the feature for any backend when the user does\n>>> not explicitly say the feature is wanted (either in general, or for\n>>> a specific backend), the change in the above hunk is exactly want we\n>>> want to see.\n>>>\n>>> Looking good. Let's not revert the series and disable by default.\n>>>\n>>> Should I expect an updated log message, though?\n[...]\n>> For tonight's pushout, I'll use the patch as-is and merge it in\n>> 'seen'.\n>\n> Any progress here?\n\nSorry for the delay. I should be able to send out an improved log\nmessage (more concise and summarizing the supporting info from this\nthread) later this afternoon.\n\nThanks,\nJonathan\n"},{"id":"419018","messageId":"xmqqh7lg54h4.fsf@gitster.g","threadId":"54871","inReplyTo":"YEv5d0pGvEVpepoY@google.com","subject":"Re: [PATCH] mergetool: do not enable hideResolved by default","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-12T23:36:55Z","receivedAt":"2021-03-12T23:37:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n>> Any progress here?\n>\n> Sorry for the delay. I should be able to send out an improved log\n> message (more concise and summarizing the supporting info from this\n> thread) later this afternoon.\n\nThanks. I think this is the last known regression in the -rc, and\nan update before the final happens on coming Monday is very much\nappreciated.\n"},{"id":"419027","messageId":"YEx5hM/HWby3FBJv@google.com","threadId":"54871","inReplyTo":"xmqqh7lg54h4.fsf@gitster.g","subject":"[PATCH v2 0/2] mergetool: do not enable hideResolved by default","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2021-03-13T08:36:20Z","receivedAt":"2021-03-13T08:37:17Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n> Jonathan Nieder writes:\n\n>> I should be able to send out an improved log\n>> message (more concise and summarizing the supporting info from this\n>> thread) later this afternoon.\n>\n> Thanks. I think this is the last known regression in the -rc, and\n> an update before the final happens on coming Monday is very much\n> appreciated.\n\nA little late, but here it is. Thoughts of all kinds welcome, as\nalways.\n\nJonathan Nieder (2):\n mergetool: do not enable hideResolved by default\n doc: describe mergetool configuration in git-mergetool page\n\n Documentation/config/mergetool.txt | 2 +-\n Documentation/git-mergetool.txt | 4 ++++\n git-mergetool.sh | 9 ++-------\n 3 files changed, 7 insertions(+), 8 deletions(-)\n"},{"id":"419028","messageId":"YEx6GNybrU5mrlNi@google.com","threadId":"54871","inReplyTo":"YEx5hM/HWby3FBJv@google.com","subject":"[PATCH 1/2] mergetool: do not enable hideResolved by default","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2021-03-13T08:38:48Z","receivedAt":"2021-03-13T08:39:27Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"When 98ea309b3f (mergetool: add hideResolved configuration,\n2021-02-09) introduced the mergetool.hideResolved setting to reduce\nthe clutter in viewing non-conflicted sections of files in a\nmergetool, it enabled it by default, explaining:\n\n No adverse effects were noted in a small survey of popular mergetools[1]\n so this behavior defaults to `true`.\n\nIn practice, alas, adverse effects do appear. A few issues:\n\n1. No indication is shown in the UI that the base, local, and remote\n versions shown have been modified by additional resolution. This\n is inherent in the design: the idea of mergetool.hideResolved is to\n convince a mergetool that expects pristine local, base, and remote\n files to show partially resolved verisons of those files instead;\n there is no additional source of information accessible to the\n mergetool to see where the resolution has happened.\n\n (By contrast, a mergetool generating the partial resolution from\n conflict markers for itself would be able to hilight the resolved\n sections with a different color.)\n\n A user accustomed to seeing the files without partial resolution\n gets no indication that this behavior has changed when they upgrade\n Git.\n\n2. If the computed merge did not line up the files correctly (for\n example due to repeated sections in the file), the partially\n resolved files can be misleading and do not have enough information\n to reconstruct what happened and compute the correct merge result.\n\n3. Resolving a conflict can involve information beyond the textual\n conflict. For example, if the local and remote versions added\n overlapping functionality in different ways, seeing the full\n unresolved versions of each alongside the base gives information\n about each side's intent that makes it possible to come up with a\n resolution that combines those two intents. By contrast, when\n starting with partially resolved versions of those files, one can\n produce a subtly wrong resolution that includes redundant extra\n code added by one side that is not needed in the approach taken\n on the other.\n\nAll that said, a user wanting to focus on textual conflicts with\nreduced clutter can still benefit from mergetool.hideResolved=true as\na way to deemphasize sections of the code that resolve cleanly without\nrequiring any changes to the invoked mergetool. The caveats described\nabove are reduced when the user has explicitly turned this on, because\nthen the user is aware of them.\n\nFlip the default to 'false'.\n\nReported-by: Dana Dahlstrom \nHelped-by: Seth House \nSigned-off-by: Jonathan Nieder \n---\nOnly difference from v1 is the commit message.\n\n Documentation/config/mergetool.txt | 2 +-\n git-mergetool.sh | 9 ++-------\n 2 files changed, 3 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/config/mergetool.txt b/Documentation/config/mergetool.txt\nindex 90f76f5b9ba..cafbbef46ae 100644\n--- a/Documentation/config/mergetool.txt\n+++ b/Documentation/config/mergetool.txt\n@@ -53,7 +53,7 @@ mergetool.hideResolved::\n \tresolution. This flag causes 'LOCAL' and 'REMOTE' to be overwriten so\n \tthat only the unresolved conflicts are presented to the merge tool. Can\n \tbe configured per-tool via the `mergetool..hideResolved`\n-\tconfiguration variable. Defaults to `true`.\n+\tconfiguration variable. Defaults to `false`.\n \n mergetool.keepBackup::\n \tAfter performing a merge, the original file with conflict markers\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex 911470a5b2c..f751d9cfe20 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -358,13 +358,8 @@ merge_file () {\n \t\t enabled=false\n \t\tfi\n \telse\n-\t\t# The user does not have a preference. Ask the tool.\n-\t\tif hide_resolved_enabled\n-\t\tthen\n-\t\t enabled=true\n-\t\telse\n-\t\t enabled=false\n-\t\tfi\n+\t\t# The user does not have a preference. Default to disabled.\n+\t\tenabled=false\n \tfi\n \n \tif test \"$enabled\" = true\n-- \n2.31.0.rc2.261.g7f71774620\n\n"},{"id":"419029","messageId":"YEx6ve6AbqacVTQH@google.com","threadId":"54871","inReplyTo":"YEx5hM/HWby3FBJv@google.com","subject":"[PATCH 2/2] doc: describe mergetool configuration in git-mergetool(1)","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2021-03-13T08:41:33Z","receivedAt":"2021-03-13T08:42:40Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"In particular, this describes mergetool.hideResolved, which can help\nusers discover this setting (either because it may be useful to them\nor in order to understand mergetool's behavior if they have forgotten\nsetting it in the past).\n\nTested by running\n\n\tmake -C Documentation git-mergetool.1\n\tman Documentation/git-mergetool.1\n\nand reading through the page.\n\nSigned-off-by: Jonathan Nieder \n---\n Documentation/git-mergetool.txt | 4 ++++\n 1 file changed, 4 insertions(+)\n\nThanks for reading.\n\ndiff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt\nindex 6b14702e784..e587c7763a7 100644\n--- a/Documentation/git-mergetool.txt\n+++ b/Documentation/git-mergetool.txt\n@@ -99,6 +99,10 @@ success of the resolution after the custom tool has exited.\n \t(see linkgit:git-config[1]). To cancel `diff.orderFile`,\n \tuse `-O/dev/null`.\n \n+CONFIGURATION\n+-------------\n+include::config/mergetool.txt[]\n+\n TEMPORARY FILES\n ---------------\n `git mergetool` creates `*.orig` backup files while resolving merges.\n-- \n2.31.0.rc2.261.g7f71774620\n\n"},{"id":"419061","messageId":"xmqqsg4y4ohr.fsf@gitster.g","threadId":"54871","inReplyTo":"YEx6ve6AbqacVTQH@google.com","subject":"Re: [PATCH 2/2] doc: describe mergetool configuration in git-mergetool(1)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-13T23:34:24Z","receivedAt":"2021-03-13T23:35:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n> diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt\n> index 6b14702e784..e587c7763a7 100644\n> --- a/Documentation/git-mergetool.txt\n> +++ b/Documentation/git-mergetool.txt\n> @@ -99,6 +99,10 @@ success of the resolution after the custom tool has exited.\n> \t(see linkgit:git-config[1]). To cancel `diff.orderFile`,\n> \tuse `-O/dev/null`.\n> \n> +CONFIGURATION\n> +-------------\n> +include::config/mergetool.txt[]\n> +\n\nIt is a nice touch. We don't have much explanation other than the\ndescription below ...\n\nmergetool.hideResolved::\n\tDuring a merge Git will automatically resolve as many conflicts as\n\tpossible and write the 'MERGED' file containing conflict markers around\n\tany conflicts that it cannot resolve; 'LOCAL' and 'REMOTE' normally\n\trepresent the versions of the file from before Git's conflict\n\tresolution. This flag causes 'LOCAL' and 'REMOTE' to be overwriten so\n\tthat only the unresolved conflicts are presented to the merge tool. Can\n\tbe configured per-tool via the `mergetool..hideResolved`\n\tconfiguration variable. Defaults to `false`.\n\n... which appears in the included file on the feature.\n\nThanks.\n"},{"id":"419062","messageId":"xmqqo8fm4oc6.fsf@gitster.g","threadId":"54871","inReplyTo":"YEx6ve6AbqacVTQH@google.com","subject":"Re: [PATCH 2/2] doc: describe mergetool configuration in git-mergetool(1)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-13T23:37:45Z","receivedAt":"2021-03-13T23:38:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Tested by running\n>\n> \tmake -C Documentation git-mergetool.1\n> \tman Documentation/git-mergetool.1\n>\n> and reading through the page.\n\nNice. Also applying this step and running\n\n\tcd Documentation && ./doc-diff HEAD^ HEAD\n\nwould was a trivial way to see the change ;-)\n"}]}