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

[Phillip Wood <phillip.wood@talktalk.net>]

On 27/07/18 19:27, Ævar Arnfjörð Bjarmason wrote:
> 
> On Thu, Jul 26 2018, Phillip Wood wrote:
> 
>> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>>
>> 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...

Yes, I wasn't sure whether to rebase or not and in the end I didn't. The
line below where you cut the cover letter message says it is based on
f4d35a6b49 "add -p: fix counting empty context lines in edited patches".
So you could just apply it there and test it. You can fetch it with

git fetch https://github.com/phillipwood/git add-i-select-lines-v5

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

Thanks

Phillip