[RFC PATCH v3 0/5] {checkout,reset,stash} --patch

From: Thomas Rast <trast@student.ethz.ch>
Date: Jul 25, 2009, 21:29 UTC
Message-ID: <cover.1248557241.git.trast@student.ethz.ch>
In-Reply-To: <7vzlat64u1.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:

> > ... Unfortunately I don't know how hard the necessary change will be,
> > because these two commands are now implemented in C...
> 
> That's OK.  There are others on the list who groks C, like Thomas ;-)

Well, then let's see if your trust is justified ;-)

>  - "add" goes from work tree to the index;
> 
>  - "reset" goes from commit to index.

Luckily "reset" is exactly symmetrical to "add", so that's not too hard.

>  - "checkout" goes from commit or index to the work tree; and

Unfortunately there are some subtleties here. In the 'git checkout HEAD -- $file' case, we overwrite both worktree and index. So morally, if you select a hunk for checking out it should be reverse applied to both, too. However, that may not always be possible.

[One possible change to checkout -p would be to reverse the direction of hunks as shown. Currently it's more in the "discard" spirit: you made the change shown in this hunk, saying 'y' will throw this change away.]

Similarly, "stash" has some problems: we want to encode the changes HEAD..index into one commit, and index..worktree into another. However, these patches may not apply on top of each other depending on what hunks were selected. I see three options:

* Make more a priori restrictions, such as, --patch is strictly about
  the worktree and simply refuses to stash anything if you have staged
  changes; or, we only deal with the worktree and always stash the
  index whole.  I think at least the first option would make it
  significantly less useful though.

* Hope that it works out, and catch failure later.  This is what it
  currently does.

* Expand the stash format to four commits so that, e.g.,
  stash^1..stash^2 is HEAD..index and stash^3..stash is
  index..worktree.  (Currently stash^1 is HEAD, stash^2 is index and
  stash is worktree.)  This would require more changes, and make these
  stashes backward incompatible w.r.t. application, so I'm not sure it
  is worth the trouble.

Yeah, there still aren't any tests, sorry. I'm not sure I will get around to it this weekend, but then it's -rc anyway and we're in no hurry.

Thomas Rast (5):
  git-apply--interactive: Refactor patch mode code
  builtin-add: refactor the meat of interactive_add()
  Implement 'git reset --patch'
  Implement 'git checkout --patch'
  Implement 'git stash save --patch'

 Documentation/git-checkout.txt |   13 +++-
 Documentation/git-reset.txt    |   15 +++-
 Documentation/git-stash.txt    |   10 ++-
 builtin-add.c                  |   43 ++++++---
 builtin-checkout.c             |   19 ++++
 builtin-reset.c                |   15 +++
 commit.h                       |    2 +
 git-add--interactive.perl      |  189 +++++++++++++++++++++++++++++++++-------
 git-stash.sh                   |  120 +++++++++++++++++++++-----
 9 files changed, 355 insertions(+), 71 deletions(-)