{"thread":{"id":"42197","subject":"[PATCH] git-stash: add flag to skip \"git reset --hard\"","startedAt":"2016-05-01T21:57:18Z","lastAt":"2016-05-02T22:44:27Z","messageCount":6,"participants":["Tom Anderson","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"285138","messageId":"57267BBE.9010707@google.com","threadId":"42197","inReplyTo":null,"subject":"[PATCH] git-stash: add flag to skip \"git reset --hard\"","fromName":"Tom Anderson","fromEmail":"thomasanderson@google.com","sentAt":"2016-05-01T21:57:18Z","receivedAt":"2016-05-01T21:57:18Z","isPatch":true,"sender":{"key":"thomasanderson@google.com","avatar":null},"body":"Add the \"--no-reset\" (same as \"-r\") flag to git-stash.sh. A common\nworkflow is to use \"git-stash save\" and \"git-stash apply\" to save your\nwork without modifying your changed files. This forces users to\nreload their changed files in text editors like emacs, and touches the\nmodify time on all changed files, potentially making incremental\nbuilds slower if many files in the build depend on your changed files.\n\nAdd documentation in Documentation/git-stash.txt and tests in\nt/t3903-stash.sh.\n\nSigned-off-by: Thomas Anderson \n---\n Documentation/git-stash.txt | 9 ++++++---\n git-stash.sh | 15 ++++++++++-----\n t/t3903-stash.sh | 27 +++++++++++++++++++++++++++\n 3 files changed, 43 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt\nindex 92df596..ba5ecf2 100644\n--- a/Documentation/git-stash.txt\n+++ b/Documentation/git-stash.txt\n@@ -13,8 +13,8 @@ SYNOPSIS\n 'git stash' drop [-q|--quiet] []\n 'git stash' ( pop | apply ) [--index] [-q|--quiet] []\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' [save [-p|--patch] [-r|--no-reset] [-k|--[no-]keep-index]\n+\t [-q|--quiet] [-u|--include-untracked] [-a|--all] []]\n 'git stash' clear\n 'git stash' create []\n 'git stash' store [-m|--message ] [-q|--quiet] \n@@ -44,7 +44,7 @@ is also possible).\n OPTIONS\n -------\n -save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] \n[-a|--all] [-q|--quiet] []::\n+save [-p|--patch] [-r|--no-reset] [-k|--[no-]keep-index] \n[-u|--include-untracked] [-a|--all] [-q|--quiet] []::\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@@ -61,6 +61,9 @@ stashed and then cleaned up with `git clean`, leaving \nthe working directory\n in a very clean state. If the `--all` option is used instead then the\n ignored files are stashed and cleaned in addition to the untracked files.\n +\n+If the `--no-reset` option is used, `git reset --hard` is skipped and the\n+`--[no-]keep-index`, `--include-untracked`, and `--all` flags are ignored.\n++\n With `--patch`, you can interactively select hunks from the diff\n between HEAD and the working tree to be stashed. The stash entry is\n constructed such that its index state is the same as the index state\ndiff --git a/git-stash.sh b/git-stash.sh\nindex c7c65e2..aebebb0 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -7,8 +7,9 @@ USAGE=\"list []\n or: $dashless drop [-q|--quiet] []\n or: $dashless ( pop | apply ) [--index] [-q|--quiet] []\n or: $dashless branch []\n- or: $dashless [save [--patch] [-k|--[no-]keep-index] [-q|--quiet]\n-\t\t [-u|--include-untracked] [-a|--all] []]\n+ or: $dashless [save [--patch] [-r|--no-reset] [-k|--[no-]keep-index]\n+\t\t [-q|--quiet] [-u|--include-untracked] [-a|--all]\n+\t\t []]\n or: $dashless clear\"\n SUBDIRECTORY_OK=Yes\n@@ -194,9 +195,13 @@ save_stash () {\n \tkeep_index=\n \tpatch_mode=\n \tuntracked=\n+\tno_reset=\n \twhile test $# != 0\n \tdo\n \t\tcase \"$1\" in\n+\t\t-r|--no-reset)\n+\t\t\tno_reset=t\n+\t\t\t;;\n \t\t-k|--keep-index)\n \t\t\tkeep_index=t\n \t\t\t;;\n@@ -268,7 +273,7 @@ save_stash () {\n \tdie \"$(gettext \"Cannot save the current status\")\"\n \tsay Saved working directory and index state \"$stash_msg\"\n -\tif test -z \"$patch_mode\"\n+\tif test -z \"$patch_mode\" -a -z \"$no_reset\"\n \tthen\n \t\tgit reset --hard ${GIT_QUIET:+-q}\n \t\ttest \"$untracked\" = \"all\" && CLEAN_X_OPTION=-x || CLEAN_X_OPTION=\n@@ -276,12 +281,12 @@ save_stash () {\n \t\tthen\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 \t\tthen\n \t\t\tgit read-tree --reset -u $i_tree\n \t\tfi\n-\telse\n+\telif -n \"$patch_mode\"\n+\tthen\n \t\tgit apply -R < \"$TMP-patch\" ||\n \t\tdie \"$(gettext \"Cannot remove worktree changes\")\"\n diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 2142c1f..3eba19a 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -248,6 +248,33 @@ test_expect_success 'stash --no-keep-index' '\n \ttest bar,bar2 = $(cat file),$(cat file2)\n '\n +test_expect_success 'stash -r' '\n+\tgit reset &&\n+\techo foo420 > file &&\n+\techo bar420 > file2 &&\n+\tgit add file2 &&\n+\tgit stash -r &&\n+\ttest foo420,bar420 = $(cat file),$(cat file2)\n+'\n+\n+test_expect_success 'stash --no-reset' '\n+\tgit reset &&\n+\techo bar33 > file &&\n+\techo bar44 > file2 &&\n+\tgit add file2 &&\n+\tgit stash --no-reset &&\n+\ttest bar33,bar44 = $(cat file),$(cat file2)\n+'\n+\n+test_expect_success 'stash -r with ignored options -k -u -a' '\n+\tgit reset &&\n+\techo foo55 > file &&\n+\techo bar66 > file2 &&\n+\tgit add file2 &&\n+\tgit stash -r &&\n+\ttest foo55,bar66 = $(cat file),$(cat file2)\n+'\n+\n test_expect_success 'stash --invalid-option' '\n \techo bar5 > file &&\n \techo bar6 > file2 &&\n-- \n2.8.0.rc2\n"},{"id":"285186","messageId":"xmqqeg9kti6x.fsf@gitster.mtv.corp.google.com","threadId":"42197","inReplyTo":"57267BBE.9010707@google.com","subject":"Re: [PATCH] git-stash: add flag to skip \"git reset --hard\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-02T18:42:46Z","receivedAt":"2016-05-02T18:42:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tom Anderson writes:\n\n> @@ -61,6 +61,9 @@ stashed and then cleaned up with `git clean`,\n> leaving the working directory\n> in a very clean state. If the `--all` option is used instead then the\n> ignored files are stashed and cleaned in addition to the untracked files.\n> +\n> +If the `--no-reset` option is used, `git reset --hard` is skipped and the\n> +`--[no-]keep-index`, `--include-untracked`, and `--all` flags are ignored.\n> ++\n\nI am afraid that a reader who does not read git-stash.sh script\nwould not know what you are talking about. They do not know (or\nparticularly care) where \"git reset --hard\", how often and for what\npurpose. They can tell that this option affects only \"save\",\nbecause that is where it is described, but they would not know what\nit means to \"skip reset --hard\", other than that they cannot use the\nthree features listed there.\n\nIt is unclear what problem you are trying to solve from this text,\nand the log message's mention of mtime and rebuilding makes it\nsound like an X-Y problem.\n\nIt could very well be that what you are trying to implement makes\nperfect sense and the new option is named with a stress on a wrong\naspect (i.e. named after what it uses to achieve things, rather than\nsaying what you are trying to achieve).\n\nThe workhorse used in \"git stash save\" (which is what you are\ntouching) is \"git stash create\", and that is only responsible for\nrecording a new stash entry without touch the working tree. Is that\nwhat you are after, perhaps?\n"},{"id":"285193","messageId":"5727A6DF.5020204@google.com","threadId":"42197","inReplyTo":"xmqqeg9kti6x.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] git-stash: add flag to skip \"git reset --hard\"","fromName":"Tom Anderson","fromEmail":"thomasanderson@google.com","sentAt":"2016-05-02T19:13:35Z","receivedAt":"2016-05-02T19:13:35Z","isPatch":true,"sender":{"key":"thomasanderson@google.com","avatar":null},"body":"\nOn 05/02/2016 11:42 AM, Junio C Hamano wrote:\n> Tom Anderson writes:\n>\n>> @@ -61,6 +61,9 @@ stashed and then cleaned up with `git clean`,\n>> leaving the working directory\n>> in a very clean state. If the `--all` option is used instead then the\n>> ignored files are stashed and cleaned in addition to the untracked files.\n>> +\n>> +If the `--no-reset` option is used, `git reset --hard` is skipped and the\n>> +`--[no-]keep-index`, `--include-untracked`, and `--all` flags are ignored.\n>> ++\n> I am afraid that a reader who does not read git-stash.sh script\n> would not know what you are talking about. They do not know (or\n> particularly care) where \"git reset --hard\", how often and for what\n> purpose. They can tell that this option affects only \"save\",\n> because that is where it is described, but they would not know what\n> it means to \"skip reset --hard\", other than that they cannot use the\n> three features listed there.\nThe man page for git-stash unfortunately already makes a reference to\ngit reset --hard:\n\n save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked]\n [-a|--all] [-q|--quiet] []\n Save your local modifications to a new stash, and run git reset\n --hard to revert them\n>\n> It is unclear what problem you are trying to solve from this text,\n> and the log message's mention of mtime and rebuilding makes it\n> sound like an X-Y problem.\n>\n> It could very well be that what you are trying to implement makes\n> perfect sense and the new option is named with a stress on a wrong\n> aspect (i.e. named after what it uses to achieve things, rather than\n> saying what you are trying to achieve).\nPerhaps it would be better to rename --no-reset to something more\nappropriate?\n> The workhorse used in \"git stash save\" (which is what you are\n> touching) is \"git stash create\", and that is only responsible for\n> recording a new stash entry without touch the working tree. Is that\n> what you are after, perhaps?\n>\nYes, but I like my stashes to be saved in the ref namespace. Maybe an\nalternative if it's a bad idea to add this new flag to \"git stash save\" is\nto add a new flag to \"git stash create\"? Or make a new git-stash command\nentirely?\n"},{"id":"285202","messageId":"xmqqh9egs04d.fsf@gitster.mtv.corp.google.com","threadId":"42197","inReplyTo":"5727A6DF.5020204@google.com","subject":"Re: [PATCH] git-stash: add flag to skip \"git reset --hard\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-02T19:58:26Z","receivedAt":"2016-05-02T19:58:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tom Anderson writes:\n\n> Yes, but I like my stashes to be saved in the ref namespace.\n\nIsn't that something you can do so yourself with store_stash?\n"},{"id":"285211","messageId":"5727BE65.3040004@google.com","threadId":"42197","inReplyTo":"xmqqh9egs04d.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] git-stash: add flag to skip \"git reset --hard\"","fromName":"Tom Anderson","fromEmail":"thomasanderson@google.com","sentAt":"2016-05-02T20:53:57Z","receivedAt":"2016-05-02T20:53:57Z","isPatch":true,"sender":{"key":"thomasanderson@google.com","avatar":null},"body":"What I want can be achieved using \"git stash store `git stash create`\"\n\nOn 05/02/2016 12:58 PM, Junio C Hamano wrote:\n> Tom Anderson writes:\n>\n>> Yes, but I like my stashes to be saved in the ref namespace.\n> Isn't that something you can do so yourself with store_stash?\n"},{"id":"285221","messageId":"xmqqbn4oqdv8.fsf@gitster.mtv.corp.google.com","threadId":"42197","inReplyTo":"5727BE65.3040004@google.com","subject":"Re: [PATCH] git-stash: add flag to skip \"git reset --hard\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-02T22:44:27Z","receivedAt":"2016-05-02T22:44:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tom Anderson writes:\n\n> On 05/02/2016 12:58 PM, Junio C Hamano wrote:\n>> Tom Anderson writes:\n>>\n>>> Yes, but I like my stashes to be saved in the ref namespace.\n>> Isn't that something you can do so yourself with store_stash?\n>\n> What I want can be achieved using \"git stash store `git stash create`\"\n\nOK.\n\nI am not strongly opposed to either a new option to create\n(i.e. \"create --store\" stores a new stash entry) or a new\nsubcommand.\n\nIt just felt that a new option to \"save\" that deliberately breaks\nthe basic premise of the command, i.e. \"my working tree is a mess,\nand I want to revert it to the pristine state quickly to work on\nsomething else that is urgent, but I want to store the mess away\ninstead of discarding, so that I can come back to it later\", was\ntotally out of place. I.e.\n\n\twork work work to create a mess\n\tgit stash save\n work on a quick and urgent stuff, knowing that you are\n building on a solidly committed state\n git commit\n git stash pop\n continue building on the mess\n\nis what \"save\" is about.\n\nAs your workflow is quite different (without \"reset --hard\", you no\nlonger can rely on the resulting state to be pristine, suitable to\nwork on something totally unrelated), it would have needed way way\nmore explanation in the description section to describe this quite\ndifferent mode of operation, as it is quite incompatible to the way\ntraditional \"stash save\" users would want to use the command for if\nyou do not revert the working tree to the pristine state. Instead\nyou would do something else (which was not described--if the answer\nis \"keep working, leading to the creation of the next commit\", it is\nunclear what the resulting stash entry would be used for, as it\nobviously won't apply to that state, as the stashed change is\nalready contained in that next commit).\n\n\"git stash save --keep\" is bad enough already; it does not give the\nuser pristine state wrt the current HEAD--what it does is a pristine\nstate wrt the next HEAD that _would_ have resulted if the stuff\nyou've been working on and decided are good by doing \"git add\" is\nall committed. In hindsight, it may have deserved a separate\ncommand to make it easier to explain to the end users.\n\nLet's not make it worse.\n\nThis is a tangent, but I am not sure where your aversion to \"reset\n--hard\" comes from. If something was changed, mtime changes, and\notherwise your build would break. It's not like \"reset --hard\"\ntouches mtime for every path in the working tree including unchanged\nones.\n"}]}