git-add -p: be able to undo a given hunk

One of my most frequent use case for git-add -p is when I had an intense debug session with quite a lot of debug() traces added. I then want only to select the hunks corresponding to the bugfixes and throw away the debug ones.

With this new operation, instead of not staging hunks I don't want and will eventually undo, I can just undo them.

---

I reckon this is a tad late given we're already at -rc2, but that's an itch that has scratched me for quite some time already, and I had to scratch it today...

the change looks pretty safe to me though.

The only think that looks odd in the patch is the removal of the if ($_->{USE}) clause from the TEXT copying loops, but coalesce_overlapping_hunks already ensures that only ->{USE}d hunks remain. I just have modified it to deal with ->{UNDO}ed hunks the same way.

 Documentation/git-add.txt |    1 +
 git-add--interactive.perl |   38 ++++++++++++++++++++++++++++++--------
 2 files changed, 31 insertions(+), 8 deletions(-)

index ab1943c..7b173dc 100644
@@ -254,6 +254,7 @@ patch::
 
        y - stage this hunk
        n - do not stage this hunk
+       u - do not stage this hunk and revert it
        q - quit, do not stage this hunk nor any of the remaining ones
        a - stage this and all the remaining hunks in the file
        d - do not stage this hunk nor any of the remaining hunks in the file

