Re: feature request: git add--interactive --patch on regex-matched hunks only

From: Nguyen Thai Ngoc Duy <pclouds@gmail.com>
Date: Jul 26, 2011, 05:57 UTC
Message-ID: <CACsJy8Ay1wPXAx61_rGymHDJ=YGywAy=9epiXRfJ9e68np8x6g@mail.gmail.com>
In-Reply-To: <20110726051411.GB25046@sigill.intra.peff.net>

On Tue, Jul 26, 2011 at 12:14 PM, Jeff King <peff@peff.net> wrote:

Show 17 quoted lines

> On Tue, Jul 26, 2011 at 10:03:31AM +0700, Nguyen Thai Ngoc Duy wrote:
>
>> On Tue, Jul 26, 2011 at 4:55 AM, Jeff King <peff@peff.net> wrote:
>> > On Sun, Jul 24, 2011 at 12:11:29PM +0700, Nguyen Thai Ngoc Duy wrote:
>> >
>> >> Can we have "git add--interactive --patch --match=regex" where only
>> >> (splitted if possible) hunks that match regex are shown?
>> >
>> > The patch below does this,
>>
>> Thanks!
>
> So I did this as a quick "that should be really easy" proof of concept.
> I'm not too interested personally in taking it all the way to an
> acceptable patch.  I'm happy to help out with the perl parts, though, if
> you want to do the rest (C scaffolding for calling add--interactive,
> tests, and docs).

It's my itch. I'm more than happy if you take care of the Perl part, I'll do the rest. While you're doing the Perl part, should we have both --match and --no-match? One filters in. The other filters out. I guess people may find filter-in case more helpful than the other.

Show 7 quoted lines

> The more I think about it, it is probably simpler both conceptually and
> in implementation to filter out those hunks entirely instead of marking
> them used. The latter gives the user a chance to manually find them and
> go back to them via 'J' and 'K'. But I find the chance that that is
> useful to be much less than the chance that somebody gets confused or
> annoyed by them being there (because they were iterating, or because
> they did a "/" search).

I thought of this way initially, but I needed to split hunks in parse_diff and was stuck because I was not familiar with data structure used in git-add--interactive.

> And it's not like everything needs to be in a single "add" call. The
> point of "add" is that you could call it multiple times, or even call
> it, then check what "git diff" says, then decide you want to stage some
> more.

Yes, the point of this new feature, is to get me focus on a selected parts I care.

Show 11 quoted lines

>> >  2. What should we do with non-text changes, like mode changes are
>> >     full-file deletion?
>>
>> Probably invalid use case for --match.
>
> Invalid, like we should have an error? I think that would be annoying,
> because you want to be able to do "git add -p --match=foo" even when
> there is an unrelated mode change. I think in the mindset of "this is a
> filter for things you want to see", it probably makes sense to just
> pretend they aren't there (and the user can always follow up with
> another "add", as I mentioned above).

Agreed.

Show 17 quoted lines

>> >  3. What should be shown for a file with no matching hunks? Probably
>> >     nothing (i.e., as if it had been limited away by pathname)? My
>> >     patch shows the header, but that is easy to fix.
>>
>> Printing "Nothing to add" would be nice.
>
> If we treat it like path limiting, I think we could just print nothing
> for that file. That is, doing:
>
>  git add -p foo
>
> would not even look at "bar" or print anything about it. Similarly:
>
>  git add -p --match=foo
>
> should probably say nothing whatsoever about files that didn't mention
> foo at all.

Hmm.. I did not thought of doing multiple files. Printing nothing makes sense this case.

-- 
Duy