[PATCH v5 0/6] {checkout,reset,stash} --patch

[Thomas Rast <trast@student.ethz.ch>]

Junio C Hamano wrote:
> * tr/reset-checkout-patch (Tue Jul 28 23:20:12 2009 +0200) 8 commits
[...]
> Progress?

Slow, as always.  There are three groups of changes:

1. This iteration goes the "complicated" way to mitigate Jeff's concern:

Jeff King wrote:
> Shouldn't the diff [in checkout -p] be reversed? That is, I think
> what users would like to see is "bring this hunk over from the index
> to the working tree". But we have the opposite (a hunk that is in
> the working tree that we would like to undo).

That is, the rules are now as follows:

add -p			forward application
reset -p [HEAD]		exact opposite of add -p: reverse application
reset -p other		forward application to index (**)
checkout -p		"opposite of editing": reverse application
checkout -p HEAD	"opposite of editing and staging": reverse application
checkout -p other	forward application to WT and index (**)
stash -p		"stash these edits": reverse application to WT, "forward to stash"

Those marked (**) are the only ones that changed semantics compared to
v4.  However, I adjusted the messages to look different:

add -p			Stage this hunk?
reset -p [HEAD]		Reset this hunk? (**)
reset -p other		Apply this hunk to index? (**)
checkout -p		Discard this hunk from worktree? (**)
checkout -p HEAD	Discard this hunk from index and worktree? (**)
checkout -p other	Apply this hunk to index and worktree? (**)
stash -p		Stash this hunk?

Again, (**) are the changed ones from v4.  The help message also shows
the "to/from ..." extra in the help for y/n.

I think this should now make 'reset -p' and 'checkout -p' fairly
intuitive, while at the same time making the '... other' forms easier
to wrap one's head around.  Of course, as stated earlier in the
thread, the downside with this approach is that the direction suddenly
changes when you give it an 'other'.

These changes affect all 'Implement foo --patch' patches, and the
git-apply--interactive refactoring.


2. git checkout -p HEAD fixed

Nicolas Sebrecht wrote:
> 
>   % git checkout -p HEAD
> 
> and
> 
>   % git checkout -p HEAD -- file
> 
> behave differently here in my test above.

This sadly was a rather trivial thinko on my part in the C glue for
'checkout -p', which I fixed.  I also changed the tests to cover
various ways of limiting paths.


3. Tests rewritten

I added a new 2/6 refactors the many occurences of

	test "$(cat file)" = expected_worktree &&
	test "$(git show :file)" = expected_index

to a few library functions, and rewritten all three tests to use them.
Due to the bug discussed in (2.) above, the tests also cover pathspecs
for all new commands.  Due to my own concern because this was broken
at some point during development, all commands also check if relative
paths inside a directory work.


3/6 (which was 2/5) and 7/6 (was 6/5) are unchanged, and apart from
the fix for (2.) which was a one-liner, so is all the C code.  7/6 is,
as before, based on a merge with js/stash-dwim.


Thomas Rast (7):
  git-apply--interactive: Refactor patch mode code
  Add a small patch-mode testing library
  builtin-add: refactor the meat of interactive_add()
  Implement 'git reset --patch'
  Implement 'git checkout --patch'
  Implement 'git stash save --patch'
  DWIM 'git stash save -p' for 'git stash -p'