index df9f231..945de9d 100755
@@ -693,6 +693,7 @@ sub split_hunk {
 			ADDDEL => 0,
 			POSTCTX => 0,
 			USE => undef,
+			UNDO => undef,
 		};
 
 		while (++$i < @$text) {

@@ -835,12 +836,13 @@ sub merge_hunk {
 }
 
 sub coalesce_overlapping_hunks {
+	my $field = shift;
 	my (@in) = @_;
 	my @out = ();
 
 	my ($last_o_ctx, $last_was_dirty);
 
-	for (grep { $_->{USE} } @in) {
+	for (grep { $_->{$field} } @in) {
 		my $text = $_->{TEXT};
 		my ($o_ofs) = parse_hunk_header($text->[0]);
 		if (defined $last_o_ctx &&

@@ -991,6 +993,7 @@ sub help_patch_cmd {
 	print colored $help_color, <<\EOF ;
 y - stage this hunk
 n - do not stage this hunk
+u - do not stage this hunk and revert it
 q - quit, do not stage this hunk nor any of the remaining ones
 a - stage this and all the remaining hunks in the file
 d - do not stage this hunk nor any of the remaining hunks in the file

@@ -1140,7 +1143,7 @@ sub patch_update_file {
 		}
 		print colored $prompt_color, 'Stage ',
 		  ($hunk[$ix]{TYPE} eq 'mode' ? 'mode change' : 'this hunk'),
-		  " [y,n,q,a,d,/$other,?]? ";
+		  " [y,n,u,q,a,d,/$other,?]? ";
 		my $line = prompt_single_character;
 		if ($line) {
 			if ($line =~ /^y/i) {

@@ -1149,6 +1152,10 @@ sub patch_update_file {
 			elsif ($line =~ /^n/i) {
 				$hunk[$ix]{USE} = 0;
 			}
+			elsif ($line =~ /^u/) {
+				$hunk[$ix]{USE} = 0;
+				$hunk[$ix]{UNDO} = 1;
+			}
 			elsif ($line =~ /^a/i) {
 				while ($ix < $num) {
 					if (!defined $hunk[$ix]{USE}) {

@@ -1301,14 +1308,14 @@ sub patch_update_file {
 		}
 	}
 
-	@hunk = coalesce_overlapping_hunks(@hunk);
-
 	my $n_lofs = 0;
 	my @result = ();
-	for (@hunk) {
-		if ($_->{USE}) {
-			push @result, @{$_->{TEXT}};
-		}
+	my @undo = ();
+	for (coalesce_overlapping_hunks("USE", @hunk)) {
+		push @result, @{$_->{TEXT}};
+	}
+	for (coalesce_overlapping_hunks("UNDO", @hunk)) {
+		push @undo, @{$_->{TEXT}};
 	}
 
 	if (@result) {

@@ -1326,6 +1333,21 @@ sub patch_update_file {
 		refresh();
 	}
 
+	if (@undo) {
+		my $fh;
+
+		open $fh, '| git apply -R';
+		for (@{$head->{TEXT}}, @undo) {
+			print $fh $_;
+		}
+		if (!close $fh) {
+			for (@{$head->{TEXT}}, @undo) {
+				print STDERR $_;
+			}
+		}
+		refresh();
+	}
+
 	print "\n";
 	return $quit;
 }

1.6.4.rc1.189.g9f628.dirty

This is what 'git checkout --interactive -- $file' or 'git reset --interactive --hard' would accomplish, if they existed. I wonder if there would be a way to avoid yet more command letters, and instead have "modes" that affect what happens with hunks you said 'y' to. For example:

  add -p		apply --cached
  undo -p		apply -R
  unstage -p		apply -R --cached
    [with hunks coming from diff --cached obviously]

(I picked 'undo' and 'unstage' semi-randomly, but it's not, after all, an 'add' operation any more and the user doesn't need to know that the program doing this is in fact git-add--interactive.)

you're overloading terminology a bit too much for my taste. It has nothing to do with what git-revert does, and we shouldn't confuse people more about that.

Tests?

Thomas Rast
trast@{inf,student}.ethz.ch

The point is I want to do them at once, see my use case: I want to stage the hunks from my bugfix, and remove the debugging bits of it at the same time, I don't want to run two commands.

I see git add -p as a triaging command, giving me the choice to do the three things you can do with a hunk:
- stage it now (y)
- stage it later (n)
- never stage it, it was intermediate code, debug, whatever (u).

Right, revert is probably too overloaded, let it be "forget" or "drop" instead if you want, I don't care much.

Riight, will do :)

I hesitated to do so, but yes it makes perfect sense.

Indeed.

Intersec <http://www.intersec.com>
Pierre Habouzit <[email protected]>
Tél : +33 (0)1 5570 3346
Mob : +33 (0)6 1636 8131
Fax : +33 (0)1 5570 3332
37 Rue Pierre Lhomme
92400 Courbevoie

This is just the required change to git-add--interactive. Since the use of this script is an implementation detail, the next step would be to wrap this option in other commands such as 'git reset --patch' and 'git unstage --patch'.

Since there is no frontend support whatsoever, you have to run

  git add--interactive --patch=unstage --

manually (including the --), and similarly for --patch=reset.
---

Like so, maybe. I realise that it does _not_ fill Pierre's need for a command that can do it all in one go, but if people like this I'll make a real patch series.

I'm not completely happy with the idea of reset --interactive by the way, as it's not a reset in the 'reset $commit' sense.

 git-add--interactive.perl |   60 +++++++++++++++++++++++++++++++++-----------
 1 files changed, 45 insertions(+), 15 deletions(-)

index df9f231..ad322df 100755
@@ -73,6 +73,27 @@
 # command line options
 my $patch_mode;
 
+my %patch_modes = (
+	'stage' => {
+		DIFF => 'diff-files -p',
+		APPLY => 'apply --cached',
+		VERB => 'Stage',
+		PARTICIPLE => 'Staging',
+	},
+	'unstage' => {
+		DIFF => 'diff-index -p --cached HEAD',
+		APPLY => 'apply -R --cached',
+		VERB => 'UNstage',
+		PARTICIPLE => 'UNstaging',
+	},
+	'reset' => {
+		DIFF => 'diff-files -p',
+		APPLY => 'apply -R',
+		VERB => 'RESET',
+		PARTICIPLE => 'RESETTING',
+	},
+);
+
 sub run_cmd_pipe {
 	if ($^O eq 'MSWin32' || $^O eq 'msys') {
 		my @invalid = grep {m/[":*]/} @_;

@@ -615,10 +636,11 @@
 
 sub parse_diff {
 	my ($path) = @_;
-	my @diff = run_cmd_pipe(qw(git diff-files -p --), $path);
+	my @diff_cmd = split(" ", $patch_modes{$patch_mode}{DIFF});
+	my @diff = run_cmd_pipe("git", @diff_cmd, "--", $path);
 	my @colored = ();
 	if ($diff_use_color) {
-		@colored = run_cmd_pipe(qw(git diff-files -p --color --), $path);
+		@colored = run_cmd_pipe("git", @diff_cmd, qw(--color --), $path);
 	}
 	my (@hunk) = { TEXT => [], DISPLAY => [], TYPE => 'header' };
 

@@ -877,6 +899,7 @@
 		or die "failed to open hunk edit file for writing: " . $!;
 	print $fh "# Manual hunk edit mode -- see bottom for a quick guide\n";
 	print $fh @$oldtext;
+	my $participle = $patch_modes{$patch_mode}{PARTICIPLE};
 	print $fh <<EOF;
 # ---
 # To remove '-' lines, make them ' ' lines (context).

@@ -884,7 +907,7 @@
 # Lines starting with # will be removed.
 #
 # If the patch applies cleanly, the edited hunk will immediately be
-# marked for staging. If it does not apply cleanly, you will be given
+# marked for $participle. If it does not apply cleanly, you will be given
 # an opportunity to edit again. If all lines of the hunk are removed,
 # then the edit is aborted and the hunk is left unchanged.
 EOF

@@ -918,7 +941,7 @@
 
 sub diff_applies {
 	my $fh;
-	open $fh, '| git apply --recount --cached --check';
+	open $fh, '| git '.$patch_modes{$patch_mode}{APPLY}.' --recount --check';
 	for my $h (@_) {
 		print $fh @{$h->{TEXT}};
 	}

@@ -988,12 +1011,13 @@
 }
 
 sub help_patch_cmd {
-	print colored $help_color, <<\EOF ;
-y - stage this hunk
-n - do not stage this hunk
-q - quit, do not stage this hunk nor any of the remaining ones
-a - stage this and all the remaining hunks in the file
-d - do not stage this hunk nor any of the remaining hunks in the file
+	my $verb = lc $patch_modes{$patch_mode}{VERB};
+	print colored $help_color, <<EOF ;
+y - $verb this hunk
+n - do not $verb this hunk
+q - quit, do not $verb this hunk nor any of the remaining ones
+a - $verb this and all the remaining hunks in the file
+d - do not $verb this hunk nor any of the remaining hunks in the file
 g - select a hunk to go to
 / - search for a hunk matching the given regex
 j - leave this hunk undecided, see next undecided hunk

@@ -1138,8 +1162,8 @@
 		for (@{$hunk[$ix]{DISPLAY}}) {
 			print;
 		}
-		print colored $prompt_color, 'Stage ',
-		  ($hunk[$ix]{TYPE} eq 'mode' ? 'mode change' : 'this hunk'),
+		print colored $prompt_color, $patch_modes{$patch_mode}{VERB},
+		  ($hunk[$ix]{TYPE} eq 'mode' ? ' mode change' : ' this hunk'),
 		  " [y,n,q,a,d,/$other,?]? ";
 		my $line = prompt_single_character;
 		if ($line) {

@@ -1314,7 +1338,7 @@
 	if (@result) {
 		my $fh;
 
-		open $fh, '| git apply --cached --recount';
+		open $fh, '| git '.$patch_modes{$patch_mode}{APPLY}.' --recount';
 		for (@{$head->{TEXT}}, @result) {
 			print $fh $_;
 		}

@@ -1363,8 +1387,14 @@
 sub process_args {
 	return unless @ARGV;
 	my $arg = shift @ARGV;
-	if ($arg eq "--patch") {
-		$patch_mode = 1;
+	if ($arg =~ /--patch(?:=(.*))?/) {
+		if (defined $1 && defined $patch_modes{$1}) {
+			$patch_mode = $1;
+		} elsif (defined $1) {
+			die "unknown --patch mode: $1";
+		} else {
+			$patch_mode = 'stage';
+		}
 		$arg = shift @ARGV or die "missing --";
 		die "invalid argument $arg, expecting --"
 		    unless $arg eq "--";

1.6.4.rc2.215.g4f661

The new command 'git unstage' is the precise opposite of 'git stage' (i.e., git-add). As such, it is the same as 'git reset --' unless the current branch is unborn.

---

So I decided I had wished for these frequently enough to actually implement them. I ran out of time before getting to the tests, but the rest should be there.

v1 had a bug where it would not correctly detect the changed files, this is fixed in the new patch (now 3/3).

 Documentation/git-unstage.txt |   26 ++++++++++++++++++++++++++
 Makefile                      |    1 +
 git-unstage.sh                |   26 ++++++++++++++++++++++++++
 wt-status.c                   |    6 +-----
 4 files changed, 54 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/git-unstage.txt
 create mode 100644 git-unstage.sh

new file mode 100644
index 0000000..49d09fb
@@ -0,0 +1,26 @@
+git-unstage(1)
+==============
+
+NAME
+----
+git-unstage - Remove changes to a file from the staging area
+
+
+SYNOPSIS
+--------
+[verse]
+'git unstage' <paths> ...
+
+
+DESCRIPTION
+-----------
+
+Overwrites the staged changes to the 'paths' with the values from
+HEAD, so that they are not included in the next commit.  The worktree
+is not affected.  (This is the same as `git reset \-- <paths>` unless
+you are on an unborn branch.)
+
+
+SEE ALSO
+--------
+linkgit:git-reset[1]

index 75b9dcb..9e48fdc 100644
@@ -329,6 +329,7 @@ SCRIPT_SH += git-request-pull.sh
 SCRIPT_SH += git-sh-setup.sh
 SCRIPT_SH += git-stash.sh
 SCRIPT_SH += git-submodule.sh
+SCRIPT_SH += git-unstage.sh
 SCRIPT_SH += git-web--browse.sh
 
 SCRIPT_PERL += git-add--interactive.perl

new file mode 100644
index 0000000..7f99adf
@@ -0,0 +1,26 @@
+#!/bin/sh
+
+SUBDIRECTORY_OK=Yes
+OPTIONS_KEEPDASHDASH=
+OPTIONS_SPEC="\
+git unstage file ...
+--"
+
+. git-sh-setup
+
+
+case "$1" in
+    --)
+	if [ $# -eq 1 ]; then
+	    die "You must specify at least one file to unstage"
+	fi
+	if git rev-parse -q --verify HEAD >/dev/null; then
+	    exec git reset "$@"
+	else
+	    exec git rm --cached "$@"
+	fi
+	;;
+    *)
+	usage
+	;;
+esac

index 47735d8..f1a74a4 100644
@@ -62,11 +62,7 @@ static void wt_status_print_cached_header(struct wt_status *s)
 {
 	const char *c = color(WT_STATUS_HEADER);
 	color_fprintf_ln(s->fp, c, "# Changes to be committed:");
-	if (!s->is_initial) {
-		color_fprintf_ln(s->fp, c, "#   (use \"git reset %s <file>...\" to unstage)", s->reference);
-	} else {
-		color_fprintf_ln(s->fp, c, "#   (use \"git rm --cached <file>...\" to unstage)");
-	}
+	color_fprintf_ln(s->fp, c, "#   (use \"git unstage %s <file>...\" to unstage)", s->reference);
 	color_fprintf_ln(s->fp, c, "#");
 }
 

1.6.4.rc2.217.g74c0b.dirty

Nope!

See!

Bert

Sorry, it was not obvious clear to me, that you don't shift away '--'. Does the option parsing add '--' if it is missing?

Bert

I really, really like the idea of being able to do something like unstage -p. That's sweet, thanks for working on that.

Some potential issues, though:

git unstage may cause issues for some people if they try to use it when in the middle of a merge (why reset back to HEAD rather than one of the other branches being merged?). With git reset, one can specify which commit to reset the index back to. With your 'unstage' command, you not only assume 'HEAD' but don't allow specifying any other value.
You could add a commit argument here, but that may be problematic
wording-wise, in that you're allowing to 'unstage' back to a specified commit, with the possibly perplexing result that you have 'staged' something else.

Also, you have added two new commands, unstage and discard, which are two pieces of an hg-like revert (revert edits). We still don't have a command for a full hg-like revert, though (git checkout does not cover the bases), and to cover that we'd have to add a third command. Perhaps the three could be combined?

Elijah

Why not?

"resetting index entries selectively" makes perfect sense. So do "checking out index entries selectively" and "adding to index selectively".

Quoting Junio C Hamano <[email protected]>

Are you saying that you are fine with the concept of Thomas'es patch series but you don't want to see different words used to name these operations?

In other words, do you mean the following would be a pair of better companions to "git add -p $file" than Thomas'es discard and unstage?

- "git checkout -p $file" and "git checkout -p $commit $file" that let you view the patch to bring the file in the working tree to the version in the index (or the commit) and selectively apply that to the working tree and the index, to implement "discarding changes selectively".

- "git reset -p $file" and "git reset -p $commit $file" that let you view the patch to bring the version of the file in the index to the version in the HEAD (or the commit) and selectively apply that to the index, to implement "undoing changes made to the index selectively".

I think it preserves the UI consistency better to enhance checkout and reset than adding new commands to do conceptually the same thing. Unfortunately I don't know how hard the necessary change will be, because these two commands are now implemented in C...

Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

I'd be quite interested to hear some opinions on this. I wasn't sure what to do and eventually opted for unstage/discard because I felt 'git checkout -p $file' was not an accurate description of what it does, but maybe I just have the mental blocks in the wrong places.

Thomas Rast
trast@{inf,student}.ethz.ch

Essentially, yes.

Could you please wrap your lines to a reasonable length, by the way?

I didn't read Thomas's series beyond the cover letter, but I'd say the above three (counting "add -p" in the mix) extends the existing concepts in a natural way:

- "add" goes from work tree to the index;

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

- "reset" goes from commit to index.

The "-p" variant (which exists to "add" but new to "checkout" and "reset") allows you to do these movements in a finer grained manner than per whole-file.

That's OK. There are others on the list who groks C, like Thomas ;-)

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

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

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(-)

I'm still not happy with this interface. How about the following to make it less confusing:

1b) 'stash save -p' defaults to --keep-index (which can be disabled
with a new option --no-keep-index). In --keep-index mode, it only offers hunks from the worktree.

That way, it's almost analogous to 'git add -p', but for "adding to the stash".

Thomas Rast
trast@{inf,student}.ethz.ch

I ended up implementing the even more restricted form, it always stashes the index as-is and only offers hunks from the worktree. This is because it finally dawned on me that 'git stash apply' does not merge the changesets base..index and index..worktree, but in fact (in one go) base..worktree, into the new HEAD.

That way the rules are much simpler as to what goes where. The downside of course is that the state of the worktree (with all stashed hunks discarded) compared to the index (still as before) can be a bit confusing if you stash areas of files that already have staged changes.

I also finally found some round tuits, and wrote tests. Which then of course immediately showed that neither 'git reset -p HEAD^' nor 'git checkout -p HEAD^' ever worked as advertised, so I had to fix that. Patches 1/5 and 2/5 are unaffected and still the same.

Last but not least, I rather like Dscho's patch

http://article.gmane.org/gmane.comp.version-control.git/124182

so I added a small patch to do the same DWIM logic for -p/--patch that goes on top of a merge of 1-5 and his patch. I suspect it's more work to do the merge than to just edit either 5/5 or Dscho's patch, however.

Thomas Rast (6):
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'
DWIM 'git stash save -p' for 'git stash -p'

This makes some aspects of the 'git add -p' loop configurable (within the code), so that we can later reuse git-add--interactive for other similar tools.

Most fields are fairly straightforward, but APPLY gets a subroutine (instead of just a string a la 'apply --cached') so that we can handle 'checkout -p', which will need to atomically apply the patch twice (index and worktree).

---
 git-add--interactive.perl |   79 +++++++++++++++++++++++++++++---------------
 1 files changed, 52 insertions(+), 27 deletions(-)

index df9f231..58c3332 100755
@@ -73,6 +73,21 @@
 # command line options
 my $patch_mode;
 
+sub apply_patch;
+
+my %patch_modes = (
+	'stage' => {
+		DIFF => 'diff-files -p',
+		APPLY => sub { apply_patch 'apply --cached', @_; },
+		APPLY_CHECK => 'apply --cached',
+		VERB => 'Stage',
+		PARTICIPLE => 'staging',
+		FILTER => 'file-only',
+	},
+);
+
+my %patch_mode_flavour = %{$patch_modes{stage}};
+
 sub run_cmd_pipe {
 	if ($^O eq 'MSWin32' || $^O eq 'msys') {
 		my @invalid = grep {m/[":*]/} @_;

@@ -613,12 +628,21 @@
 	print "\n";
 }
 
+sub run_git_apply {
+	my $cmd = shift;
+	my $fh;
+	open $fh, '| git ' . $cmd;
+	print $fh @_;
+	return close $fh;
+}
+
 sub parse_diff {
 	my ($path) = @_;
-	my @diff = run_cmd_pipe(qw(git diff-files -p --), $path);
+	my @diff_cmd = split(" ", $patch_mode_flavour{DIFF});
+	my @diff = run_cmd_pipe("git", @diff_cmd, "--", $path);
 	my @colored = ();
 	if ($diff_use_color) {
-		@colored = run_cmd_pipe(qw(git diff-files -p --color --), $path);
+		@colored = run_cmd_pipe("git", @diff_cmd, qw(--color --), $path);
 	}
 	my (@hunk) = { TEXT => [], DISPLAY => [], TYPE => 'header' };
 

@@ -877,6 +901,7 @@
 		or die "failed to open hunk edit file for writing: " . $!;
 	print $fh "# Manual hunk edit mode -- see bottom for a quick guide\n";
 	print $fh @$oldtext;
+	my $participle = $patch_mode_flavour{PARTICIPLE};
 	print $fh <<EOF;
 # ---
 # To remove '-' lines, make them ' ' lines (context).

@@ -884,7 +909,7 @@
 # Lines starting with # will be removed.
 #
 # If the patch applies cleanly, the edited hunk will immediately be
-# marked for staging. If it does not apply cleanly, you will be given
+# marked for $participle. If it does not apply cleanly, you will be given
 # an opportunity to edit again. If all lines of the hunk are removed,
 # then the edit is aborted and the hunk is left unchanged.
 EOF

@@ -918,11 +943,8 @@
 
 sub diff_applies {
 	my $fh;
-	open $fh, '| git apply --recount --cached --check';
-	for my $h (@_) {
-		print $fh @{$h->{TEXT}};
-	}
-	return close $fh;
+	return run_git_apply($patch_mode_flavour{APPLY_CHECK} . ' --recount --check',
+			     map { @{$_->{TEXT}} } @_);
 }
 
 sub _restore_terminal_and_die {

@@ -988,12 +1010,13 @@
 }
 
 sub help_patch_cmd {
-	print colored $help_color, <<\EOF ;
-y - stage this hunk
-n - do not stage this hunk
-q - quit, do not stage this hunk nor any of the remaining ones
-a - stage this and all the remaining hunks in the file
-d - do not stage this hunk nor any of the remaining hunks in the file
+	my $verb = lc $patch_mode_flavour{VERB};
+	print colored $help_color, <<EOF ;
+y - $verb this hunk
+n - do not $verb this hunk
+q - quit, do not $verb this hunk nor any of the remaining ones
+a - $verb this and all the remaining hunks in the file
+d - do not $verb this hunk nor any of the remaining hunks in the file
 g - select a hunk to go to
 / - search for a hunk matching the given regex
 j - leave this hunk undecided, see next undecided hunk

@@ -1006,8 +1029,17 @@
 EOF
 }
 
+sub apply_patch {
+	my $cmd = shift;
+	my $ret = run_git_apply $cmd . ' --recount', @_;
+	if (!$ret) {
+		print STDERR @_;
+	}
+	return $ret;
+}
+
 sub patch_update_cmd {
-	my @all_mods = list_modified('file-only');
+	my @all_mods = list_modified($patch_mode_flavour{FILTER});
 	my @mods = grep { !($_->{BINARY}) } @all_mods;
 	my @them;
 

@@ -1138,8 +1170,8 @@
 		for (@{$hunk[$ix]{DISPLAY}}) {
 			print;
 		}
-		print colored $prompt_color, 'Stage ',
-		  ($hunk[$ix]{TYPE} eq 'mode' ? 'mode change' : 'this hunk'),
+		print colored $prompt_color, $patch_mode_flavour{VERB},
+		  ($hunk[$ix]{TYPE} eq 'mode' ? ' mode change' : ' this hunk'),
 		  " [y,n,q,a,d,/$other,?]? ";
 		my $line = prompt_single_character;
 		if ($line) {

@@ -1313,16 +1345,9 @@
 
 	if (@result) {
 		my $fh;
-
-		open $fh, '| git apply --cached --recount';
-		for (@{$head->{TEXT}}, @result) {
-			print $fh $_;
-		}
-		if (!close $fh) {
-			for (@{$head->{TEXT}}, @result) {
-				print STDERR $_;
-			}
-		}
+		my @patch = (@{$head->{TEXT}}, @result);
+		my $apply_routine = $patch_mode_flavour{APPLY};
+		&$apply_routine(@patch);
 		refresh();
 	}
 

1.6.4.rc3.215.g18405

This moves the call setup for 'git add--interactive' to a separate function, as other users will call it without running validate_pathspec() first.

---
 builtin-add.c |   43 +++++++++++++++++++++++++++++--------------
 commit.h      |    2 ++
 2 files changed, 31 insertions(+), 14 deletions(-)

index 581a2a1..c422a62 100644
@@ -131,27 +131,27 @@ static void refresh(int verbose, const char **pathspec)
 	return pathspec;
 }
 
-int interactive_add(int argc, const char **argv, const char *prefix)
+int run_add_interactive(const char *revision, const char *patch_mode,
+			const char **pathspec)
 {
-	int status, ac;
+	int status, ac, pc = 0;
 	const char **args;
-	const char **pathspec = NULL;
 
-	if (argc) {
-		pathspec = validate_pathspec(argc, argv, prefix);
-		if (!pathspec)
-			return -1;
-	}
+	if (pathspec)
+		while (pathspec[pc])
+			pc++;
 
-	args = xcalloc(sizeof(const char *), (argc + 4));
+	args = xcalloc(sizeof(const char *), (pc + 5));
 	ac = 0;
 	args[ac++] = "add--interactive";
-	if (patch_interactive)
-		args[ac++] = "--patch";
+	if (patch_mode)
+		args[ac++] = patch_mode;
+	if (revision)
+		args[ac++] = revision;
 	args[ac++] = "--";
-	if (argc) {
-		memcpy(&(args[ac]), pathspec, sizeof(const char *) * argc);
-		ac += argc;
+	if (pc) {
+		memcpy(&(args[ac]), pathspec, sizeof(const char *) * pc);
+		ac += pc;
 	}
 	args[ac] = NULL;
 

@@ -160,6 +160,21 @@ int interactive_add(int argc, const char **argv, const char *prefix)
 	return status;
 }
 
+int interactive_add(int argc, const char **argv, const char *prefix)
+{
+	const char **pathspec = NULL;
+
+	if (argc) {
+		pathspec = validate_pathspec(argc, argv, prefix);
+		if (!pathspec)
+			return -1;
+	}
+
+	return run_add_interactive(NULL,
+				   patch_interactive ? "--patch" : NULL,
+				   pathspec);
+}
+
 static int edit_patch(int argc, const char **argv, const char *prefix)
 {
 	char *file = xstrdup(git_path("ADD_EDIT.patch"));

index 8bfdf0e..0555e80 100644
@@ -139,6 +139,8 @@ struct commit_graft {
 int in_merge_bases(struct commit *, struct commit **, int);
 
 extern int interactive_add(int argc, const char **argv, const char *prefix);
+extern int run_add_interactive(const char *revision, const char *patch_mode,
+			       const char **pathspec);
 
 static inline int single_parent(struct commit *commit)
 {

1.6.4.rc3.215.g18405

---
 Documentation/git-stash.txt |    2 +-
 git-stash.sh                |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

index 3dc16a1..1c4ed41 100644
@@ -14,7 +14,7 @@ SYNOPSIS
 'git stash' ( pop | apply ) [--index] [-q|--quiet] [<stash>]
 'git stash' branch <branchname> [<stash>]
 'git stash' [save [--patch] [-k|--[no-]keep-index] [-q|--quiet] [<message>]]
-'git stash' [-k|--keep-index]
+'git stash' [-p|--patch|-k|--keep-index]
 'git stash' clear
 'git stash' create
 

index 81a72f6..9fd7289 100755
@@ -406,8 +406,8 @@ branch)
 	apply_to_branch "$@"
 	;;
 *)
-	case $#,"$1" in
-	0,|1,-k|1,--keep-index)
+	case $#,"$1","$2" in
+	0,,|1,-k,|1,--keep-index,|1,-p,|1,--patch,|2,-p,--no-keep-index|2,--patch,--no-keep-index)
 		save_stash "$@" &&
 		say '(To restore them type "git stash apply")'
 		;;

1.6.4.rc3.215.g18405

This introduces a --patch mode for git-reset. The basic case is

  git reset --patch -- [files...]

which acts as the opposite of 'git add --patch -- [files...]': it offers hunks for *un*staging. Advanced usage is

  git reset --patch <revision> -- [files...]

which offers hunks from the diff between <revision> and the index for reverse application to the index. (That is, the basic case is just <revision> = HEAD.) This means it can be used to "undo" changes since <revision> in the index, but may be slightly confusing when <revision> is logically "newer" than the index state.

---
 Documentation/git-reset.txt |   15 +++++++++++-
 builtin-reset.c             |   19 +++++++++++++++++
 git-add--interactive.perl   |   45 ++++++++++++++++++++++++++++++++++++---
 t/t7105-reset-patch.sh      |   48 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 121 insertions(+), 6 deletions(-)
 create mode 100755 t/t7105-reset-patch.sh

index abb25d1..469cf6d 100644
@@ -10,6 +10,7 @@ SYNOPSIS
 [verse]
 'git reset' [--mixed | --soft | --hard | --merge] [-q] [<commit>]
 'git reset' [-q] [<commit>] [--] <paths>...
+'git reset' --patch [<commit>] [--] [<paths>...]
 
 DESCRIPTION
 -----------

@@ -23,8 +24,9 @@ the undo in the history.
 If you want to undo a commit other than the latest on a branch,
 linkgit:git-revert[1] is your friend.
 
-The second form with 'paths' is used to revert selected paths in
-the index from a given commit, without moving HEAD.
+The second and third forms with 'paths' and/or --patch are used to
+revert selected paths in the index from a given commit, without moving
+HEAD.
 
 
 OPTIONS

@@ -50,6 +52,15 @@ OPTIONS
 	and updates the files that are different between the named commit
 	and the current commit in the working tree.
 
+-p::
+--patch::
+	Interactively select hunks in the difference between the index
+	and <commit> (defaults to HEAD).  The chosen hunks are applied
+	in reverse to the index.
++
+This means that `git reset -p` is the opposite of `git add -p` (see
+linkgit:git-add[1]).
+
 -q::
 	Be quiet, only report errors.
 

index 5fa1789..246a127 100644
@@ -142,6 +142,17 @@ static void update_index_from_diff(struct diff_queue_struct *q,
 	}
 }
 
+static int interactive_reset(const char *revision, const char **argv,
+			     const char *prefix)
+{
+	const char **pathspec = NULL;
+
+	if (*argv)
+		pathspec = get_pathspec(prefix, argv);
+
+	return run_add_interactive(revision, "--patch=reset", pathspec);
+}
+
 static int read_from_tree(const char *prefix, const char **argv,
 		unsigned char *tree_sha1, int refresh_flags)
 {

@@ -183,6 +194,7 @@ static void prepend_reflog_action(const char *action, char *buf, size_t size)
 int cmd_reset(int argc, const char **argv, const char *prefix)
 {
 	int i = 0, reset_type = NONE, update_ref_status = 0, quiet = 0;
+	int patch_mode = 0;
 	const char *rev = "HEAD";
 	unsigned char sha1[20], *orig = NULL, sha1_orig[20],
 				*old_orig = NULL, sha1_old_orig[20];

@@ -198,6 +210,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 				"reset HEAD, index and working tree", MERGE),
 		OPT_BOOLEAN('q', NULL, &quiet,
 				"disable showing new HEAD in hard reset and progress message"),
+		OPT_BOOLEAN('p', "patch", &patch_mode, "select hunks interactively"),
 		OPT_END()
 	};
 

@@ -251,6 +264,12 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 		die("Could not parse object '%s'.", rev);
 	hashcpy(sha1, commit->object.sha1);
 
+	if (patch_mode) {
+		if (reset_type != NONE)
+			die("--patch is incompatible with --{hard,mixed,soft}");
+		return interactive_reset(rev, argv + i, prefix);
+	}
+
 	/* git reset tree [--] paths... can be used to
 	 * load chosen paths from the tree into the index without
 	 * affecting the working tree nor HEAD. */

1.6.4.rc3.215.g18405

This introduces a --patch mode for git-checkout. In the index usage

  git checkout --patch -- [files...]

it lets the user discard edits from the <files> at the granularity of hunks (by selecting hunks from 'git diff' and then reverse applying them to the worktree).

We also accept a revision argument

  git checkout --patch <revision> -- [files...]

which offers hunks from the difference between the <revision> and the worktree. The chosen hunks are then reverse applied to both index and worktree, discarding them completely. This application is done "atomically" in the sense that we first check if the patch applies to the index (it should always apply to the worktree). If it does not, we give the user a choice to either abort or apply to the worktree anyway.

---
 Documentation/git-checkout.txt |   13 ++++++-
 builtin-checkout.c             |   19 +++++++++
 git-add--interactive.perl      |   48 +++++++++++++++++++++++
 t/t2015-checkout-patch.sh      |   84 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 163 insertions(+), 1 deletions(-)
 create mode 100755 t/t2015-checkout-patch.sh

index ad4b31e..26a5447 100644
@@ -11,6 +11,7 @@ SYNOPSIS
 'git checkout' [-q] [-f] [-m] [<branch>]
 'git checkout' [-q] [-f] [-m] [-b <new_branch>] [<start_point>]
 'git checkout' [-f|--ours|--theirs|-m|--conflict=<style>] [<tree-ish>] [--] <paths>...
+'git checkout' --patch [<tree-ish>] [--] [<paths>...]
 
 DESCRIPTION
 -----------

@@ -25,7 +26,7 @@ use the --track or --no-track options, which will be passed to `git
 branch`.  As a convenience, --track without `-b` implies branch
 creation; see the description of --track below.
 
-When <paths> are given, this command does *not* switch
+When <paths> or --patch are given, this command does *not* switch
 branches.  It updates the named paths in the working tree from
 the index file, or from a named <tree-ish> (most often a commit).  In
 this case, the `-b` and `--track` options are meaningless and giving

@@ -113,6 +114,16 @@ the conflicted merge in the specified paths.
 	"merge" (default) and "diff3" (in addition to what is shown by
 	"merge" style, shows the original contents).
 
+-p::
+--patch::
+	Interactively select hunks in the difference between the
+	<tree-ish> (or the index, if unspecified) and the working
+	tree.  The chosen hunks are then applied in reverse to the
+	working tree (and if a <tree-ish> was specified, the index).
++
+This means that you can use `git checkout -p` to selectively discard
+edits from your current working tree.
+
 <branch>::
 	Branch to checkout; if it refers to a branch (i.e., a name that,
 	when prepended with "refs/heads/", is a valid ref), then that

index 446cac7..7d57741 100644
@@ -572,6 +572,13 @@ static int git_checkout_config(const char *var, const char *value, void *cb)
 	return git_xmerge_config(var, value, cb);
 }
 
+static int interactive_checkout(const char *revision, const char **pathspec,
+				struct checkout_opts *opts)
+{
+	return run_add_interactive(revision, "--patch=checkout", pathspec);
+}
+
+
 int cmd_checkout(int argc, const char **argv, const char *prefix)
 {
 	struct checkout_opts opts;

@@ -580,6 +587,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	struct branch_info new;
 	struct tree *source_tree = NULL;
 	char *conflict_style = NULL;
+	int patch_mode = 0;
 	struct option options[] = {
 		OPT__QUIET(&opts.quiet),
 		OPT_STRING('b', NULL, &opts.new_branch, "new branch", "branch"),

@@ -594,6 +602,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		OPT_BOOLEAN('m', "merge", &opts.merge, "merge"),
 		OPT_STRING(0, "conflict", &conflict_style, "style",
 			   "conflict style (merge or diff3)"),
+		OPT_BOOLEAN('p', "patch", &patch_mode, "select hunks interactively"),
 		OPT_END(),
 	};
 	int has_dash_dash;

@@ -608,6 +617,10 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix, options, checkout_usage,
 			     PARSE_OPT_KEEP_DASHDASH);
 
+	if (patch_mode && (opts.track > 0 || opts.new_branch
+			   || opts.new_branch_log || opts.merge || opts.force))
+		die ("--patch is incompatible with all other options");
+
 	/* --track without -b should DWIM */
 	if (0 < opts.track && !opts.new_branch) {
 		const char *argv0 = argv[0];

@@ -714,6 +727,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		if (!pathspec)
 			die("invalid path specification");
 
+		if (patch_mode)
+			return interactive_checkout(NULL, pathspec, &opts);
+
 		/* Checkout paths */
 		if (opts.new_branch) {
 			if (argc == 1) {

@@ -729,6 +745,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		return checkout_paths(source_tree, pathspec, &opts);
 	}
 
+	if (patch_mode)
+		return interactive_checkout(new.name, NULL, &opts);
+
 	if (opts.new_branch) {
 		struct strbuf buf = STRBUF_INIT;
 		if (strbuf_check_branch_ref(&buf, opts.new_branch))

1.6.4.rc3.215.g18405

This adds a hunk-based mode to git-stash. You can select hunks from the difference between HEAD and worktree, and git-stash will build a stash that reflects these changes. The index state of the stash is the same as your current index, and we also let --patch imply --keep-index.

Note that because the selected hunks are rolled back from the worktree but not the index, the resulting state may appear somewhat confusing if you had also staged these changes. This is not entirely satisfactory, but due to the way stashes are applied, other solutions would require a change to the stash format.

---
 Documentation/git-stash.txt |   14 ++++++-
 git-add--interactive.perl   |   13 ++++++-
 git-stash.sh                |   80 +++++++++++++++++++++++++++++++++++-------
 t/t3904-stash-patch.sh      |   66 +++++++++++++++++++++++++++++++++++
 4 files changed, 155 insertions(+), 18 deletions(-)
 create mode 100755 t/t3904-stash-patch.sh

index 1c64a02..4b15459 100644
@@ -13,7 +13,7 @@ SYNOPSIS
 'git stash' drop [-q|--quiet] [<stash>]
 'git stash' ( pop | apply ) [--index] [-q|--quiet] [<stash>]
 'git stash' branch <branchname> [<stash>]
-'git stash' [save [--keep-index] [-q|--quiet] [<message>]]
+'git stash' [save [--patch] [--[no-]keep-index] [-q|--quiet] [<message>]]
 'git stash' clear
 'git stash' create
 

@@ -42,7 +42,7 @@ is also possible).
 OPTIONS
 -------
 
-save [--keep-index] [-q|--quiet] [<message>]::
+save [--patch] [--[no-]keep-index] [-q|--quiet] [<message>]::
 
 	Save your local modifications to a new 'stash', and run `git reset
 	--hard` to revert them.  This is the default action when no

@@ -51,6 +51,16 @@ save [--keep-index] [-q|--quiet] [<message>]::
 +
 If the `--keep-index` option is used, all changes already added to the
 index are left intact.
++
+With `--patch`, you can interactively select hunks from in the diff
+between HEAD and the working tree to be stashed.  The stash entry is
+constructed such that its index state is the same as the index state
+of your repository, and its worktree contains only the changes you
+selected interactively.  The selected changes are then rolled back
+from your worktree.
++
+The `--patch` option implies `--keep-index`.  You can use
+`--no-keep-index` to override this.
 
 list [<options>]::
 

index bb76c37..91f1657 100755
@@ -76,6 +76,7 @@
 
 sub apply_patch;
 sub apply_patch_for_checkout_commit;
+sub apply_patch_for_stash;
 
 my %patch_modes = (
 	'stage' => {

@@ -86,6 +87,14 @@
 		PARTICIPLE => 'staging',
 		FILTER => 'file-only',
 	},
+	'stash' => {
+		DIFF => 'diff-index -p HEAD',
+		APPLY => sub { apply_patch 'apply --cached', @_; },
+		APPLY_CHECK => 'apply --cached',
+		VERB => 'Stash',
+		PARTICIPLE => 'stashing',
+		FILTER => undef,
+	},
 	'reset' => {
 		DIFF => 'diff-index -p --cached',
 		APPLY => sub { apply_patch 'apply -R --cached', @_; },

@@ -1465,8 +1474,8 @@
 					$patch_mode_revision = $arg;
 					$arg = shift @ARGV or die "missing --";
 				}
-			} elsif ($1 eq 'stage') {
-				$patch_mode = 'stage';
+			} elsif ($1 eq 'stage' or $1 eq 'stash') {
+				$patch_mode = $1;
 				$arg = shift @ARGV or die "missing --";
 			} else {
 				die "unknown --patch mode: $1";

1.6.4.rc3.215.g18405

I finally got a few minutes to look at this. I tried "checkout --patch" first, which was very confusing:

  $ echo old content >file && git add file && git commit -m old
  $ echo new content >>file
  $ git checkout --patch file
  diff --git a/file b/file
  index 33194a0..805c3b0 100644
  --- a/file
  +++ b/file
  @@ -1 +1,2 @@
   old content
   +new content
  Check out this hunk [y,n,q,a,d,/,e,?]?

Shouldn't the diff 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).

Maybe it is just the use of the verb "check out". I think of it as bringing _this_ hunk out. But it is really about erasing this hunk. "git reset --patch" has the same behavior, but I don't find it as bothersome, since it says "Reset this hunk".

And "stash --patch" doesn't have the issue because it goes from the working tree to the stash already.

-Peff

Well, my thinking for the initial (restricted; you couldn't say 'git checkout -p HEAD~14') version went something like this: 'reset -p' should be the opposite of 'add -p', so it offers the same hunks with the question "Reset?". Then 'checkout -p' should somehow follow suit, but asked "Discard?" (IIRC I even had it in all caps).

I'm not 100% happy with your suggested forward patches strategy because I think (particularly for people with colors enabled, and I suspect we all have), it's less confusing "my" way if they go through 'add -p' and suddenly think "oops, mistake, I need to reset that": it is much easier for the (at least for my) eye to find the same hunk again if it is really 100% the same. Probably we would have to change the verb to "discard" again, though.

OTOH this does become rather weird once you specify anything other than HEAD. And while we could of course switch the approach if the user does that, and hope that he'll understand on the grounds that it's an advanced usage, I'm not sure switching is a good idea in general.

Thomas Rast
trast@{inf,student}.ethz.ch

I agree this approach is fine. That said, I admit I've been confused at the beginning. FMPOV, asking "Discard this hunk?" is a real improvement.

In the same way I'd change "Reset?" of 'git reset -p' to "Unstage?". Otherwise, the end-user don't know if the command will discard the hunk from the WT too.

Also, I'd expect to have 'git reset --hard -p' discarding hunks from both the index and the WT (which is not possible for now unless I missed something).

Nicolas Sebrecht

Well, the unfortunate overlap between 'git reset --hard' and 'git checkout HEAD -- .' strikes again :-)

Since you can't say 'git reset --hard -- file', you have to do 'git checkout HEAD -- file' to achieve this effect. So this usage is covered by 'git checkout -p HEAD'.

Thomas Rast
trast@{inf,student}.ethz.ch

Not exactly:

Current branch: my_branch (clean)
nicolas@vidovic git % echo 'hi' > file
Current branch: my_branch (dirty: untracked:1)
nicolas@vidovic git % git add file
Current branch: my_branch (dirty: index)
nicolas@vidovic git % git checkout -p
No changes.

Same example with an already tracked file:

Current branch: my_branch (clean)
nicolas@vidovic git % echo 'hi Thomas' >> Documentation/SubmittingPatches Current branch: my_branch (dirty: working_tree) nicolas@vidovic git % git add Documentation/SubmittingPatches Current branch: my_branch (dirty: index)
nicolas@vidovic git % git checkout -p
No changes.

So, we can discard a hunk from the index with 'git reset -p' without touching the WT. And we can discard a hunk from the WT with 'git checkout -p' without touching the index. But we can't discard a hunk from both the index and the WT.

IOW, we have (from the user POV):

.git --- (reset -p) ---> index

                           index --- (checkout -p) ---> WT

What's missing:

.git ------------------ ( ??? ) --------------------> WT

Nicolas Sebrecht

                                ^^^^

I figured this makes sense:

  git checkout -- file         # copy file from index to worktree
  git checkout -p [file]       # copy hunks from index to worktree
  git checkout HEAD -- file    # copy file from HEAD to index&worktree

git checkout -p HEAD -- file # copy hunks from HEAD to index&worktree

Note that the patch application stage has no guarantees that what you picked will also apply to the index; it tries first, and if it doesn't, it asks if it should apply to the worktree anyway (and leave the index unchanged).

Thomas Rast
trast@{inf,student}.ethz.ch

I did my tests including "HEAD" but only with "-- file" too...

...and I don't have what's expected

Current branch: my_branch (clean)
nicolas@vidovic git % echo 'Hi Thomas' >> Documentation/SubmittingPatches Current branch: my_branch (dirty: working_tree)
nicolas@vidovic git % git add Documentation/SubmittingPatches Current branch: my_branch (dirty: index)
nicolas@vidovic git % git checkout -p HEAD -- Documentation/SubmittingPatches No changes.

where "checkout -p HEAD" works fine.

Ok. All of what you say above makes sense and I'm actually fine with your whole answer. That said,

% git checkout -p HEAD

and

% git checkout -p HEAD -- file

behave differently here in my test above.

Nicolas Sebrecht

Oh. Sorry, that's indeed a bug, I'll fix this. (The -- file form loses the HEAD when invoking add--interactive.)

Thomas Rast
trast@{inf,student}.ethz.ch

[...]

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

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

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

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'

<...>

Nicolas Sebrecht

Thanks for following up on this. This is something I've wanted for a while, and now my procrastination is paying off. ;)

Makes sense.

Actually, it says "Unstage this hunk", but I like that even better.

This doesn't make sense to me. For example:

  $ git show HEAD^:file
  content
  $ git show :file
  content
  with some changes
  $ git reset -p HEAD^
  diff --git a/file b/file
  index d95f3ad..60a1a4e 100644
  --- a/file
  +++ b/file
  @@ -1 +1,2 @@
   content
  +with some changes
  Apply this hunk to index [y,n,q,a,d,/,e,?]?

The hunk is _already_ in the index. You are really asking to remove it from the index. So shouldn't it say something like "Unstage this hunk" or "Remove this hunk from the index"?

Or did you intend to reverse the diff, as with "checkout -p" below?

Good, that addresses my earlier confusion.

Good. I like how it clarifies what is being touched.

I really expected this to just be the same as the "HEAD" case. That is, with "git checkout -p HEAD", you are saying "I'm not interested in these bits, discard to return back to HEAD". So if I do "git checkout -p HEAD^", that is conceptually the same thing, except going back further in time.

But I guess you are thinking of it as "pull these changes out of 'other'", in which case showing the reverse diff makes sense.

I think this may be a situation where the user has one of two mental models in issuing the command, and we don't necessarily know which. So I guess what you have is fine, but I wanted to register my surprise.

Getting greedy, is there a reason not to have "stash apply -p" as well?

-Peff

[...]

Yes, sorry :-( I apparently managed to forget the reversing here and never noticed. I'll make a fixed patch 4/6 today.

Well, as I said earlier in the thread I'd hate to reverse the direction of plain "{reset,checkout,stash} -p" because I want them to show hunks that match visually.

But then my mental model of "checkout other -- file" is already of the sort "fetch me the contents of 'file' from 'other'", and I think I use it about as frequently in a forward as in a backward application. And I just felt it would be easier to mentally go over the consequences if it shoed the diff the other way.

(I'm sufficiently confused about which way is back that I think I'll just stop saying "backward"...)

Should be doable, I'll look at it.

Thomas Rast
trast@{inf,student}.ethz.ch

I took a look at this today. It is actually a bit harder than I had hoped. The problem is that a stash is not a _state_, but rather two states whose difference is of interest to us.

So it's not right to just pick hunks out of the stash as compared with the current tree. You will get "false" hunks for the changes between the current tree and the base upon which the stash was created. In fact, we actually do a full 3-way merge with the current state and the new state with the stash base as the ancestor.

The strategy I employ in the patch below is to actually just prune the stash tree against the base, and then use that result to do the merge. For example, consider the following situation:

  $ echo base >file && git add file && git commit -m base
  $ echo stash >file && git stash
  $ echo tree >file && git add file

Now you have a stash whose base has 'base' and whose content has 'stash', and your current tree has 'tree'. Applying the stash should therefore cause a conflict (but I am using such a simple example because we can still illustrate what's happening).

You can see that ignoring the base creates an incorrect result. You would see that the stash has 'stash' and we have 'tree', and ask about applying the hunk

-tree
+stash

but that's not right. We should have a conflict.

So what I do instead is to load the base tree into a temporary index, then git-add--interactive the changes from the stashed tree, then put write the result into a new tree, which becomes our new merge point. So you will be asked if you want to apply the hunk

-base
+stash

If not, then the resulting tree will have 'base'. Since the base also has 'base', the stash has done nothing, and the 3-way merge will keep your tree contents. If you do, then the resulting tree will have 'stash', and you will have a 3-way merge with a conflict.

Now there is a slight problem with that. What if you had already applied or recreated part of the stash? In other words, instead of 'tree' in your current content, you had 'stash'? We would _still_ ask if you wanted to replace 'base' with 'stash', even though when we get to the three-way merge, it will be a no-op.

Hmm. I was about to write "so we need some clever way of integrating the interactive hunk selection with a 3-way merge". But I just had a
thought: we should do it in the reverse order. We do the three-way merge into a temporary index, and then ask the user to apply the result of _that_ tree into the working tree. Or maybe I am missing something else obvious and you can enlighten me.

I'll try to experiment with merging first, though I doubt I will have any more time for the next day or two. In the meantime, you can peek at the current patch I have below. It has two additional problems:

1. For --index mode, it actually invokes add--interactive twice. It would be nice to do both passes at the same time, but I don't think it is possible with the current add--interactive infrastructure.

2. You will notice a hack-ish 'sleep' in the test; because of the double add--interactive, when piping from stdin, the first one eats the responses in its input buffer and the second one never sees them.

-Peff

---
 git-add--interactive.perl    |   28 ++++++++++++++++++++++++++++
 git-stash.sh                 |   35 +++++++++++++++++++++++++++++------
 t/t3905-stash-apply-patch.sh |   42 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 99 insertions(+), 6 deletions(-)
 create mode 100755 t/t3905-stash-apply-patch.sh

index 392efb9..cbbf0a7 100755
@@ -97,6 +97,28 @@ my %patch_modes = (
 		PARTICIPLE => 'stashing',
 		FILTER => undef,
 	},
+	'stash-apply-tree' => {
+		DIFF => 'diff-index -R -p --cached',
+		APPLY => sub {
+			apply_patch 'apply --cached', $patch_mode_revision, @_;
+		},
+		APPLY_CHECK => 'apply --cached',
+		VERB => 'Apply',
+		TARGET => ' to working tree',
+		PARTICIPLE => 'applying',
+		FILTER => undef,
+	},
+	'stash-apply-index' => {
+		DIFF => 'diff-index -R -p --cached',
+		APPLY => sub {
+			apply_patch 'apply --cached', $patch_mode_revision, @_;
+		},
+		APPLY_CHECK => 'apply --cached',
+		VERB => 'Apply',
+		TARGET => ' to index',
+		PARTICIPLE => 'applying',
+		FILTER => undef,
+	},
 	'reset_head' => {
 		DIFF => 'diff-index -p --cached',
 		APPLY => sub { apply_patch 'apply -R --cached', @_; },

@@ -1507,6 +1529,12 @@ sub process_args {
 						       'checkout_head' : 'checkout_nothead');
 					$arg = shift @ARGV or die "missing --";
 				}
+			} elsif ($1 eq 'stash-apply-tree'
+					|| $1 eq 'stash-apply-index') {
+				$patch_mode = $1;
+				$arg = shift @ARGV or die "missing --";
+				$patch_mode_revision = $arg;
+				$arg = shift @ARGV or die "missing --";
 			} elsif ($1 eq 'stage' or $1 eq 'stash') {
 				$patch_mode = $1;
 				$arg = shift @ARGV or die "missing --";

index 9efc46d..8090bbf 100755
@@ -214,8 +214,19 @@ show_stash () {
 	git diff $flags $b_commit $w_commit
 }
 
+munge_tree_interactive () {
+	(
+	 GIT_INDEX_FILE="$TMP-index"
+	 export GIT_INDEX_FILE
+	 git read-tree --reset "$2" &&
+	 git add--interactive --patch=stash-apply-$1 "$3" -- >&3 &&
+	 git write-tree
+	)
+}
+
 apply_stash () {
 	unstash_index=
+	patch_mode=
 
 	while test $# != 0
 	do

@@ -226,6 +237,9 @@ apply_stash () {
 		-q|--quiet)
 			GIT_QUIET=t
 			;;
+		-p|--patch)
+			patch_mode=t
+			;;
 		*)
 			break
 			;;

@@ -258,12 +272,21 @@ apply_stash () {
 	if test -n "$unstash_index" && test "$b_tree" != "$i_tree" &&
 			test "$c_tree" != "$i_tree"
 	then
-		git diff-tree --binary $s^2^..$s^2 | git apply --cached
-		test $? -ne 0 &&
-			die 'Conflicts in index. Try without --index.'
-		unstashed_index_tree=$(git write-tree) ||
-			die 'Could not save index tree'
-		git reset
+		if test -n "$patch_mode"; then
+			i_tree=`munge_tree_interactive index $b_tree $i_tree` 3>&1
+		fi
+		if test "$b_tree" != "$i_tree"; then
+			git diff-tree --binary $b_tree $i_tree | git apply --cached
+			test $? -ne 0 &&
+				die 'Conflicts in index. Try without --index.'
+			unstashed_index_tree=$(git write-tree) ||
+				die 'Could not save index tree'
+			git reset
+		fi
+	fi
+
+	if test -n "$patch_mode"; then
+		w_tree=`munge_tree_interactive tree $b_tree $w_tree` 3>&1
 	fi
 
 	eval "

1.6.4.304.ge0c3be.dirty

Heh, now *my* procrastination is paying off :-)

I think that is the correct way to go about it from the user's POV. He would be confused if the patch applied to WT/index were different (because of a later merge) from the hunks he chose in the -p loop.

However, there's the issue of merge conflicts. Some options I can think of are

1) refuse to work in the face of merge problems

2) stash requires a clean WT, so we can move the user's index out of the way and use temporary index + WT to let the user resolve the conflicts

3) require both clean WT and index so we can simply use the repo to resolve

(The first one isn't quite as restrictive as it sounds; the user can always apply on top of a clean HEAD, fix conflicts and re-stash, thus doing a "stash rebase".)

Note that the 'git stash -p' in next always stashes the index whole, so the "easy" way might simply be to also unstash the index whole (if requested).

The changes will usually still be available in the worktree application, because the 3-way merge is between base and HEAD on one side and base and worktree-stash on the other side.

Thomas Rast
trast@{inf,student}.ethz.ch

Curses, I fell into your trap!

I had assumed we would do (1), just to keep things simple. Otherwise stash becomes a multi-invocation command (with a --continue feature), which really has a lot of complexity and corner cases.

Actually, we currently require that the index and WT match, so these two are equivalent. But I think they add a lot of complexity because of the continuation.

Which is really a nice way of dodging the continuation bullet, since the results after each step are well-defined in terms of currently existing steps. That is, the user could "git stash apply" and never invoke "git stash apply -p" if munging the conflicts led to the result they wanted.

OTOH, it may have been simpler for them to edit the stash beforehand to avoid the conflicts.

Hmm. Maybe we are really talking about two different commands:

1. edit the hunks that will be applied to the working tree from the stash (apply -p)

2. edit the _stash itself_, taking or leaving hunks to create a new stash

What was in my patch was basically (2), but then always followed by applying. By splitting them, you can use whichever makes sense for your situation.

That might be worthwhile. It is less flexible, but I really wonder if people actually keep stash randomly different changes in their index and worktree and want to pick through them individually. I guess we could also add a --patch-index for people who really wanted it.

-Peff

Not to me, either.

Let's say you have modified $path and ran "git add $path" earlier.

"reset -p -- $path" and "reset -p HEAD -- $path" both show what your index has relative to the commit you are resetting your index to and offer to "Unstage" [*1*]. This is consistent and feels natural.

"reset -p HEAD^ -- $path" however shows the same forward diff (i.e. how your index is different compared to the commit HEAD^ you are resetting to), but offers to "Apply".

When a user is resetting to the current HEAD (with or without an explicit HEAD parameter), it is likely that the user did "git add" earlier and is trying to reverse the effect of that. And showing a forward diff makes sense. But when a user is resetting to a different commit, it may make sense to show a reverse diff, saying "Here is a hunk you _could_ choose to use to bring the current index to a different state, do you want to apply it to do so?" Perhaps you meant to show a reverse diff and use the word "Apply".

However, that would break down rather badly when HEAD did not change $path since HEAD^. Logically what the "reset -p" would do to $path is the same, but the patch shown and the operation offered to the user are opposite.

You could compare HEAD and the commit you are resetting the index to and see if the path in question is different between the two commits, and switch the direction---if there is no change, you show forward diff and offer to "Remove this change out of the index", if there is a change, you show reverse diff and offer to "Apply this change to the index". But if the difference between HEAD and the commit you are resetting to does not overlap with the change you staged to the index earlier from your work tree, it is unclear such heuristics would yield a natural feel.

So I actually think you may be better off if you consistently showed a forward diff (i.e. what patch would have been applied to the commit in question to bring the index into its current shape), and always offer "Remove this change out of the index?"

The same comment applies to "checkout -p HEAD" vs "checkout -p HEAD^". I think the latter shouldn't show a reverse diff and offer "Apply?"; instead both should consitently show a forward diff (i.e. what patch would have been applied to the commit to bring your work tree into its current shape), and offer "Remove this change out of the index and the work tree?".

[Footnote]

*1* I actually have a slight problem with the use of word "Unstage" in this context; "to stage", at least to me, means "adding _from the work tree_ to the index", not just "modifying the index" from a random source. The command is resetting the index in this case from a tree-ish and there is no work tree involved, and the word "stage/unstage" feels out of place.

[...]

Indeed.

HEAD^ is not special. What do we do if the user resets to something that is logically further progressed in time, perhaps another branch? I just have a much better mental model of it as "apply <something> to index" than if it said: here's some diff between whatever commit you gave me, and your index; but I'm going to apply it reverse!

(v4 actually did it this way and I found it a bit confusing...)

Again (and unlike in the reset case, I can actually see myself doing this at times) the user could pass in a commit that is logically newer than HEAD.

It's not using "unstage" any more if the commit is not HEAD. If it is, then we're doing the opposite of 'add -p', so doesn't the term apply then?

Thomas Rast
trast@{inf,student}.ethz.ch

This makes some aspects of the 'git add -p' loop configurable (within the code), so that we can later reuse git-add--interactive for other similar tools.

Most fields are fairly straightforward, but APPLY gets a subroutine (instead of just a string a la 'apply --cached') so that we can handle 'checkout -p', which will need to atomically apply the patch twice (index and worktree).

---
 git-add--interactive.perl |   82 ++++++++++++++++++++++++++++++---------------
 1 files changed, 55 insertions(+), 27 deletions(-)

index df9f231..3606103 100755
@@ -73,6 +73,22 @@
 # command line options
 my $patch_mode;
 
+sub apply_patch;
+
+my %patch_modes = (
+	'stage' => {
+		DIFF => 'diff-files -p',
+		APPLY => sub { apply_patch 'apply --cached', @_; },
+		APPLY_CHECK => 'apply --cached',
+		VERB => 'Stage',
+		TARGET => '',
+		PARTICIPLE => 'staging',
+		FILTER => 'file-only',
+	},
+);
+
+my %patch_mode_flavour = %{$patch_modes{stage}};
+
 sub run_cmd_pipe {
 	if ($^O eq 'MSWin32' || $^O eq 'msys') {
 		my @invalid = grep {m/[":*]/} @_;

@@ -613,12 +629,21 @@
 	print "\n";
 }
 
+sub run_git_apply {
+	my $cmd = shift;
+	my $fh;
+	open $fh, '| git ' . $cmd;
+	print $fh @_;
+	return close $fh;
+}
+
 sub parse_diff {
 	my ($path) = @_;
-	my @diff = run_cmd_pipe(qw(git diff-files -p --), $path);
+	my @diff_cmd = split(" ", $patch_mode_flavour{DIFF});
+	my @diff = run_cmd_pipe("git", @diff_cmd, "--", $path);
 	my @colored = ();
 	if ($diff_use_color) {
-		@colored = run_cmd_pipe(qw(git diff-files -p --color --), $path);
+		@colored = run_cmd_pipe("git", @diff_cmd, qw(--color --), $path);
 	}
 	my (@hunk) = { TEXT => [], DISPLAY => [], TYPE => 'header' };
 

@@ -877,6 +902,7 @@
 		or die "failed to open hunk edit file for writing: " . $!;
 	print $fh "# Manual hunk edit mode -- see bottom for a quick guide\n";
 	print $fh @$oldtext;
+	my $participle = $patch_mode_flavour{PARTICIPLE};
 	print $fh <<EOF;
 # ---
 # To remove '-' lines, make them ' ' lines (context).

@@ -884,7 +910,7 @@
 # Lines starting with # will be removed.
 #
 # If the patch applies cleanly, the edited hunk will immediately be
-# marked for staging. If it does not apply cleanly, you will be given
+# marked for $participle. If it does not apply cleanly, you will be given
 # an opportunity to edit again. If all lines of the hunk are removed,
 # then the edit is aborted and the hunk is left unchanged.
 EOF

@@ -918,11 +944,8 @@
 
 sub diff_applies {
 	my $fh;
-	open $fh, '| git apply --recount --cached --check';
-	for my $h (@_) {
-		print $fh @{$h->{TEXT}};
-	}
-	return close $fh;
+	return run_git_apply($patch_mode_flavour{APPLY_CHECK} . ' --recount --check',
+			     map { @{$_->{TEXT}} } @_);
 }
 
 sub _restore_terminal_and_die {

@@ -988,12 +1011,14 @@
 }
 
 sub help_patch_cmd {
-	print colored $help_color, <<\EOF ;
-y - stage this hunk
-n - do not stage this hunk
-q - quit, do not stage this hunk nor any of the remaining ones
-a - stage this and all the remaining hunks in the file
-d - do not stage this hunk nor any of the remaining hunks in the file
+	my $verb = lc $patch_mode_flavour{VERB};
+	my $target = $patch_mode_flavour{TARGET};
+	print colored $help_color, <<EOF ;
+y - $verb this hunk$target
+n - do not $verb this hunk$target
+q - quit, do not $verb this hunk nor any of the remaining ones
+a - $verb this and all the remaining hunks in the file
+d - do not $verb this hunk nor any of the remaining hunks in the file
 g - select a hunk to go to
 / - search for a hunk matching the given regex
 j - leave this hunk undecided, see next undecided hunk

@@ -1006,8 +1031,17 @@
 EOF
 }
 
+sub apply_patch {
+	my $cmd = shift;
+	my $ret = run_git_apply $cmd . ' --recount', @_;
+	if (!$ret) {
+		print STDERR @_;
+	}
+	return $ret;
+}
+
 sub patch_update_cmd {
-	my @all_mods = list_modified('file-only');
+	my @all_mods = list_modified($patch_mode_flavour{FILTER});
 	my @mods = grep { !($_->{BINARY}) } @all_mods;
 	my @them;
 

@@ -1138,8 +1172,9 @@
 		for (@{$hunk[$ix]{DISPLAY}}) {
 			print;
 		}
-		print colored $prompt_color, 'Stage ',
-		  ($hunk[$ix]{TYPE} eq 'mode' ? 'mode change' : 'this hunk'),
+		print colored $prompt_color, $patch_mode_flavour{VERB},
+		  ($hunk[$ix]{TYPE} eq 'mode' ? ' mode change' : ' this hunk'),
+		  $patch_mode_flavour{TARGET},
 		  " [y,n,q,a,d,/$other,?]? ";
 		my $line = prompt_single_character;
 		if ($line) {

@@ -1313,16 +1348,9 @@
 
 	if (@result) {
 		my $fh;
-
-		open $fh, '| git apply --cached --recount';
-		for (@{$head->{TEXT}}, @result) {
-			print $fh $_;
-		}
-		if (!close $fh) {
-			for (@{$head->{TEXT}}, @result) {
-				print STDERR $_;
-			}
-		}
+		my @patch = (@{$head->{TEXT}}, @result);
+		my $apply_routine = $patch_mode_flavour{APPLY};
+		&$apply_routine(@patch);
 		refresh();
 	}
 

1.6.4.262.gbda8

The tests for {reset,commit,stash} -p will frequently have to set both worktree and index states to known values, and verify that the outcome (again both worktree and index) are what was expected.

Add a small helper library that lets us do these tasks more easily.

---
 t/lib-patch-mode.sh |   36 ++++++++++++++++++++++++++++++++++++
 1 files changed, 36 insertions(+), 0 deletions(-)
 create mode 100755 t/lib-patch-mode.sh

new file mode 100755
index 0000000..afb4b66
@@ -0,0 +1,36 @@
+. ./test-lib.sh
+
+set_state () {
+	echo "$3" > "$1" &&
+	git add "$1" &&
+	echo "$2" > "$1"
+}
+
+save_state () {
+	noslash="$(echo "$1" | tr / _)" &&
+	cat "$1" > _worktree_"$noslash" &&
+	git show :"$1" > _index_"$noslash"
+}
+
+set_and_save_state () {
+	set_state "$@" &&
+	save_state "$1"
+}
+
+verify_state () {
+	test "$(cat "$1")" = "$2" &&
+	test "$(git show :"$1")" = "$3"
+}
+
+verify_saved_state () {
+	noslash="$(echo "$1" | tr / _)" &&
+	verify_state "$1" "$(cat _worktree_"$noslash")" "$(cat _index_"$noslash")"
+}
+
+save_head () {
+	git rev-parse HEAD > _head
+}
+
+verify_saved_head () {
+	test "$(cat _head)" = "$(git rev-parse HEAD)"
+}

1.6.4.262.gbda8

This moves the call setup for 'git add--interactive' to a separate function, as other users will call it without running validate_pathspec() first.

---
 builtin-add.c |   43 +++++++++++++++++++++++++++++--------------
 commit.h      |    2 ++
 2 files changed, 31 insertions(+), 14 deletions(-)

index 581a2a1..c422a62 100644
@@ -131,27 +131,27 @@ static void refresh(int verbose, const char **pathspec)
 	return pathspec;
 }
 
-int interactive_add(int argc, const char **argv, const char *prefix)
+int run_add_interactive(const char *revision, const char *patch_mode,
+			const char **pathspec)
 {
-	int status, ac;
+	int status, ac, pc = 0;
 	const char **args;
-	const char **pathspec = NULL;
 
-	if (argc) {
-		pathspec = validate_pathspec(argc, argv, prefix);
-		if (!pathspec)
-			return -1;
-	}
+	if (pathspec)
+		while (pathspec[pc])
+			pc++;
 
-	args = xcalloc(sizeof(const char *), (argc + 4));
+	args = xcalloc(sizeof(const char *), (pc + 5));
 	ac = 0;
 	args[ac++] = "add--interactive";
-	if (patch_interactive)
-		args[ac++] = "--patch";
+	if (patch_mode)
+		args[ac++] = patch_mode;
+	if (revision)
+		args[ac++] = revision;
 	args[ac++] = "--";
-	if (argc) {
-		memcpy(&(args[ac]), pathspec, sizeof(const char *) * argc);
-		ac += argc;
+	if (pc) {
+		memcpy(&(args[ac]), pathspec, sizeof(const char *) * pc);
+		ac += pc;
 	}
 	args[ac] = NULL;
 

@@ -160,6 +160,21 @@ int interactive_add(int argc, const char **argv, const char *prefix)
 	return status;
 }
 
+int interactive_add(int argc, const char **argv, const char *prefix)
+{
+	const char **pathspec = NULL;
+
+	if (argc) {
+		pathspec = validate_pathspec(argc, argv, prefix);
+		if (!pathspec)
+			return -1;
+	}
+
+	return run_add_interactive(NULL,
+				   patch_interactive ? "--patch" : NULL,
+				   pathspec);
+}
+
 static int edit_patch(int argc, const char **argv, const char *prefix)
 {
 	char *file = xstrdup(git_path("ADD_EDIT.patch"));

index ba9f638..339f1f6 100644
@@ -137,6 +137,8 @@ struct commit_graft {
 int in_merge_bases(struct commit *, struct commit **, int);
 
 extern int interactive_add(int argc, const char **argv, const char *prefix);
+extern int run_add_interactive(const char *revision, const char *patch_mode,
+			       const char **pathspec);
 
 static inline int single_parent(struct commit *commit)
 {

1.6.4.262.gbda8

---
 Documentation/git-stash.txt |    2 +-
 git-stash.sh                |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

index 7af1840..ada16a0 100644
@@ -14,7 +14,7 @@ SYNOPSIS
 'git stash' ( pop | apply ) [--index] [-q|--quiet] [<stash>]
 'git stash' branch <branchname> [<stash>]
 'git stash' [save [--patch] [-k|--[no-]keep-index] [-q|--quiet] [<message>]]
-'git stash' [-k|--keep-index]
+'git stash' [-p|--patch|-k|--keep-index]
 'git stash' clear
 'git stash' create
 

index 81a72f6..9fd7289 100755
@@ -406,8 +406,8 @@ branch)
 	apply_to_branch "$@"
 	;;
 *)
-	case $#,"$1" in
-	0,|1,-k|1,--keep-index)
+	case $#,"$1","$2" in
+	0,,|1,-k,|1,--keep-index,|1,-p,|1,--patch,|2,-p,--no-keep-index|2,--patch,--no-keep-index)
 		save_stash "$@" &&
 		say '(To restore them type "git stash apply")'
 		;;

1.6.4.262.gbda8

This introduces a --patch mode for git-reset. The basic case is

  git reset --patch -- [files...]

which acts as the opposite of 'git add --patch -- [files...]': it offers hunks for *un*staging. Advanced usage is

  git reset --patch <revision> -- [files...]

which offers hunks from the diff between the index and <revision> for forward application to the index. (That is, the basic case is just <revision> = HEAD.)

---
 Documentation/git-reset.txt |   15 ++++++++-
 builtin-reset.c             |   19 ++++++++++++
 git-add--interactive.perl   |   57 +++++++++++++++++++++++++++++++++--
 t/t7105-reset-patch.sh      |   69 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 154 insertions(+), 6 deletions(-)
 create mode 100755 t/t7105-reset-patch.sh

index abb25d1..469cf6d 100644
@@ -10,6 +10,7 @@ SYNOPSIS
 [verse]
 'git reset' [--mixed | --soft | --hard | --merge] [-q] [<commit>]
 'git reset' [-q] [<commit>] [--] <paths>...
+'git reset' --patch [<commit>] [--] [<paths>...]
 
 DESCRIPTION
 -----------

@@ -23,8 +24,9 @@ the undo in the history.
 If you want to undo a commit other than the latest on a branch,
 linkgit:git-revert[1] is your friend.
 
-The second form with 'paths' is used to revert selected paths in
-the index from a given commit, without moving HEAD.
+The second and third forms with 'paths' and/or --patch are used to
+revert selected paths in the index from a given commit, without moving
+HEAD.
 
 
 OPTIONS

@@ -50,6 +52,15 @@ OPTIONS
 	and updates the files that are different between the named commit
 	and the current commit in the working tree.
 
+-p::
+--patch::
+	Interactively select hunks in the difference between the index
+	and <commit> (defaults to HEAD).  The chosen hunks are applied
+	in reverse to the index.
++
+This means that `git reset -p` is the opposite of `git add -p` (see
+linkgit:git-add[1]).
+
 -q::
 	Be quiet, only report errors.
 

index 5fa1789..246a127 100644
@@ -142,6 +142,17 @@ static void update_index_from_diff(struct diff_queue_struct *q,
 	}
 }
 
+static int interactive_reset(const char *revision, const char **argv,
+			     const char *prefix)
+{
+	const char **pathspec = NULL;
+
+	if (*argv)
+		pathspec = get_pathspec(prefix, argv);
+
+	return run_add_interactive(revision, "--patch=reset", pathspec);
+}
+
 static int read_from_tree(const char *prefix, const char **argv,
 		unsigned char *tree_sha1, int refresh_flags)
 {

@@ -183,6 +194,7 @@ static void prepend_reflog_action(const char *action, char *buf, size_t size)
 int cmd_reset(int argc, const char **argv, const char *prefix)
 {
 	int i = 0, reset_type = NONE, update_ref_status = 0, quiet = 0;
+	int patch_mode = 0;
 	const char *rev = "HEAD";
 	unsigned char sha1[20], *orig = NULL, sha1_orig[20],
 				*old_orig = NULL, sha1_old_orig[20];

@@ -198,6 +210,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 				"reset HEAD, index and working tree", MERGE),
 		OPT_BOOLEAN('q', NULL, &quiet,
 				"disable showing new HEAD in hard reset and progress message"),
+		OPT_BOOLEAN('p', "patch", &patch_mode, "select hunks interactively"),
 		OPT_END()
 	};
 

@@ -251,6 +264,12 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 		die("Could not parse object '%s'.", rev);
 	hashcpy(sha1, commit->object.sha1);
 
+	if (patch_mode) {
+		if (reset_type != NONE)
+			die("--patch is incompatible with --{hard,mixed,soft}");
+		return interactive_reset(rev, argv + i, prefix);
+	}
+
 	/* git reset tree [--] paths... can be used to
 	 * load chosen paths from the tree into the index without
 	 * affecting the working tree nor HEAD. */

1.6.4.262.gbda8

This introduces a --patch mode for git-reset. The basic case is

  git reset --patch -- [files...]

which acts as the opposite of 'git add --patch -- [files...]': it offers hunks for *un*staging. Advanced usage is

  git reset --patch <revision> -- [files...]

which offers hunks from the diff between the index and <revision> for forward application to the index. (That is, the basic case is just <revision> = HEAD.)

---

Jeff noticed the prompt and patch direction weren't matching:

http://article.gmane.org/gmane.comp.version-control.git/125981

This flips the direction of the patch in the 'git reset -p other' case, as I really wanted it that way around.

 Documentation/git-reset.txt |   15 ++++++++-
 builtin-reset.c             |   19 ++++++++++++
 git-add--interactive.perl   |   57 +++++++++++++++++++++++++++++++++--
 t/t7105-reset-patch.sh      |   69 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 154 insertions(+), 6 deletions(-)
 create mode 100755 t/t7105-reset-patch.sh

index abb25d1..469cf6d 100644
@@ -10,6 +10,7 @@ SYNOPSIS
 [verse]
 'git reset' [--mixed | --soft | --hard | --merge] [-q] [<commit>]
 'git reset' [-q] [<commit>] [--] <paths>...
+'git reset' --patch [<commit>] [--] [<paths>...]
 
 DESCRIPTION
 -----------

@@ -23,8 +24,9 @@ the undo in the history.
 If you want to undo a commit other than the latest on a branch,
 linkgit:git-revert[1] is your friend.
 
-The second form with 'paths' is used to revert selected paths in
-the index from a given commit, without moving HEAD.
+The second and third forms with 'paths' and/or --patch are used to
+revert selected paths in the index from a given commit, without moving
+HEAD.
 
 
 OPTIONS

@@ -50,6 +52,15 @@ OPTIONS
 	and updates the files that are different between the named commit
 	and the current commit in the working tree.
 
+-p::
+--patch::
+	Interactively select hunks in the difference between the index
+	and <commit> (defaults to HEAD).  The chosen hunks are applied
+	in reverse to the index.
++
+This means that `git reset -p` is the opposite of `git add -p` (see
+linkgit:git-add[1]).
+
 -q::
 	Be quiet, only report errors.
 

index 5fa1789..246a127 100644
@@ -142,6 +142,17 @@ static void update_index_from_diff(struct diff_queue_struct *q,
 	}
 }
 
+static int interactive_reset(const char *revision, const char **argv,
+			     const char *prefix)
+{
+	const char **pathspec = NULL;
+
+	if (*argv)
+		pathspec = get_pathspec(prefix, argv);
+
+	return run_add_interactive(revision, "--patch=reset", pathspec);
+}
+
 static int read_from_tree(const char *prefix, const char **argv,
 		unsigned char *tree_sha1, int refresh_flags)
 {

@@ -183,6 +194,7 @@ static void prepend_reflog_action(const char *action, char *buf, size_t size)
 int cmd_reset(int argc, const char **argv, const char *prefix)
 {
 	int i = 0, reset_type = NONE, update_ref_status = 0, quiet = 0;
+	int patch_mode = 0;
 	const char *rev = "HEAD";
 	unsigned char sha1[20], *orig = NULL, sha1_orig[20],
 				*old_orig = NULL, sha1_old_orig[20];

@@ -198,6 +210,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 				"reset HEAD, index and working tree", MERGE),
 		OPT_BOOLEAN('q', NULL, &quiet,
 				"disable showing new HEAD in hard reset and progress message"),
+		OPT_BOOLEAN('p', "patch", &patch_mode, "select hunks interactively"),
 		OPT_END()
 	};
 

@@ -251,6 +264,12 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 		die("Could not parse object '%s'.", rev);
 	hashcpy(sha1, commit->object.sha1);
 
+	if (patch_mode) {
+		if (reset_type != NONE)
+			die("--patch is incompatible with --{hard,mixed,soft}");
+		return interactive_reset(rev, argv + i, prefix);
+	}
+
 	/* git reset tree [--] paths... can be used to
 	 * load chosen paths from the tree into the index without
 	 * affecting the working tree nor HEAD. */

1.6.4.287.g3e02d

This introduces a --patch mode for git-checkout. In the index usage

  git checkout --patch -- [files...]

it lets the user discard edits from the <files> at the granularity of hunks (by selecting hunks from 'git diff' and then reverse applying them to the worktree).

We also accept a revision argument. In the case

  git checkout --patch HEAD -- [files...]

we offer hunks from the difference between HEAD and the worktree, and reverse applies them to both index and worktree, allowing you to discard staged changes completely. In the non-HEAD usage

  git checkout --patch <revision> -- [files...]

it offers hunks from the difference between the worktree and <revision>. The chosen hunks are then applied to both index and worktree.

The application to worktree and index is done "atomically" in the sense that we first check if the patch applies to the index (it should always apply to the worktree). If it does not, we give the user a choice to either abort or apply to the worktree anyway.

---
 Documentation/git-checkout.txt |   13 +++++-
 builtin-checkout.c             |   19 +++++++
 git-add--interactive.perl      |   61 +++++++++++++++++++++++
 t/t2015-checkout-patch.sh      |  107 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 199 insertions(+), 1 deletions(-)
 create mode 100755 t/t2015-checkout-patch.sh

index ad4b31e..26a5447 100644
@@ -11,6 +11,7 @@ SYNOPSIS
 'git checkout' [-q] [-f] [-m] [<branch>]
 'git checkout' [-q] [-f] [-m] [-b <new_branch>] [<start_point>]
 'git checkout' [-f|--ours|--theirs|-m|--conflict=<style>] [<tree-ish>] [--] <paths>...
+'git checkout' --patch [<tree-ish>] [--] [<paths>...]
 
 DESCRIPTION
 -----------

@@ -25,7 +26,7 @@ use the --track or --no-track options, which will be passed to `git
 branch`.  As a convenience, --track without `-b` implies branch
 creation; see the description of --track below.
 
-When <paths> are given, this command does *not* switch
+When <paths> or --patch are given, this command does *not* switch
 branches.  It updates the named paths in the working tree from
 the index file, or from a named <tree-ish> (most often a commit).  In
 this case, the `-b` and `--track` options are meaningless and giving

@@ -113,6 +114,16 @@ the conflicted merge in the specified paths.
 	"merge" (default) and "diff3" (in addition to what is shown by
 	"merge" style, shows the original contents).
 
+-p::
+--patch::
+	Interactively select hunks in the difference between the
+	<tree-ish> (or the index, if unspecified) and the working
+	tree.  The chosen hunks are then applied in reverse to the
+	working tree (and if a <tree-ish> was specified, the index).
++
+This means that you can use `git checkout -p` to selectively discard
+edits from your current working tree.
+
 <branch>::
 	Branch to checkout; if it refers to a branch (i.e., a name that,
 	when prepended with "refs/heads/", is a valid ref), then that

index 8a9a474..8b942ba 100644
@@ -572,6 +572,13 @@ static int git_checkout_config(const char *var, const char *value, void *cb)
 	return git_xmerge_config(var, value, cb);
 }
 
+static int interactive_checkout(const char *revision, const char **pathspec,
+				struct checkout_opts *opts)
+{
+	return run_add_interactive(revision, "--patch=checkout", pathspec);
+}
+
+
 int cmd_checkout(int argc, const char **argv, const char *prefix)
 {
 	struct checkout_opts opts;

@@ -580,6 +587,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	struct branch_info new;
 	struct tree *source_tree = NULL;
 	char *conflict_style = NULL;
+	int patch_mode = 0;
 	struct option options[] = {
 		OPT__QUIET(&opts.quiet),
 		OPT_STRING('b', NULL, &opts.new_branch, "new branch", "branch"),

@@ -594,6 +602,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		OPT_BOOLEAN('m', "merge", &opts.merge, "merge"),
 		OPT_STRING(0, "conflict", &conflict_style, "style",
 			   "conflict style (merge or diff3)"),
+		OPT_BOOLEAN('p', "patch", &patch_mode, "select hunks interactively"),
 		OPT_END(),
 	};
 	int has_dash_dash;

@@ -608,6 +617,10 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix, options, checkout_usage,
 			     PARSE_OPT_KEEP_DASHDASH);
 
+	if (patch_mode && (opts.track > 0 || opts.new_branch
+			   || opts.new_branch_log || opts.merge || opts.force))
+		die ("--patch is incompatible with all other options");
+
 	/* --track without -b should DWIM */
 	if (0 < opts.track && !opts.new_branch) {
 		const char *argv0 = argv[0];

@@ -714,6 +727,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		if (!pathspec)
 			die("invalid path specification");
 
+		if (patch_mode)
+			return interactive_checkout(new.name, pathspec, &opts);
+
 		/* Checkout paths */
 		if (opts.new_branch) {
 			if (argc == 1) {

@@ -729,6 +745,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		return checkout_paths(source_tree, pathspec, &opts);
 	}
 
+	if (patch_mode)
+		return interactive_checkout(new.name, NULL, &opts);
+
 	if (opts.new_branch) {
 		struct strbuf buf = STRBUF_INIT;
 		if (strbuf_check_branch_ref(&buf, opts.new_branch))

1.6.4.262.gbda8

This introduces a --patch mode for git-checkout. In the index usage

  git checkout --patch -- [files...]

it lets the user discard edits from the <files> at the granularity of hunks (by selecting hunks from 'git diff' and then reverse applying them to the worktree).

We also accept a revision argument. In the case

  git checkout --patch HEAD -- [files...]

we offer hunks from the difference between HEAD and the worktree, and reverse applies them to both index and worktree, allowing you to discard staged changes completely. In the non-HEAD usage

  git checkout --patch <revision> -- [files...]

it offers hunks from the difference between the worktree and <revision>. The chosen hunks are then applied to both index and worktree.

The application to worktree and index is done "atomically" in the sense that we first check if the patch applies to the index (it should always apply to the worktree). If it does not, we give the user a choice to either abort or apply to the worktree anyway.

---

I found another mismatch of the -R flags during double checking, as follows:

diff --git c/git-add--interactive.perl i/git-add--interactive.perl index 2363a77..21746d5 100755
--- c/git-add--interactive.perl
+++ i/git-add--interactive.perl
@@ -117,7 +117,7 @@
'checkout_head' => {
DIFF => 'diff-index -p',
APPLY => sub { apply_patch_for_checkout_commit '-R', @_ },
- APPLY_CHECK => 'apply',
+ APPLY_CHECK => 'apply -R',
VERB => 'Discard',
TARGET => ' from index and worktree', PARTICIPLE => 'discarding',

 Documentation/git-checkout.txt |   13 +++++-
 builtin-checkout.c             |   19 +++++++
 git-add--interactive.perl      |   61 +++++++++++++++++++++++
 t/t2015-checkout-patch.sh      |  107 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 199 insertions(+), 1 deletions(-)
 create mode 100755 t/t2015-checkout-patch.sh

index ad4b31e..26a5447 100644
@@ -11,6 +11,7 @@ SYNOPSIS
 'git checkout' [-q] [-f] [-m] [<branch>]
 'git checkout' [-q] [-f] [-m] [-b <new_branch>] [<start_point>]
 'git checkout' [-f|--ours|--theirs|-m|--conflict=<style>] [<tree-ish>] [--] <paths>...
+'git checkout' --patch [<tree-ish>] [--] [<paths>...]
 
 DESCRIPTION
 -----------

@@ -25,7 +26,7 @@ use the --track or --no-track options, which will be passed to `git
 branch`.  As a convenience, --track without `-b` implies branch
 creation; see the description of --track below.
 
-When <paths> are given, this command does *not* switch
+When <paths> or --patch are given, this command does *not* switch
 branches.  It updates the named paths in the working tree from
 the index file, or from a named <tree-ish> (most often a commit).  In
 this case, the `-b` and `--track` options are meaningless and giving

@@ -113,6 +114,16 @@ the conflicted merge in the specified paths.
 	"merge" (default) and "diff3" (in addition to what is shown by
 	"merge" style, shows the original contents).
 
+-p::
+--patch::
+	Interactively select hunks in the difference between the
+	<tree-ish> (or the index, if unspecified) and the working
+	tree.  The chosen hunks are then applied in reverse to the
+	working tree (and if a <tree-ish> was specified, the index).
++
+This means that you can use `git checkout -p` to selectively discard
+edits from your current working tree.
+
 <branch>::
 	Branch to checkout; if it refers to a branch (i.e., a name that,
 	when prepended with "refs/heads/", is a valid ref), then that

index 8a9a474..8b942ba 100644
@@ -572,6 +572,13 @@ static int git_checkout_config(const char *var, const char *value, void *cb)
 	return git_xmerge_config(var, value, cb);
 }
 
+static int interactive_checkout(const char *revision, const char **pathspec,
+				struct checkout_opts *opts)
+{
+	return run_add_interactive(revision, "--patch=checkout", pathspec);
+}
+
+
 int cmd_checkout(int argc, const char **argv, const char *prefix)
 {
 	struct checkout_opts opts;

@@ -580,6 +587,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	struct branch_info new;
 	struct tree *source_tree = NULL;
 	char *conflict_style = NULL;
+	int patch_mode = 0;
 	struct option options[] = {
 		OPT__QUIET(&opts.quiet),
 		OPT_STRING('b', NULL, &opts.new_branch, "new branch", "branch"),

@@ -594,6 +602,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		OPT_BOOLEAN('m', "merge", &opts.merge, "merge"),
 		OPT_STRING(0, "conflict", &conflict_style, "style",
 			   "conflict style (merge or diff3)"),
+		OPT_BOOLEAN('p', "patch", &patch_mode, "select hunks interactively"),
 		OPT_END(),
 	};
 	int has_dash_dash;

@@ -608,6 +617,10 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix, options, checkout_usage,
 			     PARSE_OPT_KEEP_DASHDASH);
 
+	if (patch_mode && (opts.track > 0 || opts.new_branch
+			   || opts.new_branch_log || opts.merge || opts.force))
+		die ("--patch is incompatible with all other options");
+
 	/* --track without -b should DWIM */
 	if (0 < opts.track && !opts.new_branch) {
 		const char *argv0 = argv[0];

@@ -714,6 +727,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		if (!pathspec)
 			die("invalid path specification");
 
+		if (patch_mode)
+			return interactive_checkout(new.name, pathspec, &opts);
+
 		/* Checkout paths */
 		if (opts.new_branch) {
 			if (argc == 1) {

@@ -729,6 +745,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		return checkout_paths(source_tree, pathspec, &opts);
 	}
 
+	if (patch_mode)
+		return interactive_checkout(new.name, NULL, &opts);
+
 	if (opts.new_branch) {
 		struct strbuf buf = STRBUF_INIT;
 		if (strbuf_check_branch_ref(&buf, opts.new_branch))

1.6.4.287.g3e02d

This adds a hunk-based mode to git-stash. You can select hunks from the difference between HEAD and worktree, and git-stash will build a stash that reflects these changes. The index state of the stash is the same as your current index, and we also let --patch imply --keep-index.

Note that because the selected hunks are rolled back from the worktree but not the index, the resulting state may appear somewhat confusing if you had also staged these changes. This is not entirely satisfactory, but due to the way stashes are applied, other solutions would require a change to the stash format.

---
 Documentation/git-stash.txt |   14 ++++++-
 git-add--interactive.perl   |   14 ++++++-
 git-stash.sh                |   80 +++++++++++++++++++++++++++++++++++-------
 t/t3904-stash-patch.sh      |   55 +++++++++++++++++++++++++++++
 4 files changed, 145 insertions(+), 18 deletions(-)
 create mode 100755 t/t3904-stash-patch.sh

index 2f5ca7b..d206297 100644
@@ -13,7 +13,7 @@ SYNOPSIS
 'git stash' drop [-q|--quiet] [<stash>]
 'git stash' ( pop | apply ) [--index] [-q|--quiet] [<stash>]
 'git stash' branch <branchname> [<stash>]
-'git stash' [save [--keep-index] [-q|--quiet] [<message>]]
+'git stash' [save [--patch] [--[no-]keep-index] [-q|--quiet] [<message>]]
 'git stash' clear
 'git stash' create
 

@@ -42,7 +42,7 @@ is also possible).
 OPTIONS
 -------
 
-save [--keep-index] [-q|--quiet] [<message>]::
+save [--patch] [--[no-]keep-index] [-q|--quiet] [<message>]::
 
 	Save your local modifications to a new 'stash', and run `git reset
 	--hard` to revert them.  This is the default action when no

@@ -51,6 +51,16 @@ save [--keep-index] [-q|--quiet] [<message>]::
 +
 If the `--keep-index` option is used, all changes already added to the
 index are left intact.
++
+With `--patch`, you can interactively select hunks from in the diff
+between HEAD and the working tree to be stashed.  The stash entry is
+constructed such that its index state is the same as the index state
+of your repository, and its worktree contains only the changes you
+selected interactively.  The selected changes are then rolled back
+from your worktree.
++
+The `--patch` option implies `--keep-index`.  You can use
+`--no-keep-index` to override this.
 
 list [<options>]::
 

index 9c202fc..c5e0586 100755
@@ -76,6 +76,7 @@
 
 sub apply_patch;
 sub apply_patch_for_checkout_commit;
+sub apply_patch_for_stash;
 
 my %patch_modes = (
 	'stage' => {

@@ -87,6 +88,15 @@
 		PARTICIPLE => 'staging',
 		FILTER => 'file-only',
 	},
+	'stash' => {
+		DIFF => 'diff-index -p HEAD',
+		APPLY => sub { apply_patch 'apply --cached', @_; },
+		APPLY_CHECK => 'apply --cached',
+		VERB => 'Stash',
+		TARGET => '',
+		PARTICIPLE => 'stashing',
+		FILTER => undef,
+	},
 	'reset_head' => {
 		DIFF => 'diff-index -p --cached',
 		APPLY => sub { apply_patch 'apply -R --cached', @_; },

@@ -1493,8 +1503,8 @@
 						       'checkout_head' : 'checkout_nothead');
 					$arg = shift @ARGV or die "missing --";
 				}
-			} elsif ($1 eq 'stage') {
-				$patch_mode = 'stage';
+			} elsif ($1 eq 'stage' or $1 eq 'stash') {
+				$patch_mode = $1;
 				$arg = shift @ARGV or die "missing --";
 			} else {
 				die "unknown --patch mode: $1";

1.6.4.262.gbda8

This makes some aspects of the 'git add -p' loop configurable (within the code), so that we can later reuse git-add--interactive for other similar tools.

Most fields are fairly straightforward, but APPLY gets a subroutine (instead of just a string a la 'apply --cached') so that we can handle 'checkout -p', which will need to atomically apply the patch twice (index and worktree).

---
 git-add--interactive.perl |   79 +++++++++++++++++++++++++++++---------------
 1 files changed, 52 insertions(+), 27 deletions(-)

index df9f231..58c3332 100755
@@ -73,6 +73,21 @@
 # command line options
 my $patch_mode;
 
+sub apply_patch;
+
+my %patch_modes = (
+	'stage' => {
+		DIFF => 'diff-files -p',
+		APPLY => sub { apply_patch 'apply --cached', @_; },
+		APPLY_CHECK => 'apply --cached',
+		VERB => 'Stage',
+		PARTICIPLE => 'staging',
+		FILTER => 'file-only',
+	},
+);
+
+my %patch_mode_flavour = %{$patch_modes{stage}};
+
 sub run_cmd_pipe {
 	if ($^O eq 'MSWin32' || $^O eq 'msys') {
 		my @invalid = grep {m/[":*]/} @_;

@@ -613,12 +628,21 @@
 	print "\n";
 }
 
+sub run_git_apply {
+	my $cmd = shift;
+	my $fh;
+	open $fh, '| git ' . $cmd;
+	print $fh @_;
+	return close $fh;
+}
+
 sub parse_diff {
 	my ($path) = @_;
-	my @diff = run_cmd_pipe(qw(git diff-files -p --), $path);
+	my @diff_cmd = split(" ", $patch_mode_flavour{DIFF});
+	my @diff = run_cmd_pipe("git", @diff_cmd, "--", $path);
 	my @colored = ();
 	if ($diff_use_color) {
-		@colored = run_cmd_pipe(qw(git diff-files -p --color --), $path);
+		@colored = run_cmd_pipe("git", @diff_cmd, qw(--color --), $path);
 	}
 	my (@hunk) = { TEXT => [], DISPLAY => [], TYPE => 'header' };
 

@@ -877,6 +901,7 @@
 		or die "failed to open hunk edit file for writing: " . $!;
 	print $fh "# Manual hunk edit mode -- see bottom for a quick guide\n";
 	print $fh @$oldtext;
+	my $participle = $patch_mode_flavour{PARTICIPLE};
 	print $fh <<EOF;
 # ---
 # To remove '-' lines, make them ' ' lines (context).

@@ -884,7 +909,7 @@
 # Lines starting with # will be removed.
 #
 # If the patch applies cleanly, the edited hunk will immediately be
-# marked for staging. If it does not apply cleanly, you will be given
+# marked for $participle. If it does not apply cleanly, you will be given
 # an opportunity to edit again. If all lines of the hunk are removed,
 # then the edit is aborted and the hunk is left unchanged.
 EOF

@@ -918,11 +943,8 @@
 
 sub diff_applies {
 	my $fh;
-	open $fh, '| git apply --recount --cached --check';
-	for my $h (@_) {
-		print $fh @{$h->{TEXT}};
-	}
-	return close $fh;
+	return run_git_apply($patch_mode_flavour{APPLY_CHECK} . ' --recount --check',
+			     map { @{$_->{TEXT}} } @_);
 }
 
 sub _restore_terminal_and_die {

@@ -988,12 +1010,13 @@
 }
 
 sub help_patch_cmd {
-	print colored $help_color, <<\EOF ;
-y - stage this hunk
-n - do not stage this hunk
-q - quit, do not stage this hunk nor any of the remaining ones
-a - stage this and all the remaining hunks in the file
-d - do not stage this hunk nor any of the remaining hunks in the file
+	my $verb = lc $patch_mode_flavour{VERB};
+	print colored $help_color, <<EOF ;
+y - $verb this hunk
+n - do not $verb this hunk
+q - quit, do not $verb this hunk nor any of the remaining ones
+a - $verb this and all the remaining hunks in the file
+d - do not $verb this hunk nor any of the remaining hunks in the file
 g - select a hunk to go to
 / - search for a hunk matching the given regex
 j - leave this hunk undecided, see next undecided hunk

@@ -1006,8 +1029,17 @@
 EOF
 }
 
+sub apply_patch {
+	my $cmd = shift;
+	my $ret = run_git_apply $cmd . ' --recount', @_;
+	if (!$ret) {
+		print STDERR @_;
+	}
+	return $ret;
+}
+
 sub patch_update_cmd {
-	my @all_mods = list_modified('file-only');
+	my @all_mods = list_modified($patch_mode_flavour{FILTER});
 	my @mods = grep { !($_->{BINARY}) } @all_mods;
 	my @them;
 

@@ -1138,8 +1170,8 @@
 		for (@{$hunk[$ix]{DISPLAY}}) {
 			print;
 		}
-		print colored $prompt_color, 'Stage ',
-		  ($hunk[$ix]{TYPE} eq 'mode' ? 'mode change' : 'this hunk'),
+		print colored $prompt_color, $patch_mode_flavour{VERB},
+		  ($hunk[$ix]{TYPE} eq 'mode' ? ' mode change' : ' this hunk'),
 		  " [y,n,q,a,d,/$other,?]? ";
 		my $line = prompt_single_character;
 		if ($line) {

@@ -1313,16 +1345,9 @@
 
 	if (@result) {
 		my $fh;
-
-		open $fh, '| git apply --cached --recount';
-		for (@{$head->{TEXT}}, @result) {
-			print $fh $_;
-		}
-		if (!close $fh) {
-			for (@{$head->{TEXT}}, @result) {
-				print STDERR $_;
-			}
-		}
+		my @patch = (@{$head->{TEXT}}, @result);
+		my $apply_routine = $patch_mode_flavour{APPLY};
+		&$apply_routine(@patch);
 		refresh();
 	}
 

1.6.4.rc2.227.gf5e17

This moves the call setup for 'git add--interactive' to a separate function, as other users will call it without running validate_pathspec() first.

---
 builtin-add.c |   43 +++++++++++++++++++++++++++++--------------
 commit.h      |    2 ++
 2 files changed, 31 insertions(+), 14 deletions(-)

index 581a2a1..c422a62 100644
@@ -131,27 +131,27 @@ static void refresh(int verbose, const char **pathspec)
 	return pathspec;
 }
 
-int interactive_add(int argc, const char **argv, const char *prefix)
+int run_add_interactive(const char *revision, const char *patch_mode,
+			const char **pathspec)
 {
-	int status, ac;
+	int status, ac, pc = 0;
 	const char **args;
-	const char **pathspec = NULL;
 
-	if (argc) {
-		pathspec = validate_pathspec(argc, argv, prefix);
-		if (!pathspec)
-			return -1;
-	}
+	if (pathspec)
+		while (pathspec[pc])
+			pc++;
 
-	args = xcalloc(sizeof(const char *), (argc + 4));
+	args = xcalloc(sizeof(const char *), (pc + 5));
 	ac = 0;
 	args[ac++] = "add--interactive";
-	if (patch_interactive)
-		args[ac++] = "--patch";
+	if (patch_mode)
+		args[ac++] = patch_mode;
+	if (revision)
+		args[ac++] = revision;
 	args[ac++] = "--";
-	if (argc) {
-		memcpy(&(args[ac]), pathspec, sizeof(const char *) * argc);
-		ac += argc;
+	if (pc) {
+		memcpy(&(args[ac]), pathspec, sizeof(const char *) * pc);
+		ac += pc;
 	}
 	args[ac] = NULL;
 

@@ -160,6 +160,21 @@ int interactive_add(int argc, const char **argv, const char *prefix)
 	return status;
 }
 
+int interactive_add(int argc, const char **argv, const char *prefix)
+{
+	const char **pathspec = NULL;
+
+	if (argc) {
+		pathspec = validate_pathspec(argc, argv, prefix);
+		if (!pathspec)
+			return -1;
+	}
+
+	return run_add_interactive(NULL,
+				   patch_interactive ? "--patch" : NULL,
+				   pathspec);
+}
+
 static int edit_patch(int argc, const char **argv, const char *prefix)
 {
 	char *file = xstrdup(git_path("ADD_EDIT.patch"));

index 8bfdf0e..0555e80 100644
@@ -139,6 +139,8 @@ struct commit_graft {
 int in_merge_bases(struct commit *, struct commit **, int);
 
 extern int interactive_add(int argc, const char **argv, const char *prefix);
+extern int run_add_interactive(const char *revision, const char *patch_mode,
+			       const char **pathspec);
 
 static inline int single_parent(struct commit *commit)
 {

1.6.4.rc2.227.gf5e17

This introduces a --patch mode for git-reset. The basic case is

  git reset --patch -- [files...]

which acts as the opposite of 'git add --patch -- [files...]': it offers hunks for *un*staging. Advanced usage is

  git reset --patch <revision> -- [files...]

which offers hunks from the diff between <revision> and the index for reverse application to the index. (That is, the basic case is just <revision> = HEAD.) This means it can be used to "undo" changes since <revision> in the index, but may be slightly confusing when <revision> is logically "newer" than the index state.

---
 Documentation/git-reset.txt |   15 +++++++++++++--
 builtin-reset.c             |   15 +++++++++++++++
 git-add--interactive.perl   |   36 +++++++++++++++++++++++++++++++++---
 3 files changed, 61 insertions(+), 5 deletions(-)

index abb25d1..469cf6d 100644
@@ -10,6 +10,7 @@ SYNOPSIS
 [verse]
 'git reset' [--mixed | --soft | --hard | --merge] [-q] [<commit>]
 'git reset' [-q] [<commit>] [--] <paths>...
+'git reset' --patch [<commit>] [--] [<paths>...]
 
 DESCRIPTION
 -----------

@@ -23,8 +24,9 @@ the undo in the history.
 If you want to undo a commit other than the latest on a branch,
 linkgit:git-revert[1] is your friend.
 
-The second form with 'paths' is used to revert selected paths in
-the index from a given commit, without moving HEAD.
+The second and third forms with 'paths' and/or --patch are used to
+revert selected paths in the index from a given commit, without moving
+HEAD.
 
 
 OPTIONS

@@ -50,6 +52,15 @@ OPTIONS
 	and updates the files that are different between the named commit
 	and the current commit in the working tree.
 
+-p::
+--patch::
+	Interactively select hunks in the difference between the index
+	and <commit> (defaults to HEAD).  The chosen hunks are applied
+	in reverse to the index.
++
+This means that `git reset -p` is the opposite of `git add -p` (see
+linkgit:git-add[1]).
+
 -q::
 	Be quiet, only report errors.
 

index 5fa1789..e4fef5d 100644
@@ -142,6 +142,13 @@ static void update_index_from_diff(struct diff_queue_struct *q,
 	}
 }
 
+static int interactive_reset(const char *revision, const char **argv,
+			     const char *prefix)
+{
+	return run_add_interactive(revision, "--patch=reset",
+				   get_pathspec(prefix, argv));
+}
+
 static int read_from_tree(const char *prefix, const char **argv,
 		unsigned char *tree_sha1, int refresh_flags)
 {

@@ -183,6 +190,7 @@ static void prepend_reflog_action(const char *action, char *buf, size_t size)
 int cmd_reset(int argc, const char **argv, const char *prefix)
 {
 	int i = 0, reset_type = NONE, update_ref_status = 0, quiet = 0;
+	int patch_mode = 0;
 	const char *rev = "HEAD";
 	unsigned char sha1[20], *orig = NULL, sha1_orig[20],
 				*old_orig = NULL, sha1_old_orig[20];

@@ -198,6 +206,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 				"reset HEAD, index and working tree", MERGE),
 		OPT_BOOLEAN('q', NULL, &quiet,
 				"disable showing new HEAD in hard reset and progress message"),
+		OPT_BOOLEAN('p', "patch", &patch_mode, "select hunks interactively"),
 		OPT_END()
 	};
 

@@ -251,6 +260,12 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 		die("Could not parse object '%s'.", rev);
 	hashcpy(sha1, commit->object.sha1);
 
+	if (patch_mode) {
+		if (reset_type != NONE)
+			die("--patch is incompatible with --{hard,mixed,soft}");
+		return interactive_reset(rev, argv, prefix);
+	}
+
 	/* git reset tree [--] paths... can be used to
 	 * load chosen paths from the tree into the index without
 	 * affecting the working tree nor HEAD. */

index 58c3332..b1aa846 100755
@@ -72,6 +72,7 @@
 
 # command line options
 my $patch_mode;
+my $patch_mode_revision;
 
 sub apply_patch;
 

@@ -84,6 +85,14 @@
 		PARTICIPLE => 'staging',
 		FILTER => 'file-only',
 	},
+	'reset' => {
+		DIFF => 'diff-index -p --cached',
+		APPLY => sub { apply_patch 'apply -R --cached', @_; },
+		APPLY_CHECK => 'apply -R --cached',
+		VERB => 'Reset',
+		PARTICIPLE => 'resetting',
+		FILTER => 'index-only',
+	},
 );
 
 my %patch_mode_flavour = %{$patch_modes{stage}};

@@ -639,6 +648,9 @@
 sub parse_diff {
 	my ($path) = @_;
 	my @diff_cmd = split(" ", $patch_mode_flavour{DIFF});
+	if (defined $patch_mode_revision) {
+		push @diff_cmd, $patch_mode_revision;
+	}
 	my @diff = run_cmd_pipe("git", @diff_cmd, "--", $path);
 	my @colored = ();
 	if ($diff_use_color) {

@@ -1388,11 +1400,29 @@
 sub process_args {
 	return unless @ARGV;
 	my $arg = shift @ARGV;
-	if ($arg eq "--patch") {
-		$patch_mode = 1;
-		$arg = shift @ARGV or die "missing --";
+	if ($arg =~ /--patch(?:=(.*))?/) {
+		if (defined $1) {
+			if ($1 eq 'reset') {
+				$patch_mode = 'reset';
+				$patch_mode_revision = 'HEAD';
+				$arg = shift @ARGV or die "missing --";
+				if ($arg ne '--') {
+					$patch_mode_revision = $arg;
+					$arg = shift @ARGV or die "missing --";
+				}
+			} elsif ($1 eq 'stage') {
+				$patch_mode = 'stage';
+				$arg = shift @ARGV or die "missing --";
+			} else {
+				die "unknown --patch mode: $1";
+			}
+		} else {
+			$patch_mode = 'stage';
+			$arg = shift @ARGV or die "missing --";
+		}
 		die "invalid argument $arg, expecting --"
 		    unless $arg eq "--";
+		%patch_mode_flavour = %{$patch_modes{$patch_mode}};
 	}
 	elsif ($arg ne "--") {
 		die "invalid argument $arg, expecting --";

1.6.4.rc2.227.gf5e17

This introduces a --patch mode for git-checkout. In the index usage

  git checkout --patch -- [files...]

it lets the user discard edits from the <files> at the granularity of hunks (by selecting hunks from 'git diff' and then reverse applying them to the worktree).

We also accept a revision argument

  git checkout --patch <revision> -- [files...]

which offers hunks from the difference between the <revision> and the worktree. The chosen hunks are then reverse applied to both index and worktree, discarding them completely. This application is done "atomically" in the sense that we first check if the patch applies to the index (it should always apply to the worktree). If it does not, we give the user a choice to either abort or apply to the worktree anyway.

---
 Documentation/git-checkout.txt |   13 ++++++++++-
 builtin-checkout.c             |   19 +++++++++++++++
 git-add--interactive.perl      |   48 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 79 insertions(+), 1 deletions(-)

index ad4b31e..26a5447 100644
@@ -11,6 +11,7 @@ SYNOPSIS
 'git checkout' [-q] [-f] [-m] [<branch>]
 'git checkout' [-q] [-f] [-m] [-b <new_branch>] [<start_point>]
 'git checkout' [-f|--ours|--theirs|-m|--conflict=<style>] [<tree-ish>] [--] <paths>...
+'git checkout' --patch [<tree-ish>] [--] [<paths>...]
 
 DESCRIPTION
 -----------

@@ -25,7 +26,7 @@ use the --track or --no-track options, which will be passed to `git
 branch`.  As a convenience, --track without `-b` implies branch
 creation; see the description of --track below.
 
-When <paths> are given, this command does *not* switch
+When <paths> or --patch are given, this command does *not* switch
 branches.  It updates the named paths in the working tree from
 the index file, or from a named <tree-ish> (most often a commit).  In
 this case, the `-b` and `--track` options are meaningless and giving

@@ -113,6 +114,16 @@ the conflicted merge in the specified paths.
 	"merge" (default) and "diff3" (in addition to what is shown by
 	"merge" style, shows the original contents).
 
+-p::
+--patch::
+	Interactively select hunks in the difference between the
+	<tree-ish> (or the index, if unspecified) and the working
+	tree.  The chosen hunks are then applied in reverse to the
+	working tree (and if a <tree-ish> was specified, the index).
++
+This means that you can use `git checkout -p` to selectively discard
+edits from your current working tree.
+
 <branch>::
 	Branch to checkout; if it refers to a branch (i.e., a name that,
 	when prepended with "refs/heads/", is a valid ref), then that

index 446cac7..7d57741 100644
@@ -572,6 +572,13 @@ static int git_checkout_config(const char *var, const char *value, void *cb)
 	return git_xmerge_config(var, value, cb);
 }
 
+static int interactive_checkout(const char *revision, const char **pathspec,
+				struct checkout_opts *opts)
+{
+	return run_add_interactive(revision, "--patch=checkout", pathspec);
+}
+
+
 int cmd_checkout(int argc, const char **argv, const char *prefix)
 {
 	struct checkout_opts opts;

@@ -580,6 +587,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	struct branch_info new;
 	struct tree *source_tree = NULL;
 	char *conflict_style = NULL;
+	int patch_mode = 0;
 	struct option options[] = {
 		OPT__QUIET(&opts.quiet),
 		OPT_STRING('b', NULL, &opts.new_branch, "new branch", "branch"),

@@ -594,6 +602,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		OPT_BOOLEAN('m', "merge", &opts.merge, "merge"),
 		OPT_STRING(0, "conflict", &conflict_style, "style",
 			   "conflict style (merge or diff3)"),
+		OPT_BOOLEAN('p', "patch", &patch_mode, "select hunks interactively"),
 		OPT_END(),
 	};
 	int has_dash_dash;

@@ -608,6 +617,10 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix, options, checkout_usage,
 			     PARSE_OPT_KEEP_DASHDASH);
 
+	if (patch_mode && (opts.track > 0 || opts.new_branch
+			   || opts.new_branch_log || opts.merge || opts.force))
+		die ("--patch is incompatible with all other options");
+
 	/* --track without -b should DWIM */
 	if (0 < opts.track && !opts.new_branch) {
 		const char *argv0 = argv[0];

@@ -714,6 +727,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		if (!pathspec)
 			die("invalid path specification");
 
+		if (patch_mode)
+			return interactive_checkout(NULL, pathspec, &opts);
+
 		/* Checkout paths */
 		if (opts.new_branch) {
 			if (argc == 1) {

@@ -729,6 +745,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		return checkout_paths(source_tree, pathspec, &opts);
 	}
 
+	if (patch_mode)
+		return interactive_checkout(new.name, NULL, &opts);
+
 	if (opts.new_branch) {
 		struct strbuf buf = STRBUF_INIT;
 		if (strbuf_check_branch_ref(&buf, opts.new_branch))

1.6.4.rc2.227.gf5e17

This adds a hunk-based mode to git-stash. You can select hunks from the index and the worktree, and git-stash will attempt to build a stash that reflects these changes.

Internally, we have the problem that we're trying to offer hunks from one index (the one the user sees) for inclusion in another index (used to build the stash). We solve this by letting git-add--interactive write out the hunks to a patch file, and then using git-apply with a different GIT_INDEX_FILE.

---
 Documentation/git-stash.txt |   10 +++-
 git-add--interactive.perl   |   30 ++++++++++-
 git-stash.sh                |  120 +++++++++++++++++++++++++++++++++++--------
 3 files changed, 134 insertions(+), 26 deletions(-)

index 1c64a02..e6f310a 100644
@@ -13,7 +13,7 @@ SYNOPSIS
 'git stash' drop [-q|--quiet] [<stash>]
 'git stash' ( pop | apply ) [--index] [-q|--quiet] [<stash>]
 'git stash' branch <branchname> [<stash>]
-'git stash' [save [--keep-index] [-q|--quiet] [<message>]]
+'git stash' [save [--patch] [--keep-index] [-q|--quiet] [<message>]]
 'git stash' clear
 'git stash' create
 

@@ -42,7 +42,7 @@ is also possible).
 OPTIONS
 -------
 
-save [--keep-index] [-q|--quiet] [<message>]::
+save [--patch] [--keep-index] [-q|--quiet] [<message>]::
 
 	Save your local modifications to a new 'stash', and run `git reset
 	--hard` to revert them.  This is the default action when no

@@ -51,6 +51,12 @@ save [--keep-index] [-q|--quiet] [<message>]::
 +
 If the `--keep-index` option is used, all changes already added to the
 index are left intact.
++
+With --patch, interactively select hunks of changes to be stashed.
+This first asks for the hunks to be taken from the HEAD..index
+difference, and afterwards for hunks from the index..worktree
+difference.  Then, a stash is constructed that contains these changes,
+and the changes are removed from the index and worktree, respectively.
 
 list [<options>]::
 

index 5005a8d..17100d3 100755
@@ -76,6 +76,7 @@
 
 sub apply_patch;
 sub apply_patch_for_checkout_commit;
+sub apply_patch_for_stash;
 
 my %patch_modes = (
 	'stage' => {

@@ -86,6 +87,22 @@
 		PARTICIPLE => 'staging',
 		FILTER => 'file-only',
 	},
+	'stash_index' => {
+		DIFF => 'diff-index -p --cached HEAD',
+		APPLY => \&apply_patch_for_stash,
+		APPLY_CHECK => 'apply --cached',
+		VERB => 'Stash',
+		PARTICIPLE => 'stashing',
+		FILTER => 'index-only',
+	},
+	'stash_worktree' => {
+		DIFF => 'diff-files -p',
+		APPLY => \&apply_patch_for_stash,
+		APPLY_CHECK => 'apply --cached',
+		VERB => 'Stash',
+		PARTICIPLE => 'stashing',
+		FILTER => 'file-only',
+	},
 	'reset' => {
 		DIFF => 'diff-index -p --cached',
 		APPLY => sub { apply_patch 'apply -R --cached', @_; },

@@ -1067,6 +1084,14 @@
 	return $ret;
 }
 
+sub apply_patch_for_stash {
+	my $fh;
+	open $fh, '>>', $ENV{GIT_STASH_TEMP_PATCH}
+		or die "cannot open temporary patch file: $!";
+	print $fh @_;
+	return close $fh;
+}
+
 sub apply_patch_for_checkout_commit {
 	my $applies_index = run_git_apply 'apply -R --cached --recount --check', @_;
 	my $applies_worktree = run_git_apply 'apply -R --recount --check', @_;

@@ -1458,8 +1483,9 @@
 					$patch_mode_revision = $arg;
 					$arg = shift @ARGV or die "missing --";
 				}
-			} elsif ($1 eq 'stage') {
-				$patch_mode = 'stage';
+			} elsif ($1 eq 'stage' or $1 eq 'stash_index'
+				 or $1 eq 'stash_worktree') {
+				$patch_mode = $1;
 				$arg = shift @ARGV or die "missing --";
 			} else {
 				die "unknown --patch mode: $1";

1.6.4.rc2.227.gf5e17

Heya,

Awesome! Does it also remove the stashed hunks from the worktree? From what I gather from the patch it looks like it, but the commit message doesn't say.

Cheers,

Sverre Rabbelier

Yes, and unless you used --keep-index, also the stashed index-hunks from the index.

Thomas Rast
trast@{inf,student}.ethz.ch

The new command 'git discard' is precisely the same as 'git checkout --'.

---
 Documentation/git-discard.txt |   29 +++++++++++++++++++++++++++++
 Makefile                      |    1 +
 git-discard.sh                |   22 ++++++++++++++++++++++
 wt-status.c                   |    2 +-
 4 files changed, 53 insertions(+), 1 deletions(-)
 create mode 100644 Documentation/git-discard.txt
 create mode 100644 git-discard.sh

new file mode 100644
index 0000000..4db14f0
@@ -0,0 +1,29 @@
+git-discard(1)
+==============
+
+NAME
+----
+git-discard - Remove changes to a file from the worktree
+
+
+SYNOPSIS
+--------
+[verse]
+'git discard' <paths> ...
+
+
+DESCRIPTION
+-----------
+
+Overwrites your edits to the 'paths' with the values from the staging
+area, effectively throwing them away entirely.
+
+*WARNING:* All unstaged changes to the 'paths' are *irreversibly*
+lost.
+
+(This is the same as `git checkout \-- <paths>`.)
+
+
+SEE ALSO
+--------
+linkgit:git-checkout[1]

index 9e48fdc..814d4b6 100644
@@ -312,6 +312,7 @@ TEST_PROGRAMS =
 SCRIPT_SH += git-am.sh
 SCRIPT_SH += git-bisect.sh
 SCRIPT_SH += git-difftool--helper.sh
+SCRIPT_SH += git-discard.sh
 SCRIPT_SH += git-filter-branch.sh
 SCRIPT_SH += git-lost-found.sh
 SCRIPT_SH += git-merge-octopus.sh

new file mode 100644
index 0000000..595df98
@@ -0,0 +1,22 @@
+#!/bin/sh
+
+SUBDIRECTORY_OK=Yes
+OPTIONS_KEEPDASHDASH=
+OPTIONS_SPEC="\
+git discard file ...
+--"
+
+. git-sh-setup
+
+
+case "$1" in
+    --)
+	if [ $# -eq 1 ]; then
+	    die "You must specify at least one file to discard changes from"
+	fi
+	exec git checkout "$@"
+	;;
+    *)
+	usage
+	;;
+esac

index f1a74a4..1dd4bed 100644
@@ -75,7 +75,7 @@ static void wt_status_print_dirty_header(struct wt_status *s,
 		color_fprintf_ln(s->fp, c, "#   (use \"git add <file>...\" to update what will be committed)");
 	else
 		color_fprintf_ln(s->fp, c, "#   (use \"git add/rm <file>...\" to update what will be committed)");
-	color_fprintf_ln(s->fp, c, "#   (use \"git checkout -- <file>...\" to discard changes in working directory)");
+	color_fprintf_ln(s->fp, c, "#   (use \"git discard <file>...\" to discard changes in working directory)");
 	color_fprintf_ln(s->fp, c, "#");
 }
 

1.6.4.rc2.217.g74c0b.dirty

Actually, there's an important difference:

$ git branch foo HEAD~20
$ touch foo && git add foo
$ echo hi >> foo
$ git discard foo
A     foo
Switched to branch "foo"

You really do need that "--".

You fall into the same trap as me for patch 1/3. He does not shift away the '--', so it is still in "$@".

Bert.

Indeed; I somehow missed that '--' was silently added by git. I guess that's what I get for shooting off my mouth without actually trying it. Sorry for the noise.

This implements a new optional argument --patch=<mode> in the git-add--interactive helper. The modes are:

'stage' (default, as before): offer hunks from 'git diff' and stage
the ones selected by the user.

'unstage': offer hunks from 'git diff --cached', and unstage the ones
selected.

'discard': offer hunks from 'git diff', and discard (i.e., undo the
edit in the working tree) the ones selected.

With this in hand, we can then easily add a --patch option to the git-unstage and git-discard scripts that run git-add--interactive in the corresponding modes.

---
 Documentation/git-discard.txt |    7 ++++
 Documentation/git-unstage.txt |    7 ++++
 git-add--interactive.perl     |   69 +++++++++++++++++++++++++++++++---------
 git-discard.sh                |    9 ++++-
 git-unstage.sh                |    9 ++++-
 5 files changed, 81 insertions(+), 20 deletions(-)

index 4db14f0..2c063d1 100644
@@ -24,6 +24,13 @@ lost.
 (This is the same as `git checkout \-- <paths>`.)
 
 
+OPTIONS
+-------
+-p::
+--patch::
+	Interactively select hunks to discard.
+
+
 SEE ALSO
 --------
 linkgit:git-checkout[1]

index 49d09fb..d3da3e1 100644
@@ -21,6 +21,13 @@ is not affected.  (This is the same as `git reset \-- <paths>` unless
 you are on an unborn branch.)
 
 
+OPTIONS
+-------
+-p::
+--patch::
+	Interactively select hunks to unstage.
+
+
 SEE ALSO
 --------
 linkgit:git-reset[1]

index 595df98..628346d 100644
@@ -3,13 +3,18 @@
 SUBDIRECTORY_OK=Yes
 OPTIONS_KEEPDASHDASH=
 OPTIONS_SPEC="\
-git discard file ...
---"
+git discard [-p] file ...
+--
+p,patch         interactively select hunks to discard"
 
 . git-sh-setup
 
 
 case "$1" in
+    -p|--patch)
+	shift
+	exec git add--interactive --patch=discard "$@"
+	;;
     --)
 	if [ $# -eq 1 ]; then
 	    die "You must specify at least one file to discard changes from"

index 7f99adf..921e12b 100644
@@ -3,13 +3,18 @@
 SUBDIRECTORY_OK=Yes
 OPTIONS_KEEPDASHDASH=
 OPTIONS_SPEC="\
-git unstage file ...
---"
+git unstage [-p] file ...
+--
+p,patch         interactively select hunks to unstage"
 
 . git-sh-setup
 
 
 case "$1" in
+    -p|--patch)
+	shift
+	exec git add--interactive --patch=unstage "$@"
+	;;
     --)
 	if [ $# -eq 1 ]; then
 	    die "You must specify at least one file to unstage"

1.6.4.rc2.217.g74c0b.dirty

While I do not really think even more git commands are so helpful, I like the proposed command names, and I really like that it would be possible to remove single hunks from the index. I have come to rely very much on git add -p to make self-contained and clear commits and think it would be a great feature if I could use this the other way round too.

Thanks,
Matthias

Great, I really miss this feature in git-gui, maybe its easy to add this there too. I may look into this by my self, but not this weekend.

Bert

I do not particularly like this change. "add -i", "add -p" and "add" in general are about manipulating the index. They are never meant to touch the work tree contents. Which means that even if you make a mistake in saying y/n, you won't damange the state you have in your work tree, and also means that you can recover safely by simply restarting "add -p" session if you really botched splitting of the patch.

I fear tempting a new user who sees "undo" to say "yeah, I added the change in this hunk to the index by mistake, please undo", which would lose the work. The confusion is easier to avoid if "add" only manipulates the index without harming the work tree, and the user used a different command, namely "checkout from the index", to get rid of the remaining debug cruft, once s/he added all the necessary bits to the index perhaps after a multi-stage commit session.

Okay, fair enough, this is kind of mixing stuff together, I reckon. OTOH this fills out a real need: sorting out the debug code from the non debug one, it's particularily handy, and many people at work here have applied my patch and it saves them lots of time.

So maybe we should see how to have a new command like git hunk-sort or git hunk-triage or whatever, I don't really mind.

Another way, that is non destructive is to _not_ apply the reverting patch, but only to generate it, and let the user apply it himself. E.g. something along the lines of:

$ git add -p
... select your patches ...
info: you have selected hunks for removal from you tree
info: run the following command to make them go away:
  git apply -R --recount git-add-undo-12asWED.patch
$

And let the user cut & paste the command.

_I_ could live with that, and you lose the dangerous factor.

Intersec <http://www.intersec.com>
Pierre Habouzit <[email protected]>
Tél : +33 (0)1 5570 3346
Mob : +33 (0)6 1636 8131
Fax : +33 (0)1 5570 3332
37 Rue Pierre Lhomme
92400 Courbevoie

Quoting Junio C Hamano <[email protected]>

I can see your argument that this might introduce more danger for newbies. As you said yourself number of times, nobody will stay being a newbie forever, and I don't think it is wise to reject a feature that is very handy for experts based solely on such a fear.

Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

It is true that as new people learn they gain proficiency, and you are also right to point out that I'd usually choose to optimize the interface for making the life of experts easier rather than welding training wheels to the system.

BUT

As new people gain proficiency in git, three things happen.

(1) It becomes a lot less likely for them to make mistakes in choosing commands. I mentioned that they may misunderstand what Pierre's "undo" would do, and that fear may be alleviated because of this.

(2) It does _not_ become less likely for them to make typos when giving the command they chose. The chance of saying 'u' when you mean 'y' does not decrease that much as you become more used to using git. Especially with interactive.singlekey, the consequence of such a typo is devastating.

(3) They form a better mental model of how the world works.

The high level view of the git workflow is for you to:

(a) prepare good changes, together with some changes that are not quite ready, in your work tree; and

(b) use "git add" to add only good changes suitable for the next commit to the index; and finally

(c) make the next commit out of the index. Repeat (b) and (c) as necessary to create multiple commits.

If you botch the "git add" step during this process, because "git add" promises not to touch the work tree, you can safely reset the index entry to its previous state and redo the "git add" step, without having to fear that you may lose your work.

In your arsenal, you have "git add -p" to help you sift good pieces from other parts in finer grained manner, instead of having to make an all or nothing decision per file basis (i.e. "git add file"). But "git add -p" (and "git add -i") is still about the "git add" step in the above high level view. You have a mixture of good and not so good changes in your work tree, and you pick only good pieces to add to the index, _knowing_ that you can go back and redo this step safely exactly because your work tree will stay the same even if you did make mistakes.

The proposed change breaks this expectation you would have naturally gained during the course of becoming more and more proficient in using git.

In other words, I do not think you can say that the change will not harm the experts due to both the points 2 (experts can easily make typo) and 3 above (the change breaks the mental model of the world experts would have formed).

Having said all that, it indeed would be useful to selectively revert changes from the work tree files.

Even though you could add good bits interactively, making multiple commits, and remove the remaining debugging cruft at the very end with "git checkout $files" or "git reset --hard", if there are debugging crufts for two or more phases of development, this alternative procedure would not work well, compared to the workflow using Pierre's 'u', which allows you to add necessary bits to commit one phase, remove the debugging bits for that phase (but keeping other debugging bits for the remaining good parts), then continue working and repeat committing and cleaning second and subsequent phases.

It might be enough to change the command key to uppercase "U" to avoid unintended mistakes, and document the fact prominently that this action is an oddball exception to the principle of "git add", while describing why it is an oddball, along the lines of the above discussion, if necessary.

Perhaps it makes sense to have an interactive stash rather than an interactive revert? Then the reverts that you make are still being saved somewhere, and you can recover from an error by applying the stash. Not to mention that interactive stash is useful in its own right.

The downside is that if you are the sort of person who keeps a clean stash list (and I am not such a person), then you have this "to-be-deleted" cruft on the top of your stash (whereas with a true revert, it just goes away).

-Peff

Very cool idea, it's even better than creating a revert patch.

Note that to undermine the "dirty stash list" effect we could have quite easily different stash queues if it's badly needed, so it sounds like a moot point to me.

Intersec <http://www.intersec.com>
Pierre Habouzit <[email protected]>
Tél : +33 (0)1 5570 3346
Mob : +33 (0)6 1636 8131
Fax : +33 (0)1 5570 3332
37 Rue Pierre Lhomme
92400 Courbevoie

Yeah, such a stash entry would be more like "trash can". It is not "to-be-deleted" but "have been deleted, but you _could_ resurrect".

It may not be a bad idea to do it that way, or perhaps "git checkout -p" can automatically create such a trash can while undoing the local changes in the work tree.

I'd rather implement this as part of the generic "worktree/index log" that was proposed a while back, where any index- or worktree-overwriting operation that actually discards data would save the lost state in a special reflog. That way people won't complain because 'git checkout -p -- $file' <press a> saved their state, but 'git checkout -- $file' didn't, even though they should be equivalent.

Thomas Rast
trast@{inf,student}.ethz.ch

FWIW it's what I was doing so far, and it's not very efficient for many patterns, you talked about the bit where you want to keep some of the debug, for this one I used to do that:

while I have meaning full commits to do:

    git add -p; commit;

git add -p the things I want to trash and commit
git stash
git reset --hard HEAD~1
git stash apply

That sucks.

Another thing is that: if you have _many_ commits to do, you have to refuse the same hunks (or worse edit) all over the place. That's awful. With an "undo", you look at them once only. That's a lot of thinking saved, and when you have a lot of hunks, less chances to mess an answer (who here hasn't typed 'y' where he meant 'n' at least once ...)

For all those reasons I believe it's a good thing to be able to have something to remove hunks from the working-directory. Jeff's suggestions to move them to some stash is the best suggestion so far, and is safe.

Intersec <http://www.intersec.com>
Pierre Habouzit <[email protected]>
Tél : +33 (0)1 5570 3346
Mob : +33 (0)6 1636 8131
Fax : +33 (0)1 5570 3332
37 Rue Pierre Lhomme
92400 Courbevoie

Here's kind of a weird idea I've been considering. Feel free to write it off as insane ranting.

My two complaints about using stash for separating changes are:

- it lacks the tool support for splitting changes that we have for making commits (like "add -p"), and it lacks the ability to build up a set of changes over multiple commands (like we can do for commits)

- it works as a single destination. You stash and delete a change from the working tree, or you leave it. It's hard to say "there are 3 different types of change here" and sort them all at once.

My idea is to instead have a general set of "registers" that contain states, each of which is basically an index. You can copy state from register to register, from working tree to register, or from register to register. You can also do any of those moves by looking at differences between two states and saying "move this change" (i.e., like what "add -p" does for the regular index).

So one way of splitting changes would be to say:

1. Set registers 'a' and 'b' to the same state as HEAD

2. Pick changes from the working tree to go to 'a'

3. Pick changes from the working tree to go to 'b'

4. Commit 'a' on top of HEAD

5. Commit 'b' on top of new HEAD (and this would probably actually mean the changes from 'b' to the old HEAD, not setting the new HEAD state to what's in 'b').

So it's sort of a generalized form of the index, where you have N "index registers" and you sort your changes into them. And during steps 2 and 3, you could also make more changes, pick them out, etc.

The workflow you want maps into that pretty simply: you would sort your changes into "stuff you want to commit" and "stuff that is debugging cruft". And then you would just throw away the latter register (or use a special "trash" register).

And the workflow I described is "pick the changes for 'a', then for 'a'". But there's no reason you couldn't go through the changes, sorting each into "put this one into 'a', and this one into 'b'". Which is what you asked for.

Is this really that different from what you proposed? No, I don't really think so in terms of implementation, but it is really about a different mental model:

1. You never delete things. You only copy or move them into registers.

2. The interface should be the same whether you are moving between registers, or to/from the working tree.

3. It extends naturally to multiple registers.

Anyway, just some stray thoughts. No code, so feel free to ignore. ;)

-Peff

No it's not, in the sense that what I propose is a subset of your proposal, functionaly speaking ;)

I like the general idea, I'm unsure what the UI for such tools would look like though.

Intersec <http://www.intersec.com>
Pierre Habouzit <[email protected]>
Tél : +33 (0)1 5570 3346
Mob : +33 (0)6 1636 8131
Fax : +33 (0)1 5570 3332
37 Rue Pierre Lhomme
92400 Courbevoie

I was thinking of unifying the multiple interfaces that we use to move content and changes around into a single "git sort" and "git sort -i" (and yes, I just made those names up, so feel free to call them crappy), which would take a source and a destination (which would probably default to the working tree and index respectively).

So you could do the equivalent of:

- git add foo => git sort foo
- git add -p => git sort -i foo
- git checkout -- foo => git sort --from=index --to=tree foo
- git checkout HEAD -- foo => git sort --from=HEAD --to=tree foo
- git reset --mixed => git sort --from=HEAD --to=index
- git reset --mixed foo => git sort --from=HEAD --to=index foo (note that this reset doesn't actually exist now, but is something that people try to do)
- git stash save => git sort --to=%mystash
(and note that I just made up some "this is a register" syntax;
we maybe would really just want these as refs like refs/registers, so you would specify registers/mystash)
- git stash -i => git sort --to=%mystash -i
(interactive stash doesn't exist yet, of course)
- git stash foo => git sort --to=%mystash foo
(partial stash doesn't exist yet)
- git diff HEAD stash -- foo | git apply => git sort --from=%mystash foo

So really, it could be a new way of interacting with the _regular_ index, as well, though perhaps it is overboard to completely redesign the git interface. :) I just think it introduces a consistency to the interface around the single concept of "moving your content around". So in a sense it would be a good candidate for an alternative porcelain. But do note that many common operations are more typing (like the checkout replacements); that would be something to fix.

Again, just thinking out loud. Feel free to ignore, but if you think there is anything interesting to pick out, let me know.

-Peff

I think the parenthetical remark actually contradicts the notion that it's an index. It's more like a place to hold a patch. Which then makes it rather similar to a temporary branch and cherry-pick, or interactive rebase, or whatever.

Granted, the register idea does not directly map to interactive rebase because that cannot (automatically) add changes to an older commit. So I frequently wind up making a series of commits along the lines of

WIP implement foo
WIP implement bar
WIP fix foo some
WIP docs for bar
WIP docs for foo
WIP tests for foo

and then have to sort and squash them with rebase -i.

Thomas Rast
trast@{inf,student}.ethz.ch

Sort of. It's not an index in the sense that you might make a tree and commit directly from it. But I think of it as an index in that it holds a particular state, and you can diff that state against other things. I would probably implement it as an index via GIT_INDEX_FILE (though I guess performance would not be as nice as if it shared the cache parts of the main index).

Implicit in my thinking was that you could actually get rid of the concept of "the index" and simply replace it with such a register (which maybe would be the "default register" or something). So whether you committed directly from it, or whether you applied its diff would be decided not at creation time, but at the time you wanted to commit. And that would depend on what operations you were doing (simply making a commit, sorting changes for multiple commits, etc).

Yes, I do that a lot. And maybe this whole thing is a stupid idea; git already has lots of tools for working with _commits_, so maybe registers should really just be commits. So in that sense what I am asking for is just a multi-headed stash, and tools for doing interactive, incremental stashing to those heads.

-Peff