Re: [PATCH] git-add -p: be able to undo a given hunk

From: Thomas Rast <trast@student.ethz.ch>
Date: Jul 23, 2009, 08:41 UTC
Message-ID: <200907231041.32567.trast@student.ethz.ch>
In-Reply-To: <20090723074104.GI4750@laphroaig.corp>

Pierre Habouzit wrote:

Show 7 quoted lines

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

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

> +       u - do not stage this hunk and revert it

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.

>  Documentation/git-add.txt |    1 +
>  git-add--interactive.perl |   38 ++++++++++++++++++++++++++++++--------

Tests?

Show 6 quoted lines

> @@ -693,6 +693,7 @@ sub split_hunk {
>  			ADDDEL => 0,
>  			POSTCTX => 0,
>  			USE => undef,
> +			UNDO => undef,
>  		};

Why not fold this into a single field? It could, say, take values 0, 1, or '-R'. It could probably be renamed to ACTION, but USE would be fine if you want to avoid the code churn. Then you shouldn't need _completely_ separate handling during application.

> @@ -1149,6 +1152,10 @@ sub patch_update_file {
>  			elsif ($line =~ /^n/i) {
>  				$hunk[$ix]{USE} = 0;

+				$hunk[$ix]{UNDO} = 0;

and similarly for [yad] too, on the grounds that the user can go back and change his choices with [KJ]. Of course that is not necessary if you go the ACTION way outlined above.

> +	if (@undo) {
> +		my $fh;
> +
> +		open $fh, '| git apply -R';

This probably needs a --recount to cope with the case where the hunk headers became stale/invalid through user [e]diting.

Show 10 quoted lines

> +		for (@{$head->{TEXT}}, @undo) {
> +			print $fh $_;
> +		}
> +		if (!close $fh) {
> +			for (@{$head->{TEXT}}, @undo) {
> +				print STDERR $_;
> +			}
> +		}
> +		refresh();
> +	}

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