Re: [PATCH v2 1/3] stash: don't show internal implementation details
- From
Jeff King <peff@peff.net>
- Date
- Mar 21, 2017, 22:14 UTC
- Message-ID
- <20170321221445.3jpaoce7qbshtdoq@sigill.intra.peff.net>
- In-Reply-To
- <20170321221219.28041-2-t.gummerer@gmail.com>
On Tue, Mar 21, 2017 at 10:12:17PM +0000, Thomas Gummerer wrote:
Show 12 quoted lines
> git stash push uses other git commands internally. Currently it only > passes the -q flag to those if the -q flag is passed to git stash. when > using 'git stash push -p -q --no-keep-index', it doesn't even pass the > flag on to the internal reset at all. > > It really is enough for the user to know that the stash is created, > without bothering them with the internal details of what's happening. > Always pass the -q flag to the internal git clean and git reset > commands, to avoid unnecessary and potentially confusing output. > > Reported-by: Jeff King <peff@peff.net> > Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
I think combining these is fine. The incorrect output with pathspecs isn't mentioned anymore, but I think that's OK.
Show 29 quoted lines
> diff --git a/git-stash.sh b/git-stash.sh
> index 9c70662cc8..ba86d84321 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -299,12 +299,12 @@ push_stash () {
> then
> if test $# != 0
> then
> - git reset ${GIT_QUIET:+-q} -- "$@"
> + git reset -q -- "$@"
> git ls-files -z --modified -- "$@" |
> git checkout-index -z --force --stdin
> - git clean --force ${GIT_QUIET:+-q} -d -- "$@"
> + git clean --force -q -d -- "$@"
> else
> - git reset --hard ${GIT_QUIET:+-q}
> + git reset --hard -q
> fi
> test "$untracked" = "all" && CLEAN_X_OPTION=-x || CLEAN_X_OPTION=
> if test -n "$untracked"
> @@ -322,7 +322,7 @@ push_stash () {
>
> if test "$keep_index" != "t"
> then
> - git reset
> + git reset -q
> fi
> fi
> }These all look fine.
Show 12 quoted lines
> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh > index 89877e4b52..ea8e5c7818 100755 > --- a/t/t3903-stash.sh > +++ b/t/t3903-stash.sh > @@ -663,7 +663,7 @@ test_expect_success 'stash apply shows status same as git status (relative to cu > sane_unset GIT_MERGE_VERBOSITY && > git stash apply > ) | > - sed -e 1,2d >actual && # drop "Saved..." and "HEAD is now..." > + sed -e 1,1d >actual && # drop "Saved..." > test_i18ncmp expect actual > '
This too, though I think "1d" would be the more usual way to say it.
-peff