{"thread":{"id":"28569","subject":"[PATCH] git-difftool: allow skipping file by typing 'n' at prompt","startedAt":"2011-10-04T10:53:33Z","lastAt":"2011-10-10T23:39:41Z","messageCount":14,"participants":["Sitaram Chamarty","Junio C Hamano","Jeff King","Phil Hord","Charles Bailey"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"176838","messageId":"20111004105333.GA24331@atcmail.atc.tcs.com","threadId":"28569","inReplyTo":null,"subject":"[PATCH] git-difftool: allow skipping file by typing 'n' at prompt","fromName":"Sitaram Chamarty","fromEmail":"sitaram@atc.tcs.com","sentAt":"2011-10-04T10:53:33Z","receivedAt":"2011-10-04T10:53:33Z","isPatch":true,"sender":{"key":"sitaram@atc.tcs.com","avatar":null},"body":"Signed-off-by: Sitaram Chamarty <sitaram@atc.tcs.com>\n---\n\nI'm using what is pretty much a universal convention to\nsignify that the default choice is \"y\"; I hope documentation\nfor something so small is not needed but if it is, let me\nknow and I'll do that also.\n\n git-difftool--helper.sh |    9 +++++----\n 1 files changed, 5 insertions(+), 4 deletions(-)\n\ndiff --git a/git-difftool--helper.sh b/git-difftool--helper.sh\nindex 8452890..bc1b098 100755\n--- a/git-difftool--helper.sh\n+++ b/git-difftool--helper.sh\n@@ -38,15 +38,16 @@ launch_merge_tool () {\n \n \t# $LOCAL and $REMOTE are temporary files so prompt\n \t# the user with the real $MERGED name before launching $merge_tool.\n+\tans=y\n \tif should_prompt\n \tthen\n \t\tprintf \"\\nViewing: '$MERGED'\\n\"\n \t\tif use_ext_cmd\n \t\tthen\n-\t\t\tprintf \"Hit return to launch '%s': \" \\\n+\t\t\tprintf \"Launch '%s' [y]/n: \" \\\n \t\t\t\t\"$GIT_DIFFTOOL_EXTCMD\"\n \t\telse\n-\t\t\tprintf \"Hit return to launch '%s': \" \"$merge_tool\"\n+\t\t\tprintf \"Launch '%s' [y]/n: \" \"$merge_tool\"\n \t\tfi\n \t\tread ans\n \tfi\n@@ -54,9 +55,9 @@ launch_merge_tool () {\n \tif use_ext_cmd\n \tthen\n \t\texport BASE\n-\t\teval $GIT_DIFFTOOL_EXTCMD '\"$LOCAL\"' '\"$REMOTE\"'\n+\t\ttest \"$ans\" != \"n\" && eval $GIT_DIFFTOOL_EXTCMD '\"$LOCAL\"' '\"$REMOTE\"'\n \telse\n-\t\trun_merge_tool \"$merge_tool\"\n+\t\ttest \"$ans\" != \"n\" && run_merge_tool \"$merge_tool\"\n \tfi\n }\n \n-- \n1.7.6\n"},{"id":"176853","messageId":"7vbotwdbjg.fsf@alter.siamese.dyndns.org","threadId":"28569","inReplyTo":"20111004105333.GA24331@atcmail.atc.tcs.com","subject":"Re: [PATCH] git-difftool: allow skipping file by typing 'n' at prompt","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-04T15:25:07Z","receivedAt":"2011-10-04T15:25:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sitaram Chamarty <sitaram@atc.tcs.com> writes:\n\n> Signed-off-by: Sitaram Chamarty <sitaram@atc.tcs.com>\n> ---\n>\n> I'm using what is pretty much a universal convention to\n> signify that the default choice is \"y\"; I hope documentation\n> for something so small is not needed but if it is, let me\n> know and I'll do that also.\n>\n> -\t\t\tprintf \"Hit return to launch '%s': \" \\\n> +\t\t\tprintf \"Launch '%s' [y]/n: \" \\\n\nI think I've seen this done as: \"do this? [Y/n]\" elsewhere.\n\nNot telling you what to do, but trying to feel what others may think.\n"},{"id":"176857","messageId":"20111004174937.GA31671@sigill.intra.peff.net","threadId":"28569","inReplyTo":"7vbotwdbjg.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git-difftool: allow skipping file by typing 'n' at prompt","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-10-04T17:49:37Z","receivedAt":"2011-10-04T17:49:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 04, 2011 at 08:25:07AM -0700, Junio C Hamano wrote:\n\n> Sitaram Chamarty <sitaram@atc.tcs.com> writes:\n> \n> > Signed-off-by: Sitaram Chamarty <sitaram@atc.tcs.com>\n> > ---\n> >\n> > I'm using what is pretty much a universal convention to\n> > signify that the default choice is \"y\"; I hope documentation\n> > for something so small is not needed but if it is, let me\n> > know and I'll do that also.\n> >\n> > -\t\t\tprintf \"Hit return to launch '%s': \" \\\n> > +\t\t\tprintf \"Launch '%s' [y]/n: \" \\\n> \n> I think I've seen this done as: \"do this? [Y/n]\" elsewhere.\n> \n> Not telling you what to do, but trying to feel what others may think.\n\nYes, that was my immediate thought, too (or even [Yn]).\n\n-Peff\n"},{"id":"176860","messageId":"CABURp0qmYWRJzHZZwZreKnj0ymFyM_AYXWXqwy=vTZspoPvvvg@mail.gmail.com","threadId":"28569","inReplyTo":"7vbotwdbjg.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git-difftool: allow skipping file by typing 'n' at prompt","fromName":"Phil Hord","fromEmail":"phil.hord@gmail.com","sentAt":"2011-10-04T18:02:54Z","receivedAt":"2011-10-04T18:02:54Z","isPatch":true,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"On Tue, Oct 4, 2011 at 11:25 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Sitaram Chamarty <sitaram@atc.tcs.com> writes:\n>\n>> Signed-off-by: Sitaram Chamarty <sitaram@atc.tcs.com>\n>> ---\n>>\n>> I'm using what is pretty much a universal convention to\n>> signify that the default choice is \"y\"; I hope documentation\n>> for something so small is not needed but if it is, let me\n>> know and I'll do that also.\n>>\n>> -                     printf \"Hit return to launch '%s': \" \\\n>> +                     printf \"Launch '%s' [y]/n: \" \\\n>\n> I think I've seen this done as: \"do this? [Y/n]\" elsewhere.\n>\n> Not telling you what to do, but trying to feel what others may think.\n\nI think so, too.  The [y]/n syntax is not clear enough for me to\nconfidently know what the default value will be.\n\nPhil\n"},{"id":"176865","messageId":"7vty7oblpu.fsf@alter.siamese.dyndns.org","threadId":"28569","inReplyTo":"CABURp0qmYWRJzHZZwZreKnj0ymFyM_AYXWXqwy=vTZspoPvvvg@mail.gmail.com","subject":"Re: [PATCH] git-difftool: allow skipping file by typing 'n' at prompt","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-04T19:28:13Z","receivedAt":"2011-10-04T19:28:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phil Hord <phil.hord@gmail.com> writes:\n\n> On Tue, Oct 4, 2011 at 11:25 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>\n>> I think I've seen this done as: \"do this? [Y/n]\" elsewhere.\n>>\n>> Not telling you what to do, but trying to feel what others may think.\n>\n> I think so, too.  The [y]/n syntax is not clear enough for me to\n> confidently know what the default value will be.\n\nOne downside of \"do this [Y,n,m,o,p,q]? \" is that it limits us to\nlowercase responses, which means we cannot assign 'q' for quitting from\nthe innermost nested context and assign 'Q' for quitting from the whole\ninteractive loop (e.g. \"git add -p\").\n\n    \"do this [y,n,m,o,p,q] (default=y)? \"\n\nmay have been a better choice in hindsight.\n\nNo matter what we end up doing, let's try to be consistent.\n\nThanks.\n"},{"id":"176889","messageId":"CAMK1S_gssgpy7nF46c1roJUCN5yvQaOYfVE_-ZrvMfHGWKvk0w@mail.gmail.com","threadId":"28569","inReplyTo":"7vty7oblpu.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git-difftool: allow skipping file by typing 'n' at prompt","fromName":"Sitaram Chamarty","fromEmail":"sitaramc@gmail.com","sentAt":"2011-10-04T23:05:44Z","receivedAt":"2011-10-04T23:05:44Z","isPatch":true,"sender":{"key":"sitaramc@gmail.com","avatar":"https://avatars.githubusercontent.com/u/43316?v=4"},"body":"On Wed, Oct 5, 2011 at 12:58 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Phil Hord <phil.hord@gmail.com> writes:\n>\n>> On Tue, Oct 4, 2011 at 11:25 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>>\n>>> I think I've seen this done as: \"do this? [Y/n]\" elsewhere.\n>>>\n>>> Not telling you what to do, but trying to feel what others may think.\n>>\n>> I think so, too.  The [y]/n syntax is not clear enough for me to\n>> confidently know what the default value will be.\n>\n> One downside of \"do this [Y,n,m,o,p,q]? \" is that it limits us to\n> lowercase responses, which means we cannot assign 'q' for quitting from\n> the innermost nested context and assign 'Q' for quitting from the whole\n> interactive loop (e.g. \"git add -p\").\n>\n>    \"do this [y,n,m,o,p,q] (default=y)? \"\n\nDoes this even make a difference in this case?  I was going to send\nout a new patch using [Y/n] instead of my original [y]/n.  There's\nonly one loop in this thing, and till now people have been presumably\nhitting Ctrl-C to get out of it.  I see no real need to make that more\nelegant; all I set out to do is add one teeny weeny bit of\nfunctionality to a prompt that -- other than giving you a chance to\nhit that Ctrl-C -- was not actually doing anything useful at all.\n\n>\n> may have been a better choice in hindsight.\n>\n> No matter what we end up doing, let's try to be consistent.\n\nThe only other part of git where I have ever used a prompt is 'git add\n-p'.  Consistency with *that* prompt, to me, would mean colors.  And\nhelp text.  And I'm not sure what else, really, since I only used it\nsuperficially.\n\nIsn't that overkill for this case?\n\nI'll wait a few hours for any further comments then send out a patch\nthat is the same as my original one except it uses [Y/n] instead of\n[y]/n.\n"},{"id":"177017","messageId":"20111006125658.GB18709@sita-lt.atc.tcs.com","threadId":"28569","inReplyTo":"CAMK1S_gssgpy7nF46c1roJUCN5yvQaOYfVE_-ZrvMfHGWKvk0w@mail.gmail.com","subject":"[PATCH] git-difftool: allow skipping file by typing 'n' at prompt","fromName":"Sitaram Chamarty","fromEmail":"sitaramc@gmail.com","sentAt":"2011-10-06T12:56:58Z","receivedAt":"2011-10-06T12:56:58Z","isPatch":true,"sender":{"key":"sitaramc@gmail.com","avatar":"https://avatars.githubusercontent.com/u/43316?v=4"},"body":"Signed-off-by: Sitaram Chamarty <sitaram@atc.tcs.com>\n---\n\n(re-rolled according to earlier discussion)\n\n git-difftool--helper.sh |    9 +++++----\n 1 files changed, 5 insertions(+), 4 deletions(-)\n\ndiff --git a/git-difftool--helper.sh b/git-difftool--helper.sh\nindex 8452890..0468446 100755\n--- a/git-difftool--helper.sh\n+++ b/git-difftool--helper.sh\n@@ -38,15 +38,16 @@ launch_merge_tool () {\n \n \t# $LOCAL and $REMOTE are temporary files so prompt\n \t# the user with the real $MERGED name before launching $merge_tool.\n+\tans=y\n \tif should_prompt\n \tthen\n \t\tprintf \"\\nViewing: '$MERGED'\\n\"\n \t\tif use_ext_cmd\n \t\tthen\n-\t\t\tprintf \"Hit return to launch '%s': \" \\\n+\t\t\tprintf \"Launch '%s' [Y/n]: \" \\\n \t\t\t\t\"$GIT_DIFFTOOL_EXTCMD\"\n \t\telse\n-\t\t\tprintf \"Hit return to launch '%s': \" \"$merge_tool\"\n+\t\t\tprintf \"Launch '%s' [Y/n]: \" \"$merge_tool\"\n \t\tfi\n \t\tread ans\n \tfi\n@@ -54,9 +55,9 @@ launch_merge_tool () {\n \tif use_ext_cmd\n \tthen\n \t\texport BASE\n-\t\teval $GIT_DIFFTOOL_EXTCMD '\"$LOCAL\"' '\"$REMOTE\"'\n+\t\ttest \"$ans\" != \"n\" && eval $GIT_DIFFTOOL_EXTCMD '\"$LOCAL\"' '\"$REMOTE\"'\n \telse\n-\t\trun_merge_tool \"$merge_tool\"\n+\t\ttest \"$ans\" != \"n\" && run_merge_tool \"$merge_tool\"\n \tfi\n }\n \n-- \n1.7.6\n"},{"id":"177075","messageId":"7v62k210pj.fsf@alter.siamese.dyndns.org","threadId":"28569","inReplyTo":"20111006125658.GB18709@sita-lt.atc.tcs.com","subject":"Re: [PATCH] git-difftool: allow skipping file by typing 'n' at prompt","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-06T17:36:40Z","receivedAt":"2011-10-06T17:36:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sitaram Chamarty <sitaramc@gmail.com> writes:\n\n> Signed-off-by: Sitaram Chamarty <sitaram@atc.tcs.com>\n> ---\n>\n> (re-rolled according to earlier discussion)\n\nThanks. It is clear from the subject and the patch text that you are\nchanging \"hit return to unconditionally launch\" into \"launch it if you\nwant to\", but can you give justification why a choice not to launch is\nneeded in the log message?\n"},{"id":"177080","messageId":"20111006181522.GA2936@sita-lt.atc.tcs.com","threadId":"28569","inReplyTo":"7v62k210pj.fsf@alter.siamese.dyndns.org","subject":"[PATCH] git-difftool: allow skipping file by typing 'n' at prompt","fromName":"Sitaram Chamarty","fromEmail":"sitaramc@gmail.com","sentAt":"2011-10-06T18:15:22Z","receivedAt":"2011-10-06T18:15:22Z","isPatch":true,"sender":{"key":"sitaramc@gmail.com","avatar":"https://avatars.githubusercontent.com/u/43316?v=4"},"body":"This is useful if you forgot to restrict the diff to the paths you want\nto see, or selecting precisely the ones you want is too much typing.\n\nSigned-off-by: Sitaram Chamarty <sitaram@atc.tcs.com>\n---\n\nOn Thu, Oct 06, 2011 at 10:36:40AM -0700, Junio C Hamano wrote:\n\n> Thanks. It is clear from the subject and the patch text that you are\n> changing \"hit return to unconditionally launch\" into \"launch it if you\n> want to\", but can you give justification why a choice not to launch is\n> needed in the log message?\n\nOK; done.\n\n git-difftool--helper.sh |    9 +++++----\n 1 files changed, 5 insertions(+), 4 deletions(-)\n\ndiff --git a/git-difftool--helper.sh b/git-difftool--helper.sh\nindex 8452890..0468446 100755\n--- a/git-difftool--helper.sh\n+++ b/git-difftool--helper.sh\n@@ -38,15 +38,16 @@ launch_merge_tool () {\n \n \t# $LOCAL and $REMOTE are temporary files so prompt\n \t# the user with the real $MERGED name before launching $merge_tool.\n+\tans=y\n \tif should_prompt\n \tthen\n \t\tprintf \"\\nViewing: '$MERGED'\\n\"\n \t\tif use_ext_cmd\n \t\tthen\n-\t\t\tprintf \"Hit return to launch '%s': \" \\\n+\t\t\tprintf \"Launch '%s' [Y/n]: \" \\\n \t\t\t\t\"$GIT_DIFFTOOL_EXTCMD\"\n \t\telse\n-\t\t\tprintf \"Hit return to launch '%s': \" \"$merge_tool\"\n+\t\t\tprintf \"Launch '%s' [Y/n]: \" \"$merge_tool\"\n \t\tfi\n \t\tread ans\n \tfi\n@@ -54,9 +55,9 @@ launch_merge_tool () {\n \tif use_ext_cmd\n \tthen\n \t\texport BASE\n-\t\teval $GIT_DIFFTOOL_EXTCMD '\"$LOCAL\"' '\"$REMOTE\"'\n+\t\ttest \"$ans\" != \"n\" && eval $GIT_DIFFTOOL_EXTCMD '\"$LOCAL\"' '\"$REMOTE\"'\n \telse\n-\t\trun_merge_tool \"$merge_tool\"\n+\t\ttest \"$ans\" != \"n\" && run_merge_tool \"$merge_tool\"\n \tfi\n }\n \n-- \n1.7.6\n"},{"id":"177179","messageId":"7vwrcgtvh4.fsf@alter.siamese.dyndns.org","threadId":"28569","inReplyTo":"20111006181522.GA2936@sita-lt.atc.tcs.com","subject":"Re: [PATCH] git-difftool: allow skipping file by typing 'n' at prompt","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-07T20:09:11Z","receivedAt":"2011-10-07T20:09:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sitaram Chamarty <sitaramc@gmail.com> writes:\n\n> This is useful if you forgot to restrict the diff to the paths you want\n> to see, or selecting precisely the ones you want is too much typing.\n>\n> Signed-off-by: Sitaram Chamarty <sitaram@atc.tcs.com>\n> ---\n>\n> On Thu, Oct 06, 2011 at 10:36:40AM -0700, Junio C Hamano wrote:\n>\n>> Thanks. It is clear from the subject and the patch text that you are\n>> changing \"hit return to unconditionally launch\" into \"launch it if you\n>> want to\", but can you give justification why a choice not to launch is\n>> needed in the log message?\n>\n> OK; done.\n\nLooks OK from a cursory viewing. Do we want some additional tests?\n\nFor that matter, have you run the test suite with this patch applied (I\nhaven't)?\n\n>  git-difftool--helper.sh |    9 +++++----\n>  1 files changed, 5 insertions(+), 4 deletions(-)\n>\n> diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh\n> index 8452890..0468446 100755\n> --- a/git-difftool--helper.sh\n> +++ b/git-difftool--helper.sh\n> @@ -38,15 +38,16 @@ launch_merge_tool () {\n>  \n>  \t# $LOCAL and $REMOTE are temporary files so prompt\n>  \t# the user with the real $MERGED name before launching $merge_tool.\n> +\tans=y\n>  \tif should_prompt\n>  \tthen\n>  \t\tprintf \"\\nViewing: '$MERGED'\\n\"\n>  \t\tif use_ext_cmd\n>  \t\tthen\n> -\t\t\tprintf \"Hit return to launch '%s': \" \\\n> +\t\t\tprintf \"Launch '%s' [Y/n]: \" \\\n>  \t\t\t\t\"$GIT_DIFFTOOL_EXTCMD\"\n>  \t\telse\n> -\t\t\tprintf \"Hit return to launch '%s': \" \"$merge_tool\"\n> +\t\t\tprintf \"Launch '%s' [Y/n]: \" \"$merge_tool\"\n>  \t\tfi\n>  \t\tread ans\n>  \tfi\n> @@ -54,9 +55,9 @@ launch_merge_tool () {\n>  \tif use_ext_cmd\n>  \tthen\n>  \t\texport BASE\n> -\t\teval $GIT_DIFFTOOL_EXTCMD '\"$LOCAL\"' '\"$REMOTE\"'\n> +\t\ttest \"$ans\" != \"n\" && eval $GIT_DIFFTOOL_EXTCMD '\"$LOCAL\"' '\"$REMOTE\"'\n>  \telse\n> -\t\trun_merge_tool \"$merge_tool\"\n> +\t\ttest \"$ans\" != \"n\" && run_merge_tool \"$merge_tool\"\n>  \tfi\n>  }\n"},{"id":"177216","messageId":"20111008131015.GA28213@sita-lt.atc.tcs.com","threadId":"28569","inReplyTo":"7vwrcgtvh4.fsf@alter.siamese.dyndns.org","subject":"[PATCH] git-difftool: allow skipping file by typing 'n' at prompt","fromName":"Sitaram Chamarty","fromEmail":"sitaramc@gmail.com","sentAt":"2011-10-08T13:10:15Z","receivedAt":"2011-10-08T13:10:15Z","isPatch":true,"sender":{"key":"sitaramc@gmail.com","avatar":"https://avatars.githubusercontent.com/u/43316?v=4"},"body":"This is useful if you forgot to restrict the diff to the paths you want\nto see, or selecting precisely the ones you want is too much typing.\n\nSigned-off-by: Sitaram Chamarty <sitaram@atc.tcs.com>\n---\n\nOn Fri, Oct 07, 2011 at 01:09:11PM -0700, Junio C Hamano wrote:\n\n> Looks OK from a cursory viewing. Do we want some additional tests?\n> \n> For that matter, have you run the test suite with this patch applied (I\n> haven't)?\n\nOK; done.  I got some \"broken\" but nothing \"failed\":\n\n    make aggregate-results\n    make[3]: Entering directory `/home/sitaram/clones/git/t'\n    for f in test-results/t*-*.counts; do \\\n            echo \"$f\"; \\\n    done | '/bin/sh' ./aggregate-results.sh\n    fixed   0\n    success 7377\n    failed  0\n    broken  49\n    total   7461\n\nHope that is not a problem.\n\nHowever, I'm not sure the file names that 'git difftool'\ncomes up with are in a predictable order.  That would mess\nup the test, but I can neither make it fail not find\ndefinitive information on the order in which the changed\nfiles are processed.\n\n git-difftool--helper.sh |    9 +++++----\n t/t7800-difftool.sh     |   44 +++++++++++++++++++++++++++++++++++++++++++-\n 2 files changed, 48 insertions(+), 5 deletions(-)\n\ndiff --git a/git-difftool--helper.sh b/git-difftool--helper.sh\nindex 8452890..0468446 100755\n--- a/git-difftool--helper.sh\n+++ b/git-difftool--helper.sh\n@@ -38,15 +38,16 @@ launch_merge_tool () {\n \n \t# $LOCAL and $REMOTE are temporary files so prompt\n \t# the user with the real $MERGED name before launching $merge_tool.\n+\tans=y\n \tif should_prompt\n \tthen\n \t\tprintf \"\\nViewing: '$MERGED'\\n\"\n \t\tif use_ext_cmd\n \t\tthen\n-\t\t\tprintf \"Hit return to launch '%s': \" \\\n+\t\t\tprintf \"Launch '%s' [Y/n]: \" \\\n \t\t\t\t\"$GIT_DIFFTOOL_EXTCMD\"\n \t\telse\n-\t\t\tprintf \"Hit return to launch '%s': \" \"$merge_tool\"\n+\t\t\tprintf \"Launch '%s' [Y/n]: \" \"$merge_tool\"\n \t\tfi\n \t\tread ans\n \tfi\n@@ -54,9 +55,9 @@ launch_merge_tool () {\n \tif use_ext_cmd\n \tthen\n \t\texport BASE\n-\t\teval $GIT_DIFFTOOL_EXTCMD '\"$LOCAL\"' '\"$REMOTE\"'\n+\t\ttest \"$ans\" != \"n\" && eval $GIT_DIFFTOOL_EXTCMD '\"$LOCAL\"' '\"$REMOTE\"'\n \telse\n-\t\trun_merge_tool \"$merge_tool\"\n+\t\ttest \"$ans\" != \"n\" && run_merge_tool \"$merge_tool\"\n \tfi\n }\n \ndiff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\nindex 395adfc..f547e0b 100755\n--- a/t/t7800-difftool.sh\n+++ b/t/t7800-difftool.sh\n@@ -38,7 +38,18 @@ restore_test_defaults()\n prompt_given()\n {\n \tprompt=\"$1\"\n-\ttest \"$prompt\" = \"Hit return to launch 'test-tool': branch\"\n+\ttest \"$prompt\" = \"Launch 'test-tool' [Y/n]: branch\"\n+}\n+\n+stdin_contains()\n+{\n+\tgrep >/dev/null \"$1\"\n+}\n+\n+stdin_doesnot_contain()\n+{\n+\tgrep >/dev/null \"$1\" && return 1\n+\treturn 0\n }\n \n # Create a file on master and change it on branch\n@@ -265,4 +276,35 @@ test_expect_success PERL 'difftool --extcmd cat arg2' '\n \ttest \"$diff\" = branch\n '\n \n+# Create a second file on master and a different version on branch\n+test_expect_success PERL 'setup with 2 files different' '\n+\techo m2 >file2 &&\n+\tgit add file2 &&\n+\tgit commit -m \"added file2\" &&\n+\n+\tgit checkout branch &&\n+\techo br2 >file2 &&\n+\tgit add file2 &&\n+\tgit commit -a -m \"branch changed file2\" &&\n+\tgit checkout master\n+'\n+\n+test_expect_success PERL 'say no to the first file' '\n+\tdiff=$((echo n; echo) | git difftool -x cat branch) &&\n+\n+\techo \"$diff\" | stdin_contains m2 &&\n+\techo \"$diff\" | stdin_contains br2 &&\n+\techo \"$diff\" | stdin_doesnot_contain master &&\n+\techo \"$diff\" | stdin_doesnot_contain branch\n+'\n+\n+test_expect_success PERL 'say no to the second file' '\n+\tdiff=$((echo; echo n) | git difftool -x cat branch) &&\n+\n+\techo \"$diff\" | stdin_contains master &&\n+\techo \"$diff\" | stdin_contains branch &&\n+\techo \"$diff\" | stdin_doesnot_contain m2 &&\n+\techo \"$diff\" | stdin_doesnot_contain br2\n+'\n+\n test_done\n-- \n1.7.6\n"},{"id":"177272","messageId":"20111009112623.GA30585@hashpling.org","threadId":"28569","inReplyTo":"20111008131015.GA28213@sita-lt.atc.tcs.com","subject":"Re: [PATCH] git-difftool: allow skipping file by typing 'n' at prompt","fromName":"Charles Bailey","fromEmail":"charles@hashpling.org","sentAt":"2011-10-09T11:26:23Z","receivedAt":"2011-10-09T11:26:23Z","isPatch":true,"sender":{"key":"charles@hashpling.org","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"On Sat, Oct 08, 2011 at 06:40:15PM +0530, Sitaram Chamarty wrote:\n> \n>  git-difftool--helper.sh |    9 +++++----\n>  t/t7800-difftool.sh     |   44 +++++++++++++++++++++++++++++++++++++++++++-\n>  2 files changed, 48 insertions(+), 5 deletions(-)\n> \n> diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh\n> index 8452890..0468446 100755\n> --- a/git-difftool--helper.sh\n> +++ b/git-difftool--helper.sh\n> @@ -38,15 +38,16 @@ launch_merge_tool () {\n>  \n>  \t# $LOCAL and $REMOTE are temporary files so prompt\n>  \t# the user with the real $MERGED name before launching $merge_tool.\n> +\tans=y\n>  \tif should_prompt\n>  \tthen\n>  \t\tprintf \"\\nViewing: '$MERGED'\\n\"\n>  \t\tif use_ext_cmd\n>  \t\tthen\n> -\t\t\tprintf \"Hit return to launch '%s': \" \\\n> +\t\t\tprintf \"Launch '%s' [Y/n]: \" \\\n>  \t\t\t\t\"$GIT_DIFFTOOL_EXTCMD\"\n>  \t\telse\n> -\t\t\tprintf \"Hit return to launch '%s': \" \"$merge_tool\"\n> +\t\t\tprintf \"Launch '%s' [Y/n]: \" \"$merge_tool\"\n>  \t\tfi\n>  \t\tread ans\n>  \tfi\n> @@ -54,9 +55,9 @@ launch_merge_tool () {\n>  \tif use_ext_cmd\n>  \tthen\n>  \t\texport BASE\n> -\t\teval $GIT_DIFFTOOL_EXTCMD '\"$LOCAL\"' '\"$REMOTE\"'\n> +\t\ttest \"$ans\" != \"n\" && eval $GIT_DIFFTOOL_EXTCMD '\"$LOCAL\"' '\"$REMOTE\"'\n>  \telse\n> -\t\trun_merge_tool \"$merge_tool\"\n> +\t\ttest \"$ans\" != \"n\" && run_merge_tool \"$merge_tool\"\n>  \tfi\n>  }\n\nIt's a minor point but for me, this looks a little more difficult to\nfollow than it needs to be.\n\nWhy do we need to hold on to 'ans' for so long? With the new prompt,\nif we ever 'read ans' we always want to return from the\nlaunch_merge_tool without doing anything else if we read \"n\". I think\nit's easier to follow if we just change 'read ans' and leave the 'if\nuse_ext_cmd' clauses alone. Perhaps some people don't like the early\nreturn, though?\n\nCharles.\n\n\nE.g. (for discussion, untested):\n\ndiff --git a/git-difftool--helper.sh b/git-difftool--helper.sh\nindex 8452890..b668a12 100755\n--- a/git-difftool--helper.sh\n+++ b/git-difftool--helper.sh\n@@ -43,12 +43,16 @@ launch_merge_tool () {\n                printf \"\\nViewing: '$MERGED'\\n\"\n                if use_ext_cmd\n                then\n-                       printf \"Hit return to launch '%s': \" \\\n+                       printf \"Launch '%s' [Y/n]: \" \\\n                                \"$GIT_DIFFTOOL_EXTCMD\"\n                else\n-                       printf \"Hit return to launch '%s': \" \"$merge_tool\"\n+                       printf \"Launch '%s' [Y/n]: \" \"$merge_tool\"\n+               fi\n+\n+               if read ans && test \"$ans\" = \"n\"\n+               then\n+                       return\n                fi\n-               read ans\n        fi\n\n        if use_ext_cmd\n"},{"id":"177327","messageId":"7v8voslg4l.fsf@alter.siamese.dyndns.org","threadId":"28569","inReplyTo":"20111008131015.GA28213@sita-lt.atc.tcs.com","subject":"Re: [PATCH] git-difftool: allow skipping file by typing 'n' at prompt","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-10T20:56:58Z","receivedAt":"2011-10-10T20:56:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sitaram Chamarty <sitaramc@gmail.com> writes:\n\n> However, I'm not sure the file names that 'git difftool'\n> comes up with are in a predictable order.  That would mess\n> up the test, but I can neither make it fail not find\n> definitive information on the order in which the changed\n> files are processed.\n\nHmm, that may be an issue, I would think.\n\n>  git-difftool--helper.sh |    9 +++++----\n>  t/t7800-difftool.sh     |   44 +++++++++++++++++++++++++++++++++++++++++++-\n>  2 files changed, 48 insertions(+), 5 deletions(-)\n>\n> diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh\n> index 8452890..0468446 100755\n> --- a/git-difftool--helper.sh\n> +++ b/git-difftool--helper.sh\n> @@ -38,15 +38,16 @@ launch_merge_tool () {\n>  \n>  \t# $LOCAL and $REMOTE are temporary files so prompt\n>  \t# the user with the real $MERGED name before launching $merge_tool.\n> +\tans=y\n>  \tif should_prompt\n>  \tthen\n>  \t\tprintf \"\\nViewing: '$MERGED'\\n\"\n>  \t\tif use_ext_cmd\n>  \t\tthen\n> -\t\t\tprintf \"Hit return to launch '%s': \" \\\n> +\t\t\tprintf \"Launch '%s' [Y/n]: \" \\\n>  \t\t\t\t\"$GIT_DIFFTOOL_EXTCMD\"\n>  \t\telse\n> -\t\t\tprintf \"Hit return to launch '%s': \" \"$merge_tool\"\n> +\t\t\tprintf \"Launch '%s' [Y/n]: \" \"$merge_tool\"\n>  \t\tfi\n>  \t\tread ans\n>  \tfi\n> @@ -54,9 +55,9 @@ launch_merge_tool () {\n>  \tif use_ext_cmd\n>  \tthen\n>  \t\texport BASE\n> -\t\teval $GIT_DIFFTOOL_EXTCMD '\"$LOCAL\"' '\"$REMOTE\"'\n> +\t\ttest \"$ans\" != \"n\" && eval $GIT_DIFFTOOL_EXTCMD '\"$LOCAL\"' '\"$REMOTE\"'\n>  \telse\n> -\t\trun_merge_tool \"$merge_tool\"\n> +\t\ttest \"$ans\" != \"n\" && run_merge_tool \"$merge_tool\"\n>  \tfi\n>  }\n\nI also found suggestion by Charles Bailey to return from the launch\nfunction when the user says \"no\" easier to follow.\n\n> diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\n> index 395adfc..f547e0b 100755\n> --- a/t/t7800-difftool.sh\n> +++ b/t/t7800-difftool.sh\n> @@ -38,7 +38,18 @@ restore_test_defaults()\n>  prompt_given()\n>  {\n>  \tprompt=\"$1\"\n> -\ttest \"$prompt\" = \"Hit return to launch 'test-tool': branch\"\n> +\ttest \"$prompt\" = \"Launch 'test-tool' [Y/n]: branch\"\n> +}\n> +\n> +stdin_contains()\n> +{\n> +\tgrep >/dev/null \"$1\"\n> +}\n> +\n> +stdin_doesnot_contain()\n> +{\n> +\tgrep >/dev/null \"$1\" && return 1\n> +\treturn 0\n>  }\n\nDoesn't\n\n\t! grep >/dev/null \"$1\"\n\nwork in this case?        \n\nI also wondered if this is easier to read:\n\n\tpipe | stdin_contains m2 &&\n\t! pipe | stdin_contains master\n\nbut I do not think it is (we cannot say \"pipe | ! stdin_contains master\").\n\nIn any case, here is what I ended up queuing.  Thanks.\n\n-- >8 --\nFrom: Sitaram Chamarty <sitaramc@gmail.com>\nDate: Sat, 8 Oct 2011 18:40:15 +0530\nSubject: [PATCH] git-difftool: allow skipping file by typing 'n' at prompt\n\nThis is useful if you forgot to restrict the diff to the paths you want\nto see, or selecting precisely the ones you want is too much typing.\n\n[jc: with a change to return from the function upon 'n' by Charles Bailey\nand a small tweak in stdin_doesnot_contain() in the test]\n\nSigned-off-by: Sitaram Chamarty <sitaram@atc.tcs.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n git-difftool--helper.sh |    9 ++++++---\n t/t7800-difftool.sh     |   43 ++++++++++++++++++++++++++++++++++++++++++-\n 2 files changed, 48 insertions(+), 4 deletions(-)\n\ndiff --git a/git-difftool--helper.sh b/git-difftool--helper.sh\nindex 8452890..e6558d1 100755\n--- a/git-difftool--helper.sh\n+++ b/git-difftool--helper.sh\n@@ -43,12 +43,15 @@ launch_merge_tool () {\n \t\tprintf \"\\nViewing: '$MERGED'\\n\"\n \t\tif use_ext_cmd\n \t\tthen\n-\t\t\tprintf \"Hit return to launch '%s': \" \\\n+\t\t\tprintf \"Launch '%s' [Y/n]: \" \\\n \t\t\t\t\"$GIT_DIFFTOOL_EXTCMD\"\n \t\telse\n-\t\t\tprintf \"Hit return to launch '%s': \" \"$merge_tool\"\n+\t\t\tprintf \"Launch '%s' [Y/n]: \" \"$merge_tool\"\n+\t\tfi\n+\t\tif read ans && test \"$ans\" = n\n+\t\tthen\n+\t\t\treturn\n \t\tfi\n-\t\tread ans\n \tfi\n \n \tif use_ext_cmd\ndiff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\nindex 395adfc..7fc2b3a 100755\n--- a/t/t7800-difftool.sh\n+++ b/t/t7800-difftool.sh\n@@ -38,7 +38,17 @@ restore_test_defaults()\n prompt_given()\n {\n \tprompt=\"$1\"\n-\ttest \"$prompt\" = \"Hit return to launch 'test-tool': branch\"\n+\ttest \"$prompt\" = \"Launch 'test-tool' [Y/n]: branch\"\n+}\n+\n+stdin_contains()\n+{\n+\tgrep >/dev/null \"$1\"\n+}\n+\n+stdin_doesnot_contain()\n+{\n+\t! stdin_contains \"$1\"\n }\n \n # Create a file on master and change it on branch\n@@ -265,4 +275,35 @@ test_expect_success PERL 'difftool --extcmd cat arg2' '\n \ttest \"$diff\" = branch\n '\n \n+# Create a second file on master and a different version on branch\n+test_expect_success PERL 'setup with 2 files different' '\n+\techo m2 >file2 &&\n+\tgit add file2 &&\n+\tgit commit -m \"added file2\" &&\n+\n+\tgit checkout branch &&\n+\techo br2 >file2 &&\n+\tgit add file2 &&\n+\tgit commit -a -m \"branch changed file2\" &&\n+\tgit checkout master\n+'\n+\n+test_expect_success PERL 'say no to the first file' '\n+\tdiff=$((echo n; echo) | git difftool -x cat branch) &&\n+\n+\techo \"$diff\" | stdin_contains m2 &&\n+\techo \"$diff\" | stdin_contains br2 &&\n+\techo \"$diff\" | stdin_doesnot_contain master &&\n+\techo \"$diff\" | stdin_doesnot_contain branch\n+'\n+\n+test_expect_success PERL 'say no to the second file' '\n+\tdiff=$((echo; echo n) | git difftool -x cat branch) &&\n+\n+\techo \"$diff\" | stdin_contains master &&\n+\techo \"$diff\" | stdin_contains branch &&\n+\techo \"$diff\" | stdin_doesnot_contain m2 &&\n+\techo \"$diff\" | stdin_doesnot_contain br2\n+'\n+\n test_done\n-- \n1.7.7.138.g7f41b6\n"},{"id":"177342","messageId":"CAMK1S_jNhB_cuTV0u+o_RwOdMKa-xXNZp8KGQ63yqFc70zTm5g@mail.gmail.com","threadId":"28569","inReplyTo":"7v8voslg4l.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git-difftool: allow skipping file by typing 'n' at prompt","fromName":"Sitaram Chamarty","fromEmail":"sitaramc@gmail.com","sentAt":"2011-10-10T23:39:41Z","receivedAt":"2011-10-10T23:39:41Z","isPatch":true,"sender":{"key":"sitaramc@gmail.com","avatar":"https://avatars.githubusercontent.com/u/43316?v=4"},"body":"On Tue, Oct 11, 2011 at 2:26 AM, Junio C Hamano <gitster@pobox.com> wrote:\n\n> I also wondered if this is easier to read:\n>\n>        pipe | stdin_contains m2 &&\n>        ! pipe | stdin_contains master\n\n> but I do not think it is (we cannot say \"pipe | ! stdin_contains master\").\n\nAgreed on both counts.\n\n\"pipe | ( ! grep master )\" does work, but I suspect that is an\ninconsistency in the shell so I didn't want to use it.  IIRC the \"(\nlist )\" constrict is not supposed to make *that* much difference.\nHave to check when I have time.\n\n> In any case, here is what I ended up queuing.  Thanks.\n\n> +stdin_doesnot_contain()\n> +{\n> +       ! stdin_contains \"$1\"\n>  }\n\n(facepalm) Why didn't I think of that!\n\nThanks :-)\n"}]}