* [PATCH v6 1/6] stash: introduce push verb
2017-02-19 11:03 ` [PATCH v6 " Thomas Gummerer
@ 2017-02-19 11:03 ` Thomas Gummerer
2017-02-19 11:03 ` [PATCH v6 2/6] stash: add test for the create command line arguments Thomas Gummerer
` (8 subsequent siblings)
9 siblings, 0 replies; 61+ messages in thread
From: Thomas Gummerer @ 2017-02-19 11:03 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Johannes Schindelin,
Øyvind A. Holm, Jakub Narębski, Matthieu Moy,
Thomas Gummerer
Introduce a new git stash push verb in addition to git stash save. The
push verb is used to transition from the current command line arguments
to a more conventional way, in which the message is given as an argument
to the -m option.
This allows us to have pathspecs at the end of the command line
arguments like other Git commands do, so that the user can say which
subset of paths to stash (and leave others behind).
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
Documentation/git-stash.txt | 3 +++
git-stash.sh | 46 ++++++++++++++++++++++++++++++++++++++++++---
t/t3903-stash.sh | 9 +++++++++
3 files changed, 55 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 2e9cef06e6..0f602ea0c8 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -15,6 +15,8 @@ SYNOPSIS
'git stash' branch <branchname> [<stash>]
'git stash' [save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
[-u|--include-untracked] [-a|--all] [<message>]]
+'git stash' push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
+ [-u|--include-untracked] [-a|--all] [-m|--message <message>]]
'git stash' clear
'git stash' create [<message>]
'git stash' store [-m|--message <message>] [-q|--quiet] <commit>
@@ -46,6 +48,7 @@ OPTIONS
-------
save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [<message>]::
+push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [-m|--message <message>]::
Save your local modifications to a new 'stash', and run `git reset
--hard` to revert them. The <message> part is optional and gives
diff --git a/git-stash.sh b/git-stash.sh
index 10c284d1aa..8365ebba2a 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -9,6 +9,8 @@ USAGE="list [<options>]
or: $dashless branch <branchname> [<stash>]
or: $dashless [save [--patch] [-k|--[no-]keep-index] [-q|--quiet]
[-u|--include-untracked] [-a|--all] [<message>]]
+ or: $dashless push [--patch] [-k|--[no-]keep-index] [-q|--quiet]
+ [-u|--include-untracked] [-a|--all] [-m <message>]
or: $dashless clear"
SUBDIRECTORY_OK=Yes
@@ -189,10 +191,11 @@ store_stash () {
return $ret
}
-save_stash () {
+push_stash () {
keep_index=
patch_mode=
untracked=
+ stash_msg=
while test $# != 0
do
case "$1" in
@@ -216,6 +219,11 @@ save_stash () {
-a|--all)
untracked=all
;;
+ -m|--message)
+ shift
+ test -z ${1+x} && usage
+ stash_msg=$1
+ ;;
--help)
show_help
;;
@@ -251,8 +259,6 @@ save_stash () {
die "$(gettext "Can't use --patch and --include-untracked or --all at the same time")"
fi
- stash_msg="$*"
-
git update-index -q --refresh
if no_changes
then
@@ -291,6 +297,36 @@ save_stash () {
fi
}
+save_stash () {
+ push_options=
+ while test $# != 0
+ do
+ case "$1" in
+ --)
+ shift
+ break
+ ;;
+ -*)
+ # pass all options through to push_stash
+ push_options="$push_options $1"
+ ;;
+ *)
+ break
+ ;;
+ esac
+ shift
+ done
+
+ stash_msg="$*"
+
+ if test -z "$stash_msg"
+ then
+ push_stash $push_options
+ else
+ push_stash $push_options -m "$stash_msg"
+ fi
+}
+
have_stash () {
git rev-parse --verify --quiet $ref_stash >/dev/null
}
@@ -617,6 +653,10 @@ save)
shift
save_stash "$@"
;;
+push)
+ shift
+ push_stash "$@"
+ ;;
apply)
shift
apply_stash "$@"
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 2de3e18ce6..3577115807 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -775,4 +775,13 @@ test_expect_success 'stash is not confused by partial renames' '
test_path_is_missing file
'
+test_expect_success 'push -m shows right message' '
+ >foo &&
+ git add foo &&
+ git stash push -m "test message" &&
+ echo "stash@{0}: On master: test message" >expect &&
+ git stash list -1 >actual &&
+ test_cmp expect actual
+'
+
test_done
--
2.12.0.rc2.399.g0ca89a282
^ permalink raw reply related [flat|nested] 61+ messages in thread
* [PATCH v6 2/6] stash: add test for the create command line arguments
2017-02-19 11:03 ` [PATCH v6 " Thomas Gummerer
2017-02-19 11:03 ` [PATCH v6 1/6] stash: introduce push verb Thomas Gummerer
@ 2017-02-19 11:03 ` Thomas Gummerer
2017-02-19 11:03 ` [PATCH v6 3/6] stash: refactor stash_create Thomas Gummerer
` (7 subsequent siblings)
9 siblings, 0 replies; 61+ messages in thread
From: Thomas Gummerer @ 2017-02-19 11:03 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Johannes Schindelin,
Øyvind A. Holm, Jakub Narębski, Matthieu Moy,
Thomas Gummerer
Currently there is no test showing the expected behaviour of git stash
create's command line arguments. Add a test for that to show the
current expected behaviour and to make sure future refactorings don't
break those expectations.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
t/t3903-stash.sh | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 3577115807..ffe3549ea5 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -784,4 +784,22 @@ test_expect_success 'push -m shows right message' '
test_cmp expect actual
'
+test_expect_success 'create stores correct message' '
+ >foo &&
+ git add foo &&
+ STASH_ID=$(git stash create "create test message") &&
+ echo "On master: create test message" >expect &&
+ git show --pretty=%s -s ${STASH_ID} >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'create with multiple arguments for the message' '
+ >foo &&
+ git add foo &&
+ STASH_ID=$(git stash create test untracked) &&
+ echo "On master: test untracked" >expect &&
+ git show --pretty=%s -s ${STASH_ID} >actual &&
+ test_cmp expect actual
+'
+
test_done
--
2.12.0.rc2.399.g0ca89a282
^ permalink raw reply related [flat|nested] 61+ messages in thread
* [PATCH v6 3/6] stash: refactor stash_create
2017-02-19 11:03 ` [PATCH v6 " Thomas Gummerer
2017-02-19 11:03 ` [PATCH v6 1/6] stash: introduce push verb Thomas Gummerer
2017-02-19 11:03 ` [PATCH v6 2/6] stash: add test for the create command line arguments Thomas Gummerer
@ 2017-02-19 11:03 ` Thomas Gummerer
2017-02-19 11:03 ` [PATCH v6 4/6] stash: teach 'push' (and 'create_stash') to honor pathspec Thomas Gummerer
` (6 subsequent siblings)
9 siblings, 0 replies; 61+ messages in thread
From: Thomas Gummerer @ 2017-02-19 11:03 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Johannes Schindelin,
Øyvind A. Holm, Jakub Narębski, Matthieu Moy,
Thomas Gummerer
Refactor the internal stash_create function to use a -m flag for
specifying the message and -u flag to indicate whether untracked files
should be added to the stash.
This makes it easier to pass a pathspec argument to stash_create in the
next patch.
The user interface for git stash create stays the same.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
git-stash.sh | 22 ++++++++++++++++++----
1 file changed, 18 insertions(+), 4 deletions(-)
diff --git a/git-stash.sh b/git-stash.sh
index 8365ebba2a..ef5d1b45be 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -58,8 +58,22 @@ clear_stash () {
}
create_stash () {
- stash_msg="$1"
- untracked="$2"
+ stash_msg=
+ untracked=
+ while test $# != 0
+ do
+ case "$1" in
+ -m|--message)
+ shift
+ stash_msg=${1?"BUG: create_stash () -m requires an argument"}
+ ;;
+ -u|--include-untracked)
+ shift
+ untracked=${1?"BUG: create_stash () -u requires an argument"}
+ ;;
+ esac
+ shift
+ done
git update-index -q --refresh
if no_changes
@@ -268,7 +282,7 @@ push_stash () {
git reflog exists $ref_stash ||
clear_stash || die "$(gettext "Cannot initialize stash")"
- create_stash "$stash_msg" $untracked
+ create_stash -m "$stash_msg" -u "$untracked"
store_stash -m "$stash_msg" -q $w_commit ||
die "$(gettext "Cannot save the current status")"
say "$(eval_gettext "Saved working directory and index state \$stash_msg")"
@@ -667,7 +681,7 @@ clear)
;;
create)
shift
- create_stash "$*" && echo "$w_commit"
+ create_stash -m "$*" && echo "$w_commit"
;;
store)
shift
--
2.12.0.rc2.399.g0ca89a282
^ permalink raw reply related [flat|nested] 61+ messages in thread
* [PATCH v6 4/6] stash: teach 'push' (and 'create_stash') to honor pathspec
2017-02-19 11:03 ` [PATCH v6 " Thomas Gummerer
` (2 preceding siblings ...)
2017-02-19 11:03 ` [PATCH v6 3/6] stash: refactor stash_create Thomas Gummerer
@ 2017-02-19 11:03 ` Thomas Gummerer
2017-02-21 16:55 ` Junio C Hamano
2017-02-19 11:03 ` [PATCH v6 5/6] stash: use stash_push for no verb form Thomas Gummerer
` (5 subsequent siblings)
9 siblings, 1 reply; 61+ messages in thread
From: Thomas Gummerer @ 2017-02-19 11:03 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Johannes Schindelin,
Øyvind A. Holm, Jakub Narębski, Matthieu Moy,
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 <gitster@pobox.com>
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
Documentation/git-stash.txt | 9 ++++-
git-stash.sh | 48 +++++++++++++++++++------
t/t3903-stash.sh | 72 ++++++++++++++++++++++++++++++++++++++
t/t3905-stash-include-untracked.sh | 26 ++++++++++++++
4 files changed, 143 insertions(+), 12 deletions(-)
diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 0f602ea0c8..b7db939a06 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -17,6 +17,7 @@ SYNOPSIS
[-u|--include-untracked] [-a|--all] [<message>]]
'git stash' push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
[-u|--include-untracked] [-a|--all] [-m|--message <message>]]
+ [--] [<pathspec>...]
'git stash' clear
'git stash' create [<message>]
'git stash' store [-m|--message <message>] [-q|--quiet] <commit>
@@ -48,7 +49,7 @@ OPTIONS
-------
save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [<message>]::
-push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [-m|--message <message>]::
+push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [-m|--message <message>] [--] [<pathspec>...]::
Save your local modifications to a new 'stash', and run `git reset
--hard` to revert them. The <message> part is optional and gives
@@ -57,6 +58,12 @@ push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q
only <message> 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 ef5d1b45be..b55983d1fd 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -11,6 +11,7 @@ USAGE="list [<options>]
[-u|--include-untracked] [-a|--all] [<message>]]
or: $dashless push [--patch] [-k|--[no-]keep-index] [-q|--quiet]
[-u|--include-untracked] [-a|--all] [-m <message>]
+ [-- <pathspec>...]
or: $dashless clear"
SUBDIRECTORY_OK=Yes
@@ -35,15 +36,15 @@ else
fi
no_changes () {
- git diff-index --quiet --cached HEAD --ignore-submodules -- &&
- git diff-files --quiet --ignore-submodules &&
+ git diff-index --quiet --cached HEAD --ignore-submodules -- "$@" &&
+ git diff-files --quiet --ignore-submodules -- "$@" &&
(test -z "$untracked" || test -z "$(untracked_files)")
}
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 () {
@@ -71,12 +72,16 @@ create_stash () {
shift
untracked=${1?"BUG: create_stash () -u requires an argument"}
;;
+ --)
+ shift
+ break
+ ;;
esac
shift
done
git update-index -q --refresh
- if no_changes
+ if no_changes "$@"
then
exit 0
fi
@@ -108,7 +113,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" &&
@@ -131,7 +136,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"
@@ -145,7 +150,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) ||
@@ -274,7 +279,7 @@ push_stash () {
fi
git update-index -q --refresh
- if no_changes
+ if no_changes "$@"
then
say "$(gettext "No local changes to save")"
exit 0
@@ -282,18 +287,39 @@ push_stash () {
git reflog exists $ref_stash ||
clear_stash || die "$(gettext "Cannot initialize stash")"
- create_stash -m "$stash_msg" -u "$untracked"
+ create_stash -m "$stash_msg" -u "$untracked" -- "$@"
store_stash -m "$stash_msg" -q $w_commit ||
die "$(gettext "Cannot save the current status")"
say "$(eval_gettext "Saved working directory and index state \$stash_msg")"
if test -z "$patch_mode"
then
- git reset --hard ${GIT_QUIET:+-q}
+ if test $# != 0
+ then
+ saved_untracked=
+ if test -n "$(git ls-files --others -- "$@")"
+ then
+ saved_untracked=$(
+ git ls-files -z --others -- "$@" |
+ xargs -0 git stash create -u all --)
+ fi
+ git ls-files -z -- "$@" | xargs -0 git reset ${GIT_QUIET:+-q} --
+ git ls-files -z --modified -- "$@" | xargs -0 git checkout ${GIT_QUIET:+-q} HEAD --
+ if test -n "$(git ls-files -z --others -- "$@")"
+ then
+ git ls-files -z --others -- "$@" | xargs -0 git clean --force -d ${GIT_QUIET:+-q} --
+ fi
+ if test -n "$saved_untracked"
+ then
+ git stash pop -q $saved_untracked
+ fi
+ else
+ git reset --hard ${GIT_QUIET:+-q}
+ fi
test "$untracked" = "all" && CLEAN_X_OPTION=-x || CLEAN_X_OPTION=
if test -n "$untracked"
then
- git clean --force --quiet -d $CLEAN_X_OPTION
+ git clean --force --quiet -d $CLEAN_X_OPTION -- "$@"
fi
if test "$keep_index" = "t" && test -n "$i_tree"
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index ffe3549ea5..4fb800eec8 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -802,4 +802,76 @@ test_expect_success 'create with multiple arguments for the message' '
test_cmp expect actual
'
+test_expect_success 'stash -- <filename> stashes and restores the file' '
+ >foo &&
+ >bar &&
+ git add foo bar &&
+ git stash push -- foo &&
+ test_path_is_file bar &&
+ test_path_is_missing foo &&
+ git stash pop &&
+ test_path_is_file foo &&
+ test_path_is_file bar
+'
+
+test_expect_success 'stash with multiple filename arguments' '
+ >foo &&
+ >bar &&
+ >extra &&
+ git add foo bar extra &&
+ git stash push -- foo bar &&
+ test_path_is_missing bar &&
+ test_path_is_missing foo &&
+ test_path_is_file extra &&
+ git stash pop &&
+ test_path_is_file foo &&
+ test_path_is_file bar &&
+ test_path_is_file extra
+'
+
+test_expect_success 'stash with file including $IFS character' '
+ >"foo bar" &&
+ >foo &&
+ >bar &&
+ git add foo* &&
+ git stash push -- "foo b*" &&
+ test_path_is_missing "foo bar" &&
+ test_path_is_file foo &&
+ test_path_is_file bar &&
+ git stash pop &&
+ test_path_is_file "foo bar" &&
+ test_path_is_file foo &&
+ test_path_is_file bar
+'
+
+test_expect_success 'stash push -p with pathspec shows no changes only onece' '
+ >file &&
+ git add file &&
+ git stash push -p not-file >actual &&
+ echo "No local changes to save" >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success 'stash push with pathspec shows no changes when there are none' '
+ >file &&
+ git add file &&
+ git stash push not-file >actual &&
+ echo "No local changes to save" >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success 'untracked file is not removed when using pathspecs' '
+ >untracked &&
+ git stash push untracked &&
+ test_path_is_file untracked
+'
+
+test_expect_success 'untracked files are left in place when -u is not given' '
+ >file &&
+ git add file &&
+ >untracked &&
+ git stash push file &&
+ test_path_is_file untracked
+'
+
test_done
diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh
index f372fc8ca8..193adc7b68 100755
--- a/t/t3905-stash-include-untracked.sh
+++ b/t/t3905-stash-include-untracked.sh
@@ -185,4 +185,30 @@ test_expect_success 'stash save --all is stash poppable' '
test -s .gitignore
'
+test_expect_success 'stash push --include-untracked with pathspec' '
+ >foo &&
+ >bar &&
+ git stash push --include-untracked -- foo &&
+ test_path_is_file bar &&
+ test_path_is_missing foo &&
+ git stash pop &&
+ test_path_is_file bar &&
+ test_path_is_file foo
+'
+
+test_expect_success 'stash push with $IFS character' '
+ >"foo bar" &&
+ >foo &&
+ >bar &&
+ git add foo* &&
+ git stash push --include-untracked -- "foo b*" &&
+ test_path_is_missing "foo bar" &&
+ test_path_is_file foo &&
+ test_path_is_file bar &&
+ git stash pop &&
+ test_path_is_file "foo bar" &&
+ test_path_is_file foo &&
+ test_path_is_file bar
+'
+
test_done
--
2.12.0.rc2.399.g0ca89a282
^ permalink raw reply related [flat|nested] 61+ messages in thread
* Re: [PATCH v6 4/6] stash: teach 'push' (and 'create_stash') to honor pathspec
2017-02-19 11:03 ` [PATCH v6 4/6] stash: teach 'push' (and 'create_stash') to honor pathspec Thomas Gummerer
@ 2017-02-21 16:55 ` Junio C Hamano
2017-02-25 20:27 ` Thomas Gummerer
0 siblings, 1 reply; 61+ messages in thread
From: Junio C Hamano @ 2017-02-21 16:55 UTC (permalink / raw)
To: Thomas Gummerer
Cc: git, Jeff King, Johannes Schindelin, sunny, Jakub Narębski,
Matthieu Moy
Thomas Gummerer <t.gummerer@gmail.com> writes:
> - git reset --hard ${GIT_QUIET:+-q}
This hunk is probably the most important one to review in the whole
series, in the sense that these are entirely new code that didn't
exist in the original.
> + if test $# != 0
> + then
> + saved_untracked=
> + if test -n "$(git ls-files --others -- "$@")"
I notice that "ls-files -o" used in the code before this series are
almost always used with --exclude-standard but we do not set up the
standard exclude pattern here. Is there a good reason to use (or
not to use) it here as well?
> + then
> + saved_untracked=$(
> + git ls-files -z --others -- "$@" |
> + xargs -0 git stash create -u all --)
> + fi
Running the same ls-files twice look somewhat wasteful.
I suspect that we avoid "xargs -0" in our code from portability
concern (isn't it a GNU invention?)
> + git ls-files -z -- "$@" | xargs -0 git reset ${GIT_QUIET:+-q} --
Hmm, am I being naive to suspect that the above is a roundabout way
to say:
git reset ${GIT_QUIET:+-q} -- "$@"
or is an effect quite different from that intended here?
> + git ls-files -z --modified -- "$@" | xargs -0 git checkout ${GIT_QUIET:+-q} HEAD --
Likewise. Wouldn't the above be equivalent to:
git checkout ${GIT_QUIET:+-q} HEAD -- "$@"
Or is this trying to preserve paths modified in the working tree and
fully added to the index?
> + if test -n "$(git ls-files -z --others -- "$@")"
> + then
> + git ls-files -z --others -- "$@" | xargs -0 git clean --force -d ${GIT_QUIET:+-q} --
Likewise. "ls-files --others" being the major part of what "clean"
is about, I suspect the "ls-files piped to clean" is redundant. Do
you even need a test? IOW, doesn't "git clean" with a pathspec that
does not match anything silently succeed without doing anything
harmful?
> + fi
> + if test -n "$saved_untracked"
> + then
> + git stash pop -q $saved_untracked
I see this thing was "created", and the whole point of "create" is
to be different from "save/push" that automatically adds the result
to the stash reflog. Should we be "pop"ing it, or did you mean to
just call apply_stash on it?
> + fi
> + else
> + git reset --hard ${GIT_QUIET:+-q}
> + fi
^ permalink raw reply [flat|nested] 61+ messages in thread
* Re: [PATCH v6 4/6] stash: teach 'push' (and 'create_stash') to honor pathspec
2017-02-21 16:55 ` Junio C Hamano
@ 2017-02-25 20:27 ` Thomas Gummerer
0 siblings, 0 replies; 61+ messages in thread
From: Thomas Gummerer @ 2017-02-25 20:27 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Johannes Schindelin, sunny, Jakub Narębski,
Matthieu Moy
On 02/21, Junio C Hamano wrote:
> Thomas Gummerer <t.gummerer@gmail.com> writes:
>
> > - git reset --hard ${GIT_QUIET:+-q}
>
> This hunk is probably the most important one to review in the whole
> series, in the sense that these are entirely new code that didn't
> exist in the original.
>
> > + if test $# != 0
> > + then
> > + saved_untracked=
> > + if test -n "$(git ls-files --others -- "$@")"
>
> I notice that "ls-files -o" used in the code before this series are
> almost always used with --exclude-standard but we do not set up the
> standard exclude pattern here. Is there a good reason to use (or
> not to use) it here as well?
We probably should use it, not adding it was an oversight.
> > + then
> > + saved_untracked=$(
> > + git ls-files -z --others -- "$@" |
> > + xargs -0 git stash create -u all --)
> > + fi
>
> Running the same ls-files twice look somewhat wasteful.
>
> I suspect that we avoid "xargs -0" in our code from portability
> concern (isn't it a GNU invention?)
>
> > + git ls-files -z -- "$@" | xargs -0 git reset ${GIT_QUIET:+-q} --
>
> Hmm, am I being naive to suspect that the above is a roundabout way
> to say:
>
> git reset ${GIT_QUIET:+-q} -- "$@"
>
> or is an effect quite different from that intended here?
>
> > + git ls-files -z --modified -- "$@" | xargs -0 git checkout ${GIT_QUIET:+-q} HEAD --
>
> Likewise. Wouldn't the above be equivalent to:
>
> git checkout ${GIT_QUIET:+-q} HEAD -- "$@"
>
> Or is this trying to preserve paths modified in the working tree and
> fully added to the index?.
No, it's not trying to do that (all the paths we're touching here
would have been "reset" earlier, so it wouldn't change anything.
However what this code tried to do is to allow "stash push -- path
pathspec-not-in-repo", where "pathspec-not-in-repo" would end up
tripping up "checkout" which does not accept pathspecs that are not in
the index.
I think we should disallow such pathspecs in stash as well, except in
the --include-untracked case, where it still makes sense.
This means we can't get rid of the "ls-files --modified" here, but we
can in all other places, and get rid of the "stash_create"
"stash_apply" dance to store the untracked files, simplifying this
part quite a bit.
Will re-roll.
>
> > + if test -n "$(git ls-files -z --others -- "$@")"
> > + then
> > + git ls-files -z --others -- "$@" | xargs -0 git clean --force -d ${GIT_QUIET:+-q} --
>
> Likewise. "ls-files --others" being the major part of what "clean"
> is about, I suspect the "ls-files piped to clean" is redundant. Do
> you even need a test? IOW, doesn't "git clean" with a pathspec that
> does not match anything silently succeed without doing anything
> harmful?
>
> > + fi
> > + if test -n "$saved_untracked"
> > + then
> > + git stash pop -q $saved_untracked
>
> I see this thing was "created", and the whole point of "create" is
> to be different from "save/push" that automatically adds the result
> to the stash reflog. Should we be "pop"ing it, or did you mean to
> just call apply_stash on it?
>
> > + fi
> > + else
> > + git reset --hard ${GIT_QUIET:+-q}
> > + fi
>
^ permalink raw reply [flat|nested] 61+ messages in thread
* [PATCH v6 5/6] stash: use stash_push for no verb form
2017-02-19 11:03 ` [PATCH v6 " Thomas Gummerer
` (3 preceding siblings ...)
2017-02-19 11:03 ` [PATCH v6 4/6] stash: teach 'push' (and 'create_stash') to honor pathspec Thomas Gummerer
@ 2017-02-19 11:03 ` Thomas Gummerer
2017-02-19 11:03 ` [PATCH v6 6/6] stash: allow pathspecs in the " Thomas Gummerer
` (4 subsequent siblings)
9 siblings, 0 replies; 61+ messages in thread
From: Thomas Gummerer @ 2017-02-19 11:03 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Johannes Schindelin,
Øyvind A. Holm, Jakub Narębski, Matthieu Moy,
Thomas Gummerer
Now that we have stash_push, which accepts pathspec arguments, use
it instead of stash_save in git stash without any additional verbs.
Previously we allowed git stash -- -message, which is no longer allowed
after this patch. Messages starting with a hyphen was allowed since
3c2eb80f, ("stash: simplify defaulting to "save" and reject unknown
options"). However it was never the intent to allow that, but rather it
was allowed accidentally.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
Documentation/git-stash.txt | 8 ++++----
git-stash.sh | 16 ++++++++--------
t/t3903-stash.sh | 4 +---
3 files changed, 13 insertions(+), 15 deletions(-)
diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index b7db939a06..3f7fa88ddc 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -13,11 +13,11 @@ SYNOPSIS
'git stash' drop [-q|--quiet] [<stash>]
'git stash' ( pop | apply ) [--index] [-q|--quiet] [<stash>]
'git stash' branch <branchname> [<stash>]
-'git stash' [save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
- [-u|--include-untracked] [-a|--all] [<message>]]
-'git stash' push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
+'git stash' save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
+ [-u|--include-untracked] [-a|--all] [<message>]
+'git stash' [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
[-u|--include-untracked] [-a|--all] [-m|--message <message>]]
- [--] [<pathspec>...]
+ [--] [<pathspec>...]]
'git stash' clear
'git stash' create [<message>]
'git stash' store [-m|--message <message>] [-q|--quiet] <commit>
diff --git a/git-stash.sh b/git-stash.sh
index b55983d1fd..1e55cd5fdd 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -7,11 +7,11 @@ USAGE="list [<options>]
or: $dashless drop [-q|--quiet] [<stash>]
or: $dashless ( pop | apply ) [--index] [-q|--quiet] [<stash>]
or: $dashless branch <branchname> [<stash>]
- or: $dashless [save [--patch] [-k|--[no-]keep-index] [-q|--quiet]
- [-u|--include-untracked] [-a|--all] [<message>]]
- or: $dashless push [--patch] [-k|--[no-]keep-index] [-q|--quiet]
- [-u|--include-untracked] [-a|--all] [-m <message>]
- [-- <pathspec>...]
+ or: $dashless save [--patch] [-k|--[no-]keep-index] [-q|--quiet]
+ [-u|--include-untracked] [-a|--all] [<message>]
+ or: $dashless [push [--patch] [-k|--[no-]keep-index] [-q|--quiet]
+ [-u|--include-untracked] [-a|--all] [-m <message>]
+ [-- <pathspec>...]]
or: $dashless clear"
SUBDIRECTORY_OK=Yes
@@ -667,7 +667,7 @@ apply_to_branch () {
}
PARSE_CACHE='--not-parsed'
-# The default command is "save" if nothing but options are given
+# The default command is "push" if nothing but options are given
seen_non_option=
for opt
do
@@ -677,7 +677,7 @@ do
esac
done
-test -n "$seen_non_option" || set "save" "$@"
+test -n "$seen_non_option" || set "push" "$@"
# Main command set
case "$1" in
@@ -728,7 +728,7 @@ branch)
*)
case $# in
0)
- save_stash &&
+ push_stash &&
say "$(gettext "(To restore them type \"git stash apply\")")"
;;
*)
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 4fb800eec8..7f90a247b4 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -274,9 +274,7 @@ test_expect_success 'stash --invalid-option' '
git add file2 &&
test_must_fail git stash --invalid-option &&
test_must_fail git stash save --invalid-option &&
- test bar5,bar6 = $(cat file),$(cat file2) &&
- git stash -- -message-starting-with-dash &&
- test bar,bar2 = $(cat file),$(cat file2)
+ test bar5,bar6 = $(cat file),$(cat file2)
'
test_expect_success 'stash an added file' '
--
2.12.0.rc2.399.g0ca89a282
^ permalink raw reply related [flat|nested] 61+ messages in thread
* [PATCH v6 6/6] stash: allow pathspecs in the no verb form
2017-02-19 11:03 ` [PATCH v6 " Thomas Gummerer
` (4 preceding siblings ...)
2017-02-19 11:03 ` [PATCH v6 5/6] stash: use stash_push for no verb form Thomas Gummerer
@ 2017-02-19 11:03 ` Thomas Gummerer
2017-02-20 7:57 ` [PATCH v6 0/6] stash: support pathspec argument Jeff King
` (3 subsequent siblings)
9 siblings, 0 replies; 61+ messages in thread
From: Thomas Gummerer @ 2017-02-19 11:03 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Johannes Schindelin,
Øyvind A. Holm, Jakub Narębski, Matthieu Moy,
Thomas Gummerer
Now that stash_push is used in the no verb form of stash, allow
specifying the command line for this form as well. Always use -- to
disambiguate pathspecs from other non-option arguments.
Also make git stash -p an alias for git stash push -p. This allows
users to use git stash -p <pathspec>.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
Documentation/git-stash.txt | 11 +++++++----
git-stash.sh | 3 +++
t/t3903-stash.sh | 15 +++++++++++++++
3 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 3f7fa88ddc..369bfae33d 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -53,10 +53,13 @@ push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q
Save your local modifications to a new 'stash', and run `git reset
--hard` to revert them. The <message> part is optional and gives
- the description along with the stashed state. For quickly making
- a snapshot, you can omit _both_ "save" and <message>, but giving
- only <message> does not trigger this action to prevent a misspelled
- subcommand from making an unwanted stash.
+ the description along with the stashed state.
++
+For quickly making a snapshot, you can omit "push". In this mode,
+non-option arguments are not allowed to prevent a misspelled
+subcommand from making an unwanted stash. The two exceptions to this
+are `stash -p` which acts as alias for `stash push -p` and pathspecs,
+which are allowed after a double hyphen `--` for disambiguation.
+
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
diff --git a/git-stash.sh b/git-stash.sh
index 1e55cd5fdd..18aba1346f 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -666,12 +666,15 @@ apply_to_branch () {
}
}
+test "$1" = "-p" && set "push" "$@"
+
PARSE_CACHE='--not-parsed'
# The default command is "push" if nothing but options are given
seen_non_option=
for opt
do
case "$opt" in
+ --) break ;;
-*) ;;
*) seen_non_option=t; break ;;
esac
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 7f90a247b4..c0ae41e724 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -872,4 +872,19 @@ test_expect_success 'untracked files are left in place when -u is not given' '
test_path_is_file untracked
'
+test_expect_success 'stash without verb with pathspec' '
+ >"foo bar" &&
+ >foo &&
+ >bar &&
+ git add foo* &&
+ git stash -- "foo b*" &&
+ test_path_is_missing "foo bar" &&
+ test_path_is_file foo &&
+ test_path_is_file bar &&
+ git stash pop &&
+ test_path_is_file "foo bar" &&
+ test_path_is_file foo &&
+ test_path_is_file bar
+'
+
test_done
--
2.12.0.rc2.399.g0ca89a282
^ permalink raw reply related [flat|nested] 61+ messages in thread
* Re: [PATCH v6 0/6] stash: support pathspec argument
2017-02-19 11:03 ` [PATCH v6 " Thomas Gummerer
` (5 preceding siblings ...)
2017-02-19 11:03 ` [PATCH v6 6/6] stash: allow pathspecs in the " Thomas Gummerer
@ 2017-02-20 7:57 ` Jeff King
2017-02-20 8:22 ` Junio C Hamano
` (2 subsequent siblings)
9 siblings, 0 replies; 61+ messages in thread
From: Jeff King @ 2017-02-20 7:57 UTC (permalink / raw)
To: Thomas Gummerer
Cc: git, Junio C Hamano, Johannes Schindelin, Øyvind A. Holm,
Jakub Narębski, Matthieu Moy
On Sun, Feb 19, 2017 at 11:03:07AM +0000, Thomas Gummerer wrote:
> Thanks Junio and Peff for comments on the last round.
>
> Changes since then:
>
> - removed mention of the "new form" of git stash create from the
> Documentation.
> - Changed documentation for git stash without a verb, mentioning
> stash -p now being an alias for git stash push -p and that -- can be
> used as disambiguation for for pathspecs
> - Fixed ${1-...} which should have been ${1?...}
> - Removed unused new_style variable from create_stash, which was a
> leftover from perious rounds.
I gave it one more read-through, and didn't see anything objectionable.
Thanks for seeing this through!
-Peff
^ permalink raw reply [flat|nested] 61+ messages in thread
* Re: [PATCH v6 0/6] stash: support pathspec argument
2017-02-19 11:03 ` [PATCH v6 " Thomas Gummerer
` (6 preceding siblings ...)
2017-02-20 7:57 ` [PATCH v6 0/6] stash: support pathspec argument Jeff King
@ 2017-02-20 8:22 ` Junio C Hamano
2017-02-20 19:39 ` Junio C Hamano
2017-02-25 21:33 ` [PATCH v7 " Thomas Gummerer
9 siblings, 0 replies; 61+ messages in thread
From: Junio C Hamano @ 2017-02-20 8:22 UTC (permalink / raw)
To: Thomas Gummerer
Cc: git, Jeff King, Johannes Schindelin, Øyvind A Holm,
Jakub Narębski, Matthieu Moy
Thomas Gummerer <t.gummerer@gmail.com> writes:
> @@ -55,10 +53,13 @@ push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q
>
> Save your local modifications to a new 'stash', and run `git reset
> --hard` to revert them. The <message> part is optional and gives
I didn't notice this during v5 review, but the above seems to be
based on the codebase before your documentation update (which used
to be part of the series in older iteration). I had to tweak the
series to apply on top of your 20a7e06172 ("Documentation/stash:
remove mention of git reset --hard", 2017-02-12) while queuing, so
please double check the result when it is pushed out to 'pu'.
Thanks.
^ permalink raw reply [flat|nested] 61+ messages in thread
* Re: [PATCH v6 0/6] stash: support pathspec argument
2017-02-19 11:03 ` [PATCH v6 " Thomas Gummerer
` (7 preceding siblings ...)
2017-02-20 8:22 ` Junio C Hamano
@ 2017-02-20 19:39 ` Junio C Hamano
2017-02-25 15:57 ` Thomas Gummerer
2017-02-25 21:33 ` [PATCH v7 " Thomas Gummerer
9 siblings, 1 reply; 61+ messages in thread
From: Junio C Hamano @ 2017-02-20 19:39 UTC (permalink / raw)
To: Thomas Gummerer
Cc: git, Jeff King, Johannes Schindelin, Øyvind A Holm,
Jakub Narębski, Matthieu Moy
Thomas Gummerer <t.gummerer@gmail.com> writes:
> @@ -55,10 +53,13 @@ push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q
>
> Save your local modifications to a new 'stash', and run `git reset
> --hard` to revert them. The <message> part is optional and gives
I didn't notice this during v5 review, but the above seems to be
based on the codebase before your documentation update (which used
to be part of the series in older iteration). I had to tweak the
series to apply on top of your 20a7e06172 ("Documentation/stash:
remove mention of git reset --hard", 2017-02-12) while queuing, so
please double check the result when it is pushed out to 'pu'.
Thanks.
^ permalink raw reply [flat|nested] 61+ messages in thread
* Re: [PATCH v6 0/6] stash: support pathspec argument
2017-02-20 19:39 ` Junio C Hamano
@ 2017-02-25 15:57 ` Thomas Gummerer
0 siblings, 0 replies; 61+ messages in thread
From: Thomas Gummerer @ 2017-02-25 15:57 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Johannes Schindelin, sunny, Jakub Narębski,
Matthieu Moy
On 02/20, Junio C Hamano wrote:
> Thomas Gummerer <t.gummerer@gmail.com> writes:
>
> > @@ -55,10 +53,13 @@ push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q
> >
> > Save your local modifications to a new 'stash', and run `git reset
> > --hard` to revert them. The <message> part is optional and gives
>
> I didn't notice this during v5 review, but the above seems to be
> based on the codebase before your documentation update (which used
> to be part of the series in older iteration). I had to tweak the
> series to apply on top of your 20a7e06172 ("Documentation/stash:
> remove mention of git reset --hard", 2017-02-12) while queuing, so
> please double check the result when it is pushed out to 'pu'.
Sorry about that. What you queued looks good to me, I will however
send a re-roll based on your comments on 4/6. (And I'll make sure to
base it on 20a7e06172)
^ permalink raw reply [flat|nested] 61+ messages in thread
* [PATCH v7 0/6] stash: support pathspec argument
2017-02-19 11:03 ` [PATCH v6 " Thomas Gummerer
` (8 preceding siblings ...)
2017-02-20 19:39 ` Junio C Hamano
@ 2017-02-25 21:33 ` Thomas Gummerer
2017-02-25 21:33 ` [PATCH v7 1/6] stash: introduce push verb Thomas Gummerer
` (6 more replies)
9 siblings, 7 replies; 61+ messages in thread
From: Thomas Gummerer @ 2017-02-25 21:33 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Johannes Schindelin, sunny,
Jakub Narębski, Matthieu Moy, Thomas Gummerer
Thanks Junio for more comments on the last round, and Peff for reading
through it as well.
Changes since v6:
- If no --include-untracked option is given to git stash push, and a
pathspec is not in the repository, error out instead of ignoring
it. This brings it in line with things like checkout, and also
allows us to simplify the code internally.
- Simplify the code for rolling back the changes from the working
tree. This is enabled by the changes to the pathspec handling.
- There's no more "xargs -0", so there should be no more portability
concerns.
- Adjust the tests and improve some of the titles a bit
Interdiff:
diff --git a/git-stash.sh b/git-stash.sh
index 18aba1346f..28d0624c75 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -278,12 +278,15 @@ push_stash () {
die "$(gettext "Can't use --patch and --include-untracked or --all at the same time")"
fi
+ test -n "$untracked" || git ls-files --error-unmatch -- "$@" >/dev/null || exit 1
+
git update-index -q --refresh
if no_changes "$@"
then
say "$(gettext "No local changes to save")"
exit 0
fi
+
git reflog exists $ref_stash ||
clear_stash || die "$(gettext "Cannot initialize stash")"
@@ -296,23 +299,9 @@ push_stash () {
then
if test $# != 0
then
- saved_untracked=
- if test -n "$(git ls-files --others -- "$@")"
- then
- saved_untracked=$(
- git ls-files -z --others -- "$@" |
- xargs -0 git stash create -u all --)
- fi
- git ls-files -z -- "$@" | xargs -0 git reset ${GIT_QUIET:+-q} --
- git ls-files -z --modified -- "$@" | xargs -0 git checkout ${GIT_QUIET:+-q} HEAD --
- if test -n "$(git ls-files -z --others -- "$@")"
- then
- git ls-files -z --others -- "$@" | xargs -0 git clean --force -d ${GIT_QUIET:+-q} --
- fi
- if test -n "$saved_untracked"
- then
- git stash pop -q $saved_untracked
- fi
+ git reset ${GIT_QUIET:+-q} -- "$@"
+ git checkout ${GIT_QUIET:+-q} HEAD -- $(git ls-files -z --modified "$@")
+ git clean --force ${GIT_QUIET:+-q} -d -- "$@"
else
git reset --hard ${GIT_QUIET:+-q}
fi
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index c0ae41e724..f7733b4dd4 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -800,7 +800,7 @@ test_expect_success 'create with multiple arguments for the message' '
test_cmp expect actual
'
-test_expect_success 'stash -- <filename> stashes and restores the file' '
+test_expect_success 'stash -- <pathspec> stashes and restores the file' '
>foo &&
>bar &&
git add foo bar &&
@@ -812,7 +812,7 @@ test_expect_success 'stash -- <filename> stashes and restores the file' '
test_path_is_file bar
'
-test_expect_success 'stash with multiple filename arguments' '
+test_expect_success 'stash with multiple pathspec arguments' '
>foo &&
>bar &&
>extra &&
@@ -842,25 +842,29 @@ test_expect_success 'stash with file including $IFS character' '
test_path_is_file bar
'
-test_expect_success 'stash push -p with pathspec shows no changes only onece' '
- >file &&
- git add file &&
- git stash push -p not-file >actual &&
+test_expect_success 'stash push -p with pathspec shows no changes only once' '
+ >foo &&
+ git add foo &&
+ git commit -m "tmp" &&
+ git stash push -p foo >actual &&
echo "No local changes to save" >expect &&
+ git reset --hard HEAD~ &&
test_cmp expect actual
'
test_expect_success 'stash push with pathspec shows no changes when there are none' '
- >file &&
- git add file &&
- git stash push not-file >actual &&
+ >foo &&
+ git add foo &&
+ git commit -m "tmp" &&
+ git stash push foo >actual &&
echo "No local changes to save" >expect &&
+ git reset --hard HEAD~ &&
test_cmp expect actual
'
-test_expect_success 'untracked file is not removed when using pathspecs' '
+test_expect_success 'stash push with pathspec not in the repository errors out' '
>untracked &&
- git stash push untracked &&
+ test_must_fail git stash push untracked &&
test_path_is_file untracked
'
Thomas Gummerer (6):
stash: introduce push verb
stash: add test for the create command line arguments
stash: refactor stash_create
stash: teach 'push' (and 'create_stash') to honor pathspec
stash: use stash_push for no verb form
stash: allow pathspecs in the no verb form
Documentation/git-stash.txt | 25 ++++++--
git-stash.sh | 114 +++++++++++++++++++++++++++-------
t/t3903-stash.sh | 122 ++++++++++++++++++++++++++++++++++++-
t/t3905-stash-include-untracked.sh | 26 ++++++++
4 files changed, 257 insertions(+), 30 deletions(-)
--
2.11.0.301.g275aeb250c.dirty
^ permalink raw reply related [flat|nested] 61+ messages in thread
* [PATCH v7 1/6] stash: introduce push verb
2017-02-25 21:33 ` [PATCH v7 " Thomas Gummerer
@ 2017-02-25 21:33 ` Thomas Gummerer
2017-02-25 21:33 ` [PATCH v7 2/6] stash: add test for the create command line arguments Thomas Gummerer
` (5 subsequent siblings)
6 siblings, 0 replies; 61+ messages in thread
From: Thomas Gummerer @ 2017-02-25 21:33 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Johannes Schindelin, sunny,
Jakub Narębski, Matthieu Moy, Thomas Gummerer
Introduce a new git stash push verb in addition to git stash save. The
push verb is used to transition from the current command line arguments
to a more conventional way, in which the message is given as an argument
to the -m option.
This allows us to have pathspecs at the end of the command line
arguments like other Git commands do, so that the user can say which
subset of paths to stash (and leave others behind).
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
Documentation/git-stash.txt | 3 +++
git-stash.sh | 46 ++++++++++++++++++++++++++++++++++++++++++---
t/t3903-stash.sh | 9 +++++++++
3 files changed, 55 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 2e9e344cd7..d240df4af7 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -15,6 +15,8 @@ SYNOPSIS
'git stash' branch <branchname> [<stash>]
'git stash' [save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
[-u|--include-untracked] [-a|--all] [<message>]]
+'git stash' push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
+ [-u|--include-untracked] [-a|--all] [-m|--message <message>]]
'git stash' clear
'git stash' create [<message>]
'git stash' store [-m|--message <message>] [-q|--quiet] <commit>
@@ -46,6 +48,7 @@ OPTIONS
-------
save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [<message>]::
+push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [-m|--message <message>]::
Save your local modifications to a new 'stash' and roll them
back to HEAD (in the working tree and in the index).
diff --git a/git-stash.sh b/git-stash.sh
index 10c284d1aa..8365ebba2a 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -9,6 +9,8 @@ USAGE="list [<options>]
or: $dashless branch <branchname> [<stash>]
or: $dashless [save [--patch] [-k|--[no-]keep-index] [-q|--quiet]
[-u|--include-untracked] [-a|--all] [<message>]]
+ or: $dashless push [--patch] [-k|--[no-]keep-index] [-q|--quiet]
+ [-u|--include-untracked] [-a|--all] [-m <message>]
or: $dashless clear"
SUBDIRECTORY_OK=Yes
@@ -189,10 +191,11 @@ store_stash () {
return $ret
}
-save_stash () {
+push_stash () {
keep_index=
patch_mode=
untracked=
+ stash_msg=
while test $# != 0
do
case "$1" in
@@ -216,6 +219,11 @@ save_stash () {
-a|--all)
untracked=all
;;
+ -m|--message)
+ shift
+ test -z ${1+x} && usage
+ stash_msg=$1
+ ;;
--help)
show_help
;;
@@ -251,8 +259,6 @@ save_stash () {
die "$(gettext "Can't use --patch and --include-untracked or --all at the same time")"
fi
- stash_msg="$*"
-
git update-index -q --refresh
if no_changes
then
@@ -291,6 +297,36 @@ save_stash () {
fi
}
+save_stash () {
+ push_options=
+ while test $# != 0
+ do
+ case "$1" in
+ --)
+ shift
+ break
+ ;;
+ -*)
+ # pass all options through to push_stash
+ push_options="$push_options $1"
+ ;;
+ *)
+ break
+ ;;
+ esac
+ shift
+ done
+
+ stash_msg="$*"
+
+ if test -z "$stash_msg"
+ then
+ push_stash $push_options
+ else
+ push_stash $push_options -m "$stash_msg"
+ fi
+}
+
have_stash () {
git rev-parse --verify --quiet $ref_stash >/dev/null
}
@@ -617,6 +653,10 @@ save)
shift
save_stash "$@"
;;
+push)
+ shift
+ push_stash "$@"
+ ;;
apply)
shift
apply_stash "$@"
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 2de3e18ce6..3577115807 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -775,4 +775,13 @@ test_expect_success 'stash is not confused by partial renames' '
test_path_is_missing file
'
+test_expect_success 'push -m shows right message' '
+ >foo &&
+ git add foo &&
+ git stash push -m "test message" &&
+ echo "stash@{0}: On master: test message" >expect &&
+ git stash list -1 >actual &&
+ test_cmp expect actual
+'
+
test_done
--
2.11.0.301.g275aeb250c.dirty
^ permalink raw reply related [flat|nested] 61+ messages in thread
* [PATCH v7 2/6] stash: add test for the create command line arguments
2017-02-25 21:33 ` [PATCH v7 " Thomas Gummerer
2017-02-25 21:33 ` [PATCH v7 1/6] stash: introduce push verb Thomas Gummerer
@ 2017-02-25 21:33 ` Thomas Gummerer
2017-02-25 21:33 ` [PATCH v7 3/6] stash: refactor stash_create Thomas Gummerer
` (4 subsequent siblings)
6 siblings, 0 replies; 61+ messages in thread
From: Thomas Gummerer @ 2017-02-25 21:33 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Johannes Schindelin, sunny,
Jakub Narębski, Matthieu Moy, Thomas Gummerer
Currently there is no test showing the expected behaviour of git stash
create's command line arguments. Add a test for that to show the
current expected behaviour and to make sure future refactorings don't
break those expectations.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
t/t3903-stash.sh | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 3577115807..ffe3549ea5 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -784,4 +784,22 @@ test_expect_success 'push -m shows right message' '
test_cmp expect actual
'
+test_expect_success 'create stores correct message' '
+ >foo &&
+ git add foo &&
+ STASH_ID=$(git stash create "create test message") &&
+ echo "On master: create test message" >expect &&
+ git show --pretty=%s -s ${STASH_ID} >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'create with multiple arguments for the message' '
+ >foo &&
+ git add foo &&
+ STASH_ID=$(git stash create test untracked) &&
+ echo "On master: test untracked" >expect &&
+ git show --pretty=%s -s ${STASH_ID} >actual &&
+ test_cmp expect actual
+'
+
test_done
--
2.11.0.301.g275aeb250c.dirty
^ permalink raw reply related [flat|nested] 61+ messages in thread
* [PATCH v7 3/6] stash: refactor stash_create
2017-02-25 21:33 ` [PATCH v7 " Thomas Gummerer
2017-02-25 21:33 ` [PATCH v7 1/6] stash: introduce push verb Thomas Gummerer
2017-02-25 21:33 ` [PATCH v7 2/6] stash: add test for the create command line arguments Thomas Gummerer
@ 2017-02-25 21:33 ` Thomas Gummerer
2017-02-25 21:33 ` [PATCH v7 4/6] stash: teach 'push' (and 'create_stash') to honor pathspec Thomas Gummerer
` (3 subsequent siblings)
6 siblings, 0 replies; 61+ messages in thread
From: Thomas Gummerer @ 2017-02-25 21:33 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Johannes Schindelin, sunny,
Jakub Narębski, Matthieu Moy, Thomas Gummerer
Refactor the internal stash_create function to use a -m flag for
specifying the message and -u flag to indicate whether untracked files
should be added to the stash.
This makes it easier to pass a pathspec argument to stash_create in the
next patch.
The user interface for git stash create stays the same.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
git-stash.sh | 22 ++++++++++++++++++----
1 file changed, 18 insertions(+), 4 deletions(-)
diff --git a/git-stash.sh b/git-stash.sh
index 8365ebba2a..ef5d1b45be 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -58,8 +58,22 @@ clear_stash () {
}
create_stash () {
- stash_msg="$1"
- untracked="$2"
+ stash_msg=
+ untracked=
+ while test $# != 0
+ do
+ case "$1" in
+ -m|--message)
+ shift
+ stash_msg=${1?"BUG: create_stash () -m requires an argument"}
+ ;;
+ -u|--include-untracked)
+ shift
+ untracked=${1?"BUG: create_stash () -u requires an argument"}
+ ;;
+ esac
+ shift
+ done
git update-index -q --refresh
if no_changes
@@ -268,7 +282,7 @@ push_stash () {
git reflog exists $ref_stash ||
clear_stash || die "$(gettext "Cannot initialize stash")"
- create_stash "$stash_msg" $untracked
+ create_stash -m "$stash_msg" -u "$untracked"
store_stash -m "$stash_msg" -q $w_commit ||
die "$(gettext "Cannot save the current status")"
say "$(eval_gettext "Saved working directory and index state \$stash_msg")"
@@ -667,7 +681,7 @@ clear)
;;
create)
shift
- create_stash "$*" && echo "$w_commit"
+ create_stash -m "$*" && echo "$w_commit"
;;
store)
shift
--
2.11.0.301.g275aeb250c.dirty
^ permalink raw reply related [flat|nested] 61+ messages in thread
* [PATCH v7 4/6] stash: teach 'push' (and 'create_stash') to honor pathspec
2017-02-25 21:33 ` [PATCH v7 " Thomas Gummerer
` (2 preceding siblings ...)
2017-02-25 21:33 ` [PATCH v7 3/6] stash: refactor stash_create Thomas Gummerer
@ 2017-02-25 21:33 ` Thomas Gummerer
2017-02-27 20:32 ` Junio C Hamano
` (2 more replies)
2017-02-25 21:33 ` [PATCH v7 5/6] stash: use stash_push for no verb form Thomas Gummerer
` (2 subsequent siblings)
6 siblings, 3 replies; 61+ messages in thread
From: Thomas Gummerer @ 2017-02-25 21:33 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Johannes Schindelin, sunny,
Jakub Narębski, Matthieu Moy, 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 <gitster@pobox.com>
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
Documentation/git-stash.txt | 9 ++++-
git-stash.sh | 37 +++++++++++++------
t/t3903-stash.sh | 76 ++++++++++++++++++++++++++++++++++++++
t/t3905-stash-include-untracked.sh | 26 +++++++++++++
4 files changed, 136 insertions(+), 12 deletions(-)
diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index d240df4af7..88369ed8b6 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -17,6 +17,7 @@ SYNOPSIS
[-u|--include-untracked] [-a|--all] [<message>]]
'git stash' push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
[-u|--include-untracked] [-a|--all] [-m|--message <message>]]
+ [--] [<pathspec>...]
'git stash' clear
'git stash' create [<message>]
'git stash' store [-m|--message <message>] [-q|--quiet] <commit>
@@ -48,7 +49,7 @@ OPTIONS
-------
save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [<message>]::
-push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [-m|--message <message>]::
+push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [-m|--message <message>] [--] [<pathspec>...]::
Save your local modifications to a new 'stash' and roll them
back to HEAD (in the working tree and in the index).
@@ -58,6 +59,12 @@ push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q
only <message> 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 ef5d1b45be..57828f926d 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -11,6 +11,7 @@ USAGE="list [<options>]
[-u|--include-untracked] [-a|--all] [<message>]]
or: $dashless push [--patch] [-k|--[no-]keep-index] [-q|--quiet]
[-u|--include-untracked] [-a|--all] [-m <message>]
+ [-- <pathspec>...]
or: $dashless clear"
SUBDIRECTORY_OK=Yes
@@ -35,15 +36,15 @@ else
fi
no_changes () {
- git diff-index --quiet --cached HEAD --ignore-submodules -- &&
- git diff-files --quiet --ignore-submodules &&
+ git diff-index --quiet --cached HEAD --ignore-submodules -- "$@" &&
+ git diff-files --quiet --ignore-submodules -- "$@" &&
(test -z "$untracked" || test -z "$(untracked_files)")
}
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 () {
@@ -71,12 +72,16 @@ create_stash () {
shift
untracked=${1?"BUG: create_stash () -u requires an argument"}
;;
+ --)
+ shift
+ break
+ ;;
esac
shift
done
git update-index -q --refresh
- if no_changes
+ if no_changes "$@"
then
exit 0
fi
@@ -108,7 +113,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" &&
@@ -131,7 +136,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"
@@ -145,7 +150,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) ||
@@ -273,27 +278,37 @@ push_stash () {
die "$(gettext "Can't use --patch and --include-untracked or --all at the same time")"
fi
+ test -n "$untracked" || git ls-files --error-unmatch -- "$@" >/dev/null || exit 1
+
git update-index -q --refresh
- if no_changes
+ if no_changes "$@"
then
say "$(gettext "No local changes to save")"
exit 0
fi
+
git reflog exists $ref_stash ||
clear_stash || die "$(gettext "Cannot initialize stash")"
- create_stash -m "$stash_msg" -u "$untracked"
+ create_stash -m "$stash_msg" -u "$untracked" -- "$@"
store_stash -m "$stash_msg" -q $w_commit ||
die "$(gettext "Cannot save the current status")"
say "$(eval_gettext "Saved working directory and index state \$stash_msg")"
if test -z "$patch_mode"
then
- git reset --hard ${GIT_QUIET:+-q}
+ if test $# != 0
+ then
+ git reset ${GIT_QUIET:+-q} -- "$@"
+ git checkout ${GIT_QUIET:+-q} HEAD -- $(git ls-files -z --modified "$@")
+ git clean --force ${GIT_QUIET:+-q} -d -- "$@"
+ else
+ git reset --hard ${GIT_QUIET:+-q}
+ fi
test "$untracked" = "all" && CLEAN_X_OPTION=-x || CLEAN_X_OPTION=
if test -n "$untracked"
then
- git clean --force --quiet -d $CLEAN_X_OPTION
+ git clean --force --quiet -d $CLEAN_X_OPTION -- "$@"
fi
if test "$keep_index" = "t" && test -n "$i_tree"
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index ffe3549ea5..4d8a096773 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -802,4 +802,80 @@ test_expect_success 'create with multiple arguments for the message' '
test_cmp expect actual
'
+test_expect_success 'stash -- <pathspec> stashes and restores the file' '
+ >foo &&
+ >bar &&
+ git add foo bar &&
+ git stash push -- foo &&
+ test_path_is_file bar &&
+ test_path_is_missing foo &&
+ git stash pop &&
+ test_path_is_file foo &&
+ test_path_is_file bar
+'
+
+test_expect_success 'stash with multiple pathspec arguments' '
+ >foo &&
+ >bar &&
+ >extra &&
+ git add foo bar extra &&
+ git stash push -- foo bar &&
+ test_path_is_missing bar &&
+ test_path_is_missing foo &&
+ test_path_is_file extra &&
+ git stash pop &&
+ test_path_is_file foo &&
+ test_path_is_file bar &&
+ test_path_is_file extra
+'
+
+test_expect_success 'stash with file including $IFS character' '
+ >"foo bar" &&
+ >foo &&
+ >bar &&
+ git add foo* &&
+ git stash push -- "foo b*" &&
+ test_path_is_missing "foo bar" &&
+ test_path_is_file foo &&
+ test_path_is_file bar &&
+ git stash pop &&
+ test_path_is_file "foo bar" &&
+ test_path_is_file foo &&
+ test_path_is_file bar
+'
+
+test_expect_success 'stash push -p with pathspec shows no changes only once' '
+ >foo &&
+ git add foo &&
+ git commit -m "tmp" &&
+ git stash push -p foo >actual &&
+ echo "No local changes to save" >expect &&
+ git reset --hard HEAD~ &&
+ test_cmp expect actual
+'
+
+test_expect_success 'stash push with pathspec shows no changes when there are none' '
+ >foo &&
+ git add foo &&
+ git commit -m "tmp" &&
+ git stash push foo >actual &&
+ echo "No local changes to save" >expect &&
+ git reset --hard HEAD~ &&
+ test_cmp expect actual
+'
+
+test_expect_success 'stash push with pathspec not in the repository errors out' '
+ >untracked &&
+ test_must_fail git stash push untracked &&
+ test_path_is_file untracked
+'
+
+test_expect_success 'untracked files are left in place when -u is not given' '
+ >file &&
+ git add file &&
+ >untracked &&
+ git stash push file &&
+ test_path_is_file untracked
+'
+
test_done
diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh
index f372fc8ca8..193adc7b68 100755
--- a/t/t3905-stash-include-untracked.sh
+++ b/t/t3905-stash-include-untracked.sh
@@ -185,4 +185,30 @@ test_expect_success 'stash save --all is stash poppable' '
test -s .gitignore
'
+test_expect_success 'stash push --include-untracked with pathspec' '
+ >foo &&
+ >bar &&
+ git stash push --include-untracked -- foo &&
+ test_path_is_file bar &&
+ test_path_is_missing foo &&
+ git stash pop &&
+ test_path_is_file bar &&
+ test_path_is_file foo
+'
+
+test_expect_success 'stash push with $IFS character' '
+ >"foo bar" &&
+ >foo &&
+ >bar &&
+ git add foo* &&
+ git stash push --include-untracked -- "foo b*" &&
+ test_path_is_missing "foo bar" &&
+ test_path_is_file foo &&
+ test_path_is_file bar &&
+ git stash pop &&
+ test_path_is_file "foo bar" &&
+ test_path_is_file foo &&
+ test_path_is_file bar
+'
+
test_done
--
2.11.0.301.g275aeb250c.dirty
^ permalink raw reply related [flat|nested] 61+ messages in thread
* Re: [PATCH v7 4/6] stash: teach 'push' (and 'create_stash') to honor pathspec
2017-02-25 21:33 ` [PATCH v7 4/6] stash: teach 'push' (and 'create_stash') to honor pathspec Thomas Gummerer
@ 2017-02-27 20:32 ` Junio C Hamano
2017-02-27 20:35 ` Junio C Hamano
2017-02-27 21:09 ` Junio C Hamano
2 siblings, 0 replies; 61+ messages in thread
From: Junio C Hamano @ 2017-02-27 20:32 UTC (permalink / raw)
To: Thomas Gummerer
Cc: git, Jeff King, Johannes Schindelin, sunny, Jakub Narębski,
Matthieu Moy
Thomas Gummerer <t.gummerer@gmail.com> writes:
> if test -z "$patch_mode"
> then
> - git reset --hard ${GIT_QUIET:+-q}
> + if test $# != 0
> + then
> + git reset ${GIT_QUIET:+-q} -- "$@"
> + git checkout ${GIT_QUIET:+-q} HEAD -- $(git ls-files -z --modified "$@")
"ls-files -z" on the command line?
Apparently new tests do not cover the correctness of this codepath.
I wonder if this
git ls-files -z --modified "$@" |
git checkout-index -z --stdin
is what the above "checkout" wanted to do. The "reset" in the
previous step presumably updated the index entries that match
specified pathspec to those of the HEAD, so checking out the paths
that match "$@" from the index would be the same as checking them
out from the HEAD (while updating the index with them).
> + git clean --force ${GIT_QUIET:+-q} -d -- "$@"
> + else
> + git reset --hard ${GIT_QUIET:+-q}
> + fi
Thanks.
^ permalink raw reply [flat|nested] 61+ messages in thread
* Re: [PATCH v7 4/6] stash: teach 'push' (and 'create_stash') to honor pathspec
2017-02-25 21:33 ` [PATCH v7 4/6] stash: teach 'push' (and 'create_stash') to honor pathspec Thomas Gummerer
2017-02-27 20:32 ` Junio C Hamano
@ 2017-02-27 20:35 ` Junio C Hamano
2017-02-27 21:56 ` Thomas Gummerer
2017-02-27 21:09 ` Junio C Hamano
2 siblings, 1 reply; 61+ messages in thread
From: Junio C Hamano @ 2017-02-27 20:35 UTC (permalink / raw)
To: Thomas Gummerer
Cc: git, Jeff King, Johannes Schindelin, sunny, Jakub Narębski,
Matthieu Moy
Thomas Gummerer <t.gummerer@gmail.com> writes:
> + test -n "$untracked" || git ls-files --error-unmatch -- "$@" >/dev/null || exit 1
This silent "exit 1" made me scratch my head, but --error-unmatch
would have already given an error message, like
error: pathspec 'no such' did not match any file(s) known to git.
Did you forget to 'git add'?
so that would be OK.
^ permalink raw reply [flat|nested] 61+ messages in thread
* Re: [PATCH v7 4/6] stash: teach 'push' (and 'create_stash') to honor pathspec
2017-02-27 20:35 ` Junio C Hamano
@ 2017-02-27 21:56 ` Thomas Gummerer
2017-02-27 22:58 ` Junio C Hamano
0 siblings, 1 reply; 61+ messages in thread
From: Thomas Gummerer @ 2017-02-27 21:56 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Johannes Schindelin, sunny, Jakub Narębski,
Matthieu Moy
On 02/27, Junio C Hamano wrote:
> Thomas Gummerer <t.gummerer@gmail.com> writes:
>
> > + test -n "$untracked" || git ls-files --error-unmatch -- "$@" >/dev/null || exit 1
>
> This silent "exit 1" made me scratch my head, but --error-unmatch
> would have already given an error message, like
>
> error: pathspec 'no such' did not match any file(s) known to git.
> Did you forget to 'git add'?
>
> so that would be OK.
Yeah exactly, the plan was to let --error-unmatch print the error
message, as it's better at giving a good error message than we could
easily do here I think.
Maybe this deserves a comment so others won't have the same doubts
about this line?
^ permalink raw reply [flat|nested] 61+ messages in thread
* Re: [PATCH v7 4/6] stash: teach 'push' (and 'create_stash') to honor pathspec
2017-02-27 21:56 ` Thomas Gummerer
@ 2017-02-27 22:58 ` Junio C Hamano
0 siblings, 0 replies; 61+ messages in thread
From: Junio C Hamano @ 2017-02-27 22:58 UTC (permalink / raw)
To: Thomas Gummerer
Cc: git, Jeff King, Johannes Schindelin, sunny, Jakub Narębski,
Matthieu Moy
Thomas Gummerer <t.gummerer@gmail.com> writes:
> On 02/27, Junio C Hamano wrote:
>> Thomas Gummerer <t.gummerer@gmail.com> writes:
>>
>> > + test -n "$untracked" || git ls-files --error-unmatch -- "$@" >/dev/null || exit 1
>>
>> This silent "exit 1" made me scratch my head, but --error-unmatch
>> would have already given an error message, like
>>
>> error: pathspec 'no such' did not match any file(s) known to git.
>> Did you forget to 'git add'?
>>
>> so that would be OK.
>
> Yeah exactly, the plan was to let --error-unmatch print the error
> message, as it's better at giving a good error message than we could
> easily do here I think.
>
> Maybe this deserves a comment so others won't have the same doubts
> about this line?
Nah, I do not think so. It probably is obvious for those who write
(and read) "--error-unmatch". I was just being slow to realize that
the sole point of that option was to complain ;-)
^ permalink raw reply [flat|nested] 61+ messages in thread
* Re: [PATCH v7 4/6] stash: teach 'push' (and 'create_stash') to honor pathspec
2017-02-25 21:33 ` [PATCH v7 4/6] stash: teach 'push' (and 'create_stash') to honor pathspec Thomas Gummerer
2017-02-27 20:32 ` Junio C Hamano
2017-02-27 20:35 ` Junio C Hamano
@ 2017-02-27 21:09 ` Junio C Hamano
2017-02-27 21:53 ` Thomas Gummerer
2 siblings, 1 reply; 61+ messages in thread
From: Junio C Hamano @ 2017-02-27 21:09 UTC (permalink / raw)
To: Thomas Gummerer
Cc: git, Jeff King, Johannes Schindelin, sunny, Jakub Narębski,
Matthieu Moy
Thomas Gummerer <t.gummerer@gmail.com> writes:
> if test -z "$patch_mode"
> then
> - git reset --hard ${GIT_QUIET:+-q}
> + if test $# != 0
> + then
> + git reset ${GIT_QUIET:+-q} -- "$@"
> + git checkout ${GIT_QUIET:+-q} HEAD -- $(git ls-files -z --modified "$@")
"ls-files -z" on the command line?
Apparently new tests do not cover the correctness of this codepath.
I wonder if this
git ls-files -z --modified "$@" |
git checkout-index -z --force --stdin
is what the above "checkout" wanted to do. The "reset" in the
previous step presumably updated the index entries that match
specified pathspec to those of the HEAD, so checking out the paths
that match "$@" from the index would be the same as checking them
out from the HEAD (while updating the index with them).
Perhaps squash the following into an appropriate patch in the
series?
git-stash.sh | 3 ++-
t/t3903-stash.sh | 16 ++++++++++++++++
2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/git-stash.sh b/git-stash.sh
index 28d0624c75..9c70662cc8 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -300,7 +300,8 @@ push_stash () {
if test $# != 0
then
git reset ${GIT_QUIET:+-q} -- "$@"
- git checkout ${GIT_QUIET:+-q} HEAD -- $(git ls-files -z --modified "$@")
+ git ls-files -z --modified -- "$@" |
+ git checkout-index -z --force --stdin
git clean --force ${GIT_QUIET:+-q} -d -- "$@"
else
git reset --hard ${GIT_QUIET:+-q}
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index f7733b4dd4..e868aafab2 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -891,4 +891,20 @@ test_expect_success 'stash without verb with pathspec' '
test_path_is_file bar
'
+test_expect_success 'stash with pathspec matching multiple paths' '
+ echo original >file &&
+ echo original >other-file &&
+ git commit -m "two" file other-file &&
+ echo modified >file &&
+ echo modified >other-file &&
+ git stash -- "*file" &&
+ echo original >expect &&
+ test_cmp expect file &&
+ test_cmp expect other-file &&
+ git stash pop &&
+ echo modified >expect &&
+ test_cmp expect file &&
+ test_cmp expect other-file
+'
+
test_done
^ permalink raw reply related [flat|nested] 61+ messages in thread
* Re: [PATCH v7 4/6] stash: teach 'push' (and 'create_stash') to honor pathspec
2017-02-27 21:09 ` Junio C Hamano
@ 2017-02-27 21:53 ` Thomas Gummerer
0 siblings, 0 replies; 61+ messages in thread
From: Thomas Gummerer @ 2017-02-27 21:53 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Johannes Schindelin, sunny, Jakub Narębski,
Matthieu Moy
On 02/27, Junio C Hamano wrote:
> Thomas Gummerer <t.gummerer@gmail.com> writes:
>
> > if test -z "$patch_mode"
> > then
> > - git reset --hard ${GIT_QUIET:+-q}
> > + if test $# != 0
> > + then
> > + git reset ${GIT_QUIET:+-q} -- "$@"
> > + git checkout ${GIT_QUIET:+-q} HEAD -- $(git ls-files -z --modified "$@")
>
> "ls-files -z" on the command line?
>
> Apparently new tests do not cover the correctness of this codepath.
>
> I wonder if this
>
> git ls-files -z --modified "$@" |
> git checkout-index -z --force --stdin
>
> is what the above "checkout" wanted to do. The "reset" in the
> previous step presumably updated the index entries that match
> specified pathspec to those of the HEAD, so checking out the paths
> that match "$@" from the index would be the same as checking them
> out from the HEAD (while updating the index with them).
Yes, you're completely right, this is exactly what it should have
done. Sorry for being slow on getting this right.
> Perhaps squash the following into an appropriate patch in the
> series?
Thanks, I'll squash this in and re-roll.
> git-stash.sh | 3 ++-
> t/t3903-stash.sh | 16 ++++++++++++++++
> 2 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/git-stash.sh b/git-stash.sh
> index 28d0624c75..9c70662cc8 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -300,7 +300,8 @@ push_stash () {
> if test $# != 0
> then
> git reset ${GIT_QUIET:+-q} -- "$@"
> - git checkout ${GIT_QUIET:+-q} HEAD -- $(git ls-files -z --modified "$@")
> + git ls-files -z --modified -- "$@" |
> + git checkout-index -z --force --stdin
> git clean --force ${GIT_QUIET:+-q} -d -- "$@"
> else
> git reset --hard ${GIT_QUIET:+-q}
> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
> index f7733b4dd4..e868aafab2 100755
> --- a/t/t3903-stash.sh
> +++ b/t/t3903-stash.sh
> @@ -891,4 +891,20 @@ test_expect_success 'stash without verb with pathspec' '
> test_path_is_file bar
> '
>
> +test_expect_success 'stash with pathspec matching multiple paths' '
> + echo original >file &&
> + echo original >other-file &&
> + git commit -m "two" file other-file &&
> + echo modified >file &&
> + echo modified >other-file &&
> + git stash -- "*file" &&
> + echo original >expect &&
> + test_cmp expect file &&
> + test_cmp expect other-file &&
> + git stash pop &&
> + echo modified >expect &&
> + test_cmp expect file &&
> + test_cmp expect other-file
> +'
> +
> test_done
^ permalink raw reply [flat|nested] 61+ messages in thread
* [PATCH v7 5/6] stash: use stash_push for no verb form
2017-02-25 21:33 ` [PATCH v7 " Thomas Gummerer
` (3 preceding siblings ...)
2017-02-25 21:33 ` [PATCH v7 4/6] stash: teach 'push' (and 'create_stash') to honor pathspec Thomas Gummerer
@ 2017-02-25 21:33 ` Thomas Gummerer
2017-02-25 21:33 ` [PATCH v7 6/6] stash: allow pathspecs in the " Thomas Gummerer
2017-02-28 20:33 ` [PATCH v8 0/6] stash: support pathspec argument Thomas Gummerer
6 siblings, 0 replies; 61+ messages in thread
From: Thomas Gummerer @ 2017-02-25 21:33 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Johannes Schindelin, sunny,
Jakub Narębski, Matthieu Moy, Thomas Gummerer
Now that we have stash_push, which accepts pathspec arguments, use
it instead of stash_save in git stash without any additional verbs.
Previously we allowed git stash -- -message, which is no longer allowed
after this patch. Messages starting with a hyphen was allowed since
3c2eb80f, ("stash: simplify defaulting to "save" and reject unknown
options"). However it was never the intent to allow that, but rather it
was allowed accidentally.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
Documentation/git-stash.txt | 8 ++++----
git-stash.sh | 16 ++++++++--------
t/t3903-stash.sh | 4 +---
3 files changed, 13 insertions(+), 15 deletions(-)
diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 88369ed8b6..4d8d30f179 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -13,11 +13,11 @@ SYNOPSIS
'git stash' drop [-q|--quiet] [<stash>]
'git stash' ( pop | apply ) [--index] [-q|--quiet] [<stash>]
'git stash' branch <branchname> [<stash>]
-'git stash' [save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
- [-u|--include-untracked] [-a|--all] [<message>]]
-'git stash' push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
+'git stash' save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
+ [-u|--include-untracked] [-a|--all] [<message>]
+'git stash' [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
[-u|--include-untracked] [-a|--all] [-m|--message <message>]]
- [--] [<pathspec>...]
+ [--] [<pathspec>...]]
'git stash' clear
'git stash' create [<message>]
'git stash' store [-m|--message <message>] [-q|--quiet] <commit>
diff --git a/git-stash.sh b/git-stash.sh
index 57828f926d..2d7b30ec5e 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -7,11 +7,11 @@ USAGE="list [<options>]
or: $dashless drop [-q|--quiet] [<stash>]
or: $dashless ( pop | apply ) [--index] [-q|--quiet] [<stash>]
or: $dashless branch <branchname> [<stash>]
- or: $dashless [save [--patch] [-k|--[no-]keep-index] [-q|--quiet]
- [-u|--include-untracked] [-a|--all] [<message>]]
- or: $dashless push [--patch] [-k|--[no-]keep-index] [-q|--quiet]
- [-u|--include-untracked] [-a|--all] [-m <message>]
- [-- <pathspec>...]
+ or: $dashless save [--patch] [-k|--[no-]keep-index] [-q|--quiet]
+ [-u|--include-untracked] [-a|--all] [<message>]
+ or: $dashless [push [--patch] [-k|--[no-]keep-index] [-q|--quiet]
+ [-u|--include-untracked] [-a|--all] [-m <message>]
+ [-- <pathspec>...]]
or: $dashless clear"
SUBDIRECTORY_OK=Yes
@@ -656,7 +656,7 @@ apply_to_branch () {
}
PARSE_CACHE='--not-parsed'
-# The default command is "save" if nothing but options are given
+# The default command is "push" if nothing but options are given
seen_non_option=
for opt
do
@@ -666,7 +666,7 @@ do
esac
done
-test -n "$seen_non_option" || set "save" "$@"
+test -n "$seen_non_option" || set "push" "$@"
# Main command set
case "$1" in
@@ -717,7 +717,7 @@ branch)
*)
case $# in
0)
- save_stash &&
+ push_stash &&
say "$(gettext "(To restore them type \"git stash apply\")")"
;;
*)
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 4d8a096773..2f5888df0d 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -274,9 +274,7 @@ test_expect_success 'stash --invalid-option' '
git add file2 &&
test_must_fail git stash --invalid-option &&
test_must_fail git stash save --invalid-option &&
- test bar5,bar6 = $(cat file),$(cat file2) &&
- git stash -- -message-starting-with-dash &&
- test bar,bar2 = $(cat file),$(cat file2)
+ test bar5,bar6 = $(cat file),$(cat file2)
'
test_expect_success 'stash an added file' '
--
2.11.0.301.g275aeb250c.dirty
^ permalink raw reply related [flat|nested] 61+ messages in thread
* [PATCH v7 6/6] stash: allow pathspecs in the no verb form
2017-02-25 21:33 ` [PATCH v7 " Thomas Gummerer
` (4 preceding siblings ...)
2017-02-25 21:33 ` [PATCH v7 5/6] stash: use stash_push for no verb form Thomas Gummerer
@ 2017-02-25 21:33 ` Thomas Gummerer
2017-02-28 20:33 ` [PATCH v8 0/6] stash: support pathspec argument Thomas Gummerer
6 siblings, 0 replies; 61+ messages in thread
From: Thomas Gummerer @ 2017-02-25 21:33 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Johannes Schindelin, sunny,
Jakub Narębski, Matthieu Moy, Thomas Gummerer
Now that stash_push is used in the no verb form of stash, allow
specifying the command line for this form as well. Always use -- to
disambiguate pathspecs from other non-option arguments.
Also make git stash -p an alias for git stash push -p. This allows
users to use git stash -p <pathspec>.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
Documentation/git-stash.txt | 11 +++++++----
git-stash.sh | 3 +++
t/t3903-stash.sh | 15 +++++++++++++++
3 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 4d8d30f179..70191d06b6 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -54,10 +54,13 @@ push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q
Save your local modifications to a new 'stash' and roll them
back to HEAD (in the working tree and in the index).
The <message> part is optional and gives
- the description along with the stashed state. For quickly making
- a snapshot, you can omit _both_ "save" and <message>, but giving
- only <message> does not trigger this action to prevent a misspelled
- subcommand from making an unwanted stash.
+ the description along with the stashed state.
++
+For quickly making a snapshot, you can omit "push". In this mode,
+non-option arguments are not allowed to prevent a misspelled
+subcommand from making an unwanted stash. The two exceptions to this
+are `stash -p` which acts as alias for `stash push -p` and pathspecs,
+which are allowed after a double hyphen `--` for disambiguation.
+
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
diff --git a/git-stash.sh b/git-stash.sh
index 2d7b30ec5e..28d0624c75 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -655,12 +655,15 @@ apply_to_branch () {
}
}
+test "$1" = "-p" && set "push" "$@"
+
PARSE_CACHE='--not-parsed'
# The default command is "push" if nothing but options are given
seen_non_option=
for opt
do
case "$opt" in
+ --) break ;;
-*) ;;
*) seen_non_option=t; break ;;
esac
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 2f5888df0d..f7733b4dd4 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -876,4 +876,19 @@ test_expect_success 'untracked files are left in place when -u is not given' '
test_path_is_file untracked
'
+test_expect_success 'stash without verb with pathspec' '
+ >"foo bar" &&
+ >foo &&
+ >bar &&
+ git add foo* &&
+ git stash -- "foo b*" &&
+ test_path_is_missing "foo bar" &&
+ test_path_is_file foo &&
+ test_path_is_file bar &&
+ git stash pop &&
+ test_path_is_file "foo bar" &&
+ test_path_is_file foo &&
+ test_path_is_file bar
+'
+
test_done
--
2.11.0.301.g275aeb250c.dirty
^ permalink raw reply related [flat|nested] 61+ messages in thread
* [PATCH v8 0/6] stash: support pathspec argument
2017-02-25 21:33 ` [PATCH v7 " Thomas Gummerer
` (5 preceding siblings ...)
2017-02-25 21:33 ` [PATCH v7 6/6] stash: allow pathspecs in the " Thomas Gummerer
@ 2017-02-28 20:33 ` Thomas Gummerer
2017-02-28 20:33 ` [PATCH v8 1/6] stash: introduce push verb Thomas Gummerer
` (5 more replies)
6 siblings, 6 replies; 61+ messages in thread
From: Thomas Gummerer @ 2017-02-28 20:33 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Johannes Schindelin, sunny,
Jakub Narębski, Matthieu Moy, Thomas Gummerer
Thanks Junio for the review and the squashable diff with the fix for
my errors.
I changed it a tiny bit, to use git stash push instead of git stash,
so the complete diff could go into 4/6, where I think I think the test
fits best.
Interdiff below:
diff --git a/git-stash.sh b/git-stash.sh
index 28d0624c75..207c8126f4 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -300,6 +300,8 @@ push_stash () {
if test $# != 0
then
git reset ${GIT_QUIET:+-q} -- "$@"
+ git ls-files -z --modified -- "$@" |
+ git checkout-index -z --force --stdin
git checkout ${GIT_QUIET:+-q} HEAD -- $(git ls-files -z --modified "$@")
git clean --force ${GIT_QUIET:+-q} -d -- "$@"
else
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index f7733b4dd4..89877e4b52 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -842,6 +842,22 @@ test_expect_success 'stash with file including $IFS character' '
test_path_is_file bar
'
+test_expect_success 'stash with pathspec matching multiple paths' '
+ echo original >file &&
+ echo original >other-file &&
+ git commit -m "two" file other-file &&
+ echo modified >file &&
+ echo modified >other-file &&
+ git stash push -- "*file" &&
+ echo original >expect &&
+ test_cmp expect file &&
+ test_cmp expect other-file &&
+ git stash pop &&
+ echo modified >expect &&
+ test_cmp expect file &&
+ test_cmp expect other-file
+'
+
test_expect_success 'stash push -p with pathspec shows no changes only once' '
>foo &&
git add foo &&
Thomas Gummerer (6):
stash: introduce push verb
stash: add test for the create command line arguments
stash: refactor stash_create
stash: teach 'push' (and 'create_stash') to honor pathspec
stash: use stash_push for no verb form
stash: allow pathspecs in the no verb form
Documentation/git-stash.txt | 25 +++++--
git-stash.sh | 116 +++++++++++++++++++++++++------
t/t3903-stash.sh | 138 ++++++++++++++++++++++++++++++++++++-
t/t3905-stash-include-untracked.sh | 26 +++++++
4 files changed, 275 insertions(+), 30 deletions(-)
--
2.12.0.428.g67fe103aa
^ permalink raw reply related [flat|nested] 61+ messages in thread
* [PATCH v8 1/6] stash: introduce push verb
2017-02-28 20:33 ` [PATCH v8 0/6] stash: support pathspec argument Thomas Gummerer
@ 2017-02-28 20:33 ` Thomas Gummerer
2017-02-28 20:33 ` [PATCH v8 2/6] stash: add test for the create command line arguments Thomas Gummerer
` (4 subsequent siblings)
5 siblings, 0 replies; 61+ messages in thread
From: Thomas Gummerer @ 2017-02-28 20:33 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Johannes Schindelin, sunny,
Jakub Narębski, Matthieu Moy, Thomas Gummerer
Introduce a new git stash push verb in addition to git stash save. The
push verb is used to transition from the current command line arguments
to a more conventional way, in which the message is given as an argument
to the -m option.
This allows us to have pathspecs at the end of the command line
arguments like other Git commands do, so that the user can say which
subset of paths to stash (and leave others behind).
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
Documentation/git-stash.txt | 3 +++
git-stash.sh | 46 ++++++++++++++++++++++++++++++++++++++++++---
t/t3903-stash.sh | 9 +++++++++
3 files changed, 55 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 2e9e344cd7..d240df4af7 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -15,6 +15,8 @@ SYNOPSIS
'git stash' branch <branchname> [<stash>]
'git stash' [save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
[-u|--include-untracked] [-a|--all] [<message>]]
+'git stash' push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
+ [-u|--include-untracked] [-a|--all] [-m|--message <message>]]
'git stash' clear
'git stash' create [<message>]
'git stash' store [-m|--message <message>] [-q|--quiet] <commit>
@@ -46,6 +48,7 @@ OPTIONS
-------
save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [<message>]::
+push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [-m|--message <message>]::
Save your local modifications to a new 'stash' and roll them
back to HEAD (in the working tree and in the index).
diff --git a/git-stash.sh b/git-stash.sh
index 10c284d1aa..8365ebba2a 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -9,6 +9,8 @@ USAGE="list [<options>]
or: $dashless branch <branchname> [<stash>]
or: $dashless [save [--patch] [-k|--[no-]keep-index] [-q|--quiet]
[-u|--include-untracked] [-a|--all] [<message>]]
+ or: $dashless push [--patch] [-k|--[no-]keep-index] [-q|--quiet]
+ [-u|--include-untracked] [-a|--all] [-m <message>]
or: $dashless clear"
SUBDIRECTORY_OK=Yes
@@ -189,10 +191,11 @@ store_stash () {
return $ret
}
-save_stash () {
+push_stash () {
keep_index=
patch_mode=
untracked=
+ stash_msg=
while test $# != 0
do
case "$1" in
@@ -216,6 +219,11 @@ save_stash () {
-a|--all)
untracked=all
;;
+ -m|--message)
+ shift
+ test -z ${1+x} && usage
+ stash_msg=$1
+ ;;
--help)
show_help
;;
@@ -251,8 +259,6 @@ save_stash () {
die "$(gettext "Can't use --patch and --include-untracked or --all at the same time")"
fi
- stash_msg="$*"
-
git update-index -q --refresh
if no_changes
then
@@ -291,6 +297,36 @@ save_stash () {
fi
}
+save_stash () {
+ push_options=
+ while test $# != 0
+ do
+ case "$1" in
+ --)
+ shift
+ break
+ ;;
+ -*)
+ # pass all options through to push_stash
+ push_options="$push_options $1"
+ ;;
+ *)
+ break
+ ;;
+ esac
+ shift
+ done
+
+ stash_msg="$*"
+
+ if test -z "$stash_msg"
+ then
+ push_stash $push_options
+ else
+ push_stash $push_options -m "$stash_msg"
+ fi
+}
+
have_stash () {
git rev-parse --verify --quiet $ref_stash >/dev/null
}
@@ -617,6 +653,10 @@ save)
shift
save_stash "$@"
;;
+push)
+ shift
+ push_stash "$@"
+ ;;
apply)
shift
apply_stash "$@"
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 2de3e18ce6..3577115807 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -775,4 +775,13 @@ test_expect_success 'stash is not confused by partial renames' '
test_path_is_missing file
'
+test_expect_success 'push -m shows right message' '
+ >foo &&
+ git add foo &&
+ git stash push -m "test message" &&
+ echo "stash@{0}: On master: test message" >expect &&
+ git stash list -1 >actual &&
+ test_cmp expect actual
+'
+
test_done
--
2.12.0.428.g67fe103aa
^ permalink raw reply related [flat|nested] 61+ messages in thread
* [PATCH v8 2/6] stash: add test for the create command line arguments
2017-02-28 20:33 ` [PATCH v8 0/6] stash: support pathspec argument Thomas Gummerer
2017-02-28 20:33 ` [PATCH v8 1/6] stash: introduce push verb Thomas Gummerer
@ 2017-02-28 20:33 ` Thomas Gummerer
2017-02-28 20:33 ` [PATCH v8 3/6] stash: refactor stash_create Thomas Gummerer
` (3 subsequent siblings)
5 siblings, 0 replies; 61+ messages in thread
From: Thomas Gummerer @ 2017-02-28 20:33 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Johannes Schindelin, sunny,
Jakub Narębski, Matthieu Moy, Thomas Gummerer
Currently there is no test showing the expected behaviour of git stash
create's command line arguments. Add a test for that to show the
current expected behaviour and to make sure future refactorings don't
break those expectations.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
t/t3903-stash.sh | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 3577115807..ffe3549ea5 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -784,4 +784,22 @@ test_expect_success 'push -m shows right message' '
test_cmp expect actual
'
+test_expect_success 'create stores correct message' '
+ >foo &&
+ git add foo &&
+ STASH_ID=$(git stash create "create test message") &&
+ echo "On master: create test message" >expect &&
+ git show --pretty=%s -s ${STASH_ID} >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'create with multiple arguments for the message' '
+ >foo &&
+ git add foo &&
+ STASH_ID=$(git stash create test untracked) &&
+ echo "On master: test untracked" >expect &&
+ git show --pretty=%s -s ${STASH_ID} >actual &&
+ test_cmp expect actual
+'
+
test_done
--
2.12.0.428.g67fe103aa
^ permalink raw reply related [flat|nested] 61+ messages in thread
* [PATCH v8 3/6] stash: refactor stash_create
2017-02-28 20:33 ` [PATCH v8 0/6] stash: support pathspec argument Thomas Gummerer
2017-02-28 20:33 ` [PATCH v8 1/6] stash: introduce push verb Thomas Gummerer
2017-02-28 20:33 ` [PATCH v8 2/6] stash: add test for the create command line arguments Thomas Gummerer
@ 2017-02-28 20:33 ` Thomas Gummerer
2017-02-28 20:33 ` [PATCH v8 4/6] stash: teach 'push' (and 'create_stash') to honor pathspec Thomas Gummerer
` (2 subsequent siblings)
5 siblings, 0 replies; 61+ messages in thread
From: Thomas Gummerer @ 2017-02-28 20:33 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Johannes Schindelin, sunny,
Jakub Narębski, Matthieu Moy, Thomas Gummerer
Refactor the internal stash_create function to use a -m flag for
specifying the message and -u flag to indicate whether untracked files
should be added to the stash.
This makes it easier to pass a pathspec argument to stash_create in the
next patch.
The user interface for git stash create stays the same.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
git-stash.sh | 22 ++++++++++++++++++----
1 file changed, 18 insertions(+), 4 deletions(-)
diff --git a/git-stash.sh b/git-stash.sh
index 8365ebba2a..ef5d1b45be 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -58,8 +58,22 @@ clear_stash () {
}
create_stash () {
- stash_msg="$1"
- untracked="$2"
+ stash_msg=
+ untracked=
+ while test $# != 0
+ do
+ case "$1" in
+ -m|--message)
+ shift
+ stash_msg=${1?"BUG: create_stash () -m requires an argument"}
+ ;;
+ -u|--include-untracked)
+ shift
+ untracked=${1?"BUG: create_stash () -u requires an argument"}
+ ;;
+ esac
+ shift
+ done
git update-index -q --refresh
if no_changes
@@ -268,7 +282,7 @@ push_stash () {
git reflog exists $ref_stash ||
clear_stash || die "$(gettext "Cannot initialize stash")"
- create_stash "$stash_msg" $untracked
+ create_stash -m "$stash_msg" -u "$untracked"
store_stash -m "$stash_msg" -q $w_commit ||
die "$(gettext "Cannot save the current status")"
say "$(eval_gettext "Saved working directory and index state \$stash_msg")"
@@ -667,7 +681,7 @@ clear)
;;
create)
shift
- create_stash "$*" && echo "$w_commit"
+ create_stash -m "$*" && echo "$w_commit"
;;
store)
shift
--
2.12.0.428.g67fe103aa
^ permalink raw reply related [flat|nested] 61+ messages in thread
* [PATCH v8 4/6] stash: teach 'push' (and 'create_stash') to honor pathspec
2017-02-28 20:33 ` [PATCH v8 0/6] stash: support pathspec argument Thomas Gummerer
` (2 preceding siblings ...)
2017-02-28 20:33 ` [PATCH v8 3/6] stash: refactor stash_create Thomas Gummerer
@ 2017-02-28 20:33 ` Thomas Gummerer
2017-02-28 22:15 ` Junio C Hamano
2017-02-28 20:33 ` [PATCH v8 5/6] stash: use stash_push for no verb form Thomas Gummerer
2017-02-28 20:33 ` [PATCH v8 6/6] stash: allow pathspecs in the " Thomas Gummerer
5 siblings, 1 reply; 61+ messages in thread
From: Thomas Gummerer @ 2017-02-28 20:33 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Johannes Schindelin, sunny,
Jakub Narębski, Matthieu Moy, 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 <gitster@pobox.com>
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
Documentation/git-stash.txt | 9 +++-
git-stash.sh | 39 +++++++++++-----
t/t3903-stash.sh | 92 ++++++++++++++++++++++++++++++++++++++
t/t3905-stash-include-untracked.sh | 26 +++++++++++
4 files changed, 154 insertions(+), 12 deletions(-)
diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index d240df4af7..88369ed8b6 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -17,6 +17,7 @@ SYNOPSIS
[-u|--include-untracked] [-a|--all] [<message>]]
'git stash' push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
[-u|--include-untracked] [-a|--all] [-m|--message <message>]]
+ [--] [<pathspec>...]
'git stash' clear
'git stash' create [<message>]
'git stash' store [-m|--message <message>] [-q|--quiet] <commit>
@@ -48,7 +49,7 @@ OPTIONS
-------
save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [<message>]::
-push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [-m|--message <message>]::
+push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [-m|--message <message>] [--] [<pathspec>...]::
Save your local modifications to a new 'stash' and roll them
back to HEAD (in the working tree and in the index).
@@ -58,6 +59,12 @@ push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q
only <message> 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 ef5d1b45be..5892abafa3 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -11,6 +11,7 @@ USAGE="list [<options>]
[-u|--include-untracked] [-a|--all] [<message>]]
or: $dashless push [--patch] [-k|--[no-]keep-index] [-q|--quiet]
[-u|--include-untracked] [-a|--all] [-m <message>]
+ [-- <pathspec>...]
or: $dashless clear"
SUBDIRECTORY_OK=Yes
@@ -35,15 +36,15 @@ else
fi
no_changes () {
- git diff-index --quiet --cached HEAD --ignore-submodules -- &&
- git diff-files --quiet --ignore-submodules &&
+ git diff-index --quiet --cached HEAD --ignore-submodules -- "$@" &&
+ git diff-files --quiet --ignore-submodules -- "$@" &&
(test -z "$untracked" || test -z "$(untracked_files)")
}
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 () {
@@ -71,12 +72,16 @@ create_stash () {
shift
untracked=${1?"BUG: create_stash () -u requires an argument"}
;;
+ --)
+ shift
+ break
+ ;;
esac
shift
done
git update-index -q --refresh
- if no_changes
+ if no_changes "$@"
then
exit 0
fi
@@ -108,7 +113,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" &&
@@ -131,7 +136,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"
@@ -145,7 +150,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) ||
@@ -273,27 +278,39 @@ push_stash () {
die "$(gettext "Can't use --patch and --include-untracked or --all at the same time")"
fi
+ test -n "$untracked" || git ls-files --error-unmatch -- "$@" >/dev/null || exit 1
+
git update-index -q --refresh
- if no_changes
+ if no_changes "$@"
then
say "$(gettext "No local changes to save")"
exit 0
fi
+
git reflog exists $ref_stash ||
clear_stash || die "$(gettext "Cannot initialize stash")"
- create_stash -m "$stash_msg" -u "$untracked"
+ create_stash -m "$stash_msg" -u "$untracked" -- "$@"
store_stash -m "$stash_msg" -q $w_commit ||
die "$(gettext "Cannot save the current status")"
say "$(eval_gettext "Saved working directory and index state \$stash_msg")"
if test -z "$patch_mode"
then
- git reset --hard ${GIT_QUIET:+-q}
+ if test $# != 0
+ then
+ git reset ${GIT_QUIET:+-q} -- "$@"
+ git ls-files -z --modified -- "$@" |
+ git checkout-index -z --force --stdin
+ git checkout ${GIT_QUIET:+-q} HEAD -- $(git ls-files -z --modified "$@")
+ git clean --force ${GIT_QUIET:+-q} -d -- "$@"
+ else
+ git reset --hard ${GIT_QUIET:+-q}
+ fi
test "$untracked" = "all" && CLEAN_X_OPTION=-x || CLEAN_X_OPTION=
if test -n "$untracked"
then
- git clean --force --quiet -d $CLEAN_X_OPTION
+ git clean --force --quiet -d $CLEAN_X_OPTION -- "$@"
fi
if test "$keep_index" = "t" && test -n "$i_tree"
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index ffe3549ea5..f934993263 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -802,4 +802,96 @@ test_expect_success 'create with multiple arguments for the message' '
test_cmp expect actual
'
+test_expect_success 'stash -- <pathspec> stashes and restores the file' '
+ >foo &&
+ >bar &&
+ git add foo bar &&
+ git stash push -- foo &&
+ test_path_is_file bar &&
+ test_path_is_missing foo &&
+ git stash pop &&
+ test_path_is_file foo &&
+ test_path_is_file bar
+'
+
+test_expect_success 'stash with multiple pathspec arguments' '
+ >foo &&
+ >bar &&
+ >extra &&
+ git add foo bar extra &&
+ git stash push -- foo bar &&
+ test_path_is_missing bar &&
+ test_path_is_missing foo &&
+ test_path_is_file extra &&
+ git stash pop &&
+ test_path_is_file foo &&
+ test_path_is_file bar &&
+ test_path_is_file extra
+'
+
+test_expect_success 'stash with file including $IFS character' '
+ >"foo bar" &&
+ >foo &&
+ >bar &&
+ git add foo* &&
+ git stash push -- "foo b*" &&
+ test_path_is_missing "foo bar" &&
+ test_path_is_file foo &&
+ test_path_is_file bar &&
+ git stash pop &&
+ test_path_is_file "foo bar" &&
+ test_path_is_file foo &&
+ test_path_is_file bar
+'
+
+test_expect_success 'stash with pathspec matching multiple paths' '
+ echo original >file &&
+ echo original >other-file &&
+ git commit -m "two" file other-file &&
+ echo modified >file &&
+ echo modified >other-file &&
+ git stash push -- "*file" &&
+ echo original >expect &&
+ test_cmp expect file &&
+ test_cmp expect other-file &&
+ git stash pop &&
+ echo modified >expect &&
+ test_cmp expect file &&
+ test_cmp expect other-file
+'
+
+test_expect_success 'stash push -p with pathspec shows no changes only once' '
+ >foo &&
+ git add foo &&
+ git commit -m "tmp" &&
+ git stash push -p foo >actual &&
+ echo "No local changes to save" >expect &&
+ git reset --hard HEAD~ &&
+ test_cmp expect actual
+'
+
+test_expect_success 'stash push with pathspec shows no changes when there are none' '
+ >foo &&
+ git add foo &&
+ git commit -m "tmp" &&
+ git stash push foo >actual &&
+ echo "No local changes to save" >expect &&
+ git reset --hard HEAD~ &&
+ test_cmp expect actual
+'
+
+test_expect_success 'stash push with pathspec not in the repository errors out' '
+ >untracked &&
+ test_must_fail git stash push untracked &&
+ test_path_is_file untracked
+'
+
+test_expect_success 'untracked files are left in place when -u is not given' '
+ >file &&
+ git add file &&
+ >untracked &&
+ git stash push file &&
+ test_path_is_file untracked
+'
+
test_done
diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh
index f372fc8ca8..193adc7b68 100755
--- a/t/t3905-stash-include-untracked.sh
+++ b/t/t3905-stash-include-untracked.sh
@@ -185,4 +185,30 @@ test_expect_success 'stash save --all is stash poppable' '
test -s .gitignore
'
+test_expect_success 'stash push --include-untracked with pathspec' '
+ >foo &&
+ >bar &&
+ git stash push --include-untracked -- foo &&
+ test_path_is_file bar &&
+ test_path_is_missing foo &&
+ git stash pop &&
+ test_path_is_file bar &&
+ test_path_is_file foo
+'
+
+test_expect_success 'stash push with $IFS character' '
+ >"foo bar" &&
+ >foo &&
+ >bar &&
+ git add foo* &&
+ git stash push --include-untracked -- "foo b*" &&
+ test_path_is_missing "foo bar" &&
+ test_path_is_file foo &&
+ test_path_is_file bar &&
+ git stash pop &&
+ test_path_is_file "foo bar" &&
+ test_path_is_file foo &&
+ test_path_is_file bar
+'
+
test_done
--
2.12.0.428.g67fe103aa
^ permalink raw reply related [flat|nested] 61+ messages in thread
* Re: [PATCH v8 4/6] stash: teach 'push' (and 'create_stash') to honor pathspec
2017-02-28 20:33 ` [PATCH v8 4/6] stash: teach 'push' (and 'create_stash') to honor pathspec Thomas Gummerer
@ 2017-02-28 22:15 ` Junio C Hamano
2017-03-01 21:57 ` Thomas Gummerer
0 siblings, 1 reply; 61+ messages in thread
From: Junio C Hamano @ 2017-02-28 22:15 UTC (permalink / raw)
To: Thomas Gummerer
Cc: git, Jeff King, Johannes Schindelin, sunny, Jakub Narębski,
Matthieu Moy
Thomas Gummerer <t.gummerer@gmail.com> writes:
> + git reset ${GIT_QUIET:+-q} -- "$@"
> + git ls-files -z --modified -- "$@" |
> + git checkout-index -z --force --stdin
> + git checkout ${GIT_QUIET:+-q} HEAD -- $(git ls-files -z --modified "$@")
I think you forgot to remove this line, whose correction was added
as two lines immediately before it. I'll remove it while queuing.
> + git clean --force ${GIT_QUIET:+-q} -d -- "$@"
Thanks.
^ permalink raw reply [flat|nested] 61+ messages in thread
* Re: [PATCH v8 4/6] stash: teach 'push' (and 'create_stash') to honor pathspec
2017-02-28 22:15 ` Junio C Hamano
@ 2017-03-01 21:57 ` Thomas Gummerer
2017-03-01 22:43 ` Junio C Hamano
0 siblings, 1 reply; 61+ messages in thread
From: Thomas Gummerer @ 2017-03-01 21:57 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Johannes Schindelin, sunny, Jakub Narębski,
Matthieu Moy
On 02/28, Junio C Hamano wrote:
> Thomas Gummerer <t.gummerer@gmail.com> writes:
>
> > + git reset ${GIT_QUIET:+-q} -- "$@"
> > + git ls-files -z --modified -- "$@" |
> > + git checkout-index -z --force --stdin
> > + git checkout ${GIT_QUIET:+-q} HEAD -- $(git ls-files -z --modified "$@")
>
> I think you forgot to remove this line, whose correction was added
> as two lines immediately before it. I'll remove it while queuing.
Yes, sorry. What you queued looks good to me, thanks!
> > + git clean --force ${GIT_QUIET:+-q} -d -- "$@"
>
> Thanks.
^ permalink raw reply [flat|nested] 61+ messages in thread
* Re: [PATCH v8 4/6] stash: teach 'push' (and 'create_stash') to honor pathspec
2017-03-01 21:57 ` Thomas Gummerer
@ 2017-03-01 22:43 ` Junio C Hamano
0 siblings, 0 replies; 61+ messages in thread
From: Junio C Hamano @ 2017-03-01 22:43 UTC (permalink / raw)
To: Thomas Gummerer
Cc: git, Jeff King, Johannes Schindelin, sunny, Jakub Narębski,
Matthieu Moy
Thomas Gummerer <t.gummerer@gmail.com> writes:
> On 02/28, Junio C Hamano wrote:
>> Thomas Gummerer <t.gummerer@gmail.com> writes:
>>
>> > + git reset ${GIT_QUIET:+-q} -- "$@"
>> > + git ls-files -z --modified -- "$@" |
>> > + git checkout-index -z --force --stdin
>> > + git checkout ${GIT_QUIET:+-q} HEAD -- $(git ls-files -z --modified "$@")
>>
>> I think you forgot to remove this line, whose correction was added
>> as two lines immediately before it. I'll remove it while queuing.
>
> Yes, sorry. What you queued looks good to me, thanks!
Thanks for double-checking, and thanks for working on the topic. I
think this is ready for 'next'.
^ permalink raw reply [flat|nested] 61+ messages in thread
* [PATCH v8 5/6] stash: use stash_push for no verb form
2017-02-28 20:33 ` [PATCH v8 0/6] stash: support pathspec argument Thomas Gummerer
` (3 preceding siblings ...)
2017-02-28 20:33 ` [PATCH v8 4/6] stash: teach 'push' (and 'create_stash') to honor pathspec Thomas Gummerer
@ 2017-02-28 20:33 ` Thomas Gummerer
2017-02-28 20:33 ` [PATCH v8 6/6] stash: allow pathspecs in the " Thomas Gummerer
5 siblings, 0 replies; 61+ messages in thread
From: Thomas Gummerer @ 2017-02-28 20:33 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Johannes Schindelin, sunny,
Jakub Narębski, Matthieu Moy, Thomas Gummerer
Now that we have stash_push, which accepts pathspec arguments, use
it instead of stash_save in git stash without any additional verbs.
Previously we allowed git stash -- -message, which is no longer allowed
after this patch. Messages starting with a hyphen was allowed since
3c2eb80f, ("stash: simplify defaulting to "save" and reject unknown
options"). However it was never the intent to allow that, but rather it
was allowed accidentally.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
Documentation/git-stash.txt | 8 ++++----
git-stash.sh | 16 ++++++++--------
t/t3903-stash.sh | 4 +---
3 files changed, 13 insertions(+), 15 deletions(-)
diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 88369ed8b6..4d8d30f179 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -13,11 +13,11 @@ SYNOPSIS
'git stash' drop [-q|--quiet] [<stash>]
'git stash' ( pop | apply ) [--index] [-q|--quiet] [<stash>]
'git stash' branch <branchname> [<stash>]
-'git stash' [save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
- [-u|--include-untracked] [-a|--all] [<message>]]
-'git stash' push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
+'git stash' save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
+ [-u|--include-untracked] [-a|--all] [<message>]
+'git stash' [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
[-u|--include-untracked] [-a|--all] [-m|--message <message>]]
- [--] [<pathspec>...]
+ [--] [<pathspec>...]]
'git stash' clear
'git stash' create [<message>]
'git stash' store [-m|--message <message>] [-q|--quiet] <commit>
diff --git a/git-stash.sh b/git-stash.sh
index 5892abafa3..f3c2f86804 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -7,11 +7,11 @@ USAGE="list [<options>]
or: $dashless drop [-q|--quiet] [<stash>]
or: $dashless ( pop | apply ) [--index] [-q|--quiet] [<stash>]
or: $dashless branch <branchname> [<stash>]
- or: $dashless [save [--patch] [-k|--[no-]keep-index] [-q|--quiet]
- [-u|--include-untracked] [-a|--all] [<message>]]
- or: $dashless push [--patch] [-k|--[no-]keep-index] [-q|--quiet]
- [-u|--include-untracked] [-a|--all] [-m <message>]
- [-- <pathspec>...]
+ or: $dashless save [--patch] [-k|--[no-]keep-index] [-q|--quiet]
+ [-u|--include-untracked] [-a|--all] [<message>]
+ or: $dashless [push [--patch] [-k|--[no-]keep-index] [-q|--quiet]
+ [-u|--include-untracked] [-a|--all] [-m <message>]
+ [-- <pathspec>...]]
or: $dashless clear"
SUBDIRECTORY_OK=Yes
@@ -658,7 +658,7 @@ apply_to_branch () {
}
PARSE_CACHE='--not-parsed'
-# The default command is "save" if nothing but options are given
+# The default command is "push" if nothing but options are given
seen_non_option=
for opt
do
@@ -668,7 +668,7 @@ do
esac
done
-test -n "$seen_non_option" || set "save" "$@"
+test -n "$seen_non_option" || set "push" "$@"
# Main command set
case "$1" in
@@ -719,7 +719,7 @@ branch)
*)
case $# in
0)
- save_stash &&
+ push_stash &&
say "$(gettext "(To restore them type \"git stash apply\")")"
;;
*)
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index f934993263..e875fe8259 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -274,9 +274,7 @@ test_expect_success 'stash --invalid-option' '
git add file2 &&
test_must_fail git stash --invalid-option &&
test_must_fail git stash save --invalid-option &&
- test bar5,bar6 = $(cat file),$(cat file2) &&
- git stash -- -message-starting-with-dash &&
- test bar,bar2 = $(cat file),$(cat file2)
+ test bar5,bar6 = $(cat file),$(cat file2)
'
test_expect_success 'stash an added file' '
--
2.12.0.428.g67fe103aa
^ permalink raw reply related [flat|nested] 61+ messages in thread
* [PATCH v8 6/6] stash: allow pathspecs in the no verb form
2017-02-28 20:33 ` [PATCH v8 0/6] stash: support pathspec argument Thomas Gummerer
` (4 preceding siblings ...)
2017-02-28 20:33 ` [PATCH v8 5/6] stash: use stash_push for no verb form Thomas Gummerer
@ 2017-02-28 20:33 ` Thomas Gummerer
5 siblings, 0 replies; 61+ messages in thread
From: Thomas Gummerer @ 2017-02-28 20:33 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Johannes Schindelin, sunny,
Jakub Narębski, Matthieu Moy, Thomas Gummerer
Now that stash_push is used in the no verb form of stash, allow
specifying the command line for this form as well. Always use -- to
disambiguate pathspecs from other non-option arguments.
Also make git stash -p an alias for git stash push -p. This allows
users to use git stash -p <pathspec>.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
Documentation/git-stash.txt | 11 +++++++----
git-stash.sh | 3 +++
t/t3903-stash.sh | 15 +++++++++++++++
3 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 4d8d30f179..70191d06b6 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -54,10 +54,13 @@ push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q
Save your local modifications to a new 'stash' and roll them
back to HEAD (in the working tree and in the index).
The <message> part is optional and gives
- the description along with the stashed state. For quickly making
- a snapshot, you can omit _both_ "save" and <message>, but giving
- only <message> does not trigger this action to prevent a misspelled
- subcommand from making an unwanted stash.
+ the description along with the stashed state.
++
+For quickly making a snapshot, you can omit "push". In this mode,
+non-option arguments are not allowed to prevent a misspelled
+subcommand from making an unwanted stash. The two exceptions to this
+are `stash -p` which acts as alias for `stash push -p` and pathspecs,
+which are allowed after a double hyphen `--` for disambiguation.
+
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
diff --git a/git-stash.sh b/git-stash.sh
index f3c2f86804..207c8126f4 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -657,12 +657,15 @@ apply_to_branch () {
}
}
+test "$1" = "-p" && set "push" "$@"
+
PARSE_CACHE='--not-parsed'
# The default command is "push" if nothing but options are given
seen_non_option=
for opt
do
case "$opt" in
+ --) break ;;
-*) ;;
*) seen_non_option=t; break ;;
esac
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index e875fe8259..89877e4b52 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -892,4 +892,19 @@ test_expect_success 'untracked files are left in place when -u is not given' '
test_path_is_file untracked
'
+test_expect_success 'stash without verb with pathspec' '
+ >"foo bar" &&
+ >foo &&
+ >bar &&
+ git add foo* &&
+ git stash -- "foo b*" &&
+ test_path_is_missing "foo bar" &&
+ test_path_is_file foo &&
+ test_path_is_file bar &&
+ git stash pop &&
+ test_path_is_file "foo bar" &&
+ test_path_is_file foo &&
+ test_path_is_file bar
+'
+
test_done
--
2.12.0.428.g67fe103aa
^ permalink raw reply related [flat|nested] 61+ messages in thread