{"thread":{"id":"36458","subject":"[PATCH 0/2] Simple default fixes for v2.0","startedAt":"2014-04-21T00:17:32Z","lastAt":"2014-04-24T18:30:09Z","messageCount":19,"participants":["Felipe Contreras","David Aguilar","Charles Bailey","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"239197","messageId":"1398039454-31193-1-git-send-email-felipe.contreras@gmail.com","threadId":"36458","inReplyTo":null,"subject":"[PATCH 0/2] Simple default fixes for v2.0","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2014-04-21T00:17:32Z","receivedAt":"2014-04-21T00:17:32Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"I don't see why people need to set these to have sensible behavior:\n\n  merge.defaulttoupstream = true\n  mergetool.prompt = false\n\nLet's change them to sane defaults so they are not needed.\n\nFelipe Contreras (2):\n  merge: enable defaulttoupstream by default\n  mergetool: run prompt only if guessed tool\n\n Documentation/git-merge.txt |  5 ++---\n builtin/merge.c             |  2 +-\n git-mergetool.sh            | 14 +++++++++++---\n 3 files changed, 14 insertions(+), 7 deletions(-)\n\n-- \n1.9.2+fc1.1.g5c924db\n"},{"id":"239199","messageId":"1398039454-31193-2-git-send-email-felipe.contreras@gmail.com","threadId":"36458","inReplyTo":"1398039454-31193-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 1/2] merge: enable defaulttoupstream by default","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2014-04-21T00:17:33Z","receivedAt":"2014-04-21T00:17:33Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"There's no point in this:\n\n% git merge\nfatal: No commit specified and merge.defaultToUpstream not set.\n\nWe know the most likely scenario is that the user wants to merge the\nupstream, and if not, he can set merge.defaultToUpstream to false.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n Documentation/git-merge.txt | 5 ++---\n builtin/merge.c             | 2 +-\n 2 files changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt\nindex a3c1fa3..cf2c374 100644\n--- a/Documentation/git-merge.txt\n+++ b/Documentation/git-merge.txt\n@@ -101,9 +101,8 @@ commit or stash your changes before running 'git merge'.\n \tSpecifying more than one commit will create a merge with\n \tmore than two parents (affectionately called an Octopus merge).\n +\n-If no commit is given from the command line, and if `merge.defaultToUpstream`\n-configuration variable is set, merge the remote-tracking branches\n-that the current branch is configured to use as its upstream.\n+If no commit is given from the command line, merge the remote-tracking\n+branches that the current branch is configured to use as its upstream.\n See also the configuration section of this manual page.\n \n \ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 66d8843..1fc9319 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -63,7 +63,7 @@ static int verbosity;\n static int allow_rerere_auto;\n static int abort_current_merge;\n static int show_progress = -1;\n-static int default_to_upstream;\n+static int default_to_upstream = 1;\n static const char *sign_commit;\n \n static struct strategy all_strategy[] = {\n-- \n1.9.2+fc1.1.g5c924db\n"},{"id":"239198","messageId":"1398039454-31193-3-git-send-email-felipe.contreras@gmail.com","threadId":"36458","inReplyTo":"1398039454-31193-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 2/2] mergetool: run prompt only if guessed tool","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2014-04-21T00:17:34Z","receivedAt":"2014-04-21T00:17:34Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"It's annoying to see the prompt:\n\n  Hit return to start merge resolution tool (foo):\n\nEvery time the user does 'git mergetool' even if the user already\nconfigured 'foo' as the wanted tool.\n\nDisplay this prompt only when the user hasn't explicitly configured a\ntool.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n git-mergetool.sh | 14 +++++++++++---\n 1 file changed, 11 insertions(+), 3 deletions(-)\n\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex 332528f..d08dc92 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -277,7 +277,7 @@ merge_file () {\n \techo \"Normal merge conflict for '$MERGED':\"\n \tdescribe_file \"$local_mode\" \"local\" \"$LOCAL\"\n \tdescribe_file \"$remote_mode\" \"remote\" \"$REMOTE\"\n-\tif \"$prompt\" = true\n+\tif test \"$guessed_merge_tool\" = true || test \"$prompt\" = true\n \tthen\n \t\tprintf \"Hit return to start merge resolution tool (%s): \" \"$merge_tool\"\n \t\tread ans || return 1\n@@ -315,7 +315,8 @@ merge_file () {\n \treturn 0\n }\n \n-prompt=$(git config --bool mergetool.prompt || echo true)\n+prompt=$(git config --bool mergetool.prompt)\n+guessed_merge_tool=false\n \n while test $# != 0\n do\n@@ -373,7 +374,14 @@ prompt_after_failed_merge () {\n \n if test -z \"$merge_tool\"\n then\n-\tmerge_tool=$(get_merge_tool \"$merge_tool\") || exit\n+\t# Check if a merge tool has been configured\n+\tmerge_tool=$(get_configured_merge_tool)\n+\t# Try to guess an appropriate merge tool if no tool has been set.\n+\tif test -z \"$merge_tool\"\n+\tthen\n+\t\tmerge_tool=$(guess_merge_tool) || exit\n+\t\tguessed_merge_tool=true\n+\tfi\n fi\n merge_keep_backup=\"$(git config --bool mergetool.keepBackup || echo true)\"\n merge_keep_temporaries=\"$(git config --bool mergetool.keepTemporaries || echo false)\"\n-- \n1.9.2+fc1.1.g5c924db\n"},{"id":"239288","messageId":"20140422045951.GA60610@gmail.com","threadId":"36458","inReplyTo":"1398039454-31193-3-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 2/2] mergetool: run prompt only if guessed tool","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2014-04-22T04:59:52Z","receivedAt":"2014-04-22T04:59:52Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"[Cc:ing Charles in case he has an opinion, this behavior dates back to the original MT]\n\nOn Sun, Apr 20, 2014 at 07:17:34PM -0500, Felipe Contreras wrote:\n> It's annoying to see the prompt:\n> \n>   Hit return to start merge resolution tool (foo):\n> \n> Every time the user does 'git mergetool' even if the user already\n> configured 'foo' as the wanted tool.\n> \n> Display this prompt only when the user hasn't explicitly configured a\n> tool.\n\nI agree this is probably helpful.\nMost users I've met end up configuring mergetool.prompt=false.\n\nThanks\n\n> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n> ---\n>  git-mergetool.sh | 14 +++++++++++---\n>  1 file changed, 11 insertions(+), 3 deletions(-)\n> \n> diff --git a/git-mergetool.sh b/git-mergetool.sh\n> index 332528f..d08dc92 100755\n> --- a/git-mergetool.sh\n> +++ b/git-mergetool.sh\n> @@ -277,7 +277,7 @@ merge_file () {\n>  \techo \"Normal merge conflict for '$MERGED':\"\n>  \tdescribe_file \"$local_mode\" \"local\" \"$LOCAL\"\n>  \tdescribe_file \"$remote_mode\" \"remote\" \"$REMOTE\"\n> -\tif \"$prompt\" = true\n> +\tif test \"$guessed_merge_tool\" = true || test \"$prompt\" = true\n>  \tthen\n>  \t\tprintf \"Hit return to start merge resolution tool (%s): \" \"$merge_tool\"\n>  \t\tread ans || return 1\n> @@ -315,7 +315,8 @@ merge_file () {\n>  \treturn 0\n>  }\n>  \n> -prompt=$(git config --bool mergetool.prompt || echo true)\n> +prompt=$(git config --bool mergetool.prompt)\n> +guessed_merge_tool=false\n>  \n>  while test $# != 0\n>  do\n> @@ -373,7 +374,14 @@ prompt_after_failed_merge () {\n>  \n>  if test -z \"$merge_tool\"\n>  then\n> -\tmerge_tool=$(get_merge_tool \"$merge_tool\") || exit\n> +\t# Check if a merge tool has been configured\n> +\tmerge_tool=$(get_configured_merge_tool)\n> +\t# Try to guess an appropriate merge tool if no tool has been set.\n> +\tif test -z \"$merge_tool\"\n> +\tthen\n> +\t\tmerge_tool=$(guess_merge_tool) || exit\n> +\t\tguessed_merge_tool=true\n> +\tfi\n>  fi\n>  merge_keep_backup=\"$(git config --bool mergetool.keepBackup || echo true)\"\n>  merge_keep_temporaries=\"$(git config --bool mergetool.keepTemporaries || echo false)\"\n> -- \n> 1.9.2+fc1.1.g5c924db\n> \n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n\n-- \nDavid\n"},{"id":"239294","messageId":"20140422060120.GA10198@hashpling.org","threadId":"36458","inReplyTo":"20140422045951.GA60610@gmail.com","subject":"Re: [PATCH 2/2] mergetool: run prompt only if guessed tool","fromName":"Charles Bailey","fromEmail":"charles@hashpling.org","sentAt":"2014-04-22T06:01:20Z","receivedAt":"2014-04-22T06:01:20Z","isPatch":true,"sender":{"key":"charles@hashpling.org","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"On Mon, Apr 21, 2014 at 09:59:52PM -0700, David Aguilar wrote:\n> [Cc:ing Charles in case he has an opinion, this behavior dates back to the original MT]\n> \n> On Sun, Apr 20, 2014 at 07:17:34PM -0500, Felipe Contreras wrote:\n> > It's annoying to see the prompt:\n> > \n> >   Hit return to start merge resolution tool (foo):\n> > \n> > Every time the user does 'git mergetool' even if the user already\n> > configured 'foo' as the wanted tool.\n> > \n> > Display this prompt only when the user hasn't explicitly configured a\n> > tool.\n> \n> I agree this is probably helpful.\n> Most users I've met end up configuring mergetool.prompt=false.\n\n>From my memory, the reason that we choose to prompt by default is to\nhelp new users or users who have just changed their choice of mergetool.\n\nBecause we never use the exit code of the tool to determine whether a\ntool \"worked\", if we don't prompt and the tool fails (bad custom\ncommand, requires X when no X available, etc.) then we'll repeatedly run\nthe command for all paths requiring resolution leading to, potentially,\na lot of trace with whatever error the tool might happen to report.\n\nWe prompt by default to give users a chance to abort the mergetool\nsession at the first opportunity that they see that the configured tool\nis not working.\n\nPersonally, I leave mergetool.prompt unset (default true) because one\nextra click in a complex merge resolution is relatively low overhead and\nto catch myself when I forget that I'm in a no-X environment.\n\nI glanced at the patch and it seems to subvert this intent for users\nwho have  configured a merge tool. Is my understanding correct?\n"},{"id":"239296","messageId":"53560b09bbe96_2400128531085@nysa.notmuch","threadId":"36458","inReplyTo":"20140422060120.GA10198@hashpling.org","subject":"Re: [PATCH 2/2] mergetool: run prompt only if guessed tool","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2014-04-22T06:24:09Z","receivedAt":"2014-04-22T06:24:09Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Charles Bailey wrote:\n> On Mon, Apr 21, 2014 at 09:59:52PM -0700, David Aguilar wrote:\n> > [Cc:ing Charles in case he has an opinion, this behavior dates back to the original MT]\n> > \n> > On Sun, Apr 20, 2014 at 07:17:34PM -0500, Felipe Contreras wrote:\n> > > It's annoying to see the prompt:\n> > > \n> > >   Hit return to start merge resolution tool (foo):\n> > > \n> > > Every time the user does 'git mergetool' even if the user already\n> > > configured 'foo' as the wanted tool.\n> > > \n> > > Display this prompt only when the user hasn't explicitly configured a\n> > > tool.\n> > \n> > I agree this is probably helpful.\n> > Most users I've met end up configuring mergetool.prompt=false.\n> \n> From my memory, the reason that we choose to prompt by default is to\n> help new users or users who have just changed their choice of mergetool.\n> \n> Because we never use the exit code of the tool to determine whether a\n> tool \"worked\", if we don't prompt and the tool fails (bad custom\n> command, requires X when no X available, etc.) then we'll repeatedly run\n> the command for all paths requiring resolution leading to, potentially,\n> a lot of trace with whatever error the tool might happen to report.\n> \n> We prompt by default to give users a chance to abort the mergetool\n> session at the first opportunity that they see that the configured tool\n> is not working.\n\nThis is what I get when a tool is not working:\n\n  Documentation/config.txt seems unchanged.\n  Was the merge successful? [y/n]\n\n> Personally, I leave mergetool.prompt unset (default true) because one\n> extra click in a complex merge resolution is relatively low overhead and\n> to catch myself when I forget that I'm in a no-X environment.\n> \n> I glanced at the patch and it seems to subvert this intent for users\n> who have  configured a merge tool. Is my understanding correct?\n\nYes, that's correct. If the user has *manually* configured a tool, why would\nyou ask him again? We are annoying the overwhelming the vast majority of users\nwho already configured the right tool in order to avoid issues with a minute\nminority that might potentially but unlikely have a problem once or twice.\n\nThat's not productive.\n\n-- \nFelipe Contreras\n"},{"id":"239302","messageId":"535611fac1b7b_268bd0b308f5@nysa.notmuch","threadId":"36458","inReplyTo":"20140422065549.GA11224@hashpling.org","subject":"Re: [PATCH 2/2] mergetool: run prompt only if guessed tool","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2014-04-22T06:53:46Z","receivedAt":"2014-04-22T06:53:46Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Charles Bailey wrote:\n> On Tue, Apr 22, 2014 at 01:24:09AM -0500, Felipe Contreras wrote:\n> > \n> > This is what I get when a tool is not working:\n> > \n> >   Documentation/config.txt seems unchanged.\n> >   Was the merge successful? [y/n]\n> \n> Does this happen now even with merge tools for which we do trust the\n> exit code? If so, my original concern is addressed.\n\nWhich tools are those?\n\n> > > Personally, I leave mergetool.prompt unset (default true) because one\n> > > extra click in a complex merge resolution is relatively low overhead and\n> > > to catch myself when I forget that I'm in a no-X environment.\n> > > \n> > > I glanced at the patch and it seems to subvert this intent for users\n> > > who have  configured a merge tool. Is my understanding correct?\n> > \n> > Yes, that's correct. If the user has *manually* configured a tool, why would\n> > you ask him again? We are annoying the overwhelming the vast majority of users\n> > who already configured the right tool in order to avoid issues with a minute\n> > minority that might potentially but unlikely have a problem once or twice.\n> > \n> > That's not productive.\n> \n> We're asking to user to signal that he's happy to launch the mergetool\n> and implicitly giving him an opportunity to break out of the session.\n> I don't see anything wrong with having this behaviour.\n\nYou don't see anything wrong with asking the user *every single time* he runs\n`git mergetool`, even though he *already told us* which tool to use?\n \nIf so, I'm pretty sure everybody else disagrees with you.\n\n-- \nFelipe Contreras\n"},{"id":"239300","messageId":"20140422065549.GA11224@hashpling.org","threadId":"36458","inReplyTo":"53560b09bbe96_2400128531085@nysa.notmuch","subject":"Re: [PATCH 2/2] mergetool: run prompt only if guessed tool","fromName":"Charles Bailey","fromEmail":"charles@hashpling.org","sentAt":"2014-04-22T06:55:49Z","receivedAt":"2014-04-22T06:55:49Z","isPatch":true,"sender":{"key":"charles@hashpling.org","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"On Tue, Apr 22, 2014 at 01:24:09AM -0500, Felipe Contreras wrote:\n> \n> This is what I get when a tool is not working:\n> \n>   Documentation/config.txt seems unchanged.\n>   Was the merge successful? [y/n]\n\nDoes this happen now even with merge tools for which we do trust the\nexit code? If so, my original concern is addressed.\n\n> > Personally, I leave mergetool.prompt unset (default true) because one\n> > extra click in a complex merge resolution is relatively low overhead and\n> > to catch myself when I forget that I'm in a no-X environment.\n> > \n> > I glanced at the patch and it seems to subvert this intent for users\n> > who have  configured a merge tool. Is my understanding correct?\n> \n> Yes, that's correct. If the user has *manually* configured a tool, why would\n> you ask him again? We are annoying the overwhelming the vast majority of users\n> who already configured the right tool in order to avoid issues with a minute\n> minority that might potentially but unlikely have a problem once or twice.\n> \n> That's not productive.\n\nWe're asking to user to signal that he's happy to launch the mergetool\nand implicitly giving him an opportunity to break out of the session.\nI don't see anything wrong with having this behaviour.\n\nSo long as we don't hit the loop-with-lots-of-error-trace for users who\nhaven't set mergetool.prompt I've no strong objections to changing the\ndefault so long as an explictly set mergetool.prompt is respected.\n\nIdeally, I think I'd like the prompt to accept a \"launch/skip/abort\"\nrange of options but that's a wider scoped thing. Sometimes when I'm\nresolving a lot of things and not specifying any paths I come across\nsomething that know I don't want to attempt a resolve with my currently\nconfigured tool and I just want to skip it for now. Currently, I have to\neither abort the session or let the mergetool fire up and close it again\nneither of which are optimal.\n"},{"id":"239307","messageId":"20140422073008.GA11584@hashpling.org","threadId":"36458","inReplyTo":"535611fac1b7b_268bd0b308f5@nysa.notmuch","subject":"Re: [PATCH 2/2] mergetool: run prompt only if guessed tool","fromName":"Charles Bailey","fromEmail":"charles@hashpling.org","sentAt":"2014-04-22T07:30:08Z","receivedAt":"2014-04-22T07:30:08Z","isPatch":true,"sender":{"key":"charles@hashpling.org","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"On Tue, Apr 22, 2014 at 01:53:46AM -0500, Felipe Contreras wrote:\n> Charles Bailey wrote:\n> > On Tue, Apr 22, 2014 at 01:24:09AM -0500, Felipe Contreras wrote:\n> > > \n> > > This is what I get when a tool is not working:\n> > > \n> > >   Documentation/config.txt seems unchanged.\n> > >   Was the merge successful? [y/n]\n> > \n> > Does this happen now even with merge tools for which we do trust the\n> > exit code? If so, my original concern is addressed.\n> \n> Which tools are those?\n> \n\nI didn't remember off hand, but checking the mergetools directory\nsuggests that kdiff3 is one (there's no call to check_unchanged). I\nstopped checking after I found one.\n\n> You don't see anything wrong with asking the user *every single time* he runs\n> `git mergetool`, even though he *already told us* which tool to use?\n>  \n> If so, I'm pretty sure everybody else disagrees with you.\n\nI think that you may have misunderstood me. As I said, I've no\nparticular objections to changing the default (subject a few concerns\nthat may or may not still apply).\n\nHaving said that, the fact that the user has configured the merge tool\ndoesn't mean that he necessarily doesn't want to see the prompt. In a\npart of my reply which you snipped, I said that it's sometimes the\nparticular file that's due to be resolved that might prompt a user to\nwant to skip launching the tool.\n\nEither way, I think we shouldn't unconditionally override an explicitly\nset mergetool.prompt and if we are (effectively) changing the default we\nshould probably update the documentation to say so as well. \n\n(Yes, I didn't introduce the first \"no prompt\" option patch and yes, I\nhave since changed my mind about whether I have it set, but that's just\na personal preference.)\n"},{"id":"239309","messageId":"535620a6e8274_3d2216372ec9d@nysa.notmuch","threadId":"36458","inReplyTo":"20140422073008.GA11584@hashpling.org","subject":"Re: [PATCH 2/2] mergetool: run prompt only if guessed tool","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2014-04-22T07:56:22Z","receivedAt":"2014-04-22T07:56:22Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Charles Bailey wrote:\n> On Tue, Apr 22, 2014 at 01:53:46AM -0500, Felipe Contreras wrote:\n> > Charles Bailey wrote:\n> > > On Tue, Apr 22, 2014 at 01:24:09AM -0500, Felipe Contreras wrote:\n> > > > \n> > > > This is what I get when a tool is not working:\n> > > > \n> > > >   Documentation/config.txt seems unchanged.\n> > > >   Was the merge successful? [y/n]\n> > > \n> > > Does this happen now even with merge tools for which we do trust the\n> > > exit code? If so, my original concern is addressed.\n> > \n> > Which tools are those?\n> \n> I didn't remember off hand, but checking the mergetools directory\n> suggests that kdiff3 is one (there's no call to check_unchanged). I\n> stopped checking after I found one.\n\nSo:\n\n\t% cat > ~/bin/kdiff3 <<-\\EOF\n\t#!/bin/sh\n\tfalse\n\tEOF\n\t% chmod +x ~/bin/kdiff3\n  % git -c merge.tool=kdiff3 mergetool\n  merge of Documentation/config.txt failed\n  Continue merging other unresolved paths (y/n) ?\n\n> > You don't see anything wrong with asking the user *every single time* he runs\n> > `git mergetool`, even though he *already told us* which tool to use?\n> >  \n> > If so, I'm pretty sure everybody else disagrees with you.\n> \n> I think that you may have misunderstood me. As I said, I've no\n> particular objections to changing the default (subject a few concerns\n> that may or may not still apply).\n> \n> Having said that, the fact that the user has configured the merge tool\n> doesn't mean that he necessarily doesn't want to see the prompt.\n\nNot necessarily, but in 99% of the cases it does. And for the remaining there's\nalways mergetool.prompt = true.\n\n> In a part of my reply which you snipped, I said that it's sometimes the\n> particular file that's due to be resolved that might prompt a user to want to\n> skip launching the tool.\n\nThat's a possibility, however, in almost all the situations I've wanted to stop\na merge, it's *after* I've seen the actual conflicts in the file.\n\nAnyway, I've revisited the code, and it's only now that I've realized that this:\n\n  Hit return to start merge resolution tool (kdiff3):\n\nIs not actually asking me for the tool I want to use; the value in parenthesis\nis not the default, and I can't type in another tool.\n\nSo the purpose of the prompt is very different from what I was thinking, yet I\nstill think the value of such prompt is marginal at best.\n\n> Either way, I think we shouldn't unconditionally override an explicitly\n> set mergetool.prompt and if we are (effectively) changing the default we\n> should probably update the documentation to say so as well. \n\nAn explicitly set mergetool.prompt = true would override the default. See the\npatch.\n\nI looked, the documentation doesn't mention any default. We could add it, but I\ndon't think it's necesarily part of this patch.\n\n-- \nFelipe Contreras\n"},{"id":"239351","messageId":"xmqqbnvt703s.fsf@gitster.dls.corp.google.com","threadId":"36458","inReplyTo":"20140422045951.GA60610@gmail.com","subject":"Re: [PATCH 2/2] mergetool: run prompt only if guessed tool","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-22T17:19:51Z","receivedAt":"2014-04-22T17:19:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Aguilar <davvid@gmail.com> writes:\n\n> [Cc:ing Charles in case he has an opinion, this behavior dates back to the original MT]\n>\n> On Sun, Apr 20, 2014 at 07:17:34PM -0500, Felipe Contreras wrote:\n>> It's annoying to see the prompt:\n>> \n>>   Hit return to start merge resolution tool (foo):\n>> \n>> Every time the user does 'git mergetool' even if the user already\n>> configured 'foo' as the wanted tool.\n>> \n>> Display this prompt only when the user hasn't explicitly configured a\n>> tool.\n>\n> I agree this is probably helpful.\n> Most users I've met end up configuring mergetool.prompt=false.\n>\n> Thanks\n\nOK, is it an Ack?\n\nThanks for CC'ing Charles, by the way.  I think his point about\nmentioning the change of default somewhere in the documentation\nhas some merits, and it can be done in a follow-up patch on top.\n\n\n\n\n>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n>> ---\n>>  git-mergetool.sh | 14 +++++++++++---\n>>  1 file changed, 11 insertions(+), 3 deletions(-)\n>> \n>> diff --git a/git-mergetool.sh b/git-mergetool.sh\n>> index 332528f..d08dc92 100755\n>> --- a/git-mergetool.sh\n>> +++ b/git-mergetool.sh\n>> @@ -277,7 +277,7 @@ merge_file () {\n>>  \techo \"Normal merge conflict for '$MERGED':\"\n>>  \tdescribe_file \"$local_mode\" \"local\" \"$LOCAL\"\n>>  \tdescribe_file \"$remote_mode\" \"remote\" \"$REMOTE\"\n>> -\tif \"$prompt\" = true\n>> +\tif test \"$guessed_merge_tool\" = true || test \"$prompt\" = true\n>>  \tthen\n>>  \t\tprintf \"Hit return to start merge resolution tool (%s): \" \"$merge_tool\"\n>>  \t\tread ans || return 1\n>> @@ -315,7 +315,8 @@ merge_file () {\n>>  \treturn 0\n>>  }\n>>  \n>> -prompt=$(git config --bool mergetool.prompt || echo true)\n>> +prompt=$(git config --bool mergetool.prompt)\n>> +guessed_merge_tool=false\n>>  \n>>  while test $# != 0\n>>  do\n>> @@ -373,7 +374,14 @@ prompt_after_failed_merge () {\n>>  \n>>  if test -z \"$merge_tool\"\n>>  then\n>> -\tmerge_tool=$(get_merge_tool \"$merge_tool\") || exit\n>> +\t# Check if a merge tool has been configured\n>> +\tmerge_tool=$(get_configured_merge_tool)\n>> +\t# Try to guess an appropriate merge tool if no tool has been set.\n>> +\tif test -z \"$merge_tool\"\n>> +\tthen\n>> +\t\tmerge_tool=$(guess_merge_tool) || exit\n>> +\t\tguessed_merge_tool=true\n>> +\tfi\n>>  fi\n>>  merge_keep_backup=\"$(git config --bool mergetool.keepBackup || echo true)\"\n>>  merge_keep_temporaries=\"$(git config --bool mergetool.keepTemporaries || echo false)\"\n>> -- \n>> 1.9.2+fc1.1.g5c924db\n>> \n>> --\n>> To unsubscribe from this list: send the line \"unsubscribe git\" in\n>> the body of a message to majordomo@vger.kernel.org\n>> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n"},{"id":"239353","messageId":"xmqq38h56zj0.fsf@gitster.dls.corp.google.com","threadId":"36458","inReplyTo":"20140422045951.GA60610@gmail.com","subject":"Re: [PATCH 2/2] mergetool: run prompt only if guessed tool","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-22T17:32:19Z","receivedAt":"2014-04-22T17:32:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Aguilar <davvid@gmail.com> writes:\n\n> [Cc:ing Charles in case he has an opinion, this behavior dates back to the original MT]\n>\n> On Sun, Apr 20, 2014 at 07:17:34PM -0500, Felipe Contreras wrote:\n>> It's annoying to see the prompt:\n>> \n>>   Hit return to start merge resolution tool (foo):\n>> \n>> Every time the user does 'git mergetool' even if the user already\n>> configured 'foo' as the wanted tool.\n>> \n>> Display this prompt only when the user hasn't explicitly configured a\n>> tool.\n>\n> I agree this is probably helpful.\n> Most users I've met end up configuring mergetool.prompt=false.\n>\n> Thanks\n\nDo you have reaction to the other one \"[PATCH 1/2] merge: enable\ndefaulttoupstream by default\"?\n\nAs I do not think (note: I am not saying \"I do not know\" here) it is\nsensible to do these \"flip the default\" in 2.0, I am hoping that we\ncan queue them (if the area maintainer, you, agree they are good\nchanges) to 'next' and cook them for the remainder of this cycle.\n2.0 is a place where we execute the \"flip the default\" that we have\nalready disussed, designed and agreed since around 1.8.x timeframe,\nand I would like to exclude any user visible change that just got\nstarted being discussed to avoid unnecessary fallouts.\n\nThanks.\n\n>\n>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n>> ---\n>>  git-mergetool.sh | 14 +++++++++++---\n>>  1 file changed, 11 insertions(+), 3 deletions(-)\n>> \n>> diff --git a/git-mergetool.sh b/git-mergetool.sh\n>> index 332528f..d08dc92 100755\n>> --- a/git-mergetool.sh\n>> +++ b/git-mergetool.sh\n>> @@ -277,7 +277,7 @@ merge_file () {\n>>  \techo \"Normal merge conflict for '$MERGED':\"\n>>  \tdescribe_file \"$local_mode\" \"local\" \"$LOCAL\"\n>>  \tdescribe_file \"$remote_mode\" \"remote\" \"$REMOTE\"\n>> -\tif \"$prompt\" = true\n>> +\tif test \"$guessed_merge_tool\" = true || test \"$prompt\" = true\n>>  \tthen\n>>  \t\tprintf \"Hit return to start merge resolution tool (%s): \" \"$merge_tool\"\n>>  \t\tread ans || return 1\n>> @@ -315,7 +315,8 @@ merge_file () {\n>>  \treturn 0\n>>  }\n>>  \n>> -prompt=$(git config --bool mergetool.prompt || echo true)\n>> +prompt=$(git config --bool mergetool.prompt)\n>> +guessed_merge_tool=false\n>>  \n>>  while test $# != 0\n>>  do\n>> @@ -373,7 +374,14 @@ prompt_after_failed_merge () {\n>>  \n>>  if test -z \"$merge_tool\"\n>>  then\n>> -\tmerge_tool=$(get_merge_tool \"$merge_tool\") || exit\n>> +\t# Check if a merge tool has been configured\n>> +\tmerge_tool=$(get_configured_merge_tool)\n>> +\t# Try to guess an appropriate merge tool if no tool has been set.\n>> +\tif test -z \"$merge_tool\"\n>> +\tthen\n>> +\t\tmerge_tool=$(guess_merge_tool) || exit\n>> +\t\tguessed_merge_tool=true\n>> +\tfi\n>>  fi\n>>  merge_keep_backup=\"$(git config --bool mergetool.keepBackup || echo true)\"\n>>  merge_keep_temporaries=\"$(git config --bool mergetool.keepTemporaries || echo false)\"\n>> -- \n>> 1.9.2+fc1.1.g5c924db\n>> \n>> --\n>> To unsubscribe from this list: send the line \"unsubscribe git\" in\n>> the body of a message to majordomo@vger.kernel.org\n>> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n"},{"id":"239375","messageId":"xmqqmwfd3zu9.fsf@gitster.dls.corp.google.com","threadId":"36458","inReplyTo":"1398039454-31193-2-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 1/2] merge: enable defaulttoupstream by default","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-22T19:53:50Z","receivedAt":"2014-04-22T19:53:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> There's no point in this:\n>\n> % git merge\n> fatal: No commit specified and merge.defaultToUpstream not set.\n>\n> We know the most likely scenario is that the user wants to merge the\n> upstream, and if not, he can set merge.defaultToUpstream to false.\n\nAnd a new possible failure case is when there is no upstream defined\nfor the current branch, which gets perfectly good new error message:\n\n    $ git merge\n    fatal: No remote for the current branch.\n\nSo I think this is good.  We want to protect this with a new test,\nno?\n\nWill queue as-is for now.\n\n> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n> ---\n>  Documentation/git-merge.txt | 5 ++---\n>  builtin/merge.c             | 2 +-\n>  2 files changed, 3 insertions(+), 4 deletions(-)\n>\n> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt\n> index a3c1fa3..cf2c374 100644\n> --- a/Documentation/git-merge.txt\n> +++ b/Documentation/git-merge.txt\n> @@ -101,9 +101,8 @@ commit or stash your changes before running 'git merge'.\n>  \tSpecifying more than one commit will create a merge with\n>  \tmore than two parents (affectionately called an Octopus merge).\n>  +\n> -If no commit is given from the command line, and if `merge.defaultToUpstream`\n> -configuration variable is set, merge the remote-tracking branches\n> -that the current branch is configured to use as its upstream.\n> +If no commit is given from the command line, merge the remote-tracking\n> +branches that the current branch is configured to use as its upstream.\n>  See also the configuration section of this manual page.\n>  \n>  \n> diff --git a/builtin/merge.c b/builtin/merge.c\n> index 66d8843..1fc9319 100644\n> --- a/builtin/merge.c\n> +++ b/builtin/merge.c\n> @@ -63,7 +63,7 @@ static int verbosity;\n>  static int allow_rerere_auto;\n>  static int abort_current_merge;\n>  static int show_progress = -1;\n> -static int default_to_upstream;\n> +static int default_to_upstream = 1;\n>  static const char *sign_commit;\n>  \n>  static struct strategy all_strategy[] = {\n"},{"id":"239376","messageId":"xmqqioq13zrq.fsf@gitster.dls.corp.google.com","threadId":"36458","inReplyTo":"xmqq38h56zj0.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 2/2] mergetool: run prompt only if guessed tool","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-22T19:55:21Z","receivedAt":"2014-04-22T19:55:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Do you have reaction to the other one \"[PATCH 1/2] merge: enable\n> defaulttoupstream by default\"?\n\nSorry, that other one is not even about difftool/mergetool.\n\nI just checked that patch myself, and found it sensible for the\nlonger term.\n"},{"id":"239412","messageId":"20140423075601.GA18585@hashpling.org","threadId":"36458","inReplyTo":"535620a6e8274_3d2216372ec9d@nysa.notmuch","subject":"Re: [PATCH 2/2] mergetool: run prompt only if guessed tool","fromName":"Charles Bailey","fromEmail":"charles@hashpling.org","sentAt":"2014-04-23T07:56:01Z","receivedAt":"2014-04-23T07:56:01Z","isPatch":true,"sender":{"key":"charles@hashpling.org","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"On Tue, Apr 22, 2014 at 02:56:22AM -0500, Felipe Contreras wrote:\n> An explicitly set mergetool.prompt = true would override the default. See the\n> patch.\n\nI have had a chance to test the patch now and it looks good. I think\nwhen glancing at it before I missed the change that dropped \"|| echo\ntrue\" from \n\nprompt=$(git config --bool mergetool.prompt || echo true)\n\nso I wasn't sure where the implicit true / explicit true difference was\nhandled.\n\n> I looked, the documentation doesn't mention any default. We could add it, but I\n> don't think it's necesarily part of this patch.\n\nThe bit of documentation that I was thinking of is in\nDocumentation/git-mergetool.txt where it states that \"--prompt\" is the\ndefault which is now only partially true.\n"},{"id":"239413","messageId":"CAJDDKr5P7crudFiAa7Lu4CYmDWgZ-drdkiOfusFHioTF9co2Nw@mail.gmail.com","threadId":"36458","inReplyTo":"xmqqbnvt703s.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 2/2] mergetool: run prompt only if guessed tool","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2014-04-23T07:58:51Z","receivedAt":"2014-04-23T07:58:51Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Tue, Apr 22, 2014 at 10:19 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> David Aguilar <davvid@gmail.com> writes:\n>\n>> [Cc:ing Charles in case he has an opinion, this behavior dates back to the original MT]\n>>\n>> On Sun, Apr 20, 2014 at 07:17:34PM -0500, Felipe Contreras wrote:\n>>> It's annoying to see the prompt:\n>>>\n>>>   Hit return to start merge resolution tool (foo):\n>>>\n>>> Every time the user does 'git mergetool' even if the user already\n>>> configured 'foo' as the wanted tool.\n>>>\n>>> Display this prompt only when the user hasn't explicitly configured a\n>>> tool.\n>>\n>> I agree this is probably helpful.\n>> Most users I've met end up configuring mergetool.prompt=false.\n>>\n>> Thanks\n>\n> OK, is it an Ack?\n\nSure thing.\n\nAcked-by: David Aguilar <davvid@gmail.com>\n\n> Thanks for CC'ing Charles, by the way.  I think his point about\n> mentioning the change of default somewhere in the documentation\n> has some merits, and it can be done in a follow-up patch on top.\n\nAnother thing that crossed my mind is that we have -y for --no-prompt\nbecause --prompt was the original default. Maybe a -i (?) shortcut for\nthe interactive --prompt can be added to make the \"need to skip some\nwhen resolving\" use case easier to activate.\n-- \nDavid\n"},{"id":"239447","messageId":"xmqqppk80z0m.fsf@gitster.dls.corp.google.com","threadId":"36458","inReplyTo":"CAJDDKr5P7crudFiAa7Lu4CYmDWgZ-drdkiOfusFHioTF9co2Nw@mail.gmail.com","subject":"Re: [PATCH 2/2] mergetool: run prompt only if guessed tool","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-23T16:52:09Z","receivedAt":"2014-04-23T16:52:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Aguilar <davvid@gmail.com> writes:\n\n> On Tue, Apr 22, 2014 at 10:19 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> ...\n>> Thanks for CC'ing Charles, by the way.  I think his point about\n>> mentioning the change of default somewhere in the documentation\n>> has some merits, and it can be done in a follow-up patch on top.\n>\n> Another thing that crossed my mind is that we have -y for --no-prompt\n> because --prompt was the original default. Maybe a -i (?) shortcut for\n> the interactive --prompt can be added to make the \"need to skip some\n> when resolving\" use case easier to activate.\n\nHmm, perhaps, but is \"do we prompt to give a chance to the user to\nsay 'no, I am not interested in running the tool to that path'\" the\nonly interactivity in the overall end-user experience in using the\nmergetool?  To end-users, both interaction with the mergetool\nfront-end and interaction with individual back-end taken together\nwould comprise the whole end-user experience, so \"--interactive\"\noption that is implied by \"-i\" short-cut may make them expect a\nbehaviour from the backend that is more interactive than without,\nwhich would not be the case, so....\n"},{"id":"239448","messageId":"xmqqlhuw0ybb.fsf@gitster.dls.corp.google.com","threadId":"36458","inReplyTo":"20140423075601.GA18585@hashpling.org","subject":"Re: [PATCH 2/2] mergetool: run prompt only if guessed tool","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-23T17:07:20Z","receivedAt":"2014-04-23T17:07:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Charles Bailey <charles@hashpling.org> writes:\n\n> The bit of documentation that I was thinking of is in\n> Documentation/git-mergetool.txt where it states that \"--prompt\" is the\n> default which is now only partially true.\n\nThanks for being careful to help tying the loose ends.\n\nPerhaps like this?\n\nI take that your original motivation was to confirm to run a tool on\nthis particular (as opposed to another) path, but the user can also\ntake the prompt as to confirm to run this (as opposed to some other)\ntool.  The latter of which of course is irritating for those who\ntold which exact tool to use.\n\nI think the large part of the reason why you and Felipe had to have\na long back-and-forth was because it was unclear that the prompt\nserved these two purposes from the documentation, so I attempted to\nclarify the first motivation by adding a brief half-sentence.\n\n\n Documentation/git-mergetool.txt | 9 ++++++---\n 1 file changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt\nindex 07137f2..ec089ff 100644\n--- a/Documentation/git-mergetool.txt\n+++ b/Documentation/git-mergetool.txt\n@@ -71,11 +71,14 @@ success of the resolution after the custom tool has exited.\n --no-prompt::\n \tDon't prompt before each invocation of the merge resolution\n \tprogram.\n+\tThis is the default if the merge resolution program is\n+\texplicitly specified with the `--tool` option or with the\n+\t`merge.tool` configuration variable.\n \n --prompt::\n-\tPrompt before each invocation of the merge resolution program.\n-\tThis is the default behaviour; the option is provided to\n-\toverride any configuration settings.\n+\tPrompt before each invocation of the merge resolution program\n+\tto ask if it should be run for the path.\n+\n \n TEMPORARY FILES\n ---------------\n"},{"id":"239584","messageId":"xmqq1twmwpfy.fsf@gitster.dls.corp.google.com","threadId":"36458","inReplyTo":"xmqqlhuw0ybb.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 2/2] mergetool: run prompt only if guessed tool","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-24T18:30:09Z","receivedAt":"2014-04-24T18:30:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Perhaps like this?\n>\n> I take that your original motivation was to confirm to run a tool on\n> this particular (as opposed to another) path, but the user can also\n> take the prompt as to confirm to run this (as opposed to some other)\n> tool.  The latter of which of course is irritating for those who\n> told which exact tool to use.\n>\n> I think the large part of the reason why you and Felipe had to have\n> a long back-and-forth was because it was unclear that the prompt\n> served these two purposes from the documentation, so I attempted to\n> clarify the first motivation by adding a brief half-sentence.\n\nI'll queue the following as a separate documentation patch.  We may\nwant to polish the wording, so I'll keep it out of 'next' for now.\n\nI think the main patch is good for 'next' with or without doc update\nto be cooked during the remainder of this cycle, and I merged it\nthere already.\n\nThanks.\n\n-- >8 --\nSubject: [PATCH] mergetool: document the default for --[no-]prompt\n\nThe original motivation of using the prompt was to confirm to run a\ntool on this particular (as opposed to another) path, but the user\ncan also take the prompt as to confirm to run this (as opposed to\nsome other) tool.  The latter of which of course is irritating for\nthose who told which exact tool to use, which is the reason why we\nare flipping the default.\n\nDuring the review discussion of the patch, many people (including\nthe maintainer) missed that a user can find the prompt useful way to\nskip running the tool on particular paths.  Clarify it by adding a\nbrief half-sentence to the description.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/git-mergetool.txt | 8 +++++---\n 1 file changed, 5 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt\nindex 07137f2..e846c2e 100644\n--- a/Documentation/git-mergetool.txt\n+++ b/Documentation/git-mergetool.txt\n@@ -71,11 +71,13 @@ success of the resolution after the custom tool has exited.\n --no-prompt::\n \tDon't prompt before each invocation of the merge resolution\n \tprogram.\n+\tThis is the default if the merge resolution program is\n+\texplicitly specified with the `--tool` option or with the\n+\t`merge.tool` configuration variable.\n \n --prompt::\n-\tPrompt before each invocation of the merge resolution program.\n-\tThis is the default behaviour; the option is provided to\n-\toverride any configuration settings.\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-- \n2.0.0-rc0-224-g3c1c0b8\n"}]}