{"thread":{"id":"42003","subject":"[PATCH v2] difftool/mergetool: make the form of yes/no questions consistent","startedAt":"2016-04-12T14:44:20Z","lastAt":"2016-04-13T10:27:35Z","messageCount":4,"participants":["Nikola Forró","Junio C Hamano","David Aguilar"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"283186","messageId":"1460472260.3214.23.camel@redhat.com","threadId":"42003","inReplyTo":null,"subject":"[PATCH v2] difftool/mergetool: make the form of yes/no questions consistent","fromName":"Nikola Forró","fromEmail":"nforro@redhat.com","sentAt":"2016-04-12T14:44:20Z","receivedAt":"2016-04-12T14:44:20Z","isPatch":true,"sender":{"key":"nforro@redhat.com","avatar":"https://avatars.githubusercontent.com/u/14923067?v=4"},"body":"Every yes/no question in difftool/mergetool scripts has slightly\ndifferent form, and none of them is consistent with the form git\nitself uses.\n\nMake the form of all the questions consistent with the form used\nby git.\n\nReviewed-by: John Keeping \nSigned-off-by: Nikola Forró \n---\nChanges in v2: example dropped from the commit message\n\n git-difftool--helper.sh | 4 ++--\n git-mergetool--lib.sh | 2 +-\n git-mergetool.sh | 2 +-\n 3 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/git-difftool--helper.sh b/git-difftool--helper.sh\nindex 2b11b1d..84d6cc0 100755\n--- a/git-difftool--helper.sh\n+++ b/git-difftool--helper.sh\n@@ -44,10 +44,10 @@ launch_merge_tool () {\n \t\t\t\"$GIT_DIFF_PATH_TOTAL\" \"$MERGED\"\n \t\tif use_ext_cmd\n \t\tthen\n-\t\t\tprintf \"Launch '%s' [Y/n]: \" \\\n+\t\t\tprintf \"Launch '%s' [Y/n]? \" \\\n \t\t\t\t\"$GIT_DIFFTOOL_EXTCMD\"\n \t\telse\n-\t\t\tprintf \"Launch '%s' [Y/n]: \" \"$merge_tool\"\n+\t\t\tprintf \"Launch '%s' [Y/n]? \" \"$merge_tool\"\n \t\tfi\n \t\tread ans || return\n \t\tif test \"$ans\" = n\ndiff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh\nindex 54ac8e4..92adcc0 100644\n--- a/git-mergetool--lib.sh\n+++ b/git-mergetool--lib.sh\n@@ -100,7 +100,7 @@ check_unchanged () {\n \t\twhile true\n \t\tdo\n \t\t\techo \"$MERGED seems unchanged.\"\n-\t\t\tprintf \"Was the merge successful? [y/n] \"\n+\t\t\tprintf \"Was the merge successful [y/n]? \"\n \t\t\tread answer || return 1\n \t\t\tcase \"$answer\" in\n \t\t\ty*|Y*) return 0 ;;\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex 9f77e3a..2e0635a 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -396,7 +396,7 @@ done\n prompt_after_failed_merge () {\n \twhile true\n \tdo\n-\t\tprintf \"Continue merging other unresolved paths (y/n) ? \"\n+\t\tprintf \"Continue merging other unresolved paths [y/n]? \"\n \t\tread ans || return 1\n \t\tcase \"$ans\" in\n \t\t[yY]*)\n-- \n2.4.11\n"},{"id":"283219","messageId":"xmqqegaapm02.fsf@gitster.mtv.corp.google.com","threadId":"42003","inReplyTo":"1460472260.3214.23.camel@redhat.com","subject":"Re: [PATCH v2] difftool/mergetool: make the form of yes/no questions consistent","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-12T21:15:09Z","receivedAt":"2016-04-12T21:15:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nikola Forró writes:\n\n> Every yes/no question in difftool/mergetool scripts has slightly\n> different form, and none of them is consistent with the form git\n> itself uses.\n>\n> Make the form of all the questions consistent with the form used\n> by git.\n>\n> Reviewed-by: John Keeping \n> Signed-off-by: Nikola Forró \n> ---\n> Changes in v2: example dropped from the commit message\n\nThanks; have you run the test suite with this patch applied?\n\nIt is your responsibility to make sure that the expectation by\nexisting tests are still satisfied after your change, or update\ntheir expectation to match the new (and hopefully better) world\norder your patch introduces.\n\nI needed to squash this in to make the tests pass, but because I am\nnot a difftool user, I do not at all know if the prompt produced\n(and expected by the test) is sensible or not.\n\n t/t7800-difftool.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\nindex ec8bc8c..df9050f 100755\n--- a/t/t7800-difftool.sh\n+++ b/t/t7800-difftool.sh\n@@ -20,7 +20,7 @@ difftool_test_setup ()\n prompt_given ()\n {\n \tprompt=\"$1\"\n-\ttest \"$prompt\" = \"Launch 'test-tool' [Y/n]: branch\"\n+\ttest \"$prompt\" = \"Launch 'test-tool' [Y/n]? branch\"\n }\n \n # Create a file on master and change it on branch\n"},{"id":"283280","messageId":"20160413012501.GA32611@gmail.com","threadId":"42003","inReplyTo":"xmqqegaapm02.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2] difftool/mergetool: make the form of yes/no questions consistent","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2016-04-13T01:25:01Z","receivedAt":"2016-04-13T01:25:01Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Tue, Apr 12, 2016 at 02:15:09PM -0700, Junio C Hamano wrote:\n> Nikola Forró writes:\n> \n> > Every yes/no question in difftool/mergetool scripts has slightly\n> > different form, and none of them is consistent with the form git\n> > itself uses.\n> >\n> > Make the form of all the questions consistent with the form used\n> > by git.\n> >\n> > Reviewed-by: John Keeping \n> > Signed-off-by: Nikola Forró \n> > ---\n> > Changes in v2: example dropped from the commit message\n> \n> Thanks; have you run the test suite with this patch applied?\n> \n> It is your responsibility to make sure that the expectation by\n> existing tests are still satisfied after your change, or update\n> their expectation to match the new (and hopefully better) world\n> order your patch introduces.\n> \n> I needed to squash this in to make the tests pass, but because I am\n> not a difftool user, I do not at all know if the prompt produced\n> (and expected by the test) is sensible or not.\n> \n> t/t7800-difftool.sh | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\n> index ec8bc8c..df9050f 100755\n> --- a/t/t7800-difftool.sh\n> +++ b/t/t7800-difftool.sh\n> @@ -20,7 +20,7 @@ difftool_test_setup ()\n> prompt_given ()\n> {\n> \tprompt=\"$1\"\n> -\ttest \"$prompt\" = \"Launch 'test-tool' [Y/n]: branch\"\n> +\ttest \"$prompt\" = \"Launch 'test-tool' [Y/n]? branch\"\n> }\n> \n> # Create a file on master and change it on branch\n\nThat looks correct to me. Sorry 'bout that, I'll remember to\nrun the tests with the patches applied next time.\n\ncheers,\n-- \nDavid\n"},{"id":"283308","messageId":"1460543255.3228.13.camel@redhat.com","threadId":"42003","inReplyTo":"xmqqegaapm02.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2] difftool/mergetool: make the form of yes/no questions consistent","fromName":"Nikola Forró","fromEmail":"nforro@redhat.com","sentAt":"2016-04-13T10:27:35Z","receivedAt":"2016-04-13T10:27:35Z","isPatch":true,"sender":{"key":"nforro@redhat.com","avatar":"https://avatars.githubusercontent.com/u/14923067?v=4"},"body":"On Tue, 2016-04-12 at 14:15 -0700, Junio C Hamano wrote:\n> Thanks; have you run the test suite with this patch applied?\n\nSorry, I totally forgot. I'll make sure to do that next time.\n\nThanks,\nNikola\n"}]}