Re: [BUG] "git stash -- path" reports wrong unstaged changes
From: Thomas Gummerer <hidden>
Date: 2017-03-18 18:43:30
On 03/17, Jeff King wrote:
I used "git stash -- path" for the first time today and happened to notice an oddity. If I run: git init -q repo cd repo for i in one two; do echo content >$i git add $i done git commit -qm base for i in one two; do echo change >$i done git stash -- one it says: Saved working directory and index state WIP on master: 20cfadf base Unstaged changes after reset: M one M two Even though "one" no longer has unstaged changes.
Yeah, this is clearly not right. Thanks for catching this before it got into any release.
quoted hunk ↗ jump to hunk
If I run with GIT_TRACE=1, that message is generated by: git reset -- one which makes sense. At that stage we've just reset the index, but the working tree file still has modifications. In the non-pathspec case we run "git reset --hard", which takes care of the index and the working tree. It's really "checkout-index" that cleans the working tree, but it doesn't have porcelain finery like an "Unstaged changes" message. I think the patch below would fix it, but I wonder if we can do it in a way that doesn't involve calling diff-files twice. -Peff ---diff --git a/git-stash.sh b/git-stash.sh index 9c70662cc..9a4bb503a 100755 --- a/git-stash.sh +++ b/git-stash.sh@@ -299,10 +299,15 @@ 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 -- "$@" + if test -z "$GIT_QUIET" && ! git diff-files --quiet + then + say "$(gettext "Unstaged changes after reset:")" + git diff-files --name-status + fi else git reset --hard ${GIT_QUIET:+-q} fi
This would mean the user gets something like in your case above:
Unstaged changes after reset:
M two
As a user that doesn't know the internal implementation of push_stash,
this would make me wonder why git stash would mention a file that is
not provided as pathspec, but not the one that was provided in the
pathspec argument.
I think one option would be to to just keep quiet about the exact
changes that git stash push makes, similar to what we do in the
--include-untracked and in the -p case. The other option would be to
find the files that are affected and print them, but that would
probably be a bit too noisy especially in cases such as
git stash push -- docs/*.
Also from reading the code in the -p case, when --keep-index is given,
the git reset there doesn't respect $GIT_QUIET at all, and also
doesn't respect the pathspec argument, which seems like another bug.
I can submit a patch series for those, but I won't get to it before
tomorrow :)