Re: [RFC PATCH v5 2/4] add -p: select modified lines correctly

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

 



Hi Junio, thanks for the comments

On 26/07/18 20:30, Junio C Hamano wrote:
> Phillip Wood <phillip.wood@xxxxxxxxxxxx> writes:
> 
> An interesting problem you are solving ;-)
> 
>> For example given the hunk
>>       1 -* a longer description of the
>>       2 -  first item
>>       3 -* second
>>       4 -* third
>>       5 +* first
>>       6 +  second item
>>       7 +* the third item
>>
>> If the user selects 1,2,4–5,7 then we should generate
>> 	-* a longer description of the
>> 	-  first item
>> 	+* first
>> 	 * second
>> 	-* third
>> 	+* the third item
> 
> I understood this example as "3 that is removal and 6 that is
> addition are excluded---we consider that these two lines (one in the
> pre-image and the other in the post-image) are _matching" As we> are excluding a deletion, it becomes the common context line, and
> any removal or addition that appear before that must stay to happen
> before the common context line (i.e. removal of 1 and 2, and
> addition of 5, both precede common context line "second") and any
> removal or addition that appear after that must stay after the
> common context (i.e. removal of "third" and addition of "the third
> item" come after "second").
> 
> But then it is not clear to me what you mean by "group" below.  What
> groups does the above example have?  Ones before the retained
> "second" (i.e. removal 1, 2, 4 and addition 5) form one group and
> ones after it (i.e. removal 4 and addition 7) form another group?

The code actually looks at the lines that are selected rather than
omitted. So in the example above it groups them as [1,2] (because they
are contiguous), [4],[5] (these are split because one is an insertion
and one a deletion) and [7]. It then sees that there are two groups of
deletions ([1,2],[4]) and two groups of insertions ([5],[7]) and so
pairs up the deletions in [12] with the insertion in [5] and likewise
with [4] and [7]. Lines 3 and 6 are never explicitly paired, although
they basically behave as if they were. One the insertions are all paired
up it walks over the list and creates a new hunk where the paired
insertions come immediately after their corresponding deletions,
unselected deletions are converted to context lines and unselected
additions are dropped.

> 
>> Reported-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
> 
> Is this fixing any bug?  I usually see "Reported-by" only for a
> bugfix patch but this seems to be adding a new feature (and lack of
> feature is usually not a bug).

I guess I meant that the previous series was effectively buggy as it
would give the wrong result for modified lines. I wanted to acknowledge
that Ævar spent some time testing it and pointed that out.

Best Wishes

Phillip




[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]

  Powered by Linux