{"thread":{"id":"17731","subject":"[PATCH] stash: --keep option just saves","startedAt":"2009-02-11T21:25:14Z","lastAt":"2009-02-13T07:07:24Z","messageCount":8,"participants":["Nanako Shiraishi","Junio C Hamano","Miles Bader","Björn Steinbrink"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"104301","messageId":"20090212062514.6117@nanako3.lavabit.com","threadId":"17731","inReplyTo":null,"subject":"[PATCH] stash: --keep option just saves","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2009-02-11T21:25:14Z","receivedAt":"2009-02-11T21:25:14Z","isPatch":true,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"The \"save\" subcommand usually removes the changes it stashed from the\nindex and the work tree. Existing --keep-index option however keeps the\nchanges to the index. This new option keeps the changes made to both the\nindex and the work tree.\n\nSigned-off-by: Nanako Shiraishi <nanako3@lavabit.com>\n---\n Documentation/git-stash.txt |    7 +++++--\n git-stash.sh                |   35 +++++++++++++++++++++++++++++------\n t/t3903-stash.sh            |   22 ++++++++++++++++++++++\n 3 files changed, 56 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt\nindex 051f94d..ddd68ef 100644\n--- a/Documentation/git-stash.txt\n+++ b/Documentation/git-stash.txt\n@@ -12,7 +12,7 @@ SYNOPSIS\n 'git stash' (show | drop | pop ) [<stash>]\n 'git stash' apply [--index] [<stash>]\n 'git stash' branch <branchname> [<stash>]\n-'git stash' [save [--keep-index] [<message>]]\n+'git stash' [save [--keep | --keep-index] [<message>]]\n 'git stash' clear\n 'git stash' create\n \n@@ -41,13 +41,16 @@ is also possible).\n OPTIONS\n -------\n \n-save [--keep-index] [<message>]::\n+save [--keep | --keep-index] [<message>]::\n \n \tSave your local modifications to a new 'stash', and run `git reset\n \t--hard` to revert them.  This is the default action when no\n \tsubcommand is given. The <message> part is optional and gives\n \tthe description along with the stashed state.\n +\n+If the `--keep` option is used, `git reset --hard` is omitted and your\n+changes to the index and the work tree will be kept.\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 b9ace99..0d5efaa 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -6,7 +6,7 @@ USAGE=\"list [<options>]\n    or: $dashless (show | drop | pop ) [<stash>]\n    or: $dashless apply [--index] [<stash>]\n    or: $dashless branch <branchname> [<stash>]\n-   or: $dashless [save [--keep-index] [<message>]]\n+   or: $dashless [save [--keep | --keep-index] [<message>]]\n    or: $dashless clear\"\n \n SUBDIRECTORY_OK=Yes\n@@ -93,12 +93,31 @@ create_stash () {\n }\n \n save_stash () {\n+\tkeep=\n \tkeep_index=\n-\tcase \"$1\" in\n-\t--keep-index)\n-\t\tkeep_index=t\n+\twhile test $# != 0\n+\tdo\n+\t\tcase \"$1\" in\n+\t\t--keep-index)\n+\t\t\tkeep_index=t\n+\t\t\t;;\n+\t\t--keep)\n+\t\t\tkeep=t\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\tbreak\n+\t\t\t;;\n+\t\tesac\n \t\tshift\n-\tesac\n+\tdone\n+\tif test \"$keep$keep_index\" = tt\n+\tthen\n+\t\tdie \"Cannot use --keep and --keep-index at the same time\"\n+\tfi\n \n \tstash_msg=\"$*\"\n \n@@ -120,8 +139,12 @@ save_stash () {\n \t\tdie \"Cannot save the current status\"\n \tprintf 'Saved working directory and index state \"%s\"\\n' \"$stash_msg\"\n \n-\tgit reset --hard\n+\tif test -n \"$keep\"\n+\tthen\n+\t\treturn\n+\tfi\n \n+\tgit reset --hard\n \tif test -n \"$keep_index\" && test -n $i_tree\n \tthen\n \t\tgit read-tree --reset -u $i_tree\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 7484cbe..90f0902 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -177,4 +177,26 @@ test_expect_success 'stash branch' '\n \ttest 0 = $(git stash list | wc -l)\n '\n \n+test_expect_success 'stash --keep-index' '\n+\tgit stash clear &&\n+\techo modified > file &&\n+\tgit add file &&\n+\techo changed > file &&\n+\tgit stash save --keep-index test &&\n+\tgit diff --exit-code &&\n+\ttest modified = \"$(cat file)\" &&\n+\tgit diff stash^ stash | grep \"^+changed\"\n+'\n+\n+test_expect_success 'stash --keep' '\n+\tgit stash clear &&\n+\techo modified > file &&\n+\tgit add file &&\n+\techo changed > file &&\n+\tgit stash save --keep test &&\n+\ttest changed = \"$(cat file)\" &&\n+\tgit diff --exit-code stash &&\n+\ttest modified = \"$(git show :file)\"\n+'\n+\n test_done\n-- \n1.6.2.rc0\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n"},{"id":"104304","messageId":"7vljscbp60.fsf@gitster.siamese.dyndns.org","threadId":"17731","inReplyTo":"20090212062514.6117@nanako3.lavabit.com","subject":"Re: [PATCH] stash: --keep option just saves","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-11T22:10:31Z","receivedAt":"2009-02-11T22:10:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nanako Shiraishi <nanako3@lavabit.com> writes:\n\n> The \"save\" subcommand usually removes the changes it stashed from the\n> index and the work tree. Existing --keep-index option however keeps the\n> changes to the index. This new option keeps the changes made to both the\n> index and the work tree.\n\nI saw --no-reset mentioned earlier but probably this is a more consistent\nname for the feature.\n\nBut I somewhat doubt if this line of change is a good idea to begin with.\n\nThe earlier --keep-index feature had a clear definition of what workflow\nbenefits from it, and also made it clear that the workflow it helped was a\ngood workflow.  You build what you would consider a good change in the\nindex bit by bit, but you would want a final test of the whole tree,\nwithout the changes that you are still not sure and are not in the index.\nBefore --keep-index, we did not have a good way to do so.\n\nThis one, the \"snapshot\", and various other related topics, are quite\ndifferent.  The workflow the --keep (and for that matter, \"snapshot\")\nwould support I can think of does not sound a very good one we would want\nto recommend (--untracked is a different issue; I haven't formed an\nopinion).\n\nYou build on a branch, but you are forever in the state of indecision, and\ninstead of committing, you keep saying \"save --keep\" number of times to\nleave a checkpoint on your stash.  After number of iterations, you may\nhave many stashes in \"git stash list\", but what you can do with them is\n\"git reset --hard && git stash apply stash@{$n}\" to go back to any of the\nstate, but that is about it.\n\nIf the topic you are working on is that involved, and if you are afraid of\ncontaminating the branch you started off of (which is groundless given the\nnature of git that is distributed --- the act of committing is explicitly\ndisconnected from the act of publishing), then you are much better off\nforking the original branch to another branch on which you do your own\nwork, and make normal commits to checkpoint your states.  That way, you\ncan use the usual history rewriting tools such as \"rebase --interactive\"\nand \"merge --squash\" to finish it off once you reached a good state.\n\nAll talks about using stash --no-rest and snapshot share this problem.  By\nnot making regular commits, you are denying yourself the rich set of tools\ngit offers you to use on regular commits and the ancestry chains between\nthem, and nobody explained what the benefits of not using normal commits\non normal branches are, nor how the inconveniences from this aversion to\nbranches forces you are justified by those unexplained benefits.\n\nThis topic won't go beyond 'next' during this round because it started way\nafter -rc0 was tagged.  It is not urgent to decide what will happen to the\nrecent \"snapshot\" related topics, and we have plenty of time to toss ideas\naround, but currently I have to say that I am not very enthused about any\nof the causes mentioned in various discussion threads.\n"},{"id":"104343","messageId":"buovdrgckpa.fsf@dhlpc061.dev.necel.com","threadId":"17731","inReplyTo":"7vljscbp60.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] stash: --keep option just saves","fromName":"Miles Bader","fromEmail":"miles@gnu.org","sentAt":"2009-02-12T05:01:37Z","receivedAt":"2009-02-12T05:01:37Z","isPatch":true,"sender":{"key":"miles@gnu.org","avatar":"https://gravatar.com/avatar/01069b69593af7bff28e2f97afeb3644ae6fe2f5f56cb3a8cf34c5fb8c36efe5?d=mp&s=160"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n> This one, the \"snapshot\", and various other related topics, are quite\n> different.  The workflow the --keep (and for that matter, \"snapshot\")\n> would support I can think of does not sound a very good one we would want\n> to recommend (--untracked is a different issue; I haven't formed an\n> opinion).\n>\n> You build on a branch, but you are forever in the state of indecision, and\n> instead of committing, you keep saying \"save --keep\" number of times to\n> leave a checkpoint on your stash.  After number of iterations, you may\n> have many stashes in \"git stash list\", but what you can do with them is\n> \"git reset --hard && git stash apply stash@{$n}\" to go back to any of the\n> state, but that is about it.\n\nYeah, but that's a pretty useful thing.\n\nI often save checkpoints of my working state before starting a\ntentative/intrusive series of edits -- if they are clearly just part\nof a larger logical change, I may not want to make separate commits\n[or perhaps more commonly, I'm not entirely sure what the final commit\nwill be like, and am still \"exploring\"].\n\nOf course this can also be done in git by doing temporary commits\n(to be changed later with --amend, or rolled back before making the\nreal commit), or whatever, but I think pretty much every usage of\ngit-stash can be done fairly easily via some other means in git;\ngit-stash is really just a convenience.\n\n-miles\n\n-- \nPoliteness, n. The most acceptable hypocrisy.\n"},{"id":"104351","messageId":"20090212171732.6117@nanako3.lavabit.com","threadId":"17731","inReplyTo":"7vljscbp60.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] stash: --keep option just saves","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2009-02-12T08:17:32Z","receivedAt":"2009-02-12T08:17:32Z","isPatch":true,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Quoting Junio C Hamano <gitster@pobox.com> writes:\n\n> But I somewhat doubt if this line of change is a good idea to begin with.\n>\n> All talks about using stash --no-rest and snapshot share this problem.  By\n> not making regular commits, you are denying yourself the rich set of tools\n> git offers you to use on regular commits and the ancestry chains between\n> them, and nobody explained what the benefits of not using normal commits\n> on normal branches are, nor how the inconveniences from this aversion to\n> branches forces you are justified by those unexplained benefits.\n\nI'm sorry. I didn't think about such a negative aspect of adding the feature.\n\nI can imagine that, after using many 'stash save --keep', I'll naturally want to manipulate changes between kept stashes, like running 'git log' to view the difference between each step and cherry-picking a change between ones next to each other etc. Because a list of stashes doesn't support such operations, additional support for them is needed to make it useful, but I agree with you that such additions are not real features that are necessary, because git already supports these operations if I used commits on a normal branch instead. Use of 'stash save --keep' is making it necessary to implement duplicated features for no good reason.\n\nIt may also confuse users by unnecessarily adding another way to do the same thing. So I think you're right to point out that this is not a good thing to add.\n\n> This topic won't go beyond 'next' during this round because it started way\n> after -rc0 was tagged.  It is not urgent to decide what will happen to the\n> recent \"snapshot\" related topics, and we have plenty of time to toss ideas\n> around, but currently I have to say that I am not very enthused about any\n> of the causes mentioned in various discussion threads.\n\nYou already applied my patch on pu branch. It's OK by me if you dropped it.\n\nThank you for your comments.\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n"},{"id":"104350","messageId":"20090212171742.6117@nanako3.lavabit.com","threadId":"17731","inReplyTo":"buovdrgckpa.fsf@dhlpc061.dev.necel.com","subject":"Re: [PATCH] stash: --keep option just saves","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2009-02-12T08:17:42Z","receivedAt":"2009-02-12T08:17:42Z","isPatch":true,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Quoting Miles Bader <miles@gnu.org>:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n>> This one, the \"snapshot\", and various other related topics, are quite\n>> different.  The workflow the --keep (and for that matter, \"snapshot\")\n>> would support I can think of does not sound a very good one we would want\n>> to recommend (--untracked is a different issue; I haven't formed an\n>> opinion).\n>>\n>> You build on a branch, but you are forever in the state of indecision, and\n>> instead of committing, you keep saying \"save --keep\" number of times to\n>> leave a checkpoint on your stash.  After number of iterations, you may\n>> have many stashes in \"git stash list\", but what you can do with them is\n>> \"git reset --hard && git stash apply stash@{$n}\" to go back to any of the\n>> state, but that is about it.\n>\n> Yeah, but that's a pretty useful thing.\n>\n> I often save checkpoints of my working state before starting a\n> tentative/intrusive series of edits -- if they are clearly just part\n> of a larger logical change, I may not want to make separate commits\n> [or perhaps more commonly, I'm not entirely sure what the final commit\n> will be like, and am still \"exploring\"].\n\nI suspect you either didn't read or didn't understand the rest of Junio's message in which he explained why it isn't a good use of git.\n\n\"I am still exploring\" is exactly why git already offers cheap branches, distributed operation that makes committing and sharing separate, and rich set of commands to rewrite history, so that you don't have to worry about creating a sequence of commits that may not be ideal. \"Just part of a larger logical change\" is a sign that they belong to the same branch, and being able to rewrite your local history means the boundary between parts doesn't have to be perfect in your original set of commits.\n\nIf you created five \"checkpoints\" while working on such a \"tentative/intrusive series of edits\", you will have five commits on your tentative branch that shouldn't be separated in the way they are separated. Maybe they should be squashed to a single commit in the end, or maybe you want to split them into two commits. When you finished with them, you have a much better understanding of the issues to base your decision on how to split the changes, and that is when you use history rewriting tools to finalize what you have achieved.\n\nAn important thing to understand is that in a distributed system, commits don't have to be cast in iron when you make them, and that you don't have to be afraid of making \"separate commits\" only because they belong to one topic while you are exploring. You can combine them later, and you are in control when to declare they are finished and make them visible to others.\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n"},{"id":"104353","messageId":"fc339e4a0902120028tdbad235xad7c38d504b89bc6@mail.gmail.com","threadId":"17731","inReplyTo":"20090212171742.6117@nanako3.lavabit.com","subject":"Re: [PATCH] stash: --keep option just saves","fromName":"Miles Bader","fromEmail":"miles@gnu.org","sentAt":"2009-02-12T08:28:23Z","receivedAt":"2009-02-12T08:28:23Z","isPatch":true,"sender":{"key":"miles@gnu.org","avatar":"https://gravatar.com/avatar/01069b69593af7bff28e2f97afeb3644ae6fe2f5f56cb3a8cf34c5fb8c36efe5?d=mp&s=160"},"body":"On Thu, Feb 12, 2009 at 5:17 PM, Nanako Shiraishi <nanako3@lavabit.com> wrote:\n> I suspect you either didn't read or didn't understand the rest of Junio's message in which he explained why it isn't a good use of git.\n\nSee the final paragraph of my message.\n\n-Miles\n\n-- \nDo not taunt Happy Fun Ball.\n"},{"id":"104422","messageId":"7vskmjjrin.fsf@gitster.siamese.dyndns.org","threadId":"17731","inReplyTo":"20090212171732.6117@nanako3.lavabit.com","subject":"Re: [PATCH] stash: --keep option just saves","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-12T21:04:48Z","receivedAt":"2009-02-12T21:04:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nanako Shiraishi <nanako3@lavabit.com> writes:\n\n> I'm sorry. I didn't think about such a negative aspect of adding the\n> feature.\n\nNo need to be sorry about anything, please.  Nobody has to apologise after\ntrying a constructive contribution to solve a perceived problem.\n\n> I can imagine that, after using many 'stash save --keep', I'll naturally\n> want to manipulate changes between kept stashes, like running 'git log'\n> to view the difference between each step and cherry-picking a change\n> between ones next to each other etc. Because a list of stashes doesn't\n> support such operations, additional support for them is needed to make\n> it useful, but I agree with you that such additions are not real\n> features that are necessary, because git already supports these\n> operations if I used commits on a normal branch instead. Use of 'stash\n> save --keep' is making it necessary to implement duplicated features for\n> no good reason.\n\nThat was exactly my point.  \"save --keep\" (or --no-reset) might look as if\nit is a new convenience feature, but you will realize that it really is\nnot, once you think things through.  It is not new because there already\nis a way meant to be used to snapshot before going forward (i.e. regular\ncommits on a private branch for experiment), and it is not convenient\nbecause its results cannot be used as flexibly with existing tools as the\nresults by the other, existing approach.\n\nIt can be argued that it adds one huge convenience.  It is quicker to say\n'git stash save --keep' than 'git commit -a -m \"WIP as of $(date)\"', and\nin addition, you first have to do a \"checkout -b wip\" once, if you use the\napproach with regular commits on a exprimental branch forked from the\ntarget branch, instead of the approach with regular commits directly on\nthe target branch.\n\nBut if that convenience is the real motivation behind it, adding an option\n\"git commit --snap\" that runs 'git commit -a -m \"WIP as of $(date)\"' would\nbe a far more superiour solution that does not have the problem of feature\nduplication (and no, please do not suggest \"git alias snap ...\"; the point\nis to make it easy to do this for *everybody* out of the box).\n\n> It may also confuse users by unnecessarily adding another way to do the\n> same thing.\n\nI actually did not think of that issue, but you may be right.\n\n>> This topic won't go beyond 'next' during this round because it started way\n>> after -rc0 was tagged.  It is not urgent to decide what will happen to the\n>> recent \"snapshot\" related topics, and we have plenty of time to toss ideas\n>> around, but currently I have to say that I am not very enthused about any\n>> of the causes mentioned in various discussion threads.\n>\n> You already applied my patch on pu branch. It's OK by me if you dropped it.\n\nRather, lets let a hundred flowers blossom and see what happens.  I will\npromise it will not be followed by a purge ;-).\n"},{"id":"104484","messageId":"20090213070724.GD20552@atjola.homenet","threadId":"17731","inReplyTo":"7vskmjjrin.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] stash: --keep option just saves","fromName":"Björn Steinbrink","fromEmail":"b.steinbrink@gmx.de","sentAt":"2009-02-13T07:07:24Z","receivedAt":"2009-02-13T07:07:24Z","isPatch":true,"sender":{"key":"b.steinbrink@gmx.de","avatar":"https://avatars.githubusercontent.com/u/230962?v=4"},"body":"On 2009.02.12 13:04:48 -0800, Junio C Hamano wrote:\n> It can be argued that it adds one huge convenience.  It is quicker to say\n> 'git stash save --keep' than 'git commit -a -m \"WIP as of $(date)\"', and\n> in addition, you first have to do a \"checkout -b wip\" once, if you use the\n> approach with regular commits on a exprimental branch forked from the\n> target branch, instead of the approach with regular commits directly on\n> the target branch.\n\nAnother thing that came to my mind is that stash --keep would not affect\n\"git diff\". I often look at the current diff to reassure myself of what\nI'm actually doing, and when you do intermediate commits, you can no\nlonger use a plain \"git diff\", but need to use, for example, \"git diff\nmaster\", assuming that your current branch is exclusively for this thing\nyou're working on. But the whole idea of taking snapshots feels more\nlike it is for a use case that assumes that you end up with a single\ncommit, that might in the end be a part of some bigger commit series. So\nfor the \"git diff <whatever>\" to be convenient, you might need a\n(temporary) branch for just this one commit to be, or you need to tag\nyour starting point.\n\nAnd (in part) you also lose the ability to dynamically mark parts of\nyour changes as \"good\" by staging and unstaging them. I use that quite\noften to \"trim\" the diff output to the currently interesting part. When\nI refactor some function, that might need a lot of callers to be\ntrivially changed, and I don't want to be bothered with those trivial\nchanges when I look at the diff to review my changes. So I temporarily\nstage that trivial stuff, do a review of the \"interesting\" part, change\nsomething here and there and might eventually unstage all the stuff\nagain. Often that's more comfortable than passing a list of interesting\nfiles to each \"git diff\" command, and sometime that's the only way to\nget things filtered down enough to make me happy, because the trivial\nand the interesting changes are in the same file.\n\nI think it could make sense to have that \"snapshot\" thing committing to\nanother branch (or some ref in /refs/snapshots/, or whatever). Say you\nhave topicA checked out, and work on some change \"foo\" that's likely\ngoing to be a single commit, but you want to test several versions of\nthat thing, while always keeping your changes uncommitted as far as HEAD\nis concerned. Then \"git snapshot foo\", could do something like:\n\nexport GIT_INDEX_FILE=.git/snap_index\n\nparent_ref=$(git rev-parse --symbolic-full-name HEAD)\nparent_ref=${parent_ref#refs/heads/}\n\nsnap_ref=\"refs/snapshots/$parent_ref/$1\"\n\ngit read-tree HEAD\ngit add -u # Or maybe git add -A?\n\nparent=$(git rev-parse --verify --quiet \"$snap_ref\" || echo HEAD)\ncommit=$(echo \"Snapshot\" | git commit-tree $(git write-tree) -p \"$parent\")\n\ngit update-ref \"$snap_ref\" \"$commit\"\nrm \"$GIT_INDEX_FILE\"\n\n\nSo you get the benefits of having your snapshots stored much like\nintermediate commits, but don't lose the \"benefits\" of having\nuncommitted changes, like the plain \"git diff\" invocation instead of\n\"git diff <what's_my_base_today>\".\n\nBjörn\n"}]}