On Tue, Mar 21, 2017 at 10:12:17PM +0000, Thomas Gummerer wrote:

> 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 <[email protected]>
> Signed-off-by: Thomas Gummerer <[email protected]>

I think combining these is fine. The incorrect output with pathspecs
isn't mentioned anymore, but I think that's OK.

> 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.

> 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

Reply via email to