[prev in list] [next in list] [prev in thread] [next in thread]
List: git
Subject: Re: [PATCH] git-add -p: be able to undo a given hunk
From: Pierre Habouzit <madcoder () madism ! org>
Date: 2009-07-23 8:50:47
Message-ID: 20090723085046.GK4750 () laphroaig ! corp
[Download RAW message or body]
On Thu, Jul 23, 2009 at 10:41:31AM +0200, Thomas Rast wrote:
> Pierre Habouzit wrote:
> > 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.)
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).
> > + 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.
Right, revert is probably too overloaded, let it be "forget" or "drop"
instead if you want, I don't care much.
> > Documentation/git-add.txt | 1 +
> > git-add--interactive.perl | 38 ++++++++++++++++++++++++++++++--------
>
> Tests?
Riight, will do :)
> > @@ -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.
I hesitated to do so, but yes it makes perfect sense.
> > + 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.
Indeed.
--
Intersec <http://www.intersec.com>
Pierre Habouzit <pierre.habouzit@intersec.com>
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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic