{"thread":{"id":"44933","subject":"[PATCH 0/3] stash: support filename argument","startedAt":"2017-01-21T20:08:02Z","lastAt":"2017-02-14T21:36:06Z","messageCount":57,"participants":["Thomas Gummerer","Øyvind A. Holm","Junio C Hamano","Johannes Schindelin","Jakub Narębski","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"309900","messageId":"20170121200804.19009-1-t.gummerer@gmail.com","threadId":"44933","inReplyTo":null,"subject":"[PATCH 0/3] stash: support filename argument","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2017-01-21T20:08:01Z","receivedAt":"2017-01-21T20:08:02Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"This is the first try to implement the RFC I posted a week ago [1]. It\nintroduces a new push verb for git stash. I couldn't come up with\nany better name that wasn't already taken. If anyone has ideas I'd be\nvery happy to hear them.\n\nThanks everyone for your input in the previous thread.\n\n[1]: https://public-inbox.org/git/20170115142542.11999-1-t.gummerer@gmail.com/T/\n\nThomas Gummerer (3):\n Documentation/stash: remove mention of git reset --hard\n stash: introduce push verb\n stash: support filename argument\n\n Documentation/git-stash.txt | 13 ++++-\n git-stash.sh | 123 ++++++++++++++++++++++++++++++++++++++++----\n t/t3903-stash.sh | 36 +++++++++++++\n 3 files changed, 159 insertions(+), 13 deletions(-)\n\n-- \n2.11.0.483.g087da7b7c\n\n"},{"id":"309901","messageId":"20170121200804.19009-2-t.gummerer@gmail.com","threadId":"44933","inReplyTo":"20170121200804.19009-1-t.gummerer@gmail.com","subject":"[PATCH 1/3] Documentation/stash: remove mention of git reset --hard","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2017-01-21T20:08:02Z","receivedAt":"2017-01-21T20:08:06Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"Don't mention git reset --hard in the documentation for git stash save.\nIt's an implementation detail that doesn't matter to the end user and\nthus shouldn't be exposed to them.\n\nSigned-off-by: Thomas Gummerer \n---\n Documentation/git-stash.txt | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt\nindex 2e9cef06e6..0ad5335a3e 100644\n--- a/Documentation/git-stash.txt\n+++ b/Documentation/git-stash.txt\n@@ -47,8 +47,9 @@ OPTIONS\n \n save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] []::\n \n-\tSave your local modifications to a new 'stash', and run `git reset\n-\t--hard` to revert them. The part is optional and gives\n+\tSave your local modifications to a new 'stash', and revert the\n+\tthe changes in the working tree to match the index.\n+\tThe part is optional and gives\n \tthe description along with the stashed state. For quickly making\n \ta snapshot, you can omit _both_ \"save\" and , but giving\n \tonly does not trigger this action to prevent a misspelled\n-- \n2.11.0.483.g087da7b7c\n\n"},{"id":"309902","messageId":"20170121200804.19009-3-t.gummerer@gmail.com","threadId":"44933","inReplyTo":"20170121200804.19009-1-t.gummerer@gmail.com","subject":"[PATCH 2/3] stash: introduce push verb","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2017-01-21T20:08:03Z","receivedAt":"2017-01-21T20:08:09Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"Introduce a new git stash push verb in addition to git stash save. The\npush verb is used to transition from the current command line arguments\nto a more conventional way, in which the message is specified after a -m\nparameter instead of being a positional argument.\n\nThis allows introducing a new filename argument to stash single files.\nUsing that as a positional argument is much more consistent with the\nrest of git, than using the positional argument for the message.\n\nSigned-off-by: Thomas Gummerer \n---\n git-stash.sh | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++++---\n t/t3903-stash.sh | 9 +++++++\n 2 files changed, 87 insertions(+), 3 deletions(-)\n\ndiff --git a/git-stash.sh b/git-stash.sh\nindex 10c284d1aa..d6b4ae3290 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -189,10 +189,11 @@ store_stash () {\n \treturn $ret\n }\n \n-save_stash () {\n+push_stash () {\n \tkeep_index=\n \tpatch_mode=\n \tuntracked=\n+\tstash_msg=\n \twhile test $# != 0\n \tdo\n \t\tcase \"$1\" in\n@@ -216,6 +217,10 @@ save_stash () {\n \t\t-a|--all)\n \t\t\tuntracked=all\n \t\t\t;;\n+\t\t-m|--message)\n+\t\t\tshift\n+\t\t\tstash_msg=$1\n+\t\t\t;;\n \t\t--help)\n \t\t\tshow_help\n \t\t\t;;\n@@ -251,8 +256,6 @@ save_stash () {\n \t\tdie \"$(gettext \"Can't use --patch and --include-untracked or --all at the same time\")\"\n \tfi\n \n-\tstash_msg=\"$*\"\n-\n \tgit update-index -q --refresh\n \tif no_changes\n \tthen\n@@ -291,6 +294,74 @@ save_stash () {\n \tfi\n }\n \n+save_stash () {\n+\tpush_options=\n+\twhile test $# != 0\n+\tdo\n+\t\tcase \"$1\" in\n+\t\t-k|--keep-index)\n+\t\t\tpush_options=\"-k $push_options\"\n+\t\t\t;;\n+\t\t--no-keep-index)\n+\t\t\tpush_options=\"--no-keep-index $push_options\"\n+\t\t\t;;\n+\t\t-p|--patch)\n+\t\t\tpush_options=\"-p $push_options\"\n+\t\t\t;;\n+\t\t-q|--quiet)\n+\t\t\tpush_options=\"-q $push_options\"\n+\t\t\t;;\n+\t\t-u|--include-untracked)\n+\t\t\tpush_options=\"-u $push_options\"\n+\t\t\t;;\n+\t\t-a|--all)\n+\t\t\tpush_options=\"-a $push_options\"\n+\t\t\t;;\n+\t\t--help)\n+\t\t\tshow_help\n+\t\t\t;;\n+\t\t--)\n+\t\t\tshift\n+\t\t\tbreak\n+\t\t\t;;\n+\t\t-*)\n+\t\t\toption=\"$1\"\n+\t\t\t# TRANSLATORS: $option is an invalid option, like\n+\t\t\t# `--blah-blah'. The 7 spaces at the beginning of the\n+\t\t\t# second line correspond to \"error: \". So you should line\n+\t\t\t# up the second line with however many characters the\n+\t\t\t# translation of \"error: \" takes in your language. E.g. in\n+\t\t\t# English this is:\n+\t\t\t#\n+\t\t\t# $ git stash save --blah-blah 2>&1 | head -n 2\n+\t\t\t# error: unknown option for 'stash save': --blah-blah\n+\t\t\t# To provide a message, use git stash save -- '--blah-blah'\n+\t\t\teval_gettextln \"error: unknown option for 'stash save': \\$option\n+ To provide a message, use git stash save -- '\\$option'\"\n+\t\t\tusage\n+\t\t\t;;\n+\t\t*)\n+\t\t\tbreak\n+\t\t\t;;\n+\t\tesac\n+\t\tshift\n+\tdone\n+\n+\t# if test -n \"$patch_mode\" && test -n \"$untracked\"\n+\t# then\n+\t# \tdie \"$(gettext \"Can't use --patch and --include-untracked or --all at the same time\")\"\n+\t# fi\n+\n+\tstash_msg=\"$*\"\n+\n+\tif test -z stash_msg\n+\tthen\n+\t\tpush_stash $push_options\n+\telse\n+\t\tpush_stash $push_options -m \"$stash_msg\"\n+\tfi\n+}\n+\n have_stash () {\n \tgit rev-parse --verify --quiet $ref_stash >/dev/null\n }\n@@ -617,6 +688,10 @@ save)\n \tshift\n \tsave_stash \"$@\"\n \t;;\n+push)\n+\tshift\n+\tpush_stash \"$@\"\n+\t;;\n apply)\n \tshift\n \tapply_stash \"$@\"\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 2de3e18ce6..0171b824c9 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -775,4 +775,13 @@ test_expect_success 'stash is not confused by partial renames' '\n \ttest_path_is_missing file\n '\n \n+test_expect_success 'push -m shows right message' '\n+\t>foo &&\n+\tgit add foo &&\n+\tgit stash push -m \"test message\" &&\n+\techo \"stash@{0}: On master: test message\" >expect &&\n+\tgit stash list | head -n 1 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.11.0.483.g087da7b7c\n\n"},{"id":"309903","messageId":"20170121200804.19009-4-t.gummerer@gmail.com","threadId":"44933","inReplyTo":"20170121200804.19009-1-t.gummerer@gmail.com","subject":"[PATCH 3/3] stash: support filename argument","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2017-01-21T20:08:04Z","receivedAt":"2017-01-21T20:08:12Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"While working on a repository, it's often helpful to stash the changes\nof a single or multiple files, and leave others alone. Unfortunately\ngit currently offers no such option. git stash -p can be used to work\naround this, but it's often impractical when there are a lot of changes\nover multiple files.\n\nAdd an optional filename argument to git stash push, which allows for\nstashing a single (or multiple) files.\n\nSigned-off-by: Thomas Gummerer \n---\n Documentation/git-stash.txt | 8 ++++++++\n git-stash.sh | 42 ++++++++++++++++++++++++++++++++++--------\n t/t3903-stash.sh | 27 +++++++++++++++++++++++++++\n 3 files changed, 69 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt\nindex 0ad5335a3e..871a3b246c 100644\n--- a/Documentation/git-stash.txt\n+++ b/Documentation/git-stash.txt\n@@ -15,6 +15,9 @@ SYNOPSIS\n 'git stash' branch []\n 'git stash' [save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n \t [-u|--include-untracked] [-a|--all] []]\n+'git stash' push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n+\t [-u|--include-untracked] [-a|--all] [-m|--message ]]\n+\t [--] [...]\n 'git stash' clear\n 'git stash' create []\n 'git stash' store [-m|--message ] [-q|--quiet] \n@@ -46,6 +49,7 @@ OPTIONS\n -------\n \n save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] []::\n+push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [-m|--message ] [--] [...]::\n \n \tSave your local modifications to a new 'stash', and revert the\n \tthe changes in the working tree to match the index.\n@@ -55,6 +59,10 @@ save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q\n \tonly does not trigger this action to prevent a misspelled\n \tsubcommand from making an unwanted stash.\n +\n+If the paths argument is given in 'git stash push', only these files\n+are put in the new 'stash'. In addition only the indicated files are\n+changed in the working tree to match the index.\n++\n If the `--keep-index` option is used, all changes already added to the\n index are left intact.\n +\ndiff --git a/git-stash.sh b/git-stash.sh\nindex d6b4ae3290..7dcce629bd 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -41,7 +41,7 @@ no_changes () {\n untracked_files () {\n \texcl_opt=--exclude-standard\n \ttest \"$untracked\" = \"all\" && excl_opt=\n-\tgit ls-files -o -z $excl_opt\n+\tgit ls-files -o -z $excl_opt -- $1\n }\n \n clear_stash () {\n@@ -56,6 +56,23 @@ clear_stash () {\n }\n \n create_stash () {\n+\tfiles=\n+\twhile test $# != 0\n+\tdo\n+\t\tcase \"$1\" in\n+\t\t--)\n+\t\t\tshift\n+\t\t\tbreak\n+\t\t\t;;\n+\t\t--files)\n+\t\t\t;;\n+\t\t*)\n+\t\t\tfiles=\"$1 $files\"\n+\t\t\t;;\n+\t\tesac\n+\t\tshift\n+\tdone\n+\n \tstash_msg=\"$1\"\n \tuntracked=\"$2\"\n \n@@ -92,7 +109,7 @@ create_stash () {\n \t\t# Untracked files are stored by themselves in a parentless commit, for\n \t\t# ease of unpacking later.\n \t\tu_commit=$(\n-\t\t\tuntracked_files | (\n+\t\t\tuntracked_files $files | (\n \t\t\t\tGIT_INDEX_FILE=\"$TMPindex\" &&\n \t\t\t\texport GIT_INDEX_FILE &&\n \t\t\t\trm -f \"$TMPindex\" &&\n@@ -115,7 +132,7 @@ create_stash () {\n \t\t\tgit read-tree --index-output=\"$TMPindex\" -m $i_tree &&\n \t\t\tGIT_INDEX_FILE=\"$TMPindex\" &&\n \t\t\texport GIT_INDEX_FILE &&\n-\t\t\tgit diff-index --name-only -z HEAD -- >\"$TMP-stagenames\" &&\n+\t\t\tgit diff-index --name-only -z HEAD -- $files >\"$TMP-stagenames\" &&\n \t\t\tgit update-index -z --add --remove --stdin <\"$TMP-stagenames\" &&\n \t\t\tgit write-tree &&\n \t\t\trm -f \"$TMPindex\"\n@@ -129,7 +146,7 @@ create_stash () {\n \n \t\t# find out what the user wants\n \t\tGIT_INDEX_FILE=\"$TMP-index\" \\\n-\t\t\tgit add--interactive --patch=stash -- &&\n+\t\t\tgit add--interactive --patch=stash -- $files &&\n \n \t\t# state of the working tree\n \t\tw_tree=$(GIT_INDEX_FILE=\"$TMP-index\" git write-tree) ||\n@@ -251,6 +268,8 @@ push_stash () {\n \t\tshift\n \tdone\n \n+\tfiles=\"$*\"\n+\n \tif test -n \"$patch_mode\" && test -n \"$untracked\"\n \tthen\n \t\tdie \"$(gettext \"Can't use --patch and --include-untracked or --all at the same time\")\"\n@@ -265,18 +284,25 @@ push_stash () {\n \tgit reflog exists $ref_stash ||\n \t\tclear_stash || die \"$(gettext \"Cannot initialize stash\")\"\n \n-\tcreate_stash \"$stash_msg\" $untracked\n+\tcreate_stash --files $files -- \"$stash_msg\" \"$untracked\"\n \tstore_stash -m \"$stash_msg\" -q $w_commit ||\n \tdie \"$(gettext \"Cannot save the current status\")\"\n \tsay \"$(eval_gettext \"Saved working directory and index state \\$stash_msg\")\"\n \n \tif test -z \"$patch_mode\"\n \tthen\n-\t\tgit reset --hard ${GIT_QUIET:+-q}\n+\t\tif test -n \"$files\"\n+\t\tthen\n+\t\t\tgit reset -- $files\n+\t\t\tgit checkout HEAD -- $(git ls-files --modified -- $files)\n+\t\t\tgit clean --force --quiet -- $(git ls-files --others -- $files)\n+\t\telse\n+\t\t\tgit reset --hard ${GIT_QUIET:+-q}\n+\t\tfi\n \t\ttest \"$untracked\" = \"all\" && CLEAN_X_OPTION=-x || CLEAN_X_OPTION=\n \t\tif test -n \"$untracked\"\n \t\tthen\n-\t\t\tgit clean --force --quiet -d $CLEAN_X_OPTION\n+\t\t\tgit clean --force --quiet -d $CLEAN_X_OPTION -- $files\n \t\tfi\n \n \t\tif test \"$keep_index\" = \"t\" && test -n \"$i_tree\"\n@@ -702,7 +728,7 @@ clear)\n \t;;\n create)\n \tshift\n-\tcreate_stash \"$*\" && echo \"$w_commit\"\n+\tcreate_stash -- \"$*\" && echo \"$w_commit\"\n \t;;\n store)\n \tshift\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 0171b824c9..3e763ff766 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -784,4 +784,31 @@ test_expect_success 'push -m shows right message' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'stash -- stashes and restores the file' '\n+\t>foo &&\n+\t>bar &&\n+\tgit add foo bar &&\n+\tgit stash push -- foo &&\n+\ttest_path_is_file bar &&\n+\ttest_path_is_missing foo &&\n+\tgit stash pop &&\n+\ttest_path_is_file foo &&\n+\ttest_path_is_file bar\n+'\n+\n+test_expect_success 'stash with multiple filename arguments' '\n+\t>foo &&\n+\t>bar &&\n+\t>extra &&\n+\tgit add foo bar extra &&\n+\tgit stash push -- foo bar &&\n+\ttest_path_is_missing bar &&\n+\ttest_path_is_missing foo &&\n+\ttest_path_is_file extra &&\n+\tgit stash pop &&\n+\ttest_path_is_file foo &&\n+\ttest_path_is_file bar &&\n+\ttest_path_is_file extra\n+'\n+\n test_done\n-- \n2.11.0.483.g087da7b7c\n\n"},{"id":"309913","messageId":"20170122012711.yy6dnjz3vrr7wert@sunbase.org","threadId":"44933","inReplyTo":"20170121200804.19009-2-t.gummerer@gmail.com","subject":"Re: [PATCH 1/3] Documentation/stash: remove mention of git reset --hard","fromName":"Øyvind A. Holm","fromEmail":"sunny@sunbase.org","sentAt":"2017-01-22T01:27:12Z","receivedAt":"2017-01-22T01:27:19Z","isPatch":true,"sender":{"key":"sunny@sunbase.org","avatar":"https://avatars.githubusercontent.com/u/113445?v=4"},"body":"On 2017-01-21 20:08:02, Thomas Gummerer wrote:\n> Don't mention git reset --hard in the documentation for git stash save.\n> It's an implementation detail that doesn't matter to the end user and\n> thus shouldn't be exposed to them.\n> [...]\n> +\tSave your local modifications to a new 'stash', and revert the\n> +\tthe changes in the working tree to match the index.\n\nThe patch contains a duplicated \"the\".\n\nRegards,\nØyvind\n\n+-| Øyvind A. Holm - N 60.37604° E 5.33339° |-+\n| OpenPGP: 0xFB0CBEE894A506E5 - http://www.sunbase.org/pubkey.asc |\n| Fingerprint: A006 05D6 E676 B319 55E2 E77E FB0C BEE8 94A5 06E5 |\n+------------| 42073b1c-e041-11e6-bae1-db5caa6d21d3 |-------------+\n"},{"id":"309992","messageId":"xmqqziihskbi.fsf@gitster.mtv.corp.google.com","threadId":"44933","inReplyTo":"20170121200804.19009-3-t.gummerer@gmail.com","subject":"Re: [PATCH 2/3] stash: introduce push verb","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-23T18:27:45Z","receivedAt":"2017-01-23T18:27:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Gummerer writes:\n\n> +\tstash_msg=\"$*\"\n> +\n> +\tif test -z stash_msg\n\nA dollar-sign is missing here, I think.\n\n> +\tthen\n> +\t\tpush_stash $push_options\n> +\telse\n> +\t\tpush_stash $push_options -m \"$stash_msg\"\n> +\tfi\n> +}\n"},{"id":"309998","messageId":"xmqqinp5sj98.fsf@gitster.mtv.corp.google.com","threadId":"44933","inReplyTo":"20170121200804.19009-4-t.gummerer@gmail.com","subject":"Re: [PATCH 3/3] stash: support filename argument","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-23T18:50:43Z","receivedAt":"2017-01-23T18:50:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Gummerer writes:\n\n> diff --git a/git-stash.sh b/git-stash.sh\n> index d6b4ae3290..7dcce629bd 100755\n> --- a/git-stash.sh\n> +++ b/git-stash.sh\n> @@ -41,7 +41,7 @@ no_changes () {\n> untracked_files () {\n> \texcl_opt=--exclude-standard\n> \ttest \"$untracked\" = \"all\" && excl_opt=\n> -\tgit ls-files -o -z $excl_opt\n> +\tgit ls-files -o -z $excl_opt -- $1\n\nDoes $1 need to be quoted to prevent it from split at $IFS?\n\n> @@ -56,6 +56,23 @@ clear_stash () {\n> }\n> \n> create_stash () {\n> +\tfiles=\n> +\twhile test $# != 0\n> +\tdo\n> +\t\tcase \"$1\" in\n> +\t\t--)\n> +\t\t\tshift\n> +\t\t\tbreak\n> +\t\t\t;;\n> +\t\t--files)\n> +\t\t\t;;\n> +\t\t*)\n> +\t\t\tfiles=\"$1 $files\"\n> +\t\t\t;;\n\nHmph. What is this \"no-op\" option about? Did you mean to say\nsomething like this instead?\n\n\tcase \"$1\" in\n\t...\n\t--file)\n\t\tcase $# in\n\t\t1)\n\t\t\tdie \"--file needs a pathspec\" ;;\n\t\t*)\n\t\t\tshift\n\t\t\tfiles=\"$files$1 \" ;;\n\t\tesac\n\t\t;;\n\n\nAnother thing I noticed. We won't support filenames with embedded\n$IFS characters at all?\n\nI somehow had an impression that the script was carefully done\n(e.g. by using -z option where appropriate) to add such a\nlimitation.\n\nPerhaps we have broken it over time and it no longer matters\n(i.e. there already may be existing breakages), but this troubles\nme somehow.\n\nBy the way, in addition to \"push\" thing that corrects the argument\nconvention by requiring \"-m\" before the message, we need to correct\ncreate_stash that is used internally from \"stash push\" somehow?\n"},{"id":"310082","messageId":"alpine.DEB.2.20.1701241148300.3469@virtualbox","threadId":"44933","inReplyTo":"20170121200804.19009-1-t.gummerer@gmail.com","subject":"Re: [PATCH 0/3] stash: support filename argument","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-01-24T10:56:51Z","receivedAt":"2017-01-24T10:57:15Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Thomas,\n\nOn Sat, 21 Jan 2017, Thomas Gummerer wrote:\n\n> This is the first try to implement the RFC I posted a week ago [1]. It\n> introduces a new push verb for git stash. I couldn't come up with\n> any better name that wasn't already taken. If anyone has ideas I'd be\n> very happy to hear them.\n\nI would have preferred a series of patches that essentially adds a new and\nimproved `save` syntax:\n\ngit stash [save] [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n [-u|--include-untracked] [-a|--all] [-m ]]\n [-- ...]\n\nand keeps the legacy syntax, but deprecates it:\n\ngit stash [save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n [-u|--include-untracked] [-a|--all] []]\n\nThe problem with that is, of course, that 3c2eb80fe3 (stash: simplify\ndefaulting to \"save\" and reject unknown options, 2009-08-18) in its\ninfinite wisdom *already* introduced the `--` separator to drop out of\noption parsing.\n\nOn a positive note, it is a thorn in Git's CUI that `git stash` implies\nthe `save` command, and that `save` is not at all the opposite of `apply`\nor `pop`. Your introduction of the `push` command will fix that flaw, and\nwe can *still* deprecate the `save` command.\n\nCiao,\nJohannes\n"},{"id":"310105","messageId":"c5732a5b-b6a9-2980-3d99-179a93fe6418@gmail.com","threadId":"44933","inReplyTo":"20170121200804.19009-2-t.gummerer@gmail.com","subject":"Re: [PATCH 1/3] Documentation/stash: remove mention of git reset --hard","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2017-01-24T19:51:09Z","receivedAt":"2017-01-24T19:51:28Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"W dniu 21.01.2017 o 21:08, Thomas Gummerer pisze:\n> Don't mention git reset --hard in the documentation for git stash save.\n> It's an implementation detail that doesn't matter to the end user and\n> thus shouldn't be exposed to them.\n> \n> Signed-off-by: Thomas Gummerer \n> ---\n> Documentation/git-stash.txt | 5 +++--\n> 1 file changed, 3 insertions(+), 2 deletions(-)\n> \n> diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt\n> index 2e9cef06e6..0ad5335a3e 100644\n> --- a/Documentation/git-stash.txt\n> +++ b/Documentation/git-stash.txt\n> @@ -47,8 +47,9 @@ OPTIONS\n> \n> save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] []::\n> \n> -\tSave your local modifications to a new 'stash', and run `git reset\n> -\t--hard` to revert them. The part is optional and gives\n> +\tSave your local modifications to a new 'stash', and revert the\n> +\tthe changes in the working tree to match the index.\n\nI think the following might be better:\n\n ..., and set the working tree to match the index.\n\nOr not, as it ignores problem of untracked files.\n\nAnyway, removing internal implementation detail looks like a good idea.\nOTOH the reader should be familiar with what `git reset --hard` does,\nand if not, he knows where to find the information.\n\n> +\tThe part is optional and gives\n> \tthe description along with the stashed state. For quickly making\n> \ta snapshot, you can omit _both_ \"save\" and , but giving\n> \tonly does not trigger this action to prevent a misspelled\n> \n\n-- \nJakub Narębski\n"},{"id":"310106","messageId":"20170124201415.zzyg4vt35vflwjnb@sigill.intra.peff.net","threadId":"44933","inReplyTo":"20170121200804.19009-2-t.gummerer@gmail.com","subject":"Re: [PATCH 1/3] Documentation/stash: remove mention of git reset --hard","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-01-24T20:14:15Z","receivedAt":"2017-01-24T20:14:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jan 21, 2017 at 08:08:02PM +0000, Thomas Gummerer wrote:\n\n> diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt\n> index 2e9cef06e6..0ad5335a3e 100644\n> --- a/Documentation/git-stash.txt\n> +++ b/Documentation/git-stash.txt\n> @@ -47,8 +47,9 @@ OPTIONS\n> \n> save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] []::\n> \n> -\tSave your local modifications to a new 'stash', and run `git reset\n> -\t--hard` to revert them. The part is optional and gives\n> +\tSave your local modifications to a new 'stash', and revert the\n> +\tthe changes in the working tree to match the index.\n> +\tThe part is optional and gives\n\nHrm. \"git reset --hard\" doesn't just make the working tree match the\nindex. It also resets the index to HEAD. So either the original or your\nnew description is wrong.\n\nI think it's the latter. We really do reset the index unless\n--keep-index is specified.\n\nI also wondered if it was worth avoiding the word \"revert\", as \"git\nrevert\" has a much different meaning (as opposed to \"svn revert\", which\ndoes what you're talking about here). But I see that \"git add -i\"\nalready uses the word revert in this way (and there are probably\nothers).\n\nSo it may not be worth worrying about, but \"set\" or \"reset\" probably\nserves the same purpose.\n\n-Peff\n"},{"id":"310158","messageId":"d327cfc5-2f0d-1a67-d5f5-b9b7a06f7570@gmail.com","threadId":"44933","inReplyTo":"20170124201415.zzyg4vt35vflwjnb@sigill.intra.peff.net","subject":"Re: [PATCH 1/3] Documentation/stash: remove mention of git reset --hard","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2017-01-25T13:02:32Z","receivedAt":"2017-01-25T13:02:49Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"W dniu 24.01.2017 o 21:14, Jeff King pisze:\n> On Sat, Jan 21, 2017 at 08:08:02PM +0000, Thomas Gummerer wrote:\n> \n>> diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt\n>> index 2e9cef06e6..0ad5335a3e 100644\n>> --- a/Documentation/git-stash.txt\n>> +++ b/Documentation/git-stash.txt\n>> @@ -47,8 +47,9 @@ OPTIONS\n>> \n>> save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] []::\n>> \n>> -\tSave your local modifications to a new 'stash', and run `git reset\n>> -\t--hard` to revert them. The part is optional and gives\n>> +\tSave your local modifications to a new 'stash', and revert the\n>> +\tthe changes in the working tree to match the index.\n>> +\tThe part is optional and gives\n> \n> Hrm. \"git reset --hard\" doesn't just make the working tree match the\n> index. It also resets the index to HEAD. So either the original or your\n> new description is wrong.\n> \n> I think it's the latter. We really do reset the index unless\n> --keep-index is specified.\n\nI wonder if it is worth mentioning that \"saving local modifications\"\nin 'git stash' means storing both the state of the worktree (of tracked\nfiles in it) _and_ the state of the index, before both get set to\nstate of HEAD.\n\nBest,\n-- \nJakub Narębski\n"},{"id":"310207","messageId":"xmqqfuk6alba.fsf@gitster.mtv.corp.google.com","threadId":"44933","inReplyTo":"20170124201415.zzyg4vt35vflwjnb@sigill.intra.peff.net","subject":"Re: [PATCH 1/3] Documentation/stash: remove mention of git reset --hard","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-25T21:20:25Z","receivedAt":"2017-01-25T21:20:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Sat, Jan 21, 2017 at 08:08:02PM +0000, Thomas Gummerer wrote:\n>\n>> diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt\n>> index 2e9cef06e6..0ad5335a3e 100644\n>> --- a/Documentation/git-stash.txt\n>> +++ b/Documentation/git-stash.txt\n>> @@ -47,8 +47,9 @@ OPTIONS\n>> \n>> save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] []::\n>> \n>> -\tSave your local modifications to a new 'stash', and run `git reset\n>> -\t--hard` to revert them. The part is optional and gives\n>> +\tSave your local modifications to a new 'stash', and revert the\n>> +\tthe changes in the working tree to match the index.\n>> +\tThe part is optional and gives\n>\n> Hrm. \"git reset --hard\" doesn't just make the working tree match the\n> index. It also resets the index to HEAD. So either the original or your\n> new description is wrong.\n>\n> I think it's the latter. We really do reset the index unless\n> --keep-index is specified.\n\nCorrect. So the patch is a net loss. Perhaps not requiring readers\nto know \"reset --hard\" might be an improvement (I personally do not\nthink so), but this loses crucial information from the description.\n\n\tSave your local modifications (both in the working tree and\n\tto the index) to a new 'stash', and resets the index to HEAD\n\tand makes the working tree match the index (i.e. runs \"git\n\treset --hard\").\n\nThat's one-and-a-half lines longer than the original, though.\n"},{"id":"310208","messageId":"xmqqbmuualai.fsf@gitster.mtv.corp.google.com","threadId":"44933","inReplyTo":"d327cfc5-2f0d-1a67-d5f5-b9b7a06f7570@gmail.com","subject":"Re: [PATCH 1/3] Documentation/stash: remove mention of git reset --hard","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-25T21:20:53Z","receivedAt":"2017-01-25T21:21:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narębski writes:\n\n>>> -\tSave your local modifications to a new 'stash', and run `git reset\n>>> -\t--hard` to revert them. The part is optional and gives\n>>> +\tSave your local modifications to a new 'stash', and revert the\n>>> +\tthe changes in the working tree to match the index.\n>>> +\tThe part is optional and gives\n>> \n>> Hrm. \"git reset --hard\" doesn't just make the working tree match the\n>> index. It also resets the index to HEAD. So either the original or your\n>> new description is wrong.\n>> \n>> I think it's the latter. We really do reset the index unless\n>> --keep-index is specified.\n>\n> I wonder if it is worth mentioning that \"saving local modifications\"\n> in 'git stash' means storing both the state of the worktree (of tracked\n> files in it) _and_ the state of the index, before both get set to\n> state of HEAD.\n\nYes, it is, I would say.\n"},{"id":"310490","messageId":"20170128192958.GB31189@hank","threadId":"44933","inReplyTo":"xmqqfuk6alba.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 1/3] Documentation/stash: remove mention of git reset --hard","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2017-01-28T19:30:28Z","receivedAt":"2017-01-28T20:01:37Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 01/25, Junio C Hamano wrote:\n> Jeff King writes:\n> \n> > On Sat, Jan 21, 2017 at 08:08:02PM +0000, Thomas Gummerer wrote:\n> >\n> >> diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt\n> >> index 2e9cef06e6..0ad5335a3e 100644\n> >> --- a/Documentation/git-stash.txt\n> >> +++ b/Documentation/git-stash.txt\n> >> @@ -47,8 +47,9 @@ OPTIONS\n> >> \n> >> save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] []::\n> >> \n> >> -\tSave your local modifications to a new 'stash', and run `git reset\n> >> -\t--hard` to revert them. The part is optional and gives\n> >> +\tSave your local modifications to a new 'stash', and revert the\n> >> +\tthe changes in the working tree to match the index.\n> >> +\tThe part is optional and gives\n> >\n> > Hrm. \"git reset --hard\" doesn't just make the working tree match the\n> > index. It also resets the index to HEAD. So either the original or your\n> > new description is wrong.\n> >\n> > I think it's the latter. We really do reset the index unless\n> > --keep-index is specified.\n> \n> Correct. So the patch is a net loss. Perhaps not requiring readers\n> to know \"reset --hard\" might be an improvement (I personally do not\n> think so), but this loses crucial information from the description.\n> \n> \tSave your local modifications (both in the working tree and\n> \tto the index) to a new 'stash', and resets the index to HEAD\n> \tand makes the working tree match the index (i.e. runs \"git\n> \treset --hard\").\n> \n> That's one-and-a-half lines longer than the original, though.\n\nThanks all who chimed in here. My new description is definitely not\nright. The reason I wanted to change it is part because it's an\nimplementation detail, and part because it's going to be not quite\nright when the filename argument is introduced.\n\nHow about the following:\n\n \tSave your local modifications to a new 'stash' and roll them back\n \tboth in the working tree and in the index.\n\nAs an added bonus this also matches what git stash save -p does.\n"},{"id":"310502","messageId":"20170128235455.clv5x7m6n3mzilyt@sigill.intra.peff.net","threadId":"44933","inReplyTo":"20170128192958.GB31189@hank","subject":"Re: [PATCH 1/3] Documentation/stash: remove mention of git reset --hard","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-01-28T23:54:56Z","receivedAt":"2017-01-28T23:55:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jan 28, 2017 at 07:30:28PM +0000, Thomas Gummerer wrote:\n\n> Thanks all who chimed in here. My new description is definitely not\n> right. The reason I wanted to change it is part because it's an\n> implementation detail, and part because it's going to be not quite\n> right when the filename argument is introduced.\n> \n> How about the following:\n> \n> \tSave your local modifications to a new 'stash' and roll them back\n> \tboth in the working tree and in the index.\n> \n> As an added bonus this also matches what git stash save -p does.\n\nIMHO that is both informative and accurate. You could add:\n \n (unless --keep-index was used)\n\nat the end of the sentence, though I am not sure it is necessary.\n\n-Peff\n"},{"id":"310506","messageId":"20170129123911.GC31189@hank","threadId":"44933","inReplyTo":"xmqqziihskbi.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/3] stash: introduce push verb","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2017-01-29T12:39:11Z","receivedAt":"2017-01-29T12:45:25Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 01/23, Junio C Hamano wrote:\n> Thomas Gummerer writes:\n> \n> > +\tstash_msg=\"$*\"\n> > +\n> > +\tif test -z stash_msg\n> \n> A dollar-sign is missing here, I think.\n\nYes, thanks.\n\n> > +\tthen\n> > +\t\tpush_stash $push_options\n> > +\telse\n> > +\t\tpush_stash $push_options -m \"$stash_msg\"\n> > +\tfi\n> > +}\n\n-- \nThomas\n"},{"id":"310508","messageId":"20170129132926.GD31189@hank","threadId":"44933","inReplyTo":"xmqqinp5sj98.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 3/3] stash: support filename argument","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2017-01-29T13:29:26Z","receivedAt":"2017-01-29T13:38:15Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 01/23, Junio C Hamano wrote:\n> Thomas Gummerer writes:\n> \n> > diff --git a/git-stash.sh b/git-stash.sh\n> > index d6b4ae3290..7dcce629bd 100755\n> > --- a/git-stash.sh\n> > +++ b/git-stash.sh\n> > @@ -41,7 +41,7 @@ no_changes () {\n> > untracked_files () {\n> > \texcl_opt=--exclude-standard\n> > \ttest \"$untracked\" = \"all\" && excl_opt=\n> > -\tgit ls-files -o -z $excl_opt\n> > +\tgit ls-files -o -z $excl_opt -- $1\n> \n> Does $1 need to be quoted to prevent it from split at $IFS?\n\nNot sure, I'll check and add a test for the re-roll.\n\n> > @@ -56,6 +56,23 @@ clear_stash () {\n> > }\n> > \n> > create_stash () {\n> > +\tfiles=\n> > +\twhile test $# != 0\n> > +\tdo\n> > +\t\tcase \"$1\" in\n> > +\t\t--)\n> > +\t\t\tshift\n> > +\t\t\tbreak\n> > +\t\t\t;;\n> > +\t\t--files)\n> > +\t\t\t;;\n> > +\t\t*)\n> > +\t\t\tfiles=\"$1 $files\"\n> > +\t\t\t;;\n> \n> Hmph. What is this \"no-op\" option about? Did you mean to say\n> something like this instead?\n> \n> \tcase \"$1\" in\n> \t...\n> \t--file)\n> \t\tcase $# in\n> \t\t1)\n> \t\t\tdie \"--file needs a pathspec\" ;;\n> \t\t*)\n> \t\t\tshift\n> \t\t\tfiles=\"$files$1 \" ;;\n> \t\tesac\n> \t\t;;\n>\n\nHmm that would require multiple --file arguments to create_stash,\nwhich I wanted to avoid. But probably the correct solution is to\nintroduce a new format for create_stash, which allows a -m before the\nmessage, and uses -- to disambiguate before the file name arguments.\n\nThis would be similar to what Johannes suggested in [1], deprecating\nthe old syntax in git stash create. While this didn't work in git\nstash save, it would work in git stash create, as \"--\" isn't used to\ndisambiguate anything there yet.\n\n> Another thing I noticed. We won't support filenames with embedded\n> $IFS characters at all?\n> \n> I somehow had an impression that the script was carefully done\n> (e.g. by using -z option where appropriate) to add such a\n> limitation.\n> \n> Perhaps we have broken it over time and it no longer matters\n> (i.e. there already may be existing breakages), but this troubles\n> me somehow.\n\nGood point, I didn't think about $IFS characters. Fill fix in the\nnext round.\n\n> By the way, in addition to \"push\" thing that corrects the argument\n> convention by requiring \"-m\" before the message, we need to correct\n> create_stash that is used internally from \"stash push\" somehow?\n\nYeah, I think that would make sense, see above.\n\n[1]: http://public-inbox.org/git/alpine.DEB.2.20.1701241148300.3469@virtualbox/\n"},{"id":"310512","messageId":"20170129201604.30445-4-t.gummerer@gmail.com","threadId":"44933","inReplyTo":"20170129201604.30445-1-t.gummerer@gmail.com","subject":"[PATCH v2 3/4] introduce new format for git stash create","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2017-01-29T20:16:03Z","receivedAt":"2017-01-29T20:23:07Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"git stash create currently supports a positional argument for adding a\nmessage. This is not quite in line with how git commands usually take\ncomments (using a -m flag).\n\nAdd a new syntax for adding a message to git stash create using a -m\nflag. This is with the goal of deprecating the old style git stash\ncreate with positional arguments.\n\nThis also adds a -u argument, for untracked files. This is already used\ninternally as another positional argument, but can now be used from the\ncommand line.\n\nSigned-off-by: Thomas Gummerer \n---\n Documentation/git-stash.txt | 1 +\n git-stash.sh | 50 +++++++++++++++++++++++++++++++++++++++++----\n t/t3903-stash.sh | 18 ++++++++++++++++\n 3 files changed, 65 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt\nindex 0fc23c25ee..0bce33e3fc 100644\n--- a/Documentation/git-stash.txt\n+++ b/Documentation/git-stash.txt\n@@ -17,6 +17,7 @@ SYNOPSIS\n \t [-u|--include-untracked] [-a|--all] []]\n 'git stash' clear\n 'git stash' create []\n+'git stash' create [-m ] [-u|--include-untracked ]\n 'git stash' store [-m|--message ] [-q|--quiet] \n \n DESCRIPTION\ndiff --git a/git-stash.sh b/git-stash.sh\nindex 8528708f61..5f08b43967 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -56,8 +56,50 @@ clear_stash () {\n }\n \n create_stash () {\n-\tstash_msg=\"$1\"\n-\tuntracked=\"$2\"\n+\tstash_msg=\n+\tuntracked=\n+\tnew_style=\n+\twhile test $# != 0\n+\tdo\n+\t\tcase \"$1\" in\n+\t\t-m|--message)\n+\t\t\tshift\n+\t\t\tstash_msg=\"$1\"\n+\t\t\tnew_style=t\n+\t\t\t;;\n+\t\t-u|--include-untracked)\n+\t\t\tshift\n+\t\t\tuntracked=\"$1\"\n+\t\t\tnew_style=t\n+\t\t\t;;\n+\t\t*)\n+\t\t\tif test -n \"$new_style\"\n+\t\t\tthen\n+\t\t\t\techo \"invalid argument\"\n+\t\t\t\toption=\"$1\"\n+\t\t\t\t# TRANSLATORS: $option is an invalid option, like\n+\t\t\t\t# `--blah-blah'. The 7 spaces at the beginning of the\n+\t\t\t\t# second line correspond to \"error: \". So you should line\n+\t\t\t\t# up the second line with however many characters the\n+\t\t\t\t# translation of \"error: \" takes in your language. E.g. in\n+\t\t\t\t# English this is:\n+\t\t\t\t#\n+\t\t\t\t# $ git stash save --blah-blah 2>&1 | head -n 2\n+\t\t\t\t# error: unknown option for 'stash save': --blah-blah\n+\t\t\t\t# To provide a message, use git stash save -- '--blah-blah'\n+\t\t\t\teval_gettextln \"error: unknown option for 'stash create': \\$option\"\n+\t\t\t\tusage\n+\t\t\tfi\n+\t\t\tbreak\n+\t\t\t;;\n+\t\tesac\n+\t\tshift\n+\tdone\n+\n+\tif test -z \"$new_style\"\n+\tthen\n+\t\tstash_msg=\"$*\"\n+\tfi\n \n \tgit update-index -q --refresh\n \tif no_changes\n@@ -265,7 +307,7 @@ push_stash () {\n \tgit reflog exists $ref_stash ||\n \t\tclear_stash || die \"$(gettext \"Cannot initialize stash\")\"\n \n-\tcreate_stash \"$stash_msg\" $untracked\n+\tcreate_stash -m \"$stash_msg\" -u \"$untracked\"\n \tstore_stash -m \"$stash_msg\" -q $w_commit ||\n \tdie \"$(gettext \"Cannot save the current status\")\"\n \tsay \"$(eval_gettext \"Saved working directory and index state \\$stash_msg\")\"\n@@ -697,7 +739,7 @@ clear)\n \t;;\n create)\n \tshift\n-\tcreate_stash \"$*\" && echo \"$w_commit\"\n+\tcreate_stash \"$@\" && echo \"$w_commit\"\n \t;;\n store)\n \tshift\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 0171b824c9..34e9610bb6 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -784,4 +784,22 @@ test_expect_success 'push -m shows right message' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'deprecated version of stash create stores correct message' '\n+\t>foo &&\n+\tgit add foo &&\n+\tSTASH_ID=$(git stash create \"create test message\") &&\n+\techo \"On master: create test message\" >expect &&\n+\tgit show --pretty=%s ${STASH_ID} | head -n1 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'new style stash create stores correct message' '\n+\t>foo &&\n+\tgit add foo &&\n+\tSTASH_ID=$(git stash create -m \"create test message new style\") &&\n+\techo \"On master: create test message new style\" >expect &&\n+\tgit show --pretty=%s ${STASH_ID} | head -n1 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.11.0.297.g9a2118ac0b.dirty\n\n"},{"id":"310513","messageId":"20170129201604.30445-1-t.gummerer@gmail.com","threadId":"44933","inReplyTo":"20170121200804.19009-1-t.gummerer@gmail.com","subject":"[PATCH v2 0/4] stash: create filename argument","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2017-01-29T20:16:00Z","receivedAt":"2017-01-29T20:24:58Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"Previous round is at:\nhttp://public-inbox.org/git/20170121200804.19009-1-t.gummerer@gmail.com/.\nThanks Junio, Peff, Øyvind, Jakub and Johannes for your feedback on\nthe previous round.\n\nChanges since the previous round:\n\n- Re-phrased the Documentation update.\n- Added missing $ in 2/3\n- Added an extra patch introducing a new syntax for git stash create,\n where the message can be specified with the -m flag, instead of as a\n positional argument\n- Filenames with $IFS in their name are now supported. Added a test\n for that as well.\n\nInterdiff below:\n\ndiff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt\nindex 871a3b246c..8306bac397 100644\n--- a/Documentation/git-stash.txt\n+++ b/Documentation/git-stash.txt\n@@ -20,6 +20,8 @@ SYNOPSIS\n \t [--] [...]\n 'git stash' clear\n 'git stash' create []\n+'git stash' create [-m ] [-u|--include-untracked ]\n+\t [-- ...]\n 'git stash' store [-m|--message ] [-q|--quiet] \n \n DESCRIPTION\n@@ -51,8 +53,8 @@ OPTIONS\n save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] []::\n push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [-m|--message ] [--] [...]::\n \n-\tSave your local modifications to a new 'stash', and revert the\n-\tthe changes in the working tree to match the index.\n+\tSave your local modifications to a new 'stash' and roll them\n+\tback both in the working tree and in the index.\n \tThe part is optional and gives\n \tthe description along with the stashed state. For quickly making\n \ta snapshot, you can omit _both_ \"save\" and , but giving\ndiff --git a/git-stash.sh b/git-stash.sh\nindex 7dcce629bd..0072a38b4c 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -56,25 +56,57 @@ clear_stash () {\n }\n \n create_stash () {\n+\tstash_msg=\n+\tuntracked=\n+\tnew_style=\n \tfiles=\n \twhile test $# != 0\n \tdo\n \t\tcase \"$1\" in\n+\t\t-m|--message)\n+\t\t\tshift\n+\t\t\tstash_msg=\"$1\"\n+\t\t\tnew_style=t\n+\t\t\t;;\n+\t\t-u|--include-untracked)\n+\t\t\tshift\n+\t\t\tuntracked=\"$1\"\n+\t\t\tnew_style=t\n+\t\t\t;;\n \t\t--)\n \t\t\tshift\n+\t\t\tfiles=\"$@\"\n+\t\t\tnew_style=t\n \t\t\tbreak\n \t\t\t;;\n-\t\t--files)\n-\t\t\t;;\n \t\t*)\n-\t\t\tfiles=\"$1 $files\"\n+\t\t\tif test -n \"$new_style\"\n+\t\t\tthen\n+\t\t\t\techo \"invalid argument\"\n+\t\t\t\toption=\"$1\"\n+\t\t\t\t# TRANSLATORS: $option is an invalid option, like\n+\t\t\t\t# `--blah-blah'. The 7 spaces at the beginning of the\n+\t\t\t\t# second line correspond to \"error: \". So you should line\n+\t\t\t\t# up the second line with however many characters the\n+\t\t\t\t# translation of \"error: \" takes in your language. E.g. in\n+\t\t\t\t# English this is:\n+\t\t\t\t#\n+\t\t\t\t# $ git stash save --blah-blah 2>&1 | head -n 2\n+\t\t\t\t# error: unknown option for 'stash save': --blah-blah\n+\t\t\t\t# To provide a message, use git stash save -- '--blah-blah'\n+\t\t\t\teval_gettextln \"error: unknown option for 'stash create': \\$option\"\n+\t\t\t\tusage\n+\t\t\tfi\n+\t\t\tbreak\n \t\t\t;;\n \t\tesac\n \t\tshift\n \tdone\n \n-\tstash_msg=\"$1\"\n-\tuntracked=\"$2\"\n+\tif test -z \"$new_style\"\n+\tthen\n+\t\tstash_msg=\"$*\"\n+\tfi\n \n \tgit update-index -q --refresh\n \tif no_changes\n@@ -284,7 +316,7 @@ push_stash () {\n \tgit reflog exists $ref_stash ||\n \t\tclear_stash || die \"$(gettext \"Cannot initialize stash\")\"\n \n-\tcreate_stash --files $files -- \"$stash_msg\" \"$untracked\"\n+\tcreate_stash -m \"$stash_msg\" -u \"$untracked\" -- $files\n \tstore_stash -m \"$stash_msg\" -q $w_commit ||\n \tdie \"$(gettext \"Cannot save the current status\")\"\n \tsay \"$(eval_gettext \"Saved working directory and index state \\$stash_msg\")\"\n@@ -293,9 +325,9 @@ push_stash () {\n \tthen\n \t\tif test -n \"$files\"\n \t\tthen\n-\t\t\tgit reset -- $files\n-\t\t\tgit checkout HEAD -- $(git ls-files --modified -- $files)\n-\t\t\tgit clean --force --quiet -- $(git ls-files --others -- $files)\n+\t\t\tgit ls-files -z -- \"$@\" | xargs -0 git reset --\n+\t\t\tgit ls-files -z --modified -- \"$@\" | xargs -0 git checkout HEAD --\n+\t\t\tgit ls-files -z --others -- \"$@\" | xargs -0 git clean --force --\n \t\telse\n \t\t\tgit reset --hard ${GIT_QUIET:+-q}\n \t\tfi\n@@ -373,14 +405,9 @@ save_stash () {\n \t\tshift\n \tdone\n \n-\t# if test -n \"$patch_mode\" && test -n \"$untracked\"\n-\t# then\n-\t# \tdie \"$(gettext \"Can't use --patch and --include-untracked or --all at the same time\")\"\n-\t# fi\n-\n \tstash_msg=\"$*\"\n \n-\tif test -z stash_msg\n+\tif test -z \"$stash_msg\"\n \tthen\n \t\tpush_stash $push_options\n \telse\n@@ -728,7 +755,7 @@ clear)\n \t;;\n create)\n \tshift\n-\tcreate_stash -- \"$*\" && echo \"$w_commit\"\n+\tcreate_stash \"$@\" && echo \"$w_commit\"\n \t;;\n store)\n \tshift\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 3e763ff766..ca4c44aa9c 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -784,6 +784,24 @@ test_expect_success 'push -m shows right message' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'deprecated version of stash create stores correct message' '\n+\t>foo &&\n+\tgit add foo &&\n+\tSTASH_ID=$(git stash create \"create test message\") &&\n+\techo \"On master: create test message\" >expect &&\n+\tgit show --pretty=%s ${STASH_ID} | head -n1 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'new style stash create stores correct message' '\n+\t>foo &&\n+\tgit add foo &&\n+\tSTASH_ID=$(git stash create -m \"create test message new style\") &&\n+\techo \"On master: create test message new style\" >expect &&\n+\tgit show --pretty=%s ${STASH_ID} | head -n1 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'stash -- stashes and restores the file' '\n \t>foo &&\n \t>bar &&\n@@ -811,4 +829,19 @@ test_expect_success 'stash with multiple filename arguments' '\n \ttest_path_is_file extra\n '\n \n+test_expect_success 'stash with file including $IFS character' '\n+\t>\"foo\tbar\" &&\n+\t>foo &&\n+\t>untracked &&\n+\tgit add foo* &&\n+\tgit stash push -- foo* &&\n+\ttest_path_is_missing \"foo\tbar\" &&\n+\ttest_path_is_missing foo &&\n+\ttest_path_is_file untracked &&\n+\tgit stash pop &&\n+\ttest_path_is_file \"foo\tbar\" &&\n+\ttest_path_is_file foo &&\n+\ttest_path_is_file untracked\n+'\n+\n test_done\n\nThomas Gummerer (4):\n Documentation/stash: remove mention of git reset --hard\n stash: introduce push verb\n introduce new format for git stash create\n stash: support filename argument\n\n Documentation/git-stash.txt | 14 +++-\n git-stash.sh | 154 ++++++++++++++++++++++++++++++++++++++++----\n t/t3903-stash.sh | 69 ++++++++++++++++++++\n 3 files changed, 222 insertions(+), 15 deletions(-)\n\n-- \n2.11.0.297.g9a2118ac0b.dirty\n\n"},{"id":"310514","messageId":"20170129201604.30445-3-t.gummerer@gmail.com","threadId":"44933","inReplyTo":"20170129201604.30445-1-t.gummerer@gmail.com","subject":"[PATCH v2 2/4] stash: introduce push verb","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2017-01-29T20:16:02Z","receivedAt":"2017-01-29T20:25:14Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"Introduce a new git stash push verb in addition to git stash save. The\npush verb is used to transition from the current command line arguments\nto a more conventional way, in which the message is specified after a -m\nparameter instead of being a positional argument.\n\nThis allows introducing a new filename argument to stash single files.\nUsing that as a positional argument is much more consistent with the\nrest of git, than using the positional argument for the message.\n\nSigned-off-by: Thomas Gummerer \n---\n git-stash.sh | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++---\n t/t3903-stash.sh | 9 +++++++\n 2 files changed, 82 insertions(+), 3 deletions(-)\n\ndiff --git a/git-stash.sh b/git-stash.sh\nindex 10c284d1aa..8528708f61 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -189,10 +189,11 @@ store_stash () {\n \treturn $ret\n }\n \n-save_stash () {\n+push_stash () {\n \tkeep_index=\n \tpatch_mode=\n \tuntracked=\n+\tstash_msg=\n \twhile test $# != 0\n \tdo\n \t\tcase \"$1\" in\n@@ -216,6 +217,10 @@ save_stash () {\n \t\t-a|--all)\n \t\t\tuntracked=all\n \t\t\t;;\n+\t\t-m|--message)\n+\t\t\tshift\n+\t\t\tstash_msg=$1\n+\t\t\t;;\n \t\t--help)\n \t\t\tshow_help\n \t\t\t;;\n@@ -251,8 +256,6 @@ save_stash () {\n \t\tdie \"$(gettext \"Can't use --patch and --include-untracked or --all at the same time\")\"\n \tfi\n \n-\tstash_msg=\"$*\"\n-\n \tgit update-index -q --refresh\n \tif no_changes\n \tthen\n@@ -291,6 +294,69 @@ save_stash () {\n \tfi\n }\n \n+save_stash () {\n+\tpush_options=\n+\twhile test $# != 0\n+\tdo\n+\t\tcase \"$1\" in\n+\t\t-k|--keep-index)\n+\t\t\tpush_options=\"-k $push_options\"\n+\t\t\t;;\n+\t\t--no-keep-index)\n+\t\t\tpush_options=\"--no-keep-index $push_options\"\n+\t\t\t;;\n+\t\t-p|--patch)\n+\t\t\tpush_options=\"-p $push_options\"\n+\t\t\t;;\n+\t\t-q|--quiet)\n+\t\t\tpush_options=\"-q $push_options\"\n+\t\t\t;;\n+\t\t-u|--include-untracked)\n+\t\t\tpush_options=\"-u $push_options\"\n+\t\t\t;;\n+\t\t-a|--all)\n+\t\t\tpush_options=\"-a $push_options\"\n+\t\t\t;;\n+\t\t--help)\n+\t\t\tshow_help\n+\t\t\t;;\n+\t\t--)\n+\t\t\tshift\n+\t\t\tbreak\n+\t\t\t;;\n+\t\t-*)\n+\t\t\toption=\"$1\"\n+\t\t\t# TRANSLATORS: $option is an invalid option, like\n+\t\t\t# `--blah-blah'. The 7 spaces at the beginning of the\n+\t\t\t# second line correspond to \"error: \". So you should line\n+\t\t\t# up the second line with however many characters the\n+\t\t\t# translation of \"error: \" takes in your language. E.g. in\n+\t\t\t# English this is:\n+\t\t\t#\n+\t\t\t# $ git stash save --blah-blah 2>&1 | head -n 2\n+\t\t\t# error: unknown option for 'stash save': --blah-blah\n+\t\t\t# To provide a message, use git stash save -- '--blah-blah'\n+\t\t\teval_gettextln \"error: unknown option for 'stash save': \\$option\n+ To provide a message, use git stash save -- '\\$option'\"\n+\t\t\tusage\n+\t\t\t;;\n+\t\t*)\n+\t\t\tbreak\n+\t\t\t;;\n+\t\tesac\n+\t\tshift\n+\tdone\n+\n+\tstash_msg=\"$*\"\n+\n+\tif test -z \"$stash_msg\"\n+\tthen\n+\t\tpush_stash $push_options\n+\telse\n+\t\tpush_stash $push_options -m \"$stash_msg\"\n+\tfi\n+}\n+\n have_stash () {\n \tgit rev-parse --verify --quiet $ref_stash >/dev/null\n }\n@@ -617,6 +683,10 @@ save)\n \tshift\n \tsave_stash \"$@\"\n \t;;\n+push)\n+\tshift\n+\tpush_stash \"$@\"\n+\t;;\n apply)\n \tshift\n \tapply_stash \"$@\"\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 2de3e18ce6..0171b824c9 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -775,4 +775,13 @@ test_expect_success 'stash is not confused by partial renames' '\n \ttest_path_is_missing file\n '\n \n+test_expect_success 'push -m shows right message' '\n+\t>foo &&\n+\tgit add foo &&\n+\tgit stash push -m \"test message\" &&\n+\techo \"stash@{0}: On master: test message\" >expect &&\n+\tgit stash list | head -n 1 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.11.0.297.g9a2118ac0b.dirty\n\n"},{"id":"310515","messageId":"20170129201604.30445-2-t.gummerer@gmail.com","threadId":"44933","inReplyTo":"20170129201604.30445-1-t.gummerer@gmail.com","subject":"[PATCH v2 1/4] Documentation/stash: remove mention of git reset --hard","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2017-01-29T20:16:01Z","receivedAt":"2017-01-29T20:25:17Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"Don't mention git reset --hard in the documentation for git stash save.\nIt's an implementation detail that doesn't matter to the end user and\nthus shouldn't be exposed to them. In addition it's not quite true for\ngit stash -p, and will not be true when a filename argument to limit the\nstash to a few files is introduced.\n\nSigned-off-by: Thomas Gummerer \n---\n Documentation/git-stash.txt | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt\nindex 2e9cef06e6..0fc23c25ee 100644\n--- a/Documentation/git-stash.txt\n+++ b/Documentation/git-stash.txt\n@@ -47,8 +47,9 @@ OPTIONS\n \n save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] []::\n \n-\tSave your local modifications to a new 'stash', and run `git reset\n-\t--hard` to revert them. The part is optional and gives\n+\tSave your local modifications to a new 'stash' and roll them\n+\tback both in the working tree and in the index.\n+\tThe part is optional and gives\n \tthe description along with the stashed state. For quickly making\n \ta snapshot, you can omit _both_ \"save\" and , but giving\n \tonly does not trigger this action to prevent a misspelled\n-- \n2.11.0.297.g9a2118ac0b.dirty\n\n"},{"id":"310516","messageId":"20170129201604.30445-5-t.gummerer@gmail.com","threadId":"44933","inReplyTo":"20170129201604.30445-1-t.gummerer@gmail.com","subject":"[PATCH v2 4/4] stash: support filename argument","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2017-01-29T20:16:04Z","receivedAt":"2017-01-29T20:47:25Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"While working on a repository, it's often helpful to stash the changes\nof a single or multiple files, and leave others alone. Unfortunately\ngit currently offers no such option. git stash -p can be used to work\naround this, but it's often impractical when there are a lot of changes\nover multiple files.\n\nAdd an optional filename argument to git stash push, which allows for\nstashing a single (or multiple) files.\n\nSigned-off-by: Thomas Gummerer \n---\n Documentation/git-stash.txt | 9 +++++++++\n git-stash.sh | 30 +++++++++++++++++++++++-------\n t/t3903-stash.sh | 42 ++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 74 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt\nindex 0bce33e3fc..8306bac397 100644\n--- a/Documentation/git-stash.txt\n+++ b/Documentation/git-stash.txt\n@@ -15,9 +15,13 @@ SYNOPSIS\n 'git stash' branch []\n 'git stash' [save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n \t [-u|--include-untracked] [-a|--all] []]\n+'git stash' push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n+\t [-u|--include-untracked] [-a|--all] [-m|--message ]]\n+\t [--] [...]\n 'git stash' clear\n 'git stash' create []\n 'git stash' create [-m ] [-u|--include-untracked ]\n+\t [-- ...]\n 'git stash' store [-m|--message ] [-q|--quiet] \n \n DESCRIPTION\n@@ -47,6 +51,7 @@ OPTIONS\n -------\n \n save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] []::\n+push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [-m|--message ] [--] [...]::\n \n \tSave your local modifications to a new 'stash' and roll them\n \tback both in the working tree and in the index.\n@@ -56,6 +61,10 @@ save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q\n \tonly does not trigger this action to prevent a misspelled\n \tsubcommand from making an unwanted stash.\n +\n+If the paths argument is given in 'git stash push', only these files\n+are put in the new 'stash'. In addition only the indicated files are\n+changed in the working tree to match the index.\n++\n If the `--keep-index` option is used, all changes already added to the\n index are left intact.\n +\ndiff --git a/git-stash.sh b/git-stash.sh\nindex 5f08b43967..0072a38b4c 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -41,7 +41,7 @@ no_changes () {\n untracked_files () {\n \texcl_opt=--exclude-standard\n \ttest \"$untracked\" = \"all\" && excl_opt=\n-\tgit ls-files -o -z $excl_opt\n+\tgit ls-files -o -z $excl_opt -- $1\n }\n \n clear_stash () {\n@@ -59,6 +59,7 @@ create_stash () {\n \tstash_msg=\n \tuntracked=\n \tnew_style=\n+\tfiles=\n \twhile test $# != 0\n \tdo\n \t\tcase \"$1\" in\n@@ -72,6 +73,12 @@ create_stash () {\n \t\t\tuntracked=\"$1\"\n \t\t\tnew_style=t\n \t\t\t;;\n+\t\t--)\n+\t\t\tshift\n+\t\t\tfiles=\"$@\"\n+\t\t\tnew_style=t\n+\t\t\tbreak\n+\t\t\t;;\n \t\t*)\n \t\t\tif test -n \"$new_style\"\n \t\t\tthen\n@@ -134,7 +141,7 @@ create_stash () {\n \t\t# Untracked files are stored by themselves in a parentless commit, for\n \t\t# ease of unpacking later.\n \t\tu_commit=$(\n-\t\t\tuntracked_files | (\n+\t\t\tuntracked_files $files | (\n \t\t\t\tGIT_INDEX_FILE=\"$TMPindex\" &&\n \t\t\t\texport GIT_INDEX_FILE &&\n \t\t\t\trm -f \"$TMPindex\" &&\n@@ -157,7 +164,7 @@ create_stash () {\n \t\t\tgit read-tree --index-output=\"$TMPindex\" -m $i_tree &&\n \t\t\tGIT_INDEX_FILE=\"$TMPindex\" &&\n \t\t\texport GIT_INDEX_FILE &&\n-\t\t\tgit diff-index --name-only -z HEAD -- >\"$TMP-stagenames\" &&\n+\t\t\tgit diff-index --name-only -z HEAD -- $files >\"$TMP-stagenames\" &&\n \t\t\tgit update-index -z --add --remove --stdin <\"$TMP-stagenames\" &&\n \t\t\tgit write-tree &&\n \t\t\trm -f \"$TMPindex\"\n@@ -171,7 +178,7 @@ create_stash () {\n \n \t\t# find out what the user wants\n \t\tGIT_INDEX_FILE=\"$TMP-index\" \\\n-\t\t\tgit add--interactive --patch=stash -- &&\n+\t\t\tgit add--interactive --patch=stash -- $files &&\n \n \t\t# state of the working tree\n \t\tw_tree=$(GIT_INDEX_FILE=\"$TMP-index\" git write-tree) ||\n@@ -293,6 +300,8 @@ push_stash () {\n \t\tshift\n \tdone\n \n+\tfiles=\"$*\"\n+\n \tif test -n \"$patch_mode\" && test -n \"$untracked\"\n \tthen\n \t\tdie \"$(gettext \"Can't use --patch and --include-untracked or --all at the same time\")\"\n@@ -307,18 +316,25 @@ push_stash () {\n \tgit reflog exists $ref_stash ||\n \t\tclear_stash || die \"$(gettext \"Cannot initialize stash\")\"\n \n-\tcreate_stash -m \"$stash_msg\" -u \"$untracked\"\n+\tcreate_stash -m \"$stash_msg\" -u \"$untracked\" -- $files\n \tstore_stash -m \"$stash_msg\" -q $w_commit ||\n \tdie \"$(gettext \"Cannot save the current status\")\"\n \tsay \"$(eval_gettext \"Saved working directory and index state \\$stash_msg\")\"\n \n \tif test -z \"$patch_mode\"\n \tthen\n-\t\tgit reset --hard ${GIT_QUIET:+-q}\n+\t\tif test -n \"$files\"\n+\t\tthen\n+\t\t\tgit ls-files -z -- \"$@\" | xargs -0 git reset --\n+\t\t\tgit ls-files -z --modified -- \"$@\" | xargs -0 git checkout HEAD --\n+\t\t\tgit ls-files -z --others -- \"$@\" | xargs -0 git clean --force --\n+\t\telse\n+\t\t\tgit reset --hard ${GIT_QUIET:+-q}\n+\t\tfi\n \t\ttest \"$untracked\" = \"all\" && CLEAN_X_OPTION=-x || CLEAN_X_OPTION=\n \t\tif test -n \"$untracked\"\n \t\tthen\n-\t\t\tgit clean --force --quiet -d $CLEAN_X_OPTION\n+\t\t\tgit clean --force --quiet -d $CLEAN_X_OPTION -- $files\n \t\tfi\n \n \t\tif test \"$keep_index\" = \"t\" && test -n \"$i_tree\"\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 34e9610bb6..ca4c44aa9c 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -802,4 +802,46 @@ test_expect_success 'new style stash create stores correct message' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'stash -- stashes and restores the file' '\n+\t>foo &&\n+\t>bar &&\n+\tgit add foo bar &&\n+\tgit stash push -- foo &&\n+\ttest_path_is_file bar &&\n+\ttest_path_is_missing foo &&\n+\tgit stash pop &&\n+\ttest_path_is_file foo &&\n+\ttest_path_is_file bar\n+'\n+\n+test_expect_success 'stash with multiple filename arguments' '\n+\t>foo &&\n+\t>bar &&\n+\t>extra &&\n+\tgit add foo bar extra &&\n+\tgit stash push -- foo bar &&\n+\ttest_path_is_missing bar &&\n+\ttest_path_is_missing foo &&\n+\ttest_path_is_file extra &&\n+\tgit stash pop &&\n+\ttest_path_is_file foo &&\n+\ttest_path_is_file bar &&\n+\ttest_path_is_file extra\n+'\n+\n+test_expect_success 'stash with file including $IFS character' '\n+\t>\"foo\tbar\" &&\n+\t>foo &&\n+\t>untracked &&\n+\tgit add foo* &&\n+\tgit stash push -- foo* &&\n+\ttest_path_is_missing \"foo\tbar\" &&\n+\ttest_path_is_missing foo &&\n+\ttest_path_is_file untracked &&\n+\tgit stash pop &&\n+\ttest_path_is_file \"foo\tbar\" &&\n+\ttest_path_is_file foo &&\n+\ttest_path_is_file untracked\n+'\n+\n test_done\n-- \n2.11.0.297.g9a2118ac0b.dirty\n\n"},{"id":"310557","messageId":"xmqqbmuouudg.fsf@gitster.mtv.corp.google.com","threadId":"44933","inReplyTo":"20170129201604.30445-4-t.gummerer@gmail.com","subject":"Re: [PATCH v2 3/4] introduce new format for git stash create","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-30T21:10:19Z","receivedAt":"2017-01-30T21:10:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Gummerer writes:\n\n> create_stash () {\n> -\tstash_msg=\"$1\"\n> -\tuntracked=\"$2\"\n> +\tstash_msg=\n> +\tuntracked=\n> +\tnew_style=\n> ...\n> +\twhile test $# != 0\n> +\tdo\n> +\t\tcase \"$1\" in\n> +\t\t-m|--message)\n> +\t\t\tshift\n> +\t\t\tstash_msg=\"$1\"\n\n\t${1?\"-m needs an argument\"}\n\nto error check \"git stash create -m\"?\n\n> +\tif test -z \"$new_style\"\n> +\tthen\n> +\t\tstash_msg=\"$*\"\n> +\tfi\n\nThis breaks external users who do \"git stash create\" in the old\nfashioned way, I think, but can be easily fixed with something like:\n\n\t\tstash_msg=$1 untracked=$2\n\nIf the existing tests did not catch this, I guess there is a\ncoverage gap we may want to fill. Perhaps add a new test to 3903\nthat runs \"git stash create message untracked\" and makes sure it\nstill works?\n"},{"id":"310558","messageId":"xmqqa8a8uuc9.fsf@gitster.mtv.corp.google.com","threadId":"44933","inReplyTo":"20170129201604.30445-5-t.gummerer@gmail.com","subject":"Re: [PATCH v2 4/4] stash: support filename argument","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-30T21:11:02Z","receivedAt":"2017-01-30T21:11:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Gummerer writes:\n\n> Add an optional filename argument to git stash push, which allows for\n> stashing a single (or multiple) files.\n\nYou can give pathspec with one or more elements, so \"an optional\nargument\" sounds too limiting. \n\n Allow 'git stash push' to take pathspec to specify which paths\n to stash.\n\nAlso retitle\n\n\tstash: teach 'push' (and 'create') to honor pathspec\n\nor something.\n\n> @@ -56,6 +61,10 @@ save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q\n> \tonly does not trigger this action to prevent a misspelled\n> \tsubcommand from making an unwanted stash.\n> +\n> +If the paths argument is given in 'git stash push', only these files\n> +are put in the new 'stash'. In addition only the indicated files are\n> +changed in the working tree to match the index.\n\nActually the stash contains \"all paths\". You could say that you are\nplacing _modifications_ to these paths in stash, even though that is\nnot how Git's world model works (i.e. everything is a snapshot, and\nmodifications are merely difference between two successive\nsnapshots). A technically correct version may be:\n\n\tWhen pathspec is given to 'git stash push', the new stash\n\trecords the modified states only for the files that match\n\tthe pathspec. The index entries and working tree files are\n\tthen rolled back to the state in HEAD only for these files,\n\ttoo, leaving files that do not match the pathspec intact.\n\n> diff --git a/git-stash.sh b/git-stash.sh\n> index 5f08b43967..0072a38b4c 100755\n> --- a/git-stash.sh\n> +++ b/git-stash.sh\n> @@ -41,7 +41,7 @@ no_changes () {\n> untracked_files () {\n> \texcl_opt=--exclude-standard\n> \ttest \"$untracked\" = \"all\" && excl_opt=\n> -\tgit ls-files -o -z $excl_opt\n> +\tgit ls-files -o -z $excl_opt -- $1\n\nHmph, why \"$1\" is spelled without dq, implying that it is split at\n$IFS boundary? This line alone makes me suspect that this is not\nprepared to deal correctly with $IFS. Let's read on...\n\n> @@ -59,6 +59,7 @@ create_stash () {\n> \tstash_msg=\n> \tuntracked=\n> \tnew_style=\n> +\tfiles=\n> \twhile test $# != 0\n> \tdo\n> \t\tcase \"$1\" in\n> @@ -72,6 +73,12 @@ create_stash () {\n> \t\t\tuntracked=\"$1\"\n> \t\t\tnew_style=t\n> \t\t\t;;\n> +\t\t--)\n> +\t\t\tshift\n> +\t\t\tfiles=\"$@\"\n\nIsn't this the same as writing files=\"$*\", i.e. concatenate the\nmultiple arguments with the first whitespace in $IFS in between?\n\n> @@ -134,7 +141,7 @@ create_stash () {\n> \t\t# Untracked files are stored by themselves in a parentless commit, for\n> \t\t# ease of unpacking later.\n> \t\tu_commit=$(\n> -\t\t\tuntracked_files | (\n> +\t\t\tuntracked_files $files | (\n\n... and this lets it split at $IFS again when passing it down to the\nhelper. But the helper looks only at $1 so the second and subsequent\nones will be ignored altogether.\n\nThis cannot be correct, and any hunk in the remainder of the patch\nthat mentions $files will be incorrect for the same reason.\n\nIs it possible to carry what the caller (and the end user) gave you\nin \"$@\" without molesting it at all? That would mean you do not\nneed to introduce $files variable at all, and then the places that\ndo things like this:\n\n> -\tcreate_stash -m \"$stash_msg\" -u \"$untracked\"\n> +\tcreate_stash -m \"$stash_msg\" -u \"$untracked\" -- $files\n\ncan instead do\n\n\tcreate_stash -m \"$stash_msg\" -u \"$untracked\" -- \"$@\"\n\nThat would allow you to work correctly with pathspec with $IFS\nwhitespaces in them.\n"},{"id":"310559","messageId":"xmqq8tpsuual.fsf@gitster.mtv.corp.google.com","threadId":"44933","inReplyTo":"20170129201604.30445-3-t.gummerer@gmail.com","subject":"Re: [PATCH v2 2/4] stash: introduce push verb","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-30T21:12:02Z","receivedAt":"2017-01-30T21:12:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Gummerer writes:\n\n> Introduce a new git stash push verb in addition to git stash save. The\n> push verb is used to transition from the current command line arguments\n> to a more conventional way, in which the message is specified after a -m\n> parameter instead of being a positional argument.\n\nI think the canonical way to express that is \"... the message is\ngiven as an argument to the -m option\" (i.e. some options take an\nargument, some others do not, and the \"-m\" takes one).\n\n> This allows introducing a new filename argument to stash single files.\n\nI do not want them to be \"a filename argument\", and I do not think\nyou meant them as such, either. \n\n This allows us to have pathspecs at the end of the command line\n arguments like other Git commands do, so that the user can say\n which subset of paths to stash (and leave others behind).\n\n> +save_stash () {\n> +\tpush_options=\n> +\twhile test $# != 0\n> +\tdo\n> +\t\tcase \"$1\" in\n> +\t\t-k|--keep-index)\n> +...\n> +\t\tesac\n> +\t\tshift\n> +\tdone\n\nIt is a bit unfortunate that we need to duplicate the above\ncase/esac here. I do not know if doing it this way:\n\n\tcase \"$1\" in\n\t--)\n\t\tshift\n\t\tbreak \n\t\t;;\n\t--help)\n\t\tshow_help\n\t\t;;\n\t-*)\n\t\t# pass all options through to push_stash\n\t\tpush_options=\"$push_options $1\"\n\t\t;;\n\t*)\n\t\tbreak\n ;;\n\tesac\n\nand letting push_stash complain for an unknown option is easier to\nmaintain.\n\nYou are reversing the order of the options in the loop. Don't.\n"},{"id":"310561","messageId":"xmqq7f5cuu8l.fsf@gitster.mtv.corp.google.com","threadId":"44933","inReplyTo":"20170129201604.30445-2-t.gummerer@gmail.com","subject":"Re: [PATCH v2 1/4] Documentation/stash: remove mention of git reset --hard","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-30T21:13:14Z","receivedAt":"2017-01-30T21:14:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Gummerer writes:\n\n> Don't mention git reset --hard in the documentation for git stash save.\n> It's an implementation detail that doesn't matter to the end user and\n> thus shouldn't be exposed to them.\n\nEverybody understands what \"reset --hard\" does; it can be an\neasier-to-read \"short-hand\" description to say \"reset --hard\"\ninstead of giving a lengthy description of what happens.\n\nBecause of that, I do not necessarily agree with the above\njustification. I'll read the remainder of the series before\ncommenting further on the above.\n\n> diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt\n> index 2e9cef06e6..0fc23c25ee 100644\n> --- a/Documentation/git-stash.txt\n> +++ b/Documentation/git-stash.txt\n> @@ -47,8 +47,9 @@ OPTIONS\n> \n> save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] []::\n> \n> -\tSave your local modifications to a new 'stash', and run `git reset\n> -\t--hard` to revert them. The part is optional and gives\n> +\tSave your local modifications to a new 'stash' and roll them\n> +\tback both in the working tree and in the index.\n\n\"... roll them back both ...\" is unclear where they are rolled back\nto. \n\nPerhaps \"roll them back ... to HEAD\" or something?\n"},{"id":"310851","messageId":"20170204121920.GE31189@hank","threadId":"44933","inReplyTo":"xmqqy3xsux4g.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 2/4] stash: introduce push verb","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2017-02-04T12:19:20Z","receivedAt":"2017-02-04T12:19:01Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 01/30, Junio C Hamano wrote:\n> Thomas Gummerer writes:\n> \n> > Introduce a new git stash push verb in addition to git stash save. The\n> > push verb is used to transition from the current command line arguments\n> > to a more conventional way, in which the message is specified after a -m\n> > parameter instead of being a positional argument.\n> \n> I think the canonical way to express that is \"... the message is\n> given as an argument to the -m option\" (i.e. some options take an\n> argument, some others do not, and the \"-m\" takes one).\n> \n> > This allows introducing a new filename argument to stash single files.\n> \n> I do not want them to be \"a filename argument\", and I do not think\n> you meant them as such, either. \n> \n> This allows us to have pathspecs at the end of the command line\n> arguments like other Git commands do, so that the user can say\n> which subset of paths to stash (and leave others behind).\n\nYeah, this is much better, thanks.\n\n> > +save_stash () {\n> > +\tpush_options=\n> > +\twhile test $# != 0\n> > +\tdo\n> > +\t\tcase \"$1\" in\n> > +\t\t-k|--keep-index)\n> > +...\n> > +\t\tesac\n> > +\t\tshift\n> > +\tdone\n> \n> It is a bit unfortunate that we need to duplicate the above\n> case/esac here. I do not know if doing it this way:\n> \n> \tcase \"$1\" in\n> \t--)\n> \t\tshift\n> \t\tbreak \n> \t\t;;\n> \t--help)\n> \t\tshow_help\n> \t\t;;\n> \t-*)\n> \t\t# pass all options through to push_stash\n> \t\tpush_options=\"$push_options $1\"\n> \t\t;;\n> \t*)\n> \t\tbreak\n> ;;\n> \tesac\n> \n> and letting push_stash complain for an unknown option is easier to\n> maintain.\n\nI think this will work out nicely. Will try to implement it this way.\n\n> You are reversing the order of the options in the loop. Don't.\n\n-- \nThomas\n"},{"id":"310853","messageId":"20170204131809.GF31189@hank","threadId":"44933","inReplyTo":"xmqqtw8guwfm.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 3/4] introduce new format for git stash create","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2017-02-04T13:18:09Z","receivedAt":"2017-02-04T13:17:49Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 01/30, Junio C Hamano wrote:\n> Thomas Gummerer writes:\n> \n> > create_stash () {\n> > -\tstash_msg=\"$1\"\n> > -\tuntracked=\"$2\"\n> > +\tstash_msg=\n> > +\tuntracked=\n> > +\tnew_style=\n> > ...\n> > +\twhile test $# != 0\n> > +\tdo\n> > +\t\tcase \"$1\" in\n> > +\t\t-m|--message)\n> > +\t\t\tshift\n> > +\t\t\tstash_msg=\"$1\"\n> \n> \t${1?\"-m needs an argument\"}\n> \n> to error check \"git stash create -m\"?\n> \n> > +\tif test -z \"$new_style\"\n> > +\tthen\n> > +\t\tstash_msg=\"$*\"\n> > +\tfi\n> \n> This breaks external users who do \"git stash create\" in the old\n> fashioned way, I think, but can be easily fixed with something like:\n> \n> \t\tstash_msg=$1 untracked=$2\n> \n> If the existing tests did not catch this, I guess there is a\n> coverage gap we may want to fill. Perhaps add a new test to 3903\n> that runs \"git stash create message untracked\" and makes sure it\n> still works?\n\nNo I don't think this breaks. It was never possible to add an\nuntracked argument to git stash create. The difference is in a part\nof this patch that is snipped out in your reply:\n\n@@ -697,7 +739,7 @@ clear)\n ;;\n create)\n shift\n- create_stash \"$*\" && echo \"$w_commit\"\n+ create_stash \"$@\" && echo \"$w_commit\"\n ;;\n store)\n shift\n\nIf I understand this piece correctly (I'm not very proficient in\nshell, but my testing seems to agree with me), previously we used $*,\nwhich transformed all arguments to git stash create into one argument\nin create_stash. This needed to change to $@, as otherwise we can't\npull the arguments apart for the new calling style. The two argument\nversion of create_stash was only used internally in the save_stash\nfunction.\n\n> > diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\n> > index 0171b824c9..34e9610bb6 100755\n> > --- a/t/t3903-stash.sh\n> > +++ b/t/t3903-stash.sh\n> > @@ -784,4 +784,22 @@ test_expect_success 'push -m shows right message' '\n> > \ttest_cmp expect actual\n> > '\n> > \n> > +test_expect_success 'deprecated version of stash create stores correct message' '\n> > +\t>foo &&\n> > +\tgit add foo &&\n> > +\tSTASH_ID=$(git stash create \"create test message\") &&\n> > +\techo \"On master: create test message\" >expect &&\n> > +\tgit show --pretty=%s ${STASH_ID} | head -n1 >actual &&\n> > +\ttest_cmp expect actual\n> > +'\n> > +\n> > +test_expect_success 'new style stash create stores correct message' '\n> > +\t>foo &&\n> > +\tgit add foo &&\n> > +\tSTASH_ID=$(git stash create -m \"create test message new style\") &&\n> > +\techo \"On master: create test message new style\" >expect &&\n> > +\tgit show --pretty=%s ${STASH_ID} | head -n1 >actual &&\n> > +\ttest_cmp expect actual\n> > +'\n> > +\n> > test_done\n\n-- \nThomas\n"},{"id":"310871","messageId":"20170205110233.GG31189@hank","threadId":"44933","inReplyTo":"xmqqa8a8uuc9.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 4/4] stash: support filename argument","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2017-02-05T11:02:33Z","receivedAt":"2017-02-05T11:02:22Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 01/30, Junio C Hamano wrote:\n> Thomas Gummerer writes:\n> \n> > Add an optional filename argument to git stash push, which allows for\n> > stashing a single (or multiple) files.\n> \n> You can give pathspec with one or more elements, so \"an optional\n> argument\" sounds too limiting. \n> \n> Allow 'git stash push' to take pathspec to specify which paths\n> to stash.\n> \n> Also retitle\n> \n> \tstash: teach 'push' (and 'create') to honor pathspec\n> \n> or something.\n> \n> > @@ -56,6 +61,10 @@ save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q\n> > \tonly does not trigger this action to prevent a misspelled\n> > \tsubcommand from making an unwanted stash.\n> > +\n> > +If the paths argument is given in 'git stash push', only these files\n> > +are put in the new 'stash'. In addition only the indicated files are\n> > +changed in the working tree to match the index.\n> \n> Actually the stash contains \"all paths\". You could say that you are\n> placing _modifications_ to these paths in stash, even though that is\n> not how Git's world model works (i.e. everything is a snapshot, and\n> modifications are merely difference between two successive\n> snapshots). A technically correct version may be:\n> \n> \tWhen pathspec is given to 'git stash push', the new stash\n> \trecords the modified states only for the files that match\n> \tthe pathspec. The index entries and working tree files are\n> \tthen rolled back to the state in HEAD only for these files,\n> \ttoo, leaving files that do not match the pathspec intact.\n> \n> > diff --git a/git-stash.sh b/git-stash.sh\n> > index 5f08b43967..0072a38b4c 100755\n> > --- a/git-stash.sh\n> > +++ b/git-stash.sh\n> > @@ -41,7 +41,7 @@ no_changes () {\n> > untracked_files () {\n> > \texcl_opt=--exclude-standard\n> > \ttest \"$untracked\" = \"all\" && excl_opt=\n> > -\tgit ls-files -o -z $excl_opt\n> > +\tgit ls-files -o -z $excl_opt -- $1\n> \n> Hmph, why \"$1\" is spelled without dq, implying that it is split at\n> $IFS boundary? This line alone makes me suspect that this is not\n> prepared to deal correctly with $IFS. Let's read on...\n> \n> > @@ -59,6 +59,7 @@ create_stash () {\n> > \tstash_msg=\n> > \tuntracked=\n> > \tnew_style=\n> > +\tfiles=\n> > \twhile test $# != 0\n> > \tdo\n> > \t\tcase \"$1\" in\n> > @@ -72,6 +73,12 @@ create_stash () {\n> > \t\t\tuntracked=\"$1\"\n> > \t\t\tnew_style=t\n> > \t\t\t;;\n> > +\t\t--)\n> > +\t\t\tshift\n> > +\t\t\tfiles=\"$@\"\n> \n> Isn't this the same as writing files=\"$*\", i.e. concatenate the\n> multiple arguments with the first whitespace in $IFS in between?\n> \n> > @@ -134,7 +141,7 @@ create_stash () {\n> > \t\t# Untracked files are stored by themselves in a parentless commit, for\n> > \t\t# ease of unpacking later.\n> > \t\tu_commit=$(\n> > -\t\t\tuntracked_files | (\n> > +\t\t\tuntracked_files $files | (\n> \n> ... and this lets it split at $IFS again when passing it down to the\n> helper. But the helper looks only at $1 so the second and subsequent\n> ones will be ignored altogether.\n> \n> This cannot be correct, and any hunk in the remainder of the patch\n> that mentions $files will be incorrect for the same reason.\n> \n> Is it possible to carry what the caller (and the end user) gave you\n> in \"$@\" without molesting it at all? That would mean you do not\n> need to introduce $files variable at all, and then the places that\n> do things like this:\n> \n> > -\tcreate_stash -m \"$stash_msg\" -u \"$untracked\"\n> > +\tcreate_stash -m \"$stash_msg\" -u \"$untracked\" -- $files\n> \n> can instead do\n> \n> \tcreate_stash -m \"$stash_msg\" -u \"$untracked\" -- \"$@\"\n> \n> That would allow you to work correctly with pathspec with $IFS\n> whitespaces in them.\n\nThanks for taking the time for this explanation! It cleared quite a\nfew things up in my head. I'll make these fixes in the re-roll.\n\n-- \nThomas\n"},{"id":"310873","messageId":"20170205121346.GH31189@hank","threadId":"44933","inReplyTo":"xmqq7f5cuu8l.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 1/4] Documentation/stash: remove mention of git reset --hard","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2017-02-05T12:13:47Z","receivedAt":"2017-02-05T12:13:26Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 01/30, Junio C Hamano wrote:\n> Thomas Gummerer writes:\n> \n> > Don't mention git reset --hard in the documentation for git stash save.\n> > It's an implementation detail that doesn't matter to the end user and\n> > thus shouldn't be exposed to them.\n> \n> Everybody understands what \"reset --hard\" does; it can be an\n> easier-to-read \"short-hand\" description to say \"reset --hard\"\n> instead of giving a lengthy description of what happens.\n\nWhile this is definitely true for experienced git users, it might not\nbe for some people relatively new to git, which are probably the ones\nthat need the description most.\n\n> Because of that, I do not necessarily agree with the above\n> justification. I'll read the remainder of the series before\n> commenting further on the above.\n> \n> > diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt\n> > index 2e9cef06e6..0fc23c25ee 100644\n> > --- a/Documentation/git-stash.txt\n> > +++ b/Documentation/git-stash.txt\n> > @@ -47,8 +47,9 @@ OPTIONS\n> > \n> > save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] []::\n> > \n> > -\tSave your local modifications to a new 'stash', and run `git reset\n> > -\t--hard` to revert them. The part is optional and gives\n> > +\tSave your local modifications to a new 'stash' and roll them\n> > +\tback both in the working tree and in the index.\n> \n> \"... roll them back both ...\" is unclear where they are rolled back\n> to. \n> \n> Perhaps \"roll them back ... to HEAD\" or something?\n\nYeah that makes sense, thanks.\n\n-- \nThomas\n"},{"id":"310880","messageId":"20170205202642.14216-3-t.gummerer@gmail.com","threadId":"44933","inReplyTo":"20170205202642.14216-1-t.gummerer@gmail.com","subject":"[PATCH v3 2/5] stash: introduce push verb","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2017-02-05T20:26:39Z","receivedAt":"2017-02-05T20:26:26Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"Introduce a new git stash push verb in addition to git stash save. The\npush verb is used to transition from the current command line arguments\nto a more conventional way, in which the message is given as an argument\nto the -m option.\n\nThis allows us to have pathspecs at the end of the command line\narguments like other Git commands do, so that the user can say which\nsubset of paths to stash (and leave others behind).\n\nHelped-by: Junio C Hamano \nSigned-off-by: Thomas Gummerer \n---\n git-stash.sh | 46 +++++++++++++++++++++++++++++++++++++++++++---\n t/t3903-stash.sh | 9 +++++++++\n 2 files changed, 52 insertions(+), 3 deletions(-)\n\ndiff --git a/git-stash.sh b/git-stash.sh\nindex 10c284d1aa..bf7863a846 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -189,10 +189,11 @@ store_stash () {\n \treturn $ret\n }\n \n-save_stash () {\n+push_stash () {\n \tkeep_index=\n \tpatch_mode=\n \tuntracked=\n+\tstash_msg=\n \twhile test $# != 0\n \tdo\n \t\tcase \"$1\" in\n@@ -216,6 +217,10 @@ save_stash () {\n \t\t-a|--all)\n \t\t\tuntracked=all\n \t\t\t;;\n+\t\t-m|--message)\n+\t\t\tshift\n+\t\t\tstash_msg=${1?\"-m needs an argument\"}\n+\t\t\t;;\n \t\t--help)\n \t\t\tshow_help\n \t\t\t;;\n@@ -251,8 +256,6 @@ save_stash () {\n \t\tdie \"$(gettext \"Can't use --patch and --include-untracked or --all at the same time\")\"\n \tfi\n \n-\tstash_msg=\"$*\"\n-\n \tgit update-index -q --refresh\n \tif no_changes\n \tthen\n@@ -291,6 +294,39 @@ save_stash () {\n \tfi\n }\n \n+save_stash () {\n+\tpush_options=\n+\twhile test $# != 0\n+\tdo\n+\t\tcase \"$1\" in\n+\t\t--help)\n+\t\t\tshow_help\n+\t\t\t;;\n+\t\t--)\n+\t\t\tshift\n+\t\t\tbreak\n+\t\t\t;;\n+\t\t-*)\n+\t\t\t# pass all options through to push_stash\n+\t\t\tpush_options=\"$push_options $1\"\n+\t\t\t;;\n+\t\t*)\n+\t\t\tbreak\n+\t\t\t;;\n+\t\tesac\n+\t\tshift\n+\tdone\n+\n+\tstash_msg=\"$*\"\n+\n+\tif test -z \"$stash_msg\"\n+\tthen\n+\t\tpush_stash $push_options\n+\telse\n+\t\tpush_stash $push_options -m \"$stash_msg\"\n+\tfi\n+}\n+\n have_stash () {\n \tgit rev-parse --verify --quiet $ref_stash >/dev/null\n }\n@@ -617,6 +653,10 @@ save)\n \tshift\n \tsave_stash \"$@\"\n \t;;\n+push)\n+\tshift\n+\tpush_stash \"$@\"\n+\t;;\n apply)\n \tshift\n \tapply_stash \"$@\"\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 2de3e18ce6..0171b824c9 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -775,4 +775,13 @@ test_expect_success 'stash is not confused by partial renames' '\n \ttest_path_is_missing file\n '\n \n+test_expect_success 'push -m shows right message' '\n+\t>foo &&\n+\tgit add foo &&\n+\tgit stash push -m \"test message\" &&\n+\techo \"stash@{0}: On master: test message\" >expect &&\n+\tgit stash list | head -n 1 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.12.0.rc0.208.g81c5d00b20.dirty\n\n"},{"id":"310881","messageId":"20170205202642.14216-5-t.gummerer@gmail.com","threadId":"44933","inReplyTo":"20170205202642.14216-1-t.gummerer@gmail.com","subject":"[PATCH v3 4/5] stash: introduce new format create","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2017-02-05T20:26:41Z","receivedAt":"2017-02-05T20:26:30Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"git stash create currently supports a positional argument for adding a\nmessage. This is not quite in line with how git commands usually take\ncomments (using a -m flag).\n\nAdd a new syntax for adding a message to git stash create using a -m\nflag. This is with the goal of deprecating the old style git stash\ncreate with positional arguments.\n\nThis also adds a -u argument, for untracked files. This is already used\ninternally as another positional argument, but can now be used from the\ncommand line.\n\nSigned-off-by: Thomas Gummerer \n---\n Documentation/git-stash.txt | 1 +\n git-stash.sh | 50 +++++++++++++++++++++++++++++++++++++++++----\n t/t3903-stash.sh | 9 ++++++++\n 3 files changed, 56 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt\nindex 2e9e344cd7..a138ed6a24 100644\n--- a/Documentation/git-stash.txt\n+++ b/Documentation/git-stash.txt\n@@ -17,6 +17,7 @@ SYNOPSIS\n \t [-u|--include-untracked] [-a|--all] []]\n 'git stash' clear\n 'git stash' create []\n+'git stash' create [-m ] [-u|--include-untracked ]\n 'git stash' store [-m|--message ] [-q|--quiet] \n \n DESCRIPTION\ndiff --git a/git-stash.sh b/git-stash.sh\nindex bf7863a846..33b2d0384c 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -56,8 +56,50 @@ clear_stash () {\n }\n \n create_stash () {\n-\tstash_msg=\"$1\"\n-\tuntracked=\"$2\"\n+\tstash_msg=\n+\tuntracked=\n+\tnew_style=\n+\twhile test $# != 0\n+\tdo\n+\t\tcase \"$1\" in\n+\t\t-m|--message)\n+\t\t\tshift\n+\t\t\tstash_msg=${1?\"-m needs an argument\"}\n+\t\t\tnew_style=t\n+\t\t\t;;\n+\t\t-u|--include-untracked)\n+\t\t\tshift\n+\t\t\tuntracked=\"$1\"\n+\t\t\tnew_style=t\n+\t\t\t;;\n+\t\t*)\n+\t\t\tif test -n \"$new_style\"\n+\t\t\tthen\n+\t\t\t\techo \"invalid argument\"\n+\t\t\t\toption=\"$1\"\n+\t\t\t\t# TRANSLATORS: $option is an invalid option, like\n+\t\t\t\t# `--blah-blah'. The 7 spaces at the beginning of the\n+\t\t\t\t# second line correspond to \"error: \". So you should line\n+\t\t\t\t# up the second line with however many characters the\n+\t\t\t\t# translation of \"error: \" takes in your language. E.g. in\n+\t\t\t\t# English this is:\n+\t\t\t\t#\n+\t\t\t\t# $ git stash save --blah-blah 2>&1 | head -n 2\n+\t\t\t\t# error: unknown option for 'stash save': --blah-blah\n+\t\t\t\t# To provide a message, use git stash save -- '--blah-blah'\n+\t\t\t\teval_gettextln \"error: unknown option for 'stash create': \\$option\"\n+\t\t\t\tusage\n+\t\t\tfi\n+\t\t\tbreak\n+\t\t\t;;\n+\t\tesac\n+\t\tshift\n+\tdone\n+\n+\tif test -z \"$new_style\"\n+\tthen\n+\t\tstash_msg=\"$*\"\n+\tfi\n \n \tgit update-index -q --refresh\n \tif no_changes\n@@ -265,7 +307,7 @@ push_stash () {\n \tgit reflog exists $ref_stash ||\n \t\tclear_stash || die \"$(gettext \"Cannot initialize stash\")\"\n \n-\tcreate_stash \"$stash_msg\" $untracked\n+\tcreate_stash -m \"$stash_msg\" -u \"$untracked\"\n \tstore_stash -m \"$stash_msg\" -q $w_commit ||\n \tdie \"$(gettext \"Cannot save the current status\")\"\n \tsay \"$(eval_gettext \"Saved working directory and index state \\$stash_msg\")\"\n@@ -667,7 +709,7 @@ clear)\n \t;;\n create)\n \tshift\n-\tcreate_stash \"$*\" && echo \"$w_commit\"\n+\tcreate_stash \"$@\" && echo \"$w_commit\"\n \t;;\n store)\n \tshift\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 21cb70dc74..b859e51086 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -802,4 +802,13 @@ test_expect_success 'create with multiple arguments for the message' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'new style stash create stores correct message' '\n+\t>foo &&\n+\tgit add foo &&\n+\tSTASH_ID=$(git stash create -m \"create test message new style\") &&\n+\techo \"On master: create test message new style\" >expect &&\n+\tgit show --pretty=%s ${STASH_ID} | head -n1 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.12.0.rc0.208.g81c5d00b20.dirty\n\n"},{"id":"310882","messageId":"20170205202642.14216-1-t.gummerer@gmail.com","threadId":"44933","inReplyTo":"20170129201604.30445-1-t.gummerer@gmail.com","subject":"[PATCH v3 0/5] stash: support pathspec argument","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2017-02-05T20:26:37Z","receivedAt":"2017-02-05T20:26:33Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"Thanks Junio for the review in the previous round.\n\nChanges since v2:\n\n- $IFS should now be supported by using \"$@\" everywhere instead of using\n a $files variable.\n- Added a new patch showing the old behaviour of git stash create is\n preserved.\n- Rephrased the documentation\n- Simplified the option parsing in save_stash, by leaving the\n actual parsing to push_stash instead.\n\nInterdiff below:\n\ndiff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt\nindex 8306bac397..be55e456fa 100644\n--- a/Documentation/git-stash.txt\n+++ b/Documentation/git-stash.txt\n@@ -17,7 +17,7 @@ SYNOPSIS\n \t [-u|--include-untracked] [-a|--all] []]\n 'git stash' push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n \t [-u|--include-untracked] [-a|--all] [-m|--message ]]\n-\t [--] [...]\n+\t [--] [...]\n 'git stash' clear\n 'git stash' create []\n 'git stash' create [-m ] [-u|--include-untracked ]\n@@ -51,19 +51,21 @@ OPTIONS\n -------\n \n save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] []::\n-push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [-m|--message ] [--] [...]::\n+push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [-m|--message ] [--] [...]::\n \n \tSave your local modifications to a new 'stash' and roll them\n-\tback both in the working tree and in the index.\n+\tback to HEAD (in the working tree and in the index).\n \tThe part is optional and gives\n \tthe description along with the stashed state. For quickly making\n \ta snapshot, you can omit _both_ \"save\" and , but giving\n \tonly does not trigger this action to prevent a misspelled\n \tsubcommand from making an unwanted stash.\n +\n-If the paths argument is given in 'git stash push', only these files\n-are put in the new 'stash'. In addition only the indicated files are\n-changed in the working tree to match the index.\n+When pathspec is given to 'git stash push', the new stash records the\n+modified states only for the files that match the pathspec. The index\n+entries and working tree files are then rolled back to the state in\n+HEAD only for these files, too, leaving files that do not match the\n+pathspec intact.\n +\n If the `--keep-index` option is used, all changes already added to the\n index are left intact.\ndiff --git a/git-stash.sh b/git-stash.sh\nindex 0072a38b4c..46367d40aa 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -41,7 +41,7 @@ no_changes () {\n untracked_files () {\n \texcl_opt=--exclude-standard\n \ttest \"$untracked\" = \"all\" && excl_opt=\n-\tgit ls-files -o -z $excl_opt -- $1\n+\tgit ls-files -o -z $excl_opt -- \"$@\"\n }\n \n clear_stash () {\n@@ -59,13 +59,12 @@ create_stash () {\n \tstash_msg=\n \tuntracked=\n \tnew_style=\n-\tfiles=\n \twhile test $# != 0\n \tdo\n \t\tcase \"$1\" in\n \t\t-m|--message)\n \t\t\tshift\n-\t\t\tstash_msg=\"$1\"\n+\t\t\tstash_msg=${1?\"-m needs an argument\"}\n \t\t\tnew_style=t\n \t\t\t;;\n \t\t-u|--include-untracked)\n@@ -75,7 +74,6 @@ create_stash () {\n \t\t\t;;\n \t\t--)\n \t\t\tshift\n-\t\t\tfiles=\"$@\"\n \t\t\tnew_style=t\n \t\t\tbreak\n \t\t\t;;\n@@ -106,6 +104,10 @@ create_stash () {\n \tif test -z \"$new_style\"\n \tthen\n \t\tstash_msg=\"$*\"\n+\t\twhile test $# != 0\n+\t\tdo\n+\t\t\tshift\n+\t\tdone\n \tfi\n \n \tgit update-index -q --refresh\n@@ -141,7 +143,7 @@ create_stash () {\n \t\t# Untracked files are stored by themselves in a parentless commit, for\n \t\t# ease of unpacking later.\n \t\tu_commit=$(\n-\t\t\tuntracked_files $files | (\n+\t\t\tuntracked_files \"$@\" | (\n \t\t\t\tGIT_INDEX_FILE=\"$TMPindex\" &&\n \t\t\t\texport GIT_INDEX_FILE &&\n \t\t\t\trm -f \"$TMPindex\" &&\n@@ -164,7 +166,7 @@ create_stash () {\n \t\t\tgit read-tree --index-output=\"$TMPindex\" -m $i_tree &&\n \t\t\tGIT_INDEX_FILE=\"$TMPindex\" &&\n \t\t\texport GIT_INDEX_FILE &&\n-\t\t\tgit diff-index --name-only -z HEAD -- $files >\"$TMP-stagenames\" &&\n+\t\t\tgit diff-index --name-only -z HEAD -- \"$@\" >\"$TMP-stagenames\" &&\n \t\t\tgit update-index -z --add --remove --stdin <\"$TMP-stagenames\" &&\n \t\t\tgit write-tree &&\n \t\t\trm -f \"$TMPindex\"\n@@ -178,7 +180,7 @@ create_stash () {\n \n \t\t# find out what the user wants\n \t\tGIT_INDEX_FILE=\"$TMP-index\" \\\n-\t\t\tgit add--interactive --patch=stash -- $files &&\n+\t\t\tgit add--interactive --patch=stash -- \"$@\" &&\n \n \t\t# state of the working tree\n \t\tw_tree=$(GIT_INDEX_FILE=\"$TMP-index\" git write-tree) ||\n@@ -268,7 +270,7 @@ push_stash () {\n \t\t\t;;\n \t\t-m|--message)\n \t\t\tshift\n-\t\t\tstash_msg=$1\n+\t\t\tstash_msg=${1?\"-m needs an argument\"}\n \t\t\t;;\n \t\t--help)\n \t\t\tshow_help\n@@ -300,8 +302,6 @@ push_stash () {\n \t\tshift\n \tdone\n \n-\tfiles=\"$*\"\n-\n \tif test -n \"$patch_mode\" && test -n \"$untracked\"\n \tthen\n \t\tdie \"$(gettext \"Can't use --patch and --include-untracked or --all at the same time\")\"\n@@ -316,14 +316,14 @@ push_stash () {\n \tgit reflog exists $ref_stash ||\n \t\tclear_stash || die \"$(gettext \"Cannot initialize stash\")\"\n \n-\tcreate_stash -m \"$stash_msg\" -u \"$untracked\" -- $files\n+\tcreate_stash -m \"$stash_msg\" -u \"$untracked\" -- \"$@\"\n \tstore_stash -m \"$stash_msg\" -q $w_commit ||\n \tdie \"$(gettext \"Cannot save the current status\")\"\n \tsay \"$(eval_gettext \"Saved working directory and index state \\$stash_msg\")\"\n \n \tif test -z \"$patch_mode\"\n \tthen\n-\t\tif test -n \"$files\"\n+\t\tif test $# != 0\n \t\tthen\n \t\t\tgit ls-files -z -- \"$@\" | xargs -0 git reset --\n \t\t\tgit ls-files -z --modified -- \"$@\" | xargs -0 git checkout HEAD --\n@@ -334,7 +334,7 @@ push_stash () {\n \t\ttest \"$untracked\" = \"all\" && CLEAN_X_OPTION=-x || CLEAN_X_OPTION=\n \t\tif test -n \"$untracked\"\n \t\tthen\n-\t\t\tgit clean --force --quiet -d $CLEAN_X_OPTION -- $files\n+\t\t\tgit clean --force --quiet -d $CLEAN_X_OPTION -- \"$@\"\n \t\tfi\n \n \t\tif test \"$keep_index\" = \"t\" && test -n \"$i_tree\"\n@@ -357,24 +357,6 @@ save_stash () {\n \twhile test $# != 0\n \tdo\n \t\tcase \"$1\" in\n-\t\t-k|--keep-index)\n-\t\t\tpush_options=\"-k $push_options\"\n-\t\t\t;;\n-\t\t--no-keep-index)\n-\t\t\tpush_options=\"--no-keep-index $push_options\"\n-\t\t\t;;\n-\t\t-p|--patch)\n-\t\t\tpush_options=\"-p $push_options\"\n-\t\t\t;;\n-\t\t-q|--quiet)\n-\t\t\tpush_options=\"-q $push_options\"\n-\t\t\t;;\n-\t\t-u|--include-untracked)\n-\t\t\tpush_options=\"-u $push_options\"\n-\t\t\t;;\n-\t\t-a|--all)\n-\t\t\tpush_options=\"-a $push_options\"\n-\t\t\t;;\n \t\t--help)\n \t\t\tshow_help\n \t\t\t;;\n@@ -383,20 +365,8 @@ save_stash () {\n \t\t\tbreak\n \t\t\t;;\n \t\t-*)\n-\t\t\toption=\"$1\"\n-\t\t\t# TRANSLATORS: $option is an invalid option, like\n-\t\t\t# `--blah-blah'. The 7 spaces at the beginning of the\n-\t\t\t# second line correspond to \"error: \". So you should line\n-\t\t\t# up the second line with however many characters the\n-\t\t\t# translation of \"error: \" takes in your language. E.g. in\n-\t\t\t# English this is:\n-\t\t\t#\n-\t\t\t# $ git stash save --blah-blah 2>&1 | head -n 2\n-\t\t\t# error: unknown option for 'stash save': --blah-blah\n-\t\t\t# To provide a message, use git stash save -- '--blah-blah'\n-\t\t\teval_gettextln \"error: unknown option for 'stash save': \\$option\n- To provide a message, use git stash save -- '\\$option'\"\n-\t\t\tusage\n+\t\t\t# pass all options through to push_stash\n+\t\t\tpush_options=\"$push_options $1\"\n \t\t\t;;\n \t\t*)\n \t\t\tbreak\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex ca4c44aa9c..461fe46045 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -784,7 +784,7 @@ test_expect_success 'push -m shows right message' '\n \ttest_cmp expect actual\n '\n \n-test_expect_success 'deprecated version of stash create stores correct message' '\n+test_expect_success 'create stores correct message' '\n \t>foo &&\n \tgit add foo &&\n \tSTASH_ID=$(git stash create \"create test message\") &&\n@@ -793,6 +793,15 @@ test_expect_success 'deprecated version of stash create stores correct message'\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'create with multiple arguments for the message' '\n+\t>foo &&\n+\tgit add foo &&\n+\tSTASH_ID=$(git stash create test untracked) &&\n+\techo \"On master: test untracked\" >expect &&\n+\tgit show --pretty=%s ${STASH_ID} | head -n1 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'new style stash create stores correct message' '\n \t>foo &&\n \tgit add foo &&\ndiff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh\nindex f372fc8ca8..9a98e31a3e 100755\n--- a/t/t3905-stash-include-untracked.sh\n+++ b/t/t3905-stash-include-untracked.sh\n@@ -185,4 +185,30 @@ test_expect_success 'stash save --all is stash poppable' '\n \ttest -s .gitignore\n '\n \n+test_expect_success 'stash push --include-untracked with pathspec' '\n+\t>foo &&\n+\t>bar &&\n+\tgit stash push --include-untracked -- foo &&\n+\ttest_path_is_file bar &&\n+\ttest_path_is_missing foo &&\n+\tgit stash pop &&\n+\ttest_path_is_file bar &&\n+\ttest_path_is_file foo\n+'\n+\n+test_expect_success 'stash push with $IFS character' '\n+\t>\"foo\tbar\" &&\n+\t>foo &&\n+\t>bar &&\n+\tgit add foo* &&\n+\tgit stash push --include-untracked -- foo* &&\n+\ttest_path_is_missing \"foo\tbar\" &&\n+\ttest_path_is_missing foo &&\n+\ttest_path_is_file bar &&\n+\tgit stash pop &&\n+\ttest_path_is_file \"foo\tbar\" &&\n+\ttest_path_is_file foo &&\n+\ttest_path_is_file bar\n+'\n+\n test_done\n\nThomas Gummerer (5):\n Documentation/stash: remove mention of git reset --hard\n stash: introduce push verb\n stash: add test for the create command line arguments\n stash: introduce new format create\n stash: teach 'push' (and 'create') to honor pathspec\n\n Documentation/git-stash.txt | 17 ++++-\n git-stash.sh | 124 +++++++++++++++++++++++++++++++++----\n t/t3903-stash.sh | 78 +++++++++++++++++++++++\n t/t3905-stash-include-untracked.sh | 26 ++++++++\n 4 files changed, 230 insertions(+), 15 deletions(-)\n\n-- \n2.12.0.rc0.208.g81c5d00b20.dirty\n\n"},{"id":"310883","messageId":"20170205202642.14216-4-t.gummerer@gmail.com","threadId":"44933","inReplyTo":"20170205202642.14216-1-t.gummerer@gmail.com","subject":"[PATCH v3 3/5] stash: add test for the create command line arguments","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2017-02-05T20:26:40Z","receivedAt":"2017-02-05T20:26:36Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"Currently there is no test showing the expected behaviour of git stash\ncreate's command line arguments. Add a test for that to show the\ncurrent expected behaviour and to make sure future refactorings don't\nbreak those expectations.\n\nSigned-off-by: Thomas Gummerer \n---\n t/t3903-stash.sh | 18 ++++++++++++++++++\n 1 file changed, 18 insertions(+)\n\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 0171b824c9..21cb70dc74 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -784,4 +784,22 @@ test_expect_success 'push -m shows right message' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'create stores correct message' '\n+\t>foo &&\n+\tgit add foo &&\n+\tSTASH_ID=$(git stash create \"create test message\") &&\n+\techo \"On master: create test message\" >expect &&\n+\tgit show --pretty=%s ${STASH_ID} | head -n1 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'create with multiple arguments for the message' '\n+\t>foo &&\n+\tgit add foo &&\n+\tSTASH_ID=$(git stash create test untracked) &&\n+\techo \"On master: test untracked\" >expect &&\n+\tgit show --pretty=%s ${STASH_ID} | head -n1 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.12.0.rc0.208.g81c5d00b20.dirty\n\n"},{"id":"310884","messageId":"20170205202642.14216-2-t.gummerer@gmail.com","threadId":"44933","inReplyTo":"20170205202642.14216-1-t.gummerer@gmail.com","subject":"[PATCH v3 1/5] Documentation/stash: remove mention of git reset --hard","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2017-02-05T20:26:38Z","receivedAt":"2017-02-05T20:26:38Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"Don't mention git reset --hard in the documentation for git stash save.\nIt's an implementation detail that doesn't matter to the end user and\nthus shouldn't be exposed to them. In addition it's not quite true for\ngit stash -p, and will not be true when a filename argument to limit the\nstash to a few files is introduced.\n\nSigned-off-by: Thomas Gummerer \n---\n Documentation/git-stash.txt | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt\nindex 2e9cef06e6..2e9e344cd7 100644\n--- a/Documentation/git-stash.txt\n+++ b/Documentation/git-stash.txt\n@@ -47,8 +47,9 @@ OPTIONS\n \n save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] []::\n \n-\tSave your local modifications to a new 'stash', and run `git reset\n-\t--hard` to revert them. The part is optional and gives\n+\tSave your local modifications to a new 'stash' and roll them\n+\tback to HEAD (in the working tree and in the index).\n+\tThe part is optional and gives\n \tthe description along with the stashed state. For quickly making\n \ta snapshot, you can omit _both_ \"save\" and , but giving\n \tonly does not trigger this action to prevent a misspelled\n-- \n2.12.0.rc0.208.g81c5d00b20.dirty\n\n"},{"id":"310885","messageId":"20170205202642.14216-6-t.gummerer@gmail.com","threadId":"44933","inReplyTo":"20170205202642.14216-1-t.gummerer@gmail.com","subject":"[PATCH v3 5/5] stash: teach 'push' (and 'create') to honor pathspec","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2017-02-05T20:26:42Z","receivedAt":"2017-02-05T20:26:42Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"While working on a repository, it's often helpful to stash the changes\nof a single or multiple files, and leave others alone. Unfortunately\ngit currently offers no such option. git stash -p can be used to work\naround this, but it's often impractical when there are a lot of changes\nover multiple files.\n\nAllow 'git stash push' to take pathspec to specify which paths to stash.\n\nHelped-by: Junio C Hamano \nSigned-off-by: Thomas Gummerer \n---\n Documentation/git-stash.txt | 11 ++++++++++\n git-stash.sh | 30 ++++++++++++++++++++-------\n t/t3903-stash.sh | 42 ++++++++++++++++++++++++++++++++++++++\n t/t3905-stash-include-untracked.sh | 26 +++++++++++++++++++++++\n 4 files changed, 102 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt\nindex a138ed6a24..be55e456fa 100644\n--- a/Documentation/git-stash.txt\n+++ b/Documentation/git-stash.txt\n@@ -15,9 +15,13 @@ SYNOPSIS\n 'git stash' branch []\n 'git stash' [save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n \t [-u|--include-untracked] [-a|--all] []]\n+'git stash' push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n+\t [-u|--include-untracked] [-a|--all] [-m|--message ]]\n+\t [--] [...]\n 'git stash' clear\n 'git stash' create []\n 'git stash' create [-m ] [-u|--include-untracked ]\n+\t [-- ...]\n 'git stash' store [-m|--message ] [-q|--quiet] \n \n DESCRIPTION\n@@ -47,6 +51,7 @@ OPTIONS\n -------\n \n save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] []::\n+push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [-m|--message ] [--] [...]::\n \n \tSave your local modifications to a new 'stash' and roll them\n \tback to HEAD (in the working tree and in the index).\n@@ -56,6 +61,12 @@ save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q\n \tonly does not trigger this action to prevent a misspelled\n \tsubcommand from making an unwanted stash.\n +\n+When pathspec is given to 'git stash push', the new stash records the\n+modified states only for the files that match the pathspec. The index\n+entries and working tree files are then rolled back to the state in\n+HEAD only for these files, too, leaving files that do not match the\n+pathspec intact.\n++\n If the `--keep-index` option is used, all changes already added to the\n index are left intact.\n +\ndiff --git a/git-stash.sh b/git-stash.sh\nindex 33b2d0384c..46367d40aa 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -41,7 +41,7 @@ no_changes () {\n untracked_files () {\n \texcl_opt=--exclude-standard\n \ttest \"$untracked\" = \"all\" && excl_opt=\n-\tgit ls-files -o -z $excl_opt\n+\tgit ls-files -o -z $excl_opt -- \"$@\"\n }\n \n clear_stash () {\n@@ -72,6 +72,11 @@ create_stash () {\n \t\t\tuntracked=\"$1\"\n \t\t\tnew_style=t\n \t\t\t;;\n+\t\t--)\n+\t\t\tshift\n+\t\t\tnew_style=t\n+\t\t\tbreak\n+\t\t\t;;\n \t\t*)\n \t\t\tif test -n \"$new_style\"\n \t\t\tthen\n@@ -99,6 +104,10 @@ create_stash () {\n \tif test -z \"$new_style\"\n \tthen\n \t\tstash_msg=\"$*\"\n+\t\twhile test $# != 0\n+\t\tdo\n+\t\t\tshift\n+\t\tdone\n \tfi\n \n \tgit update-index -q --refresh\n@@ -134,7 +143,7 @@ create_stash () {\n \t\t# Untracked files are stored by themselves in a parentless commit, for\n \t\t# ease of unpacking later.\n \t\tu_commit=$(\n-\t\t\tuntracked_files | (\n+\t\t\tuntracked_files \"$@\" | (\n \t\t\t\tGIT_INDEX_FILE=\"$TMPindex\" &&\n \t\t\t\texport GIT_INDEX_FILE &&\n \t\t\t\trm -f \"$TMPindex\" &&\n@@ -157,7 +166,7 @@ create_stash () {\n \t\t\tgit read-tree --index-output=\"$TMPindex\" -m $i_tree &&\n \t\t\tGIT_INDEX_FILE=\"$TMPindex\" &&\n \t\t\texport GIT_INDEX_FILE &&\n-\t\t\tgit diff-index --name-only -z HEAD -- >\"$TMP-stagenames\" &&\n+\t\t\tgit diff-index --name-only -z HEAD -- \"$@\" >\"$TMP-stagenames\" &&\n \t\t\tgit update-index -z --add --remove --stdin <\"$TMP-stagenames\" &&\n \t\t\tgit write-tree &&\n \t\t\trm -f \"$TMPindex\"\n@@ -171,7 +180,7 @@ create_stash () {\n \n \t\t# find out what the user wants\n \t\tGIT_INDEX_FILE=\"$TMP-index\" \\\n-\t\t\tgit add--interactive --patch=stash -- &&\n+\t\t\tgit add--interactive --patch=stash -- \"$@\" &&\n \n \t\t# state of the working tree\n \t\tw_tree=$(GIT_INDEX_FILE=\"$TMP-index\" git write-tree) ||\n@@ -307,18 +316,25 @@ push_stash () {\n \tgit reflog exists $ref_stash ||\n \t\tclear_stash || die \"$(gettext \"Cannot initialize stash\")\"\n \n-\tcreate_stash -m \"$stash_msg\" -u \"$untracked\"\n+\tcreate_stash -m \"$stash_msg\" -u \"$untracked\" -- \"$@\"\n \tstore_stash -m \"$stash_msg\" -q $w_commit ||\n \tdie \"$(gettext \"Cannot save the current status\")\"\n \tsay \"$(eval_gettext \"Saved working directory and index state \\$stash_msg\")\"\n \n \tif test -z \"$patch_mode\"\n \tthen\n-\t\tgit reset --hard ${GIT_QUIET:+-q}\n+\t\tif test $# != 0\n+\t\tthen\n+\t\t\tgit ls-files -z -- \"$@\" | xargs -0 git reset --\n+\t\t\tgit ls-files -z --modified -- \"$@\" | xargs -0 git checkout HEAD --\n+\t\t\tgit ls-files -z --others -- \"$@\" | xargs -0 git clean --force --\n+\t\telse\n+\t\t\tgit reset --hard ${GIT_QUIET:+-q}\n+\t\tfi\n \t\ttest \"$untracked\" = \"all\" && CLEAN_X_OPTION=-x || CLEAN_X_OPTION=\n \t\tif test -n \"$untracked\"\n \t\tthen\n-\t\t\tgit clean --force --quiet -d $CLEAN_X_OPTION\n+\t\t\tgit clean --force --quiet -d $CLEAN_X_OPTION -- \"$@\"\n \t\tfi\n \n \t\tif test \"$keep_index\" = \"t\" && test -n \"$i_tree\"\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex b859e51086..461fe46045 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -811,4 +811,46 @@ test_expect_success 'new style stash create stores correct message' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'stash -- stashes and restores the file' '\n+\t>foo &&\n+\t>bar &&\n+\tgit add foo bar &&\n+\tgit stash push -- foo &&\n+\ttest_path_is_file bar &&\n+\ttest_path_is_missing foo &&\n+\tgit stash pop &&\n+\ttest_path_is_file foo &&\n+\ttest_path_is_file bar\n+'\n+\n+test_expect_success 'stash with multiple filename arguments' '\n+\t>foo &&\n+\t>bar &&\n+\t>extra &&\n+\tgit add foo bar extra &&\n+\tgit stash push -- foo bar &&\n+\ttest_path_is_missing bar &&\n+\ttest_path_is_missing foo &&\n+\ttest_path_is_file extra &&\n+\tgit stash pop &&\n+\ttest_path_is_file foo &&\n+\ttest_path_is_file bar &&\n+\ttest_path_is_file extra\n+'\n+\n+test_expect_success 'stash with file including $IFS character' '\n+\t>\"foo\tbar\" &&\n+\t>foo &&\n+\t>untracked &&\n+\tgit add foo* &&\n+\tgit stash push -- foo* &&\n+\ttest_path_is_missing \"foo\tbar\" &&\n+\ttest_path_is_missing foo &&\n+\ttest_path_is_file untracked &&\n+\tgit stash pop &&\n+\ttest_path_is_file \"foo\tbar\" &&\n+\ttest_path_is_file foo &&\n+\ttest_path_is_file untracked\n+'\n+\n test_done\ndiff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh\nindex f372fc8ca8..9a98e31a3e 100755\n--- a/t/t3905-stash-include-untracked.sh\n+++ b/t/t3905-stash-include-untracked.sh\n@@ -185,4 +185,30 @@ test_expect_success 'stash save --all is stash poppable' '\n \ttest -s .gitignore\n '\n \n+test_expect_success 'stash push --include-untracked with pathspec' '\n+\t>foo &&\n+\t>bar &&\n+\tgit stash push --include-untracked -- foo &&\n+\ttest_path_is_file bar &&\n+\ttest_path_is_missing foo &&\n+\tgit stash pop &&\n+\ttest_path_is_file bar &&\n+\ttest_path_is_file foo\n+'\n+\n+test_expect_success 'stash push with $IFS character' '\n+\t>\"foo\tbar\" &&\n+\t>foo &&\n+\t>bar &&\n+\tgit add foo* &&\n+\tgit stash push --include-untracked -- foo* &&\n+\ttest_path_is_missing \"foo\tbar\" &&\n+\ttest_path_is_missing foo &&\n+\ttest_path_is_file bar &&\n+\tgit stash pop &&\n+\ttest_path_is_file \"foo\tbar\" &&\n+\ttest_path_is_file foo &&\n+\ttest_path_is_file bar\n+'\n+\n test_done\n-- \n2.12.0.rc0.208.g81c5d00b20.dirty\n\n"},{"id":"310912","messageId":"20170206152013.4qv6734jcdnuvekt@sigill.intra.peff.net","threadId":"44933","inReplyTo":"xmqqmvdz3ied.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 5/5] stash: teach 'push' (and 'create') to honor pathspec","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-02-06T15:20:13Z","receivedAt":"2017-02-06T15:20:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Feb 05, 2017 at 11:09:14PM -0800, Junio C Hamano wrote:\n\n> > @@ -99,6 +104,10 @@ create_stash () {\n> > \tif test -z \"$new_style\"\n> > \tthen\n> > \t\tstash_msg=\"$*\"\n> > +\t\twhile test $# != 0\n> > +\t\tdo\n> > +\t\t\tshift\n> > +\t\tdone\n> > \tfi\n> \n> The intent is correct, but I would probaly empty the \"$@\" not with\n> an iteration of shifts but with a single\n> \n> \tset x && shift\n> \n> ;-)\n\nPerhaps it is not portable, but I think \"set --\" does the same thing and\nis perhaps less obscure.\n\n-Peff\n"},{"id":"310913","messageId":"20170206152200.sszklnow332rf6fj@sigill.intra.peff.net","threadId":"44933","inReplyTo":"20170205202642.14216-2-t.gummerer@gmail.com","subject":"Re: [PATCH v3 1/5] Documentation/stash: remove mention of git reset --hard","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-02-06T15:22:00Z","receivedAt":"2017-02-06T15:22:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Feb 05, 2017 at 08:26:38PM +0000, Thomas Gummerer wrote:\n\n> diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt\n> index 2e9cef06e6..2e9e344cd7 100644\n> --- a/Documentation/git-stash.txt\n> +++ b/Documentation/git-stash.txt\n> @@ -47,8 +47,9 @@ OPTIONS\n> \n> save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] []::\n> \n> -\tSave your local modifications to a new 'stash', and run `git reset\n> -\t--hard` to revert them. The part is optional and gives\n> +\tSave your local modifications to a new 'stash' and roll them\n> +\tback to HEAD (in the working tree and in the index).\n> +\tThe part is optional and gives\n\nNice. I think what you ended up with here is concise and accurate.\n\n-Peff\n"},{"id":"310915","messageId":"20170206154628.v27z5mqhxylz22ba@sigill.intra.peff.net","threadId":"44933","inReplyTo":"20170205202642.14216-3-t.gummerer@gmail.com","subject":"Re: [PATCH v3 2/5] stash: introduce push verb","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-02-06T15:46:28Z","receivedAt":"2017-02-06T15:46:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Feb 05, 2017 at 08:26:39PM +0000, Thomas Gummerer wrote:\n\n> +\t\t-m|--message)\n> +\t\t\tshift\n> +\t\t\tstash_msg=${1?\"-m needs an argument\"}\n> +\t\t\t;;\n\nI think this is our first use of the \"?\" parameter expansion magic. It\n_is_ in POSIX, so it may be fine. We may get complaints from people on\nweird shell variants, though. If that's the only reason to avoid it, I'd\nbe inclined to try it and see, as it is much shorter.\n\nOTOH, most of the other usage errors call usage(), and this one doesn't.\nNor is the error message translatable. Perhaps we should stick to the\nlonger form (and add a helper function if necessary to reduce the\nboilerplate).\n\n> +save_stash () {\n> +\tpush_options=\n> +\twhile test $# != 0\n> +\tdo\n> +\t\tcase \"$1\" in\n> +\t\t--help)\n> +\t\t\tshow_help\n> +\t\t\t;;\n> +\t\t--)\n> +\t\t\tshift\n> +\t\t\tbreak\n> +\t\t\t;;\n> +\t\t-*)\n> +\t\t\t# pass all options through to push_stash\n> +\t\t\tpush_options=\"$push_options $1\"\n> +\t\t\t;;\n> +\t\t*)\n> +\t\t\tbreak\n> +\t\t\t;;\n> +\t\tesac\n> +\t\tshift\n> +\tdone\n\nI suspect you could just let \"--help\" get handled in the pass-through\ncase (it generally takes precedence over errors found in other options,\nbut I do not see any other parsing errors that could be found by this\nloop). It is not too bad to keep it, though (the important thing is that\nwe're not duplicating all of the push_stash options here).\n\n> +\tif test -z \"$stash_msg\"\n> +\tthen\n> +\t\tpush_stash $push_options\n> +\telse\n> +\t\tpush_stash $push_options -m \"$stash_msg\"\n> +\tfi\n\nHmm. So $push_options is subject to word-splitting here. That's\nnecessary to split the options back apart. It does the wrong thing if\nany of the options had spaces in them. But I don't think there are any\nvalid options which do so, and \"save\" would presumably not grow any new\noptions (they would go straight to \"push\").\n\nSo there is a detectable behavior change:\n\n [before]\n $ git stash \"--bogus option\"\n error: unknown option for 'stash save': --bogus option\n To provide a message, use git stash save -- '--bogus option'\n [etc...]\n\n [after]\n $ git stash \"--bogus option\"\n error: unknown option for 'stash save': --bogus\n To provide a message, use git stash save -- '--bogus'\n\nbut it's probably an acceptable casualty (the \"right\" way would be to\nshell-quote everything you stuff into $push_options and then eval the\nresult when you invoke push_stash).\n\nLikewise, it's usually a mistake to just stick a new option (like \"-m\")\nafter a list of unknown options. But it's OK here because we know we\nremoved any \"--\" or non-option arguments.\n\n-Peff\n"},{"id":"310916","messageId":"20170206155012.rb2vydjvlquchwk2@sigill.intra.peff.net","threadId":"44933","inReplyTo":"20170205202642.14216-4-t.gummerer@gmail.com","subject":"Re: [PATCH v3 3/5] stash: add test for the create command line arguments","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-02-06T15:50:13Z","receivedAt":"2017-02-06T15:50:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Feb 05, 2017 at 08:26:40PM +0000, Thomas Gummerer wrote:\n\n> +test_expect_success 'create stores correct message' '\n> +\t>foo &&\n> +\tgit add foo &&\n> +\tSTASH_ID=$(git stash create \"create test message\") &&\n> +\techo \"On master: create test message\" >expect &&\n> +\tgit show --pretty=%s ${STASH_ID} | head -n1 >actual &&\n> +\ttest_cmp expect actual\n> +'\n\nThis misses failure exit codes from \"git show\" because it's on the left\nside of a pipe. Perhaps \"git show -s\" or \"git log -1\" would get you the\nsame output without needing \"head\" (and saving a process and the time\nspent generating the diff, as a bonus).\n\nDitto in the other tests from this patch and later ones.\n\n-Peff\n"},{"id":"310917","messageId":"20170206155606.xgkmhg656vuc6uki@sigill.intra.peff.net","threadId":"44933","inReplyTo":"20170205202642.14216-5-t.gummerer@gmail.com","subject":"Re: [PATCH v3 4/5] stash: introduce new format create","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-02-06T15:56:06Z","receivedAt":"2017-02-06T15:56:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Feb 05, 2017 at 08:26:41PM +0000, Thomas Gummerer wrote:\n\n> git stash create currently supports a positional argument for adding a\n> message. This is not quite in line with how git commands usually take\n> comments (using a -m flag).\n> \n> Add a new syntax for adding a message to git stash create using a -m\n> flag. This is with the goal of deprecating the old style git stash\n> create with positional arguments.\n> \n> This also adds a -u argument, for untracked files. This is already used\n> internally as another positional argument, but can now be used from the\n> command line.\n\nHow do we tell the difference between new-style invocations, and\nold-style ones that look new-style? IOW, I think:\n\n git stash create -m works\n\ncurrently treats \"-m works\" as the full message, and it would now become\njust \"works\".\n\nThat may be an acceptable loss for the benefit we are getting. The\nalternative is to make yet another verb for create, as we did with\nsave/push). I have a feeling that hardly anybody uses \"create\", though,\nand it's mostly an implementation detail. So given the obscure nature,\nmaybe it's an acceptable level of regression. I dunno.\n\nBut either way, it should probably be in the commit message in case\nsomebody does have to track it down later.\n\n-Peff\n"},{"id":"310918","messageId":"20170206161432.zvpsqegjspaa2l5l@sigill.intra.peff.net","threadId":"44933","inReplyTo":"20170205202642.14216-1-t.gummerer@gmail.com","subject":"Re: [PATCH v3 0/5] stash: support pathspec argument","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-02-06T16:14:33Z","receivedAt":"2017-02-06T16:14:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Feb 05, 2017 at 08:26:37PM +0000, Thomas Gummerer wrote:\n\n> Thanks Junio for the review in the previous round.\n> \n> Changes since v2:\n> \n> - $IFS should now be supported by using \"$@\" everywhere instead of using\n> a $files variable.\n> - Added a new patch showing the old behaviour of git stash create is\n> preserved.\n> - Rephrased the documentation\n> - Simplified the option parsing in save_stash, by leaving the\n> actual parsing to push_stash instead.\n\nOverall, I like the direction this is heading. I raised a few issues,\nthe most major of which is whether we want to allow the minor regression\nin \"git stash create -m foo\".\n\nThis also makes \"git stash push -p \" work, which is good. I\ndon't think \"git stash -p \" does, though. I _think_ it\nwould be trivial to do on top, since we already consider that case an\nerror. That's somewhat outside the scope of your series, so I won't\ncomplain (too much :) ) if you don't dig into it, but it might just be\ntrivial on top.\n\nA few other random bits I noticed while playing with the new code:\n\n $ git init\n $ echo content >file && git add file && git commit -m file\n $ echo change >file\n\n $ git stash push -p not-file\n No changes.\n No changes selected\n\nProbably only one of those is necessary. :)\n\nLet's keep trying a few things:\n\n $ git stash push not-file\n Saved working directory and index state WIP on master: 5d5f951 file\n Unstaged changes after reset:\n M\tfile\n M\tfile\n\nThe unstaged change is mentioned twice, which is weird. But weirder\nstill is that we created a stash at all, as it's empty. Why didn't we\nhit the \"no changes selected\" path?\n\nAnd one more:\n\n $ echo foo >untracked\n $ git stash push untracked\n Saved working directory and index state WIP on master: 5d5f951 file\n Unstaged changes after reset:\n M\tfile\n M\tfile\n Removing untracked\n\nWe removed the untracked file, even though it wasn't actually stashed! I\nthought at first this was because it was mentioned as a pathspec, but it\nseems to happen even with a different name:\n\n $ echo foo >untracked\n $ git stash push does-not-exist\n ...\n Removing untracked\n\nThat seems dangerous.\n\n-Peff\n"},{"id":"311337","messageId":"20170211133305.GI31189@hank","threadId":"44933","inReplyTo":"20170206154628.v27z5mqhxylz22ba@sigill.intra.peff.net","subject":"Re: [PATCH v3 2/5] stash: introduce push verb","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2017-02-11T13:33:05Z","receivedAt":"2017-02-11T13:32:40Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"[sorry for the late responses, life is keeping me busy]\n\nOn 02/06, Jeff King wrote:\n> On Sun, Feb 05, 2017 at 08:26:39PM +0000, Thomas Gummerer wrote:\n> \n> > +\t\t-m|--message)\n> > +\t\t\tshift\n> > +\t\t\tstash_msg=${1?\"-m needs an argument\"}\n> > +\t\t\t;;\n> \n> I think this is our first use of the \"?\" parameter expansion magic. It\n> _is_ in POSIX, so it may be fine. We may get complaints from people on\n> weird shell variants, though. If that's the only reason to avoid it, I'd\n> be inclined to try it and see, as it is much shorter.\n> \n> OTOH, most of the other usage errors call usage(), and this one doesn't.\n> Nor is the error message translatable. Perhaps we should stick to the\n> longer form (and add a helper function if necessary to reduce the\n> boilerplate).\n\nYeah I do agree that calling usage is the better option here.\n\n> > +save_stash () {\n> > +\tpush_options=\n> > +\twhile test $# != 0\n> > +\tdo\n> > +\t\tcase \"$1\" in\n> > +\t\t--help)\n> > +\t\t\tshow_help\n> > +\t\t\t;;\n> > +\t\t--)\n> > +\t\t\tshift\n> > +\t\t\tbreak\n> > +\t\t\t;;\n> > +\t\t-*)\n> > +\t\t\t# pass all options through to push_stash\n> > +\t\t\tpush_options=\"$push_options $1\"\n> > +\t\t\t;;\n> > +\t\t*)\n> > +\t\t\tbreak\n> > +\t\t\t;;\n> > +\t\tesac\n> > +\t\tshift\n> > +\tdone\n> \n> I suspect you could just let \"--help\" get handled in the pass-through\n> case (it generally takes precedence over errors found in other options,\n> but I do not see any other parsing errors that could be found by this\n> loop). It is not too bad to keep it, though (the important thing is that\n> we're not duplicating all of the push_stash options here).\n\nGood point, would be good to get rid of that duplication as well.\n\n> > +\tif test -z \"$stash_msg\"\n> > +\tthen\n> > +\t\tpush_stash $push_options\n> > +\telse\n> > +\t\tpush_stash $push_options -m \"$stash_msg\"\n> > +\tfi\n> \n> Hmm. So $push_options is subject to word-splitting here. That's\n> necessary to split the options back apart. It does the wrong thing if\n> any of the options had spaces in them. But I don't think there are any\n> valid options which do so, and \"save\" would presumably not grow any new\n> options (they would go straight to \"push\").\n> \n> So there is a detectable behavior change:\n> \n> [before]\n> $ git stash \"--bogus option\"\n> error: unknown option for 'stash save': --bogus option\n> To provide a message, use git stash save -- '--bogus option'\n> [etc...]\n> \n> [after]\n> $ git stash \"--bogus option\"\n> error: unknown option for 'stash save': --bogus\n> To provide a message, use git stash save -- '--bogus'\n> \n> but it's probably an acceptable casualty (the \"right\" way would be to\n> shell-quote everything you stuff into $push_options and then eval the\n> result when you invoke push_stash).\n>\n> Likewise, it's usually a mistake to just stick a new option (like \"-m\")\n> after a list of unknown options. But it's OK here because we know we\n> removed any \"--\" or non-option arguments.\n> \n> -Peff\n\n-- \nThomas\n"},{"id":"311338","messageId":"20170211135549.GJ31189@hank","threadId":"44933","inReplyTo":"20170206155012.rb2vydjvlquchwk2@sigill.intra.peff.net","subject":"Re: [PATCH v3 3/5] stash: add test for the create command line arguments","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2017-02-11T13:55:49Z","receivedAt":"2017-02-11T13:55:23Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 02/06, Jeff King wrote:\n> On Sun, Feb 05, 2017 at 08:26:40PM +0000, Thomas Gummerer wrote:\n> \n> > +test_expect_success 'create stores correct message' '\n> > +\t>foo &&\n> > +\tgit add foo &&\n> > +\tSTASH_ID=$(git stash create \"create test message\") &&\n> > +\techo \"On master: create test message\" >expect &&\n> > +\tgit show --pretty=%s ${STASH_ID} | head -n1 >actual &&\n> > +\ttest_cmp expect actual\n> > +'\n> \n> This misses failure exit codes from \"git show\" because it's on the left\n> side of a pipe. Perhaps \"git show -s\" or \"git log -1\" would get you the\n> same output without needing \"head\" (and saving a process and the time\n> spent generating the diff, as a bonus).\n> \n> Ditto in the other tests from this patch and later ones.\n\nGood catch, will fix.\n\n> -Peff\n"},{"id":"311340","messageId":"20170211145127.GA23081@hank","threadId":"44933","inReplyTo":"20170206155606.xgkmhg656vuc6uki@sigill.intra.peff.net","subject":"Re: [PATCH v3 4/5] stash: introduce new format create","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2017-02-11T14:51:27Z","receivedAt":"2017-02-11T14:51:01Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 02/06, Jeff King wrote:\n> On Sun, Feb 05, 2017 at 08:26:41PM +0000, Thomas Gummerer wrote:\n> \n> > git stash create currently supports a positional argument for adding a\n> > message. This is not quite in line with how git commands usually take\n> > comments (using a -m flag).\n> > \n> > Add a new syntax for adding a message to git stash create using a -m\n> > flag. This is with the goal of deprecating the old style git stash\n> > create with positional arguments.\n> > \n> > This also adds a -u argument, for untracked files. This is already used\n> > internally as another positional argument, but can now be used from the\n> > command line.\n> \n> How do we tell the difference between new-style invocations, and\n> old-style ones that look new-style? IOW, I think:\n> \n> git stash create -m works\n> \n> currently treats \"-m works\" as the full message, and it would now become\n> just \"works\".\n> \n> That may be an acceptable loss for the benefit we are getting. The\n> alternative is to make yet another verb for create, as we did with\n> save/push). I have a feeling that hardly anybody uses \"create\", though,\n> and it's mostly an implementation detail. So given the obscure nature,\n> maybe it's an acceptable level of regression. I dunno.\n\nRight. So I did a quick search on google and github for this, and\nthere seems one place where git stash create -m is used [1]. From a\nquick look it does however not seem like the -m in the stash message\nis of any significance there, but rather that the intention was to use\na flag that doesn't exist.\n\nI *think* this regression is acceptable, but I'm happy to introduce\nanother verb if people think otherwise.\n\n> But either way, it should probably be in the commit message in case\n> somebody does have to track it down later.\n\nI'll add something there, thanks!\n\n> -Peff\n\n[1]: https://github.com/Andersbakken/nrdp-scripts/blob/1052fc6781c36c4b227c7dd4838a501341b0862a/bin/git-rstash#L81\n"},{"id":"311342","messageId":"20170211165017.GB23081@hank","threadId":"44933","inReplyTo":"xmqqmvdz3ied.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 5/5] stash: teach 'push' (and 'create') to honor pathspec","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2017-02-11T16:50:17Z","receivedAt":"2017-02-11T16:49:54Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 02/05, Junio C Hamano wrote:\n> Thomas Gummerer writes:\n> \n> > @@ -72,6 +72,11 @@ create_stash () {\n> > \t\t\tuntracked=\"$1\"\n> > \t\t\tnew_style=t\n> > \t\t\t;;\n> > +\t\t--)\n> > +\t\t\tshift\n> > +\t\t\tnew_style=t\n> > +\t\t\tbreak\n> > +\t\t\t;;\n> > \t\t*)\n> > \t\t\tif test -n \"$new_style\"\n> > \t\t\tthen\n> > @@ -99,6 +104,10 @@ create_stash () {\n> > \tif test -z \"$new_style\"\n> > \tthen\n> > \t\tstash_msg=\"$*\"\n> > +\t\twhile test $# != 0\n> > +\t\tdo\n> > +\t\t\tshift\n> > +\t\tdone\n> > \tfi\n> \n> The intent is correct, but I would probaly empty the \"$@\" not with\n> an iteration of shifts but with a single\n> \n> \tset x && shift\n> \n> ;-)\n\nThanks, I knew there had to be a better way, but I was unable to find\nit. I think I like Andreas suggestion of \"shift $#\" the best here.\n\n> > @@ -134,7 +143,7 @@ create_stash () {\n> > \t\t# Untracked files are stored by themselves in a parentless commit, for\n> > \t\t# ease of unpacking later.\n> > \t\tu_commit=$(\n> > -\t\t\tuntracked_files | (\n> > +\t\t\tuntracked_files \"$@\" | (\n> \n> The above (and all other uses of \"$@\" was exactly what I had in mind\n> when I gave you the \"can we leave the '$@' the user gave us as-is?\"\n> comment during the review of the previous round. \n> \n> Looks much nicer.\n> \n> > +test_expect_success 'stash push with $IFS character' '\n> > +\t>\"foo\tbar\" &&\n> \n> IIRC, a pathname with HT in it cannot be tested on MINGW. For the\n> purpose of this test, I think it is sufficient to use SP instead of\n> HT here (and matching change below in the expectation, of course).\n\nWill change.\n\n> > +\t>foo &&\n> > +\t>bar &&\n> > +\tgit add foo* &&\n> > +\tgit stash push --include-untracked -- foo* &&\n> \n> While it is good that you are testing \"foo*\", you would also want\n> another test to cover a pathspec with $IFS in it. That is the case\n> the code in the previous round had problems with. Perhaps try\n> something like\n> \n> \t>foo && >bar && >\"foo bar\" && git add . &&\n> \techo modify foo >foo &&\n> \techo modify bar >bar &&\n> \techo modify \"foo bar\" >\"foo bar\" &&\n> \tgit stash push -- \"foo b*\"\n> \n> which should result in the changes to \"foo bar\" in the resulting\n> stash, while changes to \"foo\" and \"bar\" are not (and left in the\n> working tree). And make sure that is what happens? I think with\n> the code from the previous round, the above would instead stash\n> changes to \"foo\" and \"bar\" without catching changes to \"foo bar\",\n> because the single pathspec \"foo b*\" would have been mistakenly\n> split into two, i.e. \"foo\" and \"b*\", and failed to match \"foo bar\"\n> while matching the other two.\n\nThanks, I'll add a test for that.\n"},{"id":"311369","messageId":"20170212193050.GA652@hank","threadId":"44933","inReplyTo":"20170206161432.zvpsqegjspaa2l5l@sigill.intra.peff.net","subject":"Re: [PATCH v3 0/5] stash: support pathspec argument","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2017-02-12T19:30:50Z","receivedAt":"2017-02-12T19:30:26Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"[+cc Matthieu Moy as author of a patch mentioned below]\n\nOn 02/06, Jeff King wrote:\n> On Sun, Feb 05, 2017 at 08:26:37PM +0000, Thomas Gummerer wrote:\n> \n> > Thanks Junio for the review in the previous round.\n> > \n> > Changes since v2:\n> > \n> > - $IFS should now be supported by using \"$@\" everywhere instead of using\n> > a $files variable.\n> > - Added a new patch showing the old behaviour of git stash create is\n> > preserved.\n> > - Rephrased the documentation\n> > - Simplified the option parsing in save_stash, by leaving the\n> > actual parsing to push_stash instead.\n> \n> Overall, I like the direction this is heading. I raised a few issues,\n> the most major of which is whether we want to allow the minor regression\n> in \"git stash create -m foo\".\n> \n> This also makes \"git stash push -p \" work, which is good. I\n> don't think \"git stash -p \" does, though. I _think_ it\n> would be trivial to do on top, since we already consider that case an\n> error. That's somewhat outside the scope of your series, so I won't\n> complain (too much :) ) if you don't dig into it, but it might just be\n> trivial on top.\n\nHmm good idea, I think it would indeed be nice to add that. Theres a\nfew things to consider though. First, we need to switch git stash\nwithout arguments over to use git stash push internally. This does\nintroduce a slight regression as we currently allow git stash save --\n-fmessage, (only messages starting with - are allowed). I think that\nregression would be acceptable however.\n\nThe other question is when we should allow the pathspec argument.\n3c2eb80f, (\"stash: simplify defaulting to \"save\" and reject unknown\noptions\") made sure that all option arguments are treated the same. I\nthink we could use the usual disambiguation of -- to allow pathspecs\nafter that, so stash -p wouldn't be treated specially, otherwise the\nrules could get a bit confusing again.\n\nThe other question is how we would deal with the -m flag for\nspecifying a stash message. I think we could special case it, but\nthat would allow users to do things such as git stash -m apply, which\nwould make the interface a bit more error prone. So I'm leaning\ntowards disallowing that for now.\n\n> A few other random bits I noticed while playing with the new code:\n> \n> $ git init\n> $ echo content >file && git add file && git commit -m file\n> $ echo change >file\n> \n> $ git stash push -p not-file\n> No changes.\n> No changes selected\n> \n> Probably only one of those is necessary. :)\n> \n> Let's keep trying a few things:\n> \n> $ git stash push not-file\n> Saved working directory and index state WIP on master: 5d5f951 file\n> Unstaged changes after reset:\n> M\tfile\n> M\tfile\n> \n> The unstaged change is mentioned twice, which is weird. But weirder\n> still is that we created a stash at all, as it's empty. Why didn't we\n> hit the \"no changes selected\" path?\n> \n> And one more:\n> \n> $ echo foo >untracked\n> $ git stash push untracked\n> Saved working directory and index state WIP on master: 5d5f951 file\n> Unstaged changes after reset:\n> M\tfile\n> M\tfile\n> Removing untracked\n> \n> We removed the untracked file, even though it wasn't actually stashed! I\n> thought at first this was because it was mentioned as a pathspec, but it\n> seems to happen even with a different name:\n> \n> $ echo foo >untracked\n> $ git stash push does-not-exist\n> ...\n> Removing untracked\n> \n> That seems dangerous.\n\nOuch, yeah this is clearly not good. I'll fix this, and add tests for\nthese cases.\n\n> -Peff\n"},{"id":"311428","messageId":"20170213200950.m3bcyp52wd25p737@sigill.intra.peff.net","threadId":"44933","inReplyTo":"vpq7f4uxjmo.fsf@anie.imag.fr","subject":"Re: [PATCH v3 0/5] stash: support pathspec argument","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-02-13T20:09:50Z","receivedAt":"2017-02-13T20:09:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 13, 2017 at 03:14:55PM +0100, Matthieu Moy wrote:\n\n> I don't remember doing this intentionally. The goal was really to\n> prevent typos like \"git stash -p drop\" (which would have been\n> interpreted as \"git stash save -p drop\" previously). So, let's consider\n> this \"only messages starting with - are accepted\" as a bug: moving to\n> the new \"push\" command it a nice opportunity to fix it without worrying\n> about compatibility. A bit more about this in this old thread:\n\nYeah, I consider it a bug that we treat \"-foo\" as a message there.\nFortunately I think you really have to try hard to abuse it:\n\n # doesn't work, because \"save\" complains about \"-foo\" as an option\n git stash -foo\n\n # does save stash with message \"-foo\"\n git stash -- -foo\n\n # doesn't work, because _any_ non-option argument is rejected\n git stash -- use --foo option\n\n> I think we can safely allow both\n> \n> git stash -p -- \n> git stash push -p \n> \n> But allowing the same without -- is a bit more dangerous for the reason\n> above:\n> \n> git stash -p drop\n> \n> would be interpreted as\n> \n> git stash push -p drop\n> \n> (i.e. limit the stash to file named \"drop\"), and this would almost\n> certainly not be what the user wanted.\n\nIs it really that dangerous, though? The likely outcome is Git saying\n\"nope, you don't have any changes to the file named drop\". Of course the\nuser may have meant something different, but I feel like \"-p\" is a good\nindicator that they are interested in making an actual stash.\n\nThink about this continuum of commands:\n\n 1. git stash droop\n\n 2. git stash -p drop\n\n 3. git stash -p -- drop\n\n 4. git stash push -p drop\n\nI think we can all agree that (4) is reasonable and safe. And I suspect\nwe'd all agree that (1) is suspect (you probably meant \"drop\", not \"push\ndroop\").\n\nBut between (2) and (3), I don't see much difference. The interesting\ndifference between (1) and (2) is the addition of \"-p\", which tells us\nthat the user expects to save a stash.\n\nAnother way of thinking of it is that \"-p\" as a command name is an alias\nfor \"push -p\". If you wanted to err on the conservative side, you could\nliterally implement it that way (so \"git stash -k -p foo\" would not\nwork; you'd have to use \"git stash -p -k foo\").\n\n> > The other question is how we would deal with the -m flag for\n> > specifying a stash message. I think we could special case it, but\n> > that would allow users to do things such as git stash -m apply, which\n> > would make the interface a bit more error prone. So I'm leaning\n> > towards disallowing that for now.\n> \n> We already have \"git commit -m \", so I think stash\n> should just do the same thing as commit. Or, did I miss something?\n\nThe complexity is that right now, the first-level decision of \"which\nstash sub-command am I running?\" doesn't know about any options. So \"git\nstash -m foo\" would be rejected in the name of typo prevention, unless\nthat outer decision learns about \"-m\" as an option.\n\n-Peff\n"},{"id":"311452","messageId":"20170213214521.pkjesijdlus36tnp@sigill.intra.peff.net","threadId":"44933","inReplyTo":"vpqo9y5lqos.fsf@anie.imag.fr","subject":"Re: [PATCH v3 0/5] stash: support pathspec argument","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-02-13T21:45:21Z","receivedAt":"2017-02-13T21:45:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 13, 2017 at 10:35:31PM +0100, Matthieu Moy wrote:\n\n> > Is it really that dangerous, though? The likely outcome is Git saying\n> > \"nope, you don't have any changes to the file named drop\". Of course the\n> > user may have meant something different, but I feel like \"-p\" is a good\n> > indicator that they are interested in making an actual stash.\n> \n> Indeed -p is not the best example. In the old thread, I used -q which is\n> much more problematic:\n> \n> git stash -q drop => interpreted as: git stash push -q drop\n> git stash drop -q => drop with option -q\n\nYeah, I'd agree with that. I wouldn't propose to loosen it entirely, but\nrather to treat \"-p\" specially.\n\n> It's not really \"dangerous\" at least in this case, since we misinterpret\n> a destructive command for a less destructive one, but it is rather\n> confusing that changing the order between command and options change the\n> behavior.\n> \n> I actually find it a reasonable expectation to allow swapping commands\n> and options, some programs other than git allow it.\n\nI think we may have already crossed that bridge with \"git -p stash\".\n\nNot to mention that the ordering already _is_ relevant (we disallow one\norder but not the other). If we really wanted to allow swapping, it\nwould mean making:\n\n git stash -p drop\n\nthe same as:\n\n git stash drop -p\n\nI actually find _that_ more confusing. It would perhaps make more sense\nwith something like \"-q\", which is more of a \"global\" option than a\ncommand-specific one. But I think we'd want to whitelist such global\noptions (and \"-p\" would not be on that list).\n\n> > The complexity is that right now, the first-level decision of \"which\n> > stash sub-command am I running?\" doesn't know about any options. So \"git\n> > stash -m foo\" would be rejected in the name of typo prevention, unless\n> > that outer decision learns about \"-m\" as an option.\n> \n> Ah, OK. But that's not really hard to implement: when going through the\n> option list looking for non-option, shift one more time when finding -m.\n\nNo, it's not hard conceptually. It just means implementing the\noption-parsing policy in two places. That's not too bad now, but if we\nstarted using rev-parse's options helper, then I think you have corner\ncases like \"git stash -km foo\".\n\nMy \"-p\" suggestion suffers from a similar problem if you treat it as\n\"you can omit the 'push' if you say \"-p\", rather than \"if -p is the\nfirst option, it is a synonym for 'push -p'\".\n\n-Peff\n"},{"id":"311455","messageId":"20170213215734.puoung6hhdifbgai@sigill.intra.peff.net","threadId":"44933","inReplyTo":"20170211145127.GA23081@hank","subject":"Re: [PATCH v3 4/5] stash: introduce new format create","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-02-13T21:57:34Z","receivedAt":"2017-02-13T21:57:42Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Feb 11, 2017 at 02:51:27PM +0000, Thomas Gummerer wrote:\n\n> > How do we tell the difference between new-style invocations, and\n> > old-style ones that look new-style? IOW, I think:\n> > \n> > git stash create -m works\n> > \n> > currently treats \"-m works\" as the full message, and it would now become\n> > just \"works\".\n> > \n> > That may be an acceptable loss for the benefit we are getting. The\n> > alternative is to make yet another verb for create, as we did with\n> > save/push). I have a feeling that hardly anybody uses \"create\", though,\n> > and it's mostly an implementation detail. So given the obscure nature,\n> > maybe it's an acceptable level of regression. I dunno.\n> \n> Right. So I did a quick search on google and github for this, and\n> there seems one place where git stash create -m is used [1]. From a\n> quick look it does however not seem like the -m in the stash message\n> is of any significance there, but rather that the intention was to use\n> a flag that doesn't exist.\n\nYeah, I think your patch is actually fixing that case. But your search\nis only part of the story. You found somebody using \"-m\" explicitly, but\nwhat about somebody blindly calling:\n\n git stash create $*\n\nThat's now surprising to somebody who puts \"-m\" in their message.\n\n> I *think* this regression is acceptable, but I'm happy to introduce\n> another verb if people think otherwise.\n\nDespite what I wrote above, I'm still inclined to say that this isn't an\nimportant regression. I'd be surprised if \"stash create\" is used\nindependently much at all.\n\n-Peff\n"},{"id":"311457","messageId":"20170213221630.GC652@hank","threadId":"44933","inReplyTo":"vpqlgtaz09q.fsf@anie.imag.fr","subject":"Re: [PATCH v3 2/5] stash: introduce push verb","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2017-02-13T22:16:30Z","receivedAt":"2017-02-13T22:16:07Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 02/13, Matthieu Moy wrote:\n> Thomas Gummerer writes:\n> \n> > Introduce a new git stash push verb in addition to git stash save. The\n> > push verb is used to transition from the current command line arguments\n> > to a more conventional way, in which the message is given as an argument\n> > to the -m option.\n> \n> Sorry if this has been discussed before, but I find 'push' rather\n> confusing here. It took me a while to understand that it meant \"opposite\n> of pop\", because in the context of Git, \"push\" usually means \"send to\n> remote repository\".\n\nThere wasn't much of a discussion about it, but it was pretty much the\nonly thing that came to my mind, and nobody complained or suggested\nanything different, so I just went with it. No other verb came to my\nmind yet, but if someone has a better suggestion, I'd be happy to\nchange.\n\n> Unfortunately, I didn't come up with a better name. \"create\" is already\n> taken ...\n> \n> Another think to have in mind: changing the option name to break\n> backward compatibility is something we can't do often, so if there's\n> anything else we should change about the UI, we should do it now. I\n> don't have anything particular in mind, just thinking aloud.\n\nNow that you mention this, there actually is one inconsistency that I\nintroduced, which I shouldn't have. git stash push works with\n--include-untracked and --all to decide whether or not to include\nuntracked files, and if also ignored files should be included. I also\nadded a --include-untracked {untracked,all} argument to git stash\ncreate, which is clearly inconsistent.\n\nThere really should only be one way. I'd be fine with either way, but\nI think using --include-untracked and --all is the better choice,\nbecause it's easy to understand, and also makes it easier to switch\ngit stash without a verb over to use push_stash internally.\n\n> -- \n> Matthieu Moy\n> http://www-verimag.imag.fr/~moy/\n"},{"id":"311459","messageId":"20170213223346.GD652@hank","threadId":"44933","inReplyTo":"20170213214521.pkjesijdlus36tnp@sigill.intra.peff.net","subject":"Re: [PATCH v3 0/5] stash: support pathspec argument","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2017-02-13T22:33:46Z","receivedAt":"2017-02-13T22:33:34Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 02/13, Jeff King wrote:\n> On Mon, Feb 13, 2017 at 10:35:31PM +0100, Matthieu Moy wrote:\n> \n> > > Is it really that dangerous, though? The likely outcome is Git saying\n> > > \"nope, you don't have any changes to the file named drop\". Of course the\n> > > user may have meant something different, but I feel like \"-p\" is a good\n> > > indicator that they are interested in making an actual stash.\n> > \n> > Indeed -p is not the best example. In the old thread, I used -q which is\n> > much more problematic:\n> > \n> > git stash -q drop => interpreted as: git stash push -q drop\n> > git stash drop -q => drop with option -q\n> \n> Yeah, I'd agree with that. I wouldn't propose to loosen it entirely, but\n> rather to treat \"-p\" specially.\n> \n> > It's not really \"dangerous\" at least in this case, since we misinterpret\n> > a destructive command for a less destructive one, but it is rather\n> > confusing that changing the order between command and options change the\n> > behavior.\n> > \n> > I actually find it a reasonable expectation to allow swapping commands\n> > and options, some programs other than git allow it.\n> \n> I think we may have already crossed that bridge with \"git -p stash\".\n> \n> Not to mention that the ordering already _is_ relevant (we disallow one\n> order but not the other). If we really wanted to allow swapping, it\n> would mean making:\n> \n> git stash -p drop\n> \n> the same as:\n> \n> git stash drop -p\n> \n> I actually find _that_ more confusing. It would perhaps make more sense\n> with something like \"-q\", which is more of a \"global\" option than a\n> command-specific one. But I think we'd want to whitelist such global\n> options (and \"-p\" would not be on that list).\n>\n> > > The complexity is that right now, the first-level decision of \"which\n> > > stash sub-command am I running?\" doesn't know about any options. So \"git\n> > > stash -m foo\" would be rejected in the name of typo prevention, unless\n> > > that outer decision learns about \"-m\" as an option.\n> > \n> > Ah, OK. But that's not really hard to implement: when going through the\n> > option list looking for non-option, shift one more time when finding -m.\n> \n> No, it's not hard conceptually. It just means implementing the\n> option-parsing policy in two places. That's not too bad now, but if we\n> started using rev-parse's options helper, then I think you have corner\n> cases like \"git stash -km foo\".\n> \n> My \"-p\" suggestion suffers from a similar problem if you treat it as\n> \"you can omit the 'push' if you say \"-p\", rather than \"if -p is the\n> first option, it is a synonym for 'push -p'\".\n\nI'm almost convinced of special casing \"-p\". (Maybe I'm easy to\nconvince as well, because it would be convenient ;) ) However it's a\nbit weird that now \"git stash -p file\" would work, but \"git stash -m\nmessage\" wouldn't. Maybe we should do it the other way around, and\nonly special case \"-q\", and see if there is an non option argument\nafter that? From a glance at the options that's the only one where\n\"git stash -