Re: [PATCH/RFC 3/3] stash: pass the pathspec argument to git reset

[Jeff King <peff@peff.net>]

On Sun, Mar 19, 2017 at 08:23:51PM +0000, Thomas Gummerer wrote:

> For "git stash -p --no-keep-index", the pathspec argument is currently
> not passed to "git reset".  This means that changed that are staged but
> that are excluded from the pathspec still get unstaged by git stash -p.

Yeah, I noticed this while playing around with patch 2. This seems
like an improvement to me. Unlike the other patches (which are just
tweaking quietness), I think this one really needs a test.

Also, s/changed/changes/ above.

> ---
> So this make things a bit inconsistent in for example the following
> situation:
> 
>     $ git status -s
>      M git-stash.sh
>     M  read-cache.c
> 
> where using git stash -p --no-keep-index, when only changes in
> git-stash.sh are added to the stash would reset read-cache.c, but with
> git stash -p --no-keep-index -- git-stash.sh, read-cache.c would not
> be reset.

I think it's OK. You can't select (or not select) changes from the index
anyway. TBH, I kind of wonder if "git stash -p --no-keep-index" makes
any sense at all.

> However it does bring things more in line with git stash --
> <pathspec>, so I think this is an improvement.

I did notice one other case while looking at this that I think is much
more serious. The "read-tree" call in the non-patch-mode case doesn't
use a pathspec either. So if we have our same setup where "one" and
"two" have unstaged changes and we do:

  git stash push -k one

Then we stash "one", but the change in "two" is wiped out completely!

I don't think read-tree takes pathspecs, so it would have to drop the
"-u" and replace it with checkout-index. I'm confused about why we would
need it in the first place, though. We should have already dealt with
the working tree files in the earlier block, so there should be nothing
to checkout.

The "-u" goes all the way back to 7bedebcaa (stash: introduce 'stash
save --keep-index' option, 2008-06-27). I wonder if it has always been
unnecessary, but we never noticed because it's just a noop.

-Peff