Re: [PATCH v2 3/3] stash: keep untracked files intact in stash -k

On 03/21, Jeff King wrote:

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

quoted

Currently when there are untracked changes in a file "one" and in a file
"two" in the repository and the user uses:

    git stash push -k one

all changes in "two" are wiped out completely.  That is clearly not the
intended result.  Make sure that only the files given in the pathspec
are changed when git stash push -k <pathspec> is used.

Good description.

I basically just tweaked your report here :)  thanks for that!

quoted

diff --git a/git-stash.sh b/git-stash.sh
index 13711764a9..2fb651b2b8 100755
--- a/git-stash.sh
+++ b/git-stash.sh

@@ -314,7 +314,9 @@ push_stash () {
 
 		if test "$keep_index" = "t" && test -n "$i_tree"
 		then
-			git read-tree --reset -u $i_tree
+			git read-tree --reset $i_tree
+			git ls-files -z --modified -- "$@" |
+			git checkout-index -z --force --stdin
 		fi

I briefly wondered if this needed "-q" to match the earlier commit, but
"checkout-index" isn't really chatty, so I don't think so (and the
earlier checkout-index doesn't have it either).

Yep, I had the same thoughts here.

I also wondered if this could be done in a single command as:

  git reset -q --hard $i_tree -- "$@"

But "git reset" can't handle pathspecs with "--hard" (which is why the
similar case a few lines above uses the same commands).

Yeah unfortunately, that would have made the previous patch series
quite a bit easier :)  But here it wouldn't help anyway, as git reset
without pathspecs can't handle a tree-ish either, right?  And we also
want to handle the case where no pathspecs are given to git stash.

So this looks good to me.

quoted

+test_expect_success 'stash -k -- <pathspec> leaves unstaged files intact' '
+	git reset &&
+	>foo &&
+	>bar &&
+	git add foo bar &&
+	git commit -m "test" &&
+	echo "foo" >foo &&
+	echo "bar" >bar &&
+	git stash -k -- foo &&
+	test "",bar = $(cat foo),$(cat bar) &&
+	git stash pop &&
+	test foo,bar = $(cat foo),$(cat bar)
+'

I always get nervous when I see test arguments without quotes, but I
think this is fine (and I couldn't see a shorter way of doing it with
test_cmp).

Hmm yeah I basically copied this from a different test and tweaked it
a bit to fit here.

-Peff