Re: [RFC PATCH] git-add--interactive: manual hunk editing mode v2

From: Jeff King <peff@peff.net>
Date: Jun 5, 2008, 08:56 UTC
Message-ID: <20080605085605.GA16624@sigill.intra.peff.net>
In-Reply-To: <7vprqw2t64.fsf@gitster.siamese.dyndns.org>

On Thu, Jun 05, 2008 at 01:16:19AM -0700, Junio C Hamano wrote:

Show 6 quoted lines

> > So perhaps manual hunk editing is simply something for advanced users
> > who are comfortable with the patch format.
> 
> Exactly.  To them, "git diff >patch && vi patch && git apply --cached <patch"
> would likely to be much handier, quicker and a more familiar way. That is
> one of the reasons I somewhat doubt that we would want to have this patch.

I'm not so sure. I think this patch buys you two things:

 1. It munges the hunk headers as appropriate.

 2. It fits more naturally into the "git add -p" workflow. That is, I
    don't start by saying "I need to edit this patch." Instead, I am
    staging bits of changes, and looking at a hunk I say "Oh, this needs
    to be tweaked"; this patch provides an easy route from looking at
    the hunk to editing the hunk.

    Granted, 99% of the time my tweak is "these adjacent lines are
    actually unrelated, so split them into two hunks." So maybe that is
    an argument for something like Thomas' original patch.

> For the rest of your comments, I agree with the Perl style (use of map and
> grep instead of repeated push in loops).  The end user input, what the
> code needs to parse and accept, can screw you up royally and your parsing
> needs to be careful, and the code looks fragile.

I think there are two extremes here:

 1. We provide a very limited interface. E.g., you pick a line to
    arbitrarily split the hunk.

 2. We provide a very flexible interface. You get dumped in your editor
    with the hunk text, and we try to apply the result. Powerful, but
    easy to shoot yourself in the foot.

And there is probably room for both. But I think it is important not to fall into the middle, where we have a hard-to-use but featureless interface. That is, if we dump the user in an editor, doing anything besides (2) doesn't make sense; trying to infer what happened in the user's editing session is a waste of time.

So I think we need to be not so much careful in parsing, as accepting. Or we need to dump the idea of letting the user make arbitrary edits.

-Peff