difftool/mergetool: make the form of yes/no questions consistent
PATCH 1/1 difftool/mergetool: make the form of yes/no questions consistent
Every yes/no question in difftool/mergetool scripts has slightly different form, and none of them is consistent with the form git itself uses.
Make the form of all the questions consistent with the form used by git, i.e. "Question [y/n]? ".
--- git-difftool--helper.sh | 4 ++-- git-mergetool--lib.sh | 2 +- git-mergetool.sh | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-)
git-difftool--helper.sh +2 -2
@@ -44,10 +44,10 @@ launch_merge_tool () { "$GIT_DIFF_PATH_TOTAL" "$MERGED" if use_ext_cmd then - printf "Launch '%s' [Y/n]: " \ + printf "Launch '%s' [Y/n]? " \ "$GIT_DIFFTOOL_EXTCMD" else - printf "Launch '%s' [Y/n]: " "$merge_tool" + printf "Launch '%s' [Y/n]? " "$merge_tool" fi read ans || return if test "$ans" = n
git-mergetool--lib.sh +1 -1
@@ -100,7 +100,7 @@ check_unchanged () { while true do echo "$MERGED seems unchanged." - printf "Was the merge successful? [y/n] " + printf "Was the merge successful [y/n]? " read answer || return 1 case "$answer" in y*|Y*) return 0 ;;
git-mergetool.sh +1 -1
@@ -396,7 +396,7 @@ done prompt_after_failed_merge () { while true do - printf "Continue merging other unresolved paths (y/n) ? " + printf "Continue merging other unresolved paths [y/n]? " read ans || return 1 case "$ans" in [yY]*)
Signature · 1 line
2.4.11
Quoted 19 lines · Nikola Forrrr
Every yes/no question in difftool/mergetool scripts has slightly different form, and none of them is consistent with the form git itself uses.
Make the form of all the questions consistent with the form used by git, i.e. "Question [y/n]? ".
Signed-off-by Nikola Forró <[email protected]>--- git-difftool--helper.sh | 4 ++-- git-mergetool--lib.sh | 2 +- git-mergetool.sh | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-)git-difftool--helper.sh +0
-0
git-difftool--helper.sh launch_merge_tool () { · line 50
Quoted 9 lines
git-difftool--helper.sh +2
-2@@ -44,10 +44,10 @@ launch_merge_tool () { "$GIT_DIFF_PATH_TOTAL" "$MERGED" if use_ext_cmd then - printf "Launch '%s' [Y/n]: " \ + printf "Launch '%s' [Y/n]? " \ "$GIT_DIFFTOOL_EXTCMD" else - printf "Launch '%s' [Y/n]: " "$merge_tool" + printf "Launch '%s' [Y/n]? " "$merge_tool"
I think the case in these two is correct as-is. The "Y" is capitalised because it is the default and will take effect if the user just presses ENTER.
From a quick look, the two cases below do not have this behaviour (the user must enter either "y" or "n"), so it is correct that they are not capitalized.
The change from ":" to "?" and normalization of "?" placement below seems reasonable.
Quoted 31 lines
git-difftool--helper.sh +0
-0fi read ans || return if test "$ans" = ngit-mergetool--lib.sh +1
-1@@ -100,7 +100,7 @@ check_unchanged () { while true do echo "$MERGED seems unchanged." - printf "Was the merge successful? [y/n] " + printf "Was the merge successful [y/n]? " read answer || return 1 case "$answer" in y*|Y*) return 0 ;;git-mergetool.sh +1
-1@@ -396,7 +396,7 @@ done prompt_after_failed_merge () { while true do - printf "Continue merging other unresolved paths (y/n) ? " + printf "Continue merging other unresolved paths [y/n]? " read ans || return 1 case "$ans" in [yY]*)Signature · 1 line
2.4.11
Quoted 3 lines · John Keeping
I think the case in these two is correct as-is. The "Y" is capitalised because it is the default and will take effect if the user just presses ENTER.
Thanks John, I'm aware of that. That's why the patch doesn't change the case. Maybe I should have mention that explicitly in the commit message.
John Keeping
Reviewed-byQuoted 8 lines · Nikola Forrrr
Quoted 3 lines · John Keeping
I think the case in these two is correct as-is. The "Y" is capitalised because it is the default and will take effect if the user just presses ENTER.
Thanks John, I'm aware of that. That's why the patch doesn't change the case. Maybe I should have mention that explicitly in the commit message.
Sorry, I completely missed that. Your patch does in fact look good, so:
I think I was taken in by the commit message saying 'i.e. "Question [y/n]? "' and didn't examine the patch carefully enough. It might be better just to drop the example since it's obvious what the patch does.
David Aguilar
Acked-byQuoted 10 lines · John Keeping
Quoted 8 lines · Nikola Forrrr
Quoted 3 lines · John Keeping
I think the case in these two is correct as-is. The "Y" is capitalised because it is the default and will take effect if the user just presses ENTER.
Thanks John, I'm aware of that. That's why the patch doesn't change the case. Maybe I should have mention that explicitly in the commit message.
Quoted 7 lines
Sorry, I completely missed that. Your patch does in fact look good, so:
Reviewed-by John Keeping <[email protected]>I think I was taken in by the commit message saying 'i.e. "Question [y/n]? "' and didn't examine the patch carefully enough. It might be better just to drop the example since it's obvious what the patch does.
Thanks for reviewing.
cheers,
Signature · 1 line
David
Quoted 9 lines · Nikola Forró
Every yes/no question in difftool/mergetool scripts has slightly different form, and none of them is consistent with the form git itself uses.
Make the form of all the questions consistent with the form used by git, i.e. "Question [y/n]? ".
Signed-off-by Nikola Forró <[email protected]>---
Looks good from a cursory read. David?
Quoted 48 lines
git-difftool--helper.sh | 4 ++-- git-mergetool--lib.sh | 2 +- git-mergetool.sh | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-)git-difftool--helper.sh +2
-2@@ -44,10 +44,10 @@ launch_merge_tool () { "$GIT_DIFF_PATH_TOTAL" "$MERGED" if use_ext_cmd then - printf "Launch '%s' [Y/n]: " \ + printf "Launch '%s' [Y/n]? " \ "$GIT_DIFFTOOL_EXTCMD" else - printf "Launch '%s' [Y/n]: " "$merge_tool" + printf "Launch '%s' [Y/n]? " "$merge_tool" fi read ans || return if test "$ans" = ngit-mergetool--lib.sh +1
-1@@ -100,7 +100,7 @@ check_unchanged () { while true do echo "$MERGED seems unchanged." - printf "Was the merge successful? [y/n] " + printf "Was the merge successful [y/n]? " read answer || return 1 case "$answer" in y*|Y*) return 0 ;;git-mergetool.sh +1
-1@@ -396,7 +396,7 @@ done prompt_after_failed_merge () { while true do - printf "Continue merging other unresolved paths (y/n) ? " + printf "Continue merging other unresolved paths [y/n]? " read ans || return 1 case "$ans" in [yY]*)