From: Junio C Hamano Date: Fri, 07 Oct 2011 20:09:11 GMT Subject: Re: [PATCH] git-difftool: allow skipping file by typing 'n' at prompt Message-ID: <7vwrcgtvh4.fsf@alter.siamese.dyndns.org> In-Reply-To: <20111006181522.GA2936@sita-lt.atc.tcs.com> Sitaram Chamarty writes: > This is useful if you forgot to restrict the diff to the paths you want > to see, or selecting precisely the ones you want is too much typing. > > Signed-off-by: Sitaram Chamarty > --- > > On Thu, Oct 06, 2011 at 10:36:40AM -0700, Junio C Hamano wrote: > >> Thanks. It is clear from the subject and the patch text that you are >> changing "hit return to unconditionally launch" into "launch it if you >> want to", but can you give justification why a choice not to launch is >> needed in the log message? > > OK; done. Looks OK from a cursory viewing. Do we want some additional tests? For that matter, have you run the test suite with this patch applied (I haven't)? > git-difftool--helper.sh | 9 +++++---- > 1 files changed, 5 insertions(+), 4 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 > }