Skip to content

{checkout,reset,stash} --patch

Thomas Rast git 3 versions In review

Get series with b4
b4 am [email protected]

Versions newest first · linked by author and title

  1. v5 Latest6 patches 9 repliesIn review
  2. v45 patches 9 repliesSuperseded
  3. v3 RFC5 patches 3 repliesSuperseded

Patches in v5 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/6git-apply--interactive: Refactor patch mode codegit-add--interactive.perl+55 -270 replies
2/6Add a small patch-mode testing libraryt/lib-patch-mode.sh+36 -00 replies
3/6builtin-add: refactor the meat of interactive_add()builtin-add.c commit.h+31 -140 replies
4/6Implement 'git reset --patch'Documentation/git-reset.txt builtin-reset.c git-add--interactive.perl +1 more+154 -60 replies
4/6Implement 'git reset --patch'Documentation/git-reset.txt builtin-reset.c git-add--interactive.perl +1 more+154 -60 replies
5/6Implement 'git checkout --patch'Documentation/git-checkout.txt builtin-checkout.c git-add--interactive.perl +1 more+199 -10 replies
5/6Implement 'git checkout --patch'Documentation/git-checkout.txt builtin-checkout.c git-add--interactive.perl +1 more+199 -10 replies
6/6Implement 'git stash save --patch'Documentation/git-stash.txt git-add--interactive.perl git-stash.sh +1 more+145 -180 replies

Reviews trailers per patch

None of the 8 patches has a Reviewed-by or Acked-by.

PatchNicolas Sebrecht
SeriesT-b
1/6
2/6
3/6
4/6
4/6
5/6
5/6
6/6

Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes.

Read the v5 thread

Cover letter [PATCH v5 0/6] {checkout,reset,stash} --patch
Quoted 1 line · Junio C Hamano

* tr/reset-checkout-patch (Tue Jul 28 23:20:12 2009 +0200) 8 commits

[...]

Quoted 1 line

Progress?

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

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

Quoted 4 lines · Jeff King

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

Quoted 8 lines · Nicolas Sebrecht

% 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'

Read the cover letter in its thread