Re: [PATCH] git-difftool: allow skipping file by typing 'n' at prompt
From: Charles Bailey <hidden>
Date: 2016-06-15 22:52:13
Subsystem:
the rest · Maintainer:
Linus Torvalds
Possibly related (same subject, not in this thread)
- 2016-06-15 · [PATCH] git-difftool: allow skipping file by typing 'n' at prompt · Sitaram Chamarty <hidden>
- 2016-06-15 · Re: [PATCH] git-difftool: allow skipping file by typing 'n' at prompt · Junio C Hamano <hidden>
- 2016-06-15 · [PATCH] git-difftool: allow skipping file by typing 'n' at prompt · Sitaram Chamarty <hidden>
- 2016-06-15 · [PATCH] git-difftool: allow skipping file by typing 'n' at prompt · Sitaram Chamarty <hidden>
- 2016-06-15 · Re: [PATCH] git-difftool: allow skipping file by typing 'n' at prompt · Junio C Hamano <hidden>
On Sat, Oct 08, 2011 at 06:40:15PM +0530, Sitaram Chamarty wrote:
quoted hunk ↗ jump to hunk
git-difftool--helper.sh | 9 +++++---- t/t7800-difftool.sh | 44 +++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 48 insertions(+), 5 deletions(-)diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh index 8452890..0468446 100755 --- a/git-difftool--helper.sh +++ b/git-difftool--helper.sh@@ -38,15 +38,16 @@ launch_merge_tool () { # $LOCAL and $REMOTE are temporary files so prompt # the user with the real $MERGED name before launching $merge_tool. + ans=y if should_prompt then printf "\nViewing: '$MERGED'\n" if use_ext_cmd then - printf "Hit return to launch '%s': " \ + printf "Launch '%s' [Y/n]: " \ "$GIT_DIFFTOOL_EXTCMD" else - printf "Hit return to launch '%s': " "$merge_tool" + printf "Launch '%s' [Y/n]: " "$merge_tool" fi read ans fi@@ -54,9 +55,9 @@ launch_merge_tool () { if use_ext_cmd then export BASE - eval $GIT_DIFFTOOL_EXTCMD '"$LOCAL"' '"$REMOTE"' + test "$ans" != "n" && eval $GIT_DIFFTOOL_EXTCMD '"$LOCAL"' '"$REMOTE"' else - run_merge_tool "$merge_tool" + test "$ans" != "n" && run_merge_tool "$merge_tool" fi }
It's a minor point but for me, this looks a little more difficult to follow than it needs to be. Why do we need to hold on to 'ans' for so long? With the new prompt, if we ever 'read ans' we always want to return from the launch_merge_tool without doing anything else if we read "n". I think it's easier to follow if we just change 'read ans' and leave the 'if use_ext_cmd' clauses alone. Perhaps some people don't like the early return, though? Charles. E.g. (for discussion, untested):
diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh
index 8452890..b668a12 100755
--- a/git-difftool--helper.sh
+++ b/git-difftool--helper.sh@@ -43,12 +43,16 @@ launch_merge_tool () { printf "\nViewing: '$MERGED'\n" if use_ext_cmd then - printf "Hit return to launch '%s': " \ + printf "Launch '%s' [Y/n]: " \ "$GIT_DIFFTOOL_EXTCMD" else - printf "Hit return to launch '%s': " "$merge_tool" + printf "Launch '%s' [Y/n]: " "$merge_tool" + fi + + if read ans && test "$ans" = "n" + then + return fi - read ans fi if use_ext_cmd