Re: [PATCH v2 1/3] stash: don't show internal implementation details

[Thomas Gummerer <t.gummerer@gmail.com>]

On 03/21, Jeff King wrote:
> 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 <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.

Yeah, they felt so similar now that splitting this up in different
patches would be a bit too noisy.

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

Right thanks, I'll keep that in mind for another time. (I guess just
changing this doesn't warrant a re-roll?)

> -peff