Re: [PATCH] difftool/mergetool: make the form of yes/no questions consistent
- From
John Keeping <john@keeping.me.uk>
- Date
- Apr 12, 2016, 13:27 UTC
- Message-ID
- <20160412132737.GB11013@serenity.lan>
- In-Reply-To
- <1460465982.3214.7.camel@redhat.com>
On Tue, Apr 12, 2016 at 02:59:42PM +0200, Nikola Forrrr wrote:
Show 28 quoted lines
> 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ó <nforro@redhat.com>
> ---
> git-difftool--helper.sh | 4 ++--
> git-mergetool--lib.sh | 2 +-
> git-mergetool.sh | 2 +-
> 3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh
> index 2b11b1d..84d6cc0 100755
> --- a/git-difftool--helper.sh
> +++ b/git-difftool--helper.sh
> @@ -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.
Show 31 quoted lines
> fi
> read ans || return
> if test "$ans" = n
> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> index 54ac8e4..92adcc0 100644
> --- a/git-mergetool--lib.sh
> +++ b/git-mergetool--lib.sh
> @@ -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 ;;
> diff --git a/git-mergetool.sh b/git-mergetool.sh
> index 9f77e3a..2e0635a 100755
> --- a/git-mergetool.sh
> +++ b/git-mergetool.sh
> @@ -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]*)
> --
> 2.4.11