patchstash: --keep option just saves

The "save" subcommand usually removes the changes it stashed from the index and the work tree. Existing --keep-index option however keeps the changes to the index. This new option keeps the changes made to both the index and the work tree.

Signed-off-by: Nanako Shiraishi <nanako3@lavabit.com>
---
 Documentation/git-stash.txt |    7 +++++--
 git-stash.sh                |   35 +++++++++++++++++++++++++++++------
 t/t3903-stash.sh            |   22 ++++++++++++++++++++++
 3 files changed, 56 insertions(+), 8 deletions(-)

Nanako Shiraishi <nanako3@lavabit.com> writes:

I saw --no-reset mentioned earlier but probably this is a more consistent name for the feature.

But I somewhat doubt if this line of change is a good idea to begin with.

The earlier --keep-index feature had a clear definition of what workflow benefits from it, and also made it clear that the workflow it helped was a good workflow. You build what you would consider a good change in the index bit by bit, but you would want a final test of the whole tree, without the changes that you are still not sure and are not in the index. Before --keep-index, we did not have a good way to do so.

This one, the "snapshot", and various other related topics, are quite different. The workflow the --keep (and for that matter, "snapshot") would support I can think of does not sound a very good one we would want to recommend (--untracked is a different issue; I haven't formed an opinion).

You build on a branch, but you are forever in the state of indecision, and instead of committing, you keep saying "save --keep" number of times to leave a checkpoint on your stash. After number of iterations, you may have many stashes in "git stash list", but what you can do with them is "git reset --hard && git stash apply stash@{$n}" to go back to any of the state, but that is about it.

If the topic you are working on is that involved, and if you are afraid of contaminating the branch you started off of (which is groundless given the nature of git that is distributed --- the act of committing is explicitly disconnected from the act of publishing), then you are much better off forking the original branch to another branch on which you do your own work, and make normal commits to checkpoint your states. That way, you can use the usual history rewriting tools such as "rebase --interactive" and "merge --squash" to finish it off once you reached a good state.

All talks about using stash --no-rest and snapshot share this problem. By not making regular commits, you are denying yourself the rich set of tools git offers you to use on regular commits and the ancestry chains between them, and nobody explained what the benefits of not using normal commits on normal branches are, nor how the inconveniences from this aversion to branches forces you are justified by those unexplained benefits.

This topic won't go beyond 'next' during this round because it started way after -rc0 was tagged. It is not urgent to decide what will happen to the recent "snapshot" related topics, and we have plenty of time to toss ideas around, but currently I have to say that I am not very enthused about any of the causes mentioned in various discussion threads.

Junio C Hamano <gitster@pobox.com> writes:

Yeah, but that's a pretty useful thing.

I often save checkpoints of my working state before starting a tentative/intrusive series of edits -- if they are clearly just part of a larger logical change, I may not want to make separate commits [or perhaps more commonly, I'm not entirely sure what the final commit will be like, and am still "exploring"].

Of course this can also be done in git by doing temporary commits (to be changed later with --amend, or rolled back before making the real commit), or whatever, but I think pretty much every usage of git-stash can be done fairly easily via some other means in git; git-stash is really just a convenience.

-miles

Quoting Miles Bader <miles@gnu.org>:

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.

"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.

If 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.

An 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.

On Thu, Feb 12, 2009 at 5:17 PM, Nanako Shiraishi <nanako3@lavabit.com> wrote:

See the final paragraph of my message.

-Miles

Quoting Junio C Hamano <gitster@pobox.com> writes:

I'm sorry. I didn't think about such a negative aspect of adding the feature.

I 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.

It 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.

You already applied my patch on pu branch. It's OK by me if you dropped it.

Thank you for your comments.

Nanako Shiraishi <nanako3@lavabit.com> writes:

No need to be sorry about anything, please. Nobody has to apologise after trying a constructive contribution to solve a perceived problem.

That was exactly my point. "save --keep" (or --no-reset) might look as if it is a new convenience feature, but you will realize that it really is not, once you think things through. It is not new because there already is a way meant to be used to snapshot before going forward (i.e. regular commits on a private branch for experiment), and it is not convenient because its results cannot be used as flexibly with existing tools as the results by the other, existing approach.

It can be argued that it adds one huge convenience. It is quicker to say 'git stash save --keep' than 'git commit -a -m "WIP as of $(date)"', and in addition, you first have to do a "checkout -b wip" once, if you use the approach with regular commits on a exprimental branch forked from the target branch, instead of the approach with regular commits directly on the target branch.

But if that convenience is the real motivation behind it, adding an option "git commit --snap" that runs 'git commit -a -m "WIP as of $(date)"' would be a far more superiour solution that does not have the problem of feature duplication (and no, please do not suggest "git alias snap ..."; the point is to make it easy to do this for *everybody* out of the box).

I actually did not think of that issue, but you may be right.

Rather, lets let a hundred flowers blossom and see what happens. I will promise it will not be followed by a purge ;-).

On 2009.02.12 13:04:48 -0800, Junio C Hamano wrote:

Another thing that came to my mind is that stash --keep would not affect "git diff". I often look at the current diff to reassure myself of what I'm actually doing, and when you do intermediate commits, you can no longer use a plain "git diff", but need to use, for example, "git diff master", assuming that your current branch is exclusively for this thing you're working on. But the whole idea of taking snapshots feels more like it is for a use case that assumes that you end up with a single commit, that might in the end be a part of some bigger commit series. So for the "git diff <whatever>" to be convenient, you might need a (temporary) branch for just this one commit to be, or you need to tag your starting point.

And (in part) you also lose the ability to dynamically mark parts of your changes as "good" by staging and unstaging them. I use that quite often to "trim" the diff output to the currently interesting part. When I refactor some function, that might need a lot of callers to be trivially changed, and I don't want to be bothered with those trivial changes when I look at the diff to review my changes. So I temporarily stage that trivial stuff, do a review of the "interesting" part, change something here and there and might eventually unstage all the stuff again. Often that's more comfortable than passing a list of interesting files to each "git diff" command, and sometime that's the only way to get things filtered down enough to make me happy, because the trivial and the interesting changes are in the same file.

I think it could make sense to have that "snapshot" thing committing to another branch (or some ref in /refs/snapshots/, or whatever). Say you have topicA checked out, and work on some change "foo" that's likely going to be a single commit, but you want to test several versions of that thing, while always keeping your changes uncommitted as far as HEAD is concerned. Then "git snapshot foo", could do something like:

export GIT_INDEX_FILE=.git/snap_index

parent_ref=$(git rev-parse --symbolic-full-name HEAD) parent_ref=${parent_ref#refs/heads/}

snap_ref="refs/snapshots/$parent_ref/$1"

git read-tree HEAD git add -u # Or maybe git add -A?

parent=$(git rev-parse --verify --quiet "$snap_ref" || echo HEAD) commit=$(echo "Snapshot" | git commit-tree $(git write-tree) -p "$parent")

git update-ref "$snap_ref" "$commit" rm "$GIT_INDEX_FILE"

So you get the benefits of having your snapshots stored much like intermediate commits, but don't lose the "benefits" of having uncommitted changes, like the plain "git diff" invocation instead of "git diff <what's_my_base_today>".

Björn