Subject: Re: [RFC PATCH v5 0/4] add -p: select individual hunk lines

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Fri, 27 Jul 2018 20:27:00 +0200

On Thu, Jul 26 2018, Phillip Wood wrote:

> From: Phillip Wood <[email-masked]>
>
> Unfortuantely v4 had test failures due to a suprious brace from a last
> minute edit to a comment that I forgot to test. This version fixes
> that, my applogies for the patch churn.
>
> I've updated this series based on Ævar's feedback on v3 (to paraphrase
> stop using '$_' so much and fix staging modified lines.). The first
> patch is functionally equivalent to the previous version but with a
> reworked implementation. Patch 2 is new, it implements correctly
> staging modified lines with a couple of limitations - see the commit
> message for more details, I'm keen to get some feedback on it. Patches
> 3 and 4 are essentially rebased and tweaked versions of patches 2 and
> 3 from the previous version.

I was going to review this, but can't find what it's based on, I can't
apply 1/4 to master, next or pu. It seems to be based on some older
version of master, e.g. 1/4 has this hunk:

    +			elsif ($line =~ /^l/) {
    +				unless ($other =~ /l/) {
    +					error_msg __("Cannot select line by line\n");
    +					next;
    +				}
    +				my $newhunk = select_lines_loop($hunk[$ix]);
    +				if ($newhunk) {
    +					splice @hunk, $ix, 1, $newhunk;
    +				} else {
    +					next;
    +				}
    +			}
     			elsif ($other =~ /s/ && $line =~ /^s/) {

Which seems to conflict with your 4bdd6e7ce3 ("add -p: improve error
messages", 2018-02-13). I could have tried to manually apply this, but
figured I'd bounce this back to you...

Having just skimmed through the patches themselves I agree with this
approach of handling the simple case (as discussed before) and leaving
the rest for some future change, but let's see about the details once I
have this running.