Re: [PATCH] new test fails "add -p" for adds on the top line

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <gitster@xxxxxxxxx> writes:

> Thomas Rast <trast@xxxxxxxxxxxxxxx> writes:
> ...
>> The above commit still reverts cleanly, but AFAICS merge_hunk blindly
>> trusts the hunk headers, an assumption that is no longer valid due to
>> the 'edit' feature.
>
> Heh, here is my "I told you so" moment ;-).

It never blindly trusted before the edit 'feature'; it counted carefully
and it could do so because it had all the necessary information.

I told you that 'edit' could remember the line offset and line numbers
before giving the buffer to the end user, and then recount and adjust the
count after getting the edited results back, to update the offset and
count with the same carefulness.  You (and I think there was somebody else
who was helping) didn't listen.

Fundamentally, after you remove some hunks (and worse yet, you modify
some) from the patch and feed that to "git apply --recount", it can never
do as thorough a job as you could do inside "add -p" itself.  The latter
has more information (the omitted hunks, and the hunks before/after the
user edited) necessary to reconstruct the line numbers and hunk size.  To
keep the whole process more robust and trustworthy, you must do the
necessary computation while you still have all the information about the
hunks you are not feeding to the downstream.

That was what the "I told you so" was about in my message.

It is not too late to teach the 'edit hack' to do so.  That would allow us
to remove the "$_->{DIRTY}" bit my "how about this" patch adds, and I'll
stop calling it the 'edit hack' and start calling it the 'edit feature'
when that happens ;-).

But at least the "how about this" patch should restore the original
behaviour as long as the user does not use the 'edit hack' for now.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]