Re: [PATCH v3 5/5] stash: teach 'push' (and 'create') to honor pathspec

[Thomas Gummerer]

On 02/05, Junio C Hamano wrote:
> Thomas Gummerer  writes:
> 
> > @@ -72,6 +72,11 @@ create_stash () {
> > untracked="$1"
> > new_style=t
> > ;;
> > +   --)
> > +   shift
> > +   new_style=t
> > +   break
> > +   ;;
> > *)
> > if test -n "$new_style"
> > then
> > @@ -99,6 +104,10 @@ create_stash () {
> > if test -z "$new_style"
> > then
> > stash_msg="$*"
> > +   while test $# != 0
> > +   do
> > +   shift
> > +   done
> > fi
> 
> The intent is correct, but I would probaly empty the "$@" not with
> an iteration of shifts but with a single
> 
>   set x && shift
> 
> ;-)

Thanks, I knew there had to be a better way, but I was unable to find
it.  I think I like Andreas suggestion of "shift $#" the best here.

> > @@ -134,7 +143,7 @@ create_stash () {
> > # Untracked files are stored by themselves in a parentless 
> > commit, for
> > # ease of unpacking later.
> > u_commit=$(
> > -   untracked_files | (
> > +   untracked_files "$@" | (
> 
> The above (and all other uses of "$@" was exactly what I had in mind
> when I gave you the "can we leave the '$@' the user gave us as-is?"
> comment during the review of the previous round.  
> 
> Looks much nicer.
> 
> > +test_expect_success 'stash push with $IFS character' '
> > +   >"foo   bar" &&
> 
> IIRC, a pathname with HT in it cannot be tested on MINGW.  For the
> purpose of this test, I think it is sufficient to use SP instead of
> HT here (and matching change below in the expectation, of course).

Will change.

> > +   >foo &&
> > +   >bar &&
> > +   git add foo* &&
> > +   git stash push --include-untracked -- foo* &&
> 
> While it is good that you are testing "foo*", you would also want
> another test to cover a pathspec with $IFS in it.  That is the case
> the code in the previous round had problems with.  Perhaps try
> something like
> 
>   >foo && >bar && >"foo bar" && git add . &&
>   echo modify foo >foo &&
>   echo modify bar >bar &&
>   echo modify "foo bar" >"foo bar" &&
>   git stash push -- "foo b*"
> 
> which should result in the changes to "foo bar" in the resulting
> stash, while changes to "foo" and "bar" are not (and left in the
> working tree).  And make sure that is what happens?  I think with
> the code from the previous round, the above would instead stash
> changes to "foo" and "bar" without catching changes to "foo bar",
> because the single pathspec "foo b*" would have been mistakenly
> split into two, i.e. "foo" and "b*", and failed to match "foo bar"
> while matching the other two.

Thanks, I'll add a test for that.

Re: [PATCH v3 5/5] stash: teach 'push' (and 'create') to honor pathspec

[Jeff King]

On Sun, Feb 05, 2017 at 11:09:14PM -0800, Junio C Hamano wrote:

> > @@ -99,6 +104,10 @@ create_stash () {
> > if test -z "$new_style"
> > then
> > stash_msg="$*"
> > +   while test $# != 0
> > +   do
> > +   shift
> > +   done
> > fi
> 
> The intent is correct, but I would probaly empty the "$@" not with
> an iteration of shifts but with a single
> 
>   set x && shift
> 
> ;-)

Perhaps it is not portable, but I think "set --" does the same thing and
is perhaps less obscure.

-Peff

[PATCH v3 5/5] stash: teach 'push' (and 'create') to honor pathspec

[Thomas Gummerer]

While working on a repository, it's often helpful to stash the changes
of a single or multiple files, and leave others alone.  Unfortunately
git currently offers no such option.  git stash -p can be used to work
around this, but it's often impractical when there are a lot of changes
over multiple files.

Allow 'git stash push' to take pathspec to specify which paths to stash.

Helped-by: Junio C Hamano 
Signed-off-by: Thomas Gummerer 
---
 Documentation/git-stash.txt| 11 ++
 git-stash.sh   | 30 ---
 t/t3903-stash.sh   | 42 ++
 t/t3905-stash-include-untracked.sh | 26 +++
 4 files changed, 102 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index a138ed6a24..be55e456fa 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -15,9 +15,13 @@ SYNOPSIS
 'git stash' branch  []
 'git stash' [save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
 [-u|--include-untracked] [-a|--all] []]
+'git stash' push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
+[-u|--include-untracked] [-a|--all] [-m|--message ]]
+[--] [...]
 'git stash' clear
 'git stash' create []
 'git stash' create [-m ] [-u|--include-untracked ]
+[-- ...]
 'git stash' store [-m|--message ] [-q|--quiet] 
 
 DESCRIPTION
@@ -47,6 +51,7 @@ OPTIONS
 ---
 
 save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] 
[-q|--quiet] []::
+push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] 
[-q|--quiet] [-m|--message ] [--] [...]::
 
Save your local modifications to a new 'stash' and roll them
back to HEAD (in the working tree and in the index).
@@ -56,6 +61,12 @@ save [-p|--patch] [-k|--[no-]keep-index] 
[-u|--include-untracked] [-a|--all] [-q
only  does not trigger this action to prevent a misspelled
subcommand from making an unwanted stash.
 +
+When pathspec is given to 'git stash push', the new stash records the
+modified states only for the files that match the pathspec.  The index
+entries and working tree files are then rolled back to the state in
+HEAD only for these files, too, leaving files that do not match the
+pathspec intact.
++
 If the `--keep-index` option is used, all changes already added to the
 index are left intact.
 +
diff --git a/git-stash.sh b/git-stash.sh
index 33b2d0384c..46367d40aa 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -41,7 +41,7 @@ no_changes () {
 untracked_files () {
excl_opt=--exclude-standard
test "$untracked" = "all" && excl_opt=
-   git ls-files -o -z $excl_opt
+   git ls-files -o -z $excl_opt -- "$@"
 }
 
 clear_stash () {
@@ -72,6 +72,11 @@ create_stash () {
untracked="$1"
new_style=t
;;
+   --)
+   shift
+   new_style=t
+   break
+   ;;
*)
if test -n "$new_style"
then
@@ -99,6 +104,10 @@ create_stash () {
if test -z "$new_style"
then
stash_msg="$*"
+   while test $# != 0
+   do
+   shift
+   done
fi
 
git update-index -q --refresh
@@ -134,7 +143,7 @@ create_stash () {
# Untracked files are stored by themselves in a parentless 
commit, for
# ease of unpacking later.
u_commit=$(
-   untracked_files | (
+   untracked_files "$@" | (
GIT_INDEX_FILE="$TMPindex" &&
export GIT_INDEX_FILE &&
rm -f "$TMPindex" &&
@@ -157,7 +166,7 @@ create_stash () {
git read-tree --index-output="$TMPindex" -m $i_tree &&
GIT_INDEX_FILE="$TMPindex" &&
export GIT_INDEX_FILE &&
-   git diff-index --name-only -z HEAD -- 
>"$TMP-stagenames" &&
+   git diff-index --name-only -z HEAD -- "$@" 
>"$TMP-stagenames" &&
git update-index -z --add --remove --stdin 
<"$TMP-stagenames" &&
git write-tree &&
rm -f "$TMPindex"
@@ -171,7 +180,7 @@ create_stash () {
 
# find out what the user wants
GIT_INDEX_FILE="$TMP-index" \
-   git add--interactive --patch=stash -- &&
+   git add--interactive --patch=stash -- "$@" &&
 
# state of the working tree
w_tree=$(GIT_INDEX_FILE="$TMP-index" git write-tree) ||
@@ -307,18 +316,25 @@ push_stash () {
git reflog