* [PATCH] difftool/mergetool: make the form of yes/no questions consistent
@ 2016-04-12 12:59 Nikola Forró
2016-04-12 13:27 ` John Keeping
2016-04-12 16:15 ` Junio C Hamano
0 siblings, 2 replies; 6+ messages in thread
From: Nikola Forró @ 2016-04-12 12:59 UTC (permalink / raw)
To: git; +Cc: gitster
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"
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] difftool/mergetool: make the form of yes/no questions consistent
2016-04-12 12:59 [PATCH] difftool/mergetool: make the form of yes/no questions consistent Nikola Forró
@ 2016-04-12 13:27 ` John Keeping
2016-04-12 13:53 ` Nikola Forró
2016-04-12 16:15 ` Junio C Hamano
1 sibling, 1 reply; 6+ messages in thread
From: John Keeping @ 2016-04-12 13:27 UTC (permalink / raw)
To: Nikola Forrrr; +Cc: git, gitster
On Tue, Apr 12, 2016 at 02:59:42PM +0200, Nikola Forrrr wrote:
> 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.
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] difftool/mergetool: make the form of yes/no questions consistent
2016-04-12 13:27 ` John Keeping
@ 2016-04-12 13:53 ` Nikola Forró
2016-04-12 14:01 ` John Keeping
0 siblings, 1 reply; 6+ messages in thread
From: Nikola Forró @ 2016-04-12 13:53 UTC (permalink / raw)
To: John Keeping; +Cc: git, gitster
On Tue, 2016-04-12 at 14:27 +0100, John Keeping wrote:
> 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] difftool/mergetool: make the form of yes/no questions consistent
2016-04-12 13:53 ` Nikola Forró
@ 2016-04-12 14:01 ` John Keeping
2016-04-12 18:05 ` David Aguilar
0 siblings, 1 reply; 6+ messages in thread
From: John Keeping @ 2016-04-12 14:01 UTC (permalink / raw)
To: Nikola Forrrr; +Cc: git, gitster
On Tue, Apr 12, 2016 at 03:53:12PM +0200, Nikola Forrrr wrote:
> On Tue, 2016-04-12 at 14:27 +0100, John Keeping wrote:
> > 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:
Reviewed-by: John Keeping <john@keeping.me.uk>
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] difftool/mergetool: make the form of yes/no questions consistent
2016-04-12 14:01 ` John Keeping
@ 2016-04-12 18:05 ` David Aguilar
0 siblings, 0 replies; 6+ messages in thread
From: David Aguilar @ 2016-04-12 18:05 UTC (permalink / raw)
To: John Keeping; +Cc: Nikola Forrrr, git, gitster
On Tue, Apr 12, 2016 at 03:01:19PM +0100, John Keeping wrote:
> On Tue, Apr 12, 2016 at 03:53:12PM +0200, Nikola Forrrr wrote:
> > On Tue, 2016-04-12 at 14:27 +0100, John Keeping wrote:
> > > 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:
>
> Reviewed-by: John Keeping <john@keeping.me.uk>
>
> 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.
Acked-by: David Aguilar <davvid@gmail.com>
cheers,
--
David
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] difftool/mergetool: make the form of yes/no questions consistent
2016-04-12 12:59 [PATCH] difftool/mergetool: make the form of yes/no questions consistent Nikola Forró
2016-04-12 13:27 ` John Keeping
@ 2016-04-12 16:15 ` Junio C Hamano
1 sibling, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2016-04-12 16:15 UTC (permalink / raw)
To: Nikola Forró; +Cc: git, David Aguilar
Nikola Forró <nforro@redhat.com> writes:
> 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>
> ---
Looks good from a cursory read. David?
> 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"
> 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]*)
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-04-12 18:05 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-04-12 12:59 [PATCH] difftool/mergetool: make the form of yes/no questions consistent Nikola Forró
2016-04-12 13:27 ` John Keeping
2016-04-12 13:53 ` Nikola Forró
2016-04-12 14:01 ` John Keeping
2016-04-12 18:05 ` David Aguilar
2016-04-12 16:15 ` Junio C Hamano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).