{"thread":{"id":"39648","subject":"[PATCH] mergetools: add config option to disable auto-merge","startedAt":"2015-06-16T21:35:13Z","lastAt":"2015-06-18T15:14:49Z","messageCount":6,"participants":["Michael Rappazzo","Junio C Hamano","Mike Rappazzo","David Aguilar"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"263977","messageId":"1434490514-36204-1-git-send-email-rappazzo@gmail.com","threadId":"39648","inReplyTo":null,"subject":"[PATCH] mergetools: add config option to disable auto-merge","fromName":"Michael Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2015-06-16T21:35:13Z","receivedAt":"2015-06-16T21:35:13Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"During conflict resolution, invoking a mergetool which supports\nauto-merge will pass that option by default to the underlying tool.\n\nThis patch is intended to allow one to set 'merge.automerge' to\n'false' in order to override this behavior.\n\nMichael Rappazzo (1):\n mergetools: add config option to disable auto-merge\n\n Documentation/config.txt | 6 ++++++\n Documentation/git-mergetool.txt | 19 +++++++++++++------\n mergetools/araxis | 4 +++-\n mergetools/diffmerge | 4 +++-\n mergetools/kdiff3 | 4 +++-\n 5 files changed, 28 insertions(+), 9 deletions(-)\n\n-- \n2.4.2\n"},{"id":"263976","messageId":"1434490514-36204-2-git-send-email-rappazzo@gmail.com","threadId":"39648","inReplyTo":"1434490514-36204-1-git-send-email-rappazzo@gmail.com","subject":"[PATCH] mergetools: add config option to disable auto-merge","fromName":"Michael Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2015-06-16T21:35:14Z","receivedAt":"2015-06-16T21:35:14Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"For some mergetools, the current invocation of git mergetool will\ninclude an auto-merge flag. By default the flag is included, however if\nthe git config option 'merge.automerge' is set to 'false', then that\nflag will now be omitted.\n\nSigned-off-by: Michael Rappazzo \n---\n Documentation/config.txt | 6 ++++++\n Documentation/git-mergetool.txt | 19 +++++++++++++------\n mergetools/araxis | 4 +++-\n mergetools/diffmerge | 4 +++-\n mergetools/kdiff3 | 4 +++-\n 5 files changed, 28 insertions(+), 9 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 43bb53c..658d8f7 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1864,6 +1864,12 @@ mergetool.meld.hasOutput::\n \tto `true` tells Git to unconditionally use the `--output` option,\n \tand `false` avoids using `--output`.\n \n+mergetool.automerge::\n+\tWhen invoking a custom merge tool which includes an auto-merge\n+\toption, Git will include that option by default. If this variable\n+\tis set to `false` then the auto-merge option is not used when \n+\tinvoking the custom merge tool.\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/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt\nindex e846c2e..3461d3a 100644\n--- a/Documentation/git-mergetool.txt\n+++ b/Documentation/git-mergetool.txt\n@@ -79,16 +79,23 @@ success of the resolution after the custom tool has exited.\n \tPrompt before each invocation of the merge resolution program\n \tto give the user a chance to skip the path.\n \n-TEMPORARY FILES\n----------------\n-`git mergetool` creates `*.orig` backup files while resolving merges.\n-These are safe to remove once a file has been merged and its\n-`git mergetool` session has completed.\n-\n+CONFIGURATION OPTIONS\n+---------------------\n+mergetool.keepBackup::\n+\t`git mergetool` creates `*.orig` backup files while resolving merges.\n+\tThese are safe to remove once a file has been merged and its\n+\t`git mergetool` session has completed.\n++\n Setting the `mergetool.keepBackup` configuration variable to `false`\n causes `git mergetool` to automatically remove the backup as files\n are successfully merged.\n \n+mergetool.automerge::\n+\tFor some mergetools, the current invocation of git mergetool will\n+\tinclude an auto-merge flag. By default the flag is included, however if\n+\tthe git config option `merge.automerge` is set to `false`, then that\n+\tflag will be omitted.\n+\n GIT\n ---\n Part of the linkgit:git[1] suite\ndiff --git a/mergetools/araxis b/mergetools/araxis\nindex 64f97c5..00e63da 100644\n--- a/mergetools/araxis\n+++ b/mergetools/araxis\n@@ -6,7 +6,9 @@ merge_cmd () {\n \ttouch \"$BACKUP\"\n \tif $base_present\n \tthen\n-\t\t\"$merge_tool_path\" -wait -merge -3 -a1 \\\n+\t\tautomerge=\"-merge\"\n+\t\ttest \"$(git config --get --bool mergetool.automerge)\" = false && automerge=''\n+\t\t\"$merge_tool_path\" -wait ${automerge} -3 -a1 \\\n \t\t\t\"$BASE\" \"$LOCAL\" \"$REMOTE\" \"$MERGED\" >/dev/null 2>&1\n \telse\n \t\t\"$merge_tool_path\" -wait -2 \\\ndiff --git a/mergetools/diffmerge b/mergetools/diffmerge\nindex f138cb4..287489b 100644\n--- a/mergetools/diffmerge\n+++ b/mergetools/diffmerge\n@@ -5,7 +5,9 @@ diff_cmd () {\n merge_cmd () {\n \tif $base_present\n \tthen\n-\t\t\"$merge_tool_path\" --merge --result=\"$MERGED\" \\\n+\t\tautomerge=\"--merge\"\n+\t\ttest \"$(git config --get --bool mergetool.automerge)\" = false && automerge=''\n+\t\t\"$merge_tool_path\" ${automerge} --result=\"$MERGED\" \\\n \t\t\t\"$LOCAL\" \"$BASE\" \"$REMOTE\"\n \telse\n \t\t\"$merge_tool_path\" --merge \\\ndiff --git a/mergetools/kdiff3 b/mergetools/kdiff3\nindex 793d129..8e1d063 100644\n--- a/mergetools/kdiff3\n+++ b/mergetools/kdiff3\n@@ -7,7 +7,9 @@ diff_cmd () {\n merge_cmd () {\n \tif $base_present\n \tthen\n-\t\t\"$merge_tool_path\" --auto \\\n+\t\tautomerge=\"--auto\"\n+\t\ttest \"$(git config --get --bool mergetool.automerge)\" = false && automerge=''\n+\t\t\"$merge_tool_path\" ${automerge} \\\n \t\t\t--L1 \"$MERGED (Base)\" \\\n \t\t\t--L2 \"$MERGED (Local)\" \\\n \t\t\t--L3 \"$MERGED (Remote)\" \\\n-- \n2.4.2\n"},{"id":"264069","messageId":"xmqqd20u6s2b.fsf@gitster.dls.corp.google.com","threadId":"39648","inReplyTo":"1434490514-36204-2-git-send-email-rappazzo@gmail.com","subject":"Re: [PATCH] mergetools: add config option to disable auto-merge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-17T19:41:32Z","receivedAt":"2015-06-17T19:41:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Rappazzo writes:\n\n> For some mergetools, the current invocation of git mergetool will\n> include an auto-merge flag. By default the flag is included, however if\n> the git config option 'merge.automerge' is set to 'false', then that\n> flag will now be omitted.\n\n... and why is the \"automerge\" a bad thing that user would want to\navoid triggering under which condition? That description may not\nhave to be in the proposed log message, but it would help users when\nthey decide if they want to use the configuration to describe it in\nthe mergetool.automerge configuration.\n\nAnd depending on the answer to the above question, a configuration\nvariable may turn out be a bad mechanism to customize this (namely,\nset-and-forget configuration variable is a bad match for a knob that\nis more \"per invocation\" than \"user taste\").\n\nIs this not about \"automerge\" but more about \"always-show-UI because\nI like GUI?\" Then that may be a \"user taste\" thing that is a good\nmatch for a configuration variable. I simply cannot tell from what\nwas in the message I am responding to.\n\n> -TEMPORARY FILES\n> ----------------\n> -`git mergetool` creates `*.orig` backup files while resolving merges.\n> -These are safe to remove once a file has been merged and its\n> -`git mergetool` session has completed.\n> -\n> +CONFIGURATION OPTIONS\n> +---------------------\n> +mergetool.keepBackup::\n> +\t`git mergetool` creates `*.orig` backup files while resolving merges.\n> +\tThese are safe to remove once a file has been merged and its\n> +\t`git mergetool` session has completed.\n> ++\n\nThis is an unrelated change; I think it is a good change, though.\n\nI however suspect that we would not want to repeat the configuration\ndescription in this file and instead mention these in \"see also\"\nsection referring the readers to git-config(1).\n"},{"id":"264107","messageId":"CANoM8SWN2ZzgMP_MoDRRFA8ZjzRh724iaJ7GwrD2WddrjqwuqQ@mail.gmail.com","threadId":"39648","inReplyTo":"xmqqd20u6s2b.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] mergetools: add config option to disable auto-merge","fromName":"Mike Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2015-06-18T02:27:58Z","receivedAt":"2015-06-18T02:27:58Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"On Wed, Jun 17, 2015 at 3:41 PM, Junio C Hamano wrote:\n> Michael Rappazzo writes:\n>\n>> For some mergetools, the current invocation of git mergetool will\n>> include an auto-merge flag. By default the flag is included, however if\n>> the git config option 'merge.automerge' is set to 'false', then that\n>> flag will now be omitted.\n>\n> ... and why is the \"automerge\" a bad thing that user would want to\n> avoid triggering under which condition? That description may not\n> have to be in the proposed log message, but it would help users when\n> they decide if they want to use the configuration to describe it in\n> the mergetool.automerge configuration.\n>\n> And depending on the answer to the above question, a configuration\n> variable may turn out be a bad mechanism to customize this (namely,\n> set-and-forget configuration variable is a bad match for a knob that\n> is more \"per invocation\" than \"user taste\").\n>\n> Is this not about \"automerge\" but more about \"always-show-UI because\n> I like GUI?\" Then that may be a \"user taste\" thing that is a good\n> match for a configuration variable. I simply cannot tell from what\n> was in the message I am responding to.\n\nI feel that the auto-merge takes away the context of the changes.\n\nI use araxis merge as my mergetool of choice. I almost always immediately\nundo the auto-merge. After taking a moment to look at each file, I will\nthen (usually) use the keyboard shortcut for auto-merge.\n\nIt sure would be nice to \"set-and-forget\" a config variable to remove the\nannoyance of having to undo the auto-merge. I think that this config\noption is reasonable. Perhaps my documentation leaves something to be\ndesired. I can take another stab at it.\n\n\n>\n>> -TEMPORARY FILES\n>> ----------------\n>> -`git mergetool` creates `*.orig` backup files while resolving merges.\n>> -These are safe to remove once a file has been merged and its\n>> -`git mergetool` session has completed.\n>> -\n>> +CONFIGURATION OPTIONS\n>> +---------------------\n>> +mergetool.keepBackup::\n>> + `git mergetool` creates `*.orig` backup files while resolving merges.\n>> + These are safe to remove once a file has been merged and its\n>> + `git mergetool` session has completed.\n>> ++\n>\n> This is an unrelated change; I think it is a good change, though.\n>\n> I however suspect that we would not want to repeat the configuration\n> description in this file and instead mention these in \"see also\"\n> section referring the readers to git-config(1).\n>\n\nI felt that adding a separate header for a different config option was more\nappropriate, so I went with this. Pointing to the config.txt doc is\nprobably better.\n"},{"id":"264110","messageId":"20150618084252.GA38187@gmail.com","threadId":"39648","inReplyTo":"CANoM8SWN2ZzgMP_MoDRRFA8ZjzRh724iaJ7GwrD2WddrjqwuqQ@mail.gmail.com","subject":"Re: [PATCH] mergetools: add config option to disable auto-merge","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2015-06-18T08:42:53Z","receivedAt":"2015-06-18T08:42:53Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Wed, Jun 17, 2015 at 10:27:58PM -0400, Mike Rappazzo wrote:\n> On Wed, Jun 17, 2015 at 3:41 PM, Junio C Hamano wrote:\n> > Michael Rappazzo writes:\n> >\n> >> For some mergetools, the current invocation of git mergetool will\n> >> include an auto-merge flag. By default the flag is included, however if\n> >> the git config option 'merge.automerge' is set to 'false', then that\n> >> flag will now be omitted.\n> >\n> > ... and why is the \"automerge\" a bad thing that user would want to\n> > avoid triggering under which condition? That description may not\n> > have to be in the proposed log message, but it would help users when\n> > they decide if they want to use the configuration to describe it in\n> > the mergetool.automerge configuration.\n> >\n> > And depending on the answer to the above question, a configuration\n> > variable may turn out be a bad mechanism to customize this (namely,\n> > set-and-forget configuration variable is a bad match for a knob that\n> > is more \"per invocation\" than \"user taste\").\n> >\n> > Is this not about \"automerge\" but more about \"always-show-UI because\n> > I like GUI?\" Then that may be a \"user taste\" thing that is a good\n> > match for a configuration variable. I simply cannot tell from what\n> > was in the message I am responding to.\n> \n> I feel that the auto-merge takes away the context of the changes.\n> \n> I use araxis merge as my mergetool of choice. I almost always immediately\n> undo the auto-merge. After taking a moment to look at each file, I will\n> then (usually) use the keyboard shortcut for auto-merge.\n> \n> It sure would be nice to \"set-and-forget\" a config variable to remove the\n> annoyance of having to undo the auto-merge. I think that this config\n> option is reasonable. Perhaps my documentation leaves something to be\n> desired. I can take another stab at it.\n\nIf this is the case then I would recommend making it more\ngranular. Just because Araxis' automerge is undesirable does\nnot mean that some other tools' automerge is as well.\ne.g. the config variable could be \"mergetool..automerge\"\nrather than the top-level \"mergetool.automerge\" variable.\n\nBut, as Junio suggested, having a command-line flag to skip the\nbehavior might be a better first step. Something like,\n\"git mergetool --no-automerge\".\n\nMost of Git's behavior that can be modified via configuration\ncan also be modified on the command-line, so exposing this\nfeature as a flag would probably be a good idea.\n\nEven without a config variable, it can still be fire-and-forget\nconvenient by using a git alias to supply the flag.\n\nIn lieu of any of these features, another option is that you can\noverride the default command by setting \"mergetool.araxis.cmd\",\nand \"git mergetool\" will use your definition rather than its\nbuilt-in command. We left that escape hatch in for just this\npurpose.\n\nI'm curious, is this a common use case? I'm personally not a\ngood judge of that, but I'll let the list chime in. Thoughts?\n-- \nDavid\n"},{"id":"264186","messageId":"CANoM8SXLTKSPPdByNtGXaqchStTwKF8-1J6A5YW43CT0ikPRHQ@mail.gmail.com","threadId":"39648","inReplyTo":"20150618084252.GA38187@gmail.com","subject":"Re: [PATCH] mergetools: add config option to disable auto-merge","fromName":"Mike Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2015-06-18T15:14:49Z","receivedAt":"2015-06-18T15:14:49Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"On Thu, Jun 18, 2015 at 4:43 AM David Aguilar wrote:\n>\n> On Wed, Jun 17, 2015 at 10:27:58PM -0400, Mike Rappazzo wrote:\n> >\n> > I feel that the auto-merge takes away the context of the changes.\n> >\n> > I use araxis merge as my mergetool of choice. I almost always immediately\n> > undo the auto-merge. After taking a moment to look at each file, I will\n> > then (usually) use the keyboard shortcut for auto-merge.\n> >\n> > It sure would be nice to \"set-and-forget\" a config variable to remove the\n> > annoyance of having to undo the auto-merge. I think that this config\n> > option is reasonable. Perhaps my documentation leaves something to be\n> > desired. I can take another stab at it.\n>\n> If this is the case then I would recommend making it more\n> granular. Just because Araxis' automerge is undesirable does\n> not mean that some other tools' automerge is as well.\n> e.g. the config variable could be \"mergetool..automerge\"\n> rather than the top-level \"mergetool.automerge\" variable.\n\nI don't necessarily think that araxis' automerge is bad, but I like to look\nat the before and after to understand the context of a conflict. I can't\nimagine that this is a quirk of araxis, but is probably something that\nexists for any auto-merging tool. The feature doesn't seem to be that\nwidely supported among the other tooling. I only found the three to use\nsuch a feature.\n\nSince the automerge option is not available on every merge tool, it seems\nreasonable to use \"mergetool..automerge\" instead of \"merge.automerge\".\n\n>\n>\n> But, as Junio suggested, having a command-line flag to skip the\n> behavior might be a better first step. Something like,\n> \"git mergetool --no-automerge\".\n>\n> Most of Git's behavior that can be modified via configuration\n> can also be modified on the command-line, so exposing this\n> feature as a flag would probably be a good idea.\n\n\nThis makes sense, and if this change is to go forward, I will implement the\ncommand line option.\n\n>\n> Even without a config variable, it can still be fire-and-forget\n> convenient by using a git alias to supply the flag.\n>\n> In lieu of any of these features, another option is that you can\n> override the default command by setting \"mergetool.araxis.cmd\",\n> and \"git mergetool\" will use your definition rather than its\n> built-in command. We left that escape hatch in for just this\n> purpose.\n\nI guess that if this patch does not go forward, I will have to use this\nworkaround.\n"}]